Linting

.github/workflows/go.yml

@@ -22,11 +22,6 @@ jobs:
         run: |
           cat .version >> $GITHUB_ENV
 
-      - name: Setup Go
-        uses: actions/setup-go@v4
-        with:
-          go-version: "${{env.GO_VERSION}}"
-
       - name: Run golangci-lint
         uses: golangci/golangci-lint-action@v3
         with:

.golangci.yml

@@ -8,31 +8,39 @@ run:
   build-tags:
     - e2e
   skip-files:
-    - httpbin/httpbin.go
     - middleware/delegator.go
     - utils/cobrautil/templates
     - utils/cobrautil/term
-    - internal/martian
   silent: true
 
 linters:
   enable-all: true
   disable:
+    - bodyclose
     - contextcheck
     - cyclop
+    - deadcode
+    - depguard
     - exhaustive
     - exhaustivestruct
     - exhaustruct
     - gochecknoinits
     - godox
     - goerr113
+    - golint
     - gomnd
     - gomoddirectives
+    - ifshort
+    - interfacer
+    - ireturn
     - maligned
     - nlreturn
     - nonamedreturns
     - nosnakecase
+    - scopelint
+    - structcheck
     - tagliatelle
+    - varcheck
     - varnamelen
     - wrapcheck
     - wsl
@@ -42,9 +50,10 @@ linters-settings:
   errcheck:
     check-blank: true
   funlen:
-    lines: 100
+    lines: 140
+    statements: 75
   gocognit:
-    min-complexity: 40
+    min-complexity: 50
   gocyclo:
     min-complexity: 40
   gocritic:
@@ -55,7 +64,7 @@ linters-settings:
   lll:
     line-length: 180
   nestif:
-    min-complexity: 6
+    min-complexity: 11
   goheader:
     values:
       regexp:
@@ -74,10 +83,12 @@ issues:
     - "can be `expvar.Var`"
     - "can be `fmt.Stringer`"
     - "missing cases in switch of type Scheme: TunnelScheme"
-    - "returns interface \\(github.com/dop251/goja\\.Value\\)"
-    - "returns interface \\(github.com/saucelabs/forwarder/internal/martian\\.(Request|Response)+Modifier\\)"
     - "shadow of imported from 'github.com/saucelabs/forwarder/log' package 'log'"
     - "string `https?` has \\d+ occurrences"
+    - "context-keys-type: should not use basic type string as key in context.WithValue"
+    - "SA1029: should not use built-in type string as key for value; define your own type to avoid collisions"
+    - "importShadow: shadow of imported package 'url'"
+    - "commentedOutCode: may want to remove commented-out code"
   exclude-rules:
     - path: cmd/forwarder/version/version.go
       linters:
@@ -111,6 +122,18 @@ issues:
       linters:
         - gochecknoglobals
 
+    - path: internal/martian
+      linters:
+        - goheader
+        - gochecknoglobals
+        - errcheck
+        - gosec
+
+    - path: internal/martian/h2/testing
+      linters:
+        - forbidigo
+      text: use of `fmt.Print
+
     - linters:
         - thelper
       source: "configure: func\\(t \\*testing.T"

.version

@@ -1,6 +1,6 @@
 GO_VERSION=1.21.0
 
 X_TOOLS_VERSION=v0.6.0
-GOLANGCI_LINT_VERSION=v1.51.1
+GOLANGCI_LINT_VERSION=v1.54.1
 GORELEASER_VERSION=v1.17.2
 GO_LICENSES_VERSION=v1.6.0

api.go

@@ -66,7 +66,7 @@ func NewAPIHandler(r prometheus.Gatherer, ready func(ctx context.Context) bool,
 	return a
 }
 
-func (h *APIHandler) healthz(w http.ResponseWriter, r *http.Request) {
+func (h *APIHandler) healthz(w http.ResponseWriter, _ *http.Request) {
 	w.WriteHeader(http.StatusOK)
 	w.Header().Set("Content-Type", "text/plain")
 	w.Write([]byte("OK"))
@@ -84,18 +84,18 @@ func (h *APIHandler) readyz(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
-func (h *APIHandler) configz(w http.ResponseWriter, r *http.Request) {
+func (h *APIHandler) configz(w http.ResponseWriter, _ *http.Request) {
 	w.WriteHeader(http.StatusOK)
 	w.Header().Set("Content-Type", "text/plain")
 	w.Write([]byte(h.config))
 }
 
-func (h *APIHandler) pac(w http.ResponseWriter, r *http.Request) {
+func (h *APIHandler) pac(w http.ResponseWriter, _ *http.Request) {
 	w.Header().Set("Content-Type", "application/x-ns-proxy-autoconfig")
 	w.Write([]byte(h.script))
 }
 
-func (h *APIHandler) version(w http.ResponseWriter, r *http.Request) {
+func (h *APIHandler) version(w http.ResponseWriter, _ *http.Request) {
 	w.Header().Set("Content-Type", "application/json")
 	v := struct {
 		Version string `json:"version"`
@@ -133,7 +133,7 @@ const indexTemplate = `<!DOCTYPE html>
 </html>
 `
 
-func (h *APIHandler) index(w http.ResponseWriter, r *http.Request) {
+func (h *APIHandler) index(w http.ResponseWriter, _ *http.Request) {
 	w.WriteHeader(http.StatusOK)
 	w.Header().Set("Content-Type", "text/html")
 

cmd/forwarder/httpbin/httpbin.go

@@ -1,3 +1,9 @@
+// Copyright 2023 Sauce Labs Inc. All rights reserved.
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at https://mozilla.org/MPL/2.0/.
+
 package httpbin
 
 import (
@@ -17,7 +23,7 @@ type command struct {
 	logConfig        *log.Config
 }
 
-func (c *command) RunE(cmd *cobra.Command, args []string) error {
+func (c *command) RunE(cmd *cobra.Command, _ []string) error {
 	config := bind.DescribeFlags(cmd.Flags())
 
 	if f := c.logConfig.File; f != nil {

cmd/forwarder/pac/server/server.go

@@ -30,7 +30,7 @@ type command struct {
 	logConfig           *log.Config
 }
 
-func (c *command) RunE(cmd *cobra.Command, args []string) error {
+func (c *command) RunE(cmd *cobra.Command, _ []string) error {
 	config := bind.DescribeFlags(cmd.Flags())
 
 	if f := c.logConfig.File; f != nil {

cmd/forwarder/ready/ready.go

@@ -18,7 +18,7 @@ type command struct {
 	apiAddr string
 }
 
-func (c *command) RunE(cmd *cobra.Command, args []string) error {
+func (c *command) RunE(cmd *cobra.Command, _ []string) error {
 	resp, err := http.Get("http://" + c.apiAddr + "/readyz") //nolint:noctx // net/http.Get must not be called
 	if err != nil {
 		return err

cmd/forwarder/run/run.go

@@ -42,7 +42,7 @@ type command struct {
 	goleak              bool
 }
 
-func (c *command) RunE(cmd *cobra.Command, args []string) error {
+func (c *command) RunE(cmd *cobra.Command, _ []string) error {
 	config := bind.DescribeFlags(cmd.Flags())
 
 	if f := c.logConfig.File; f != nil {

dialvia/socks5.go

@@ -36,7 +36,7 @@ func SOCKS5Proxy(dial ContextDialerFunc, proxyURL *url.URL) *SOCKS5ProxyDialer {
 	}
 }
 
-func (d *SOCKS5ProxyDialer) DialContext(ctx context.Context, network, addr string) (net.Conn, error) {
+func (d *SOCKS5ProxyDialer) DialContext(_ context.Context, network, addr string) (net.Conn, error) {
 	u := d.proxyURL.User
 	var auth *proxy.Auth
 	if u != nil {

header/header.go

@@ -41,7 +41,7 @@ var (
 func ParseHeader(val string) (Header, error) {
 	var h Header
 
-	if strings.HasPrefix(val, "-") { //nolint:nestif // the branches are very similar
+	if strings.HasPrefix(val, "-") {
 		if strings.HasSuffix(val, "*") {
 			h.Name = val[1 : len(val)-1]
 			h.Action = RemoveByPrefix

http_proxy.go

@@ -525,6 +525,6 @@ func (hp *HTTPProxy) Addr() string {
 }
 
 // Ready returns true if the server is running and ready to accept requests.
-func (hp *HTTPProxy) Ready(ctx context.Context) bool {
+func (hp *HTTPProxy) Ready(_ context.Context) bool {
 	return hp.Addr() != ""
 }

http_server.go

@@ -260,6 +260,6 @@ func (hs *HTTPServer) Addr() string {
 }
 
 // Ready returns true if the server is running and ready to accept requests.
-func (hs *HTTPServer) Ready(ctx context.Context) bool {
+func (hs *HTTPServer) Ready(_ context.Context) bool {
 	return hs.Addr() != ""
 }

internal/martian/context.go

@@ -51,15 +51,15 @@ type Session struct {
 	vals     map[string]any
 }
 
-const marianKey string = "martian.Context"
+const martianKey string = "martian.Context"
 
 // NewContext returns a context for the in-flight HTTP request.
 func NewContext(req *http.Request) *Context {
-	v := req.Context().Value(marianKey)
+	v := req.Context().Value(martianKey)
 	if v == nil {
 		return nil
 	}
-	return v.(*Context)
+	return v.(*Context) //nolint:forcetypeassert // We know the type.
 }
 
 // TestContext builds a new session and associated context and returns the context.
@@ -150,16 +150,6 @@ func (s *Session) Hijacked() bool {
 	return s.hijacked
 }
 
-// setConn resets the underlying connection and bufio.ReadWriter of the
-// session. Used by the proxy when the connection is upgraded to TLS.
-func (s *Session) setConn(conn net.Conn, brw *bufio.ReadWriter) {
-	s.mu.Lock()
-	defer s.mu.Unlock()
-
-	s.conn = conn
-	s.brw = brw
-}
-
 // Get takes key and returns the associated value from the session.
 func (s *Session) Get(key string) (any, bool) {
 	s.mu.RLock()
@@ -188,7 +178,7 @@ func (ctx *Context) addToContext(rctx context.Context) context.Context {
 	if rctx == nil {
 		rctx = context.Background()
 	}
-	return context.WithValue(rctx, marianKey, ctx)
+	return context.WithValue(rctx, martianKey, ctx)
 }
 
 // Session returns the session for the context.

internal/martian/context_test.go

@@ -17,14 +17,14 @@ package martian
 import (
 	"bufio"
 	"bytes"
-	"io/ioutil"
+	"io"
 	"net"
 	"net/http"
 	"testing"
 )
 
 func TestContexts(t *testing.T) {
-	req, err := http.NewRequest("GET", "http://example.com", nil)
+	req, err := http.NewRequest(http.MethodGet, "http://example.com", http.NoBody)
 	if err != nil {
 		t.Fatalf("http.NewRequest(): got %v, want no error", err)
 	}
@@ -72,8 +72,7 @@ func TestContexts(t *testing.T) {
 
 func TestContextHijack(t *testing.T) {
 	rc, wc := net.Pipe()
-
-	req, err := http.NewRequest("GET", "http://example.com", nil)
+	req, err := http.NewRequest(http.MethodGet, "http://example.com", http.NoBody)
 	if err != nil {
 		t.Fatalf("http.NewRequest(): got %v, want no error", err)
 	}
@@ -99,14 +98,14 @@ func TestContextHijack(t *testing.T) {
 	}
 
 	go func() {
-		brw.Write([]byte("test message"))
+		brw.WriteString("test message")
 		brw.Flush()
 		conn.Close()
 	}()
 
-	got, err := ioutil.ReadAll(wc)
+	got, err := io.ReadAll(wc)
 	if err != nil {
-		t.Fatalf("ioutil.ReadAll(): got %v, want no error", err)
+		t.Fatalf("io.ReadAll(): got %v, want no error", err)
 	}
 
 	if want := []byte("test message"); !bytes.Equal(got, want) {

internal/martian/fifo/fifo_group_test.go

@@ -31,7 +31,7 @@ func TestModifyRequest(t *testing.T) {
 
 	fg.AddRequestModifier(tm)
 
-	req, err := http.NewRequest("GET", "/", nil)
+	req, err := http.NewRequest(http.MethodGet, "/", http.NoBody)
 	if err != nil {
 		t.Fatalf("http.NewRequest(): got %v, want no error", err)
 	}
@@ -54,11 +54,11 @@ func TestModifyRequestHaltsOnError(t *testing.T) {
 	tm2 := martiantest.NewModifier()
 	fg.AddRequestModifier(tm2)
 
-	req, err := http.NewRequest("GET", "http://example.com/", nil)
+	req, err := http.NewRequest(http.MethodGet, "http://example.com/", http.NoBody)
 	if err != nil {
 		t.Fatalf("http.NewRequest(): got %v, want no error", err)
 	}
-	if err := fg.ModifyRequest(req); err != reqerr {
+	if err := fg.ModifyRequest(req); !errors.Is(err, reqerr) {
 		t.Fatalf("fg.ModifyRequest(): got %v, want %v", err, reqerr)
 	}
 
@@ -81,7 +81,7 @@ func TestModifyRequestAggregatesErrors(t *testing.T) {
 	tm2.RequestError(reqerr2)
 	fg.AddRequestModifier(tm2)
 
-	req, err := http.NewRequest("GET", "http://example.com/", nil)
+	req, err := http.NewRequest(http.MethodGet, "http://example.com/", http.NoBody)
 	if err != nil {
 		t.Fatalf("http.NewRequest(): got %v, want no error", err)
 	}
@@ -129,7 +129,7 @@ func TestModifyResponseHaltsOnError(t *testing.T) {
 	fg.AddResponseModifier(tm2)
 
 	res := proxyutil.NewResponse(200, nil, nil)
-	if err := fg.ModifyResponse(res); err != reserr {
+	if err := fg.ModifyResponse(res); !errors.Is(err, reserr) {
 		t.Fatalf("fg.ModifyResponse(): got %v, want %v", err, reserr)
 	}
 
@@ -152,7 +152,7 @@ func TestModifyResponseAggregatesErrors(t *testing.T) {
 	tm2.ResponseError(reserr2)
 	fg.AddResponseModifier(tm2)
 
-	req, err := http.NewRequest("GET", "http://example.com/", nil)
+	req, err := http.NewRequest(http.MethodGet, "http://example.com/", http.NoBody)
 	if err != nil {
 		t.Fatalf("http.NewRequest(): got %v, want no error", err)
 	}
@@ -203,7 +203,7 @@ func TestModifyResponseInlineGroupsAggregateErrors(t *testing.T) {
 		t.Fatalf("inner groups should be inlined")
 	}
 
-	req, err := http.NewRequest("GET", "http://example.com/", nil)
+	req, err := http.NewRequest(http.MethodGet, "http://example.com/", http.NoBody)
 	if err != nil {
 		t.Fatalf("http.NewRequest(): got %v, want no error", err)
 	}
@@ -255,7 +255,7 @@ func TestModifyRequestInlineGroupsAggregateErrors(t *testing.T) {
 		t.Fatalf("inner groups should be inlined")
 	}
 
-	req, err := http.NewRequest("GET", "http://example.com/", nil)
+	req, err := http.NewRequest(http.MethodGet, "http://example.com/", http.NoBody)
 	if err != nil {
 		t.Fatalf("http.NewRequest(): got %v, want no error", err)
 	}