chore: Lint tests

.golangci.yml

@@ -1,4 +1,74 @@
 linters-settings:
+  revive:
+    enable-all-rules: true
+    # Do NOT whine about the following, full explanation found in:
+    # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#description-of-available-rules
+    rules:
+      - name: use-any
+        disabled: true
+      - name: if-return
+        disabled: true
+      - name: max-public-structs
+        disabled: true
+      - name: cognitive-complexity
+        disabled: true
+      - name: argument-limit
+        disabled: true
+      - name: cyclomatic
+        disabled: true
+      - name: file-header
+        disabled: true
+      - name: function-length
+        disabled: true
+      - name: function-result-limit
+        disabled: true
+      - name: line-length-limit
+        disabled: true
+      - name: flag-parameter
+        disabled: true
+      - name: add-constant
+        disabled: true
+      - name: empty-lines
+        disabled: true
+      - name: banned-characters
+        disabled: true
+      - name: deep-exit
+        disabled: true
+      - name: confusing-results
+        disabled: true
+      - name: unused-parameter
+        disabled: true
+      - name: modifies-value-receiver
+        disabled: true
+      - name: early-return
+        disabled: true
+      - name: unnecessary-stmt
+        disabled: true
+      - name: import-shadowing
+        disabled: true
+      - name: defer
+        disabled: true
+      - name: confusing-naming
+        disabled: true
+      - name: nested-structs
+        disabled: true
+      - name: unhandled-error
+        disabled: true
+        # Arguments added below do not trigger the rule.
+        #           arguments =["os\.(Create|WriteFile|Chmod)", "fmt\.Print", "myFunction", "net\..*", "bytes\.Buffer\.Write"]
+        arguments:
+          - 'fmt\..*'
+          - 'c.Conn.Close'
+          - 'server.Serve'
+          - 'http.ListenAndServe'
+          - 'conn.Close'
+          - 'os.Stderr.Write'
+          - 'buf\.(WriteString|Write)'
+          - 'csc.Close'
+          - 'cl.Close'
+          - 'out.WriteRune'
+          - 'sb\.(WriteRune|WriteString)'
+          - 'reconn.Conn.Close'
   errcheck:
     ignore: fmt:.*,go.uber.org/zap/zapcore:^Add.*
     ignoretests: true
@@ -27,9 +97,12 @@ linters:
     - gosimple
     - govet
     - ineffassign
+    - nolintlint
     - misspell
     - prealloc
+    - revive
     - staticcheck
+    - thelper
     - typecheck
     - unconvert
     - unused
@@ -51,7 +124,6 @@ linters:
   # - godot
   # - godox
   # - goerr113
-  # - gofumpt
   # - goheader
   # - golint
   # - gomnd
@@ -88,6 +160,8 @@ output:
   print-linter-name: true
 
 issues:
+  max-issues-per-linter: 0
+  max-same-issues: 0
   exclude-rules:
     # we aren't calling unknown URL
     - text: 'G107' # G107: Url provided to HTTP request as taint input

admin.go

@@ -915,7 +915,7 @@ func (h adminHandler) checkOrigin(r *http.Request) (string, error) {
 	return origin.String(), nil
 }
 
-func (h adminHandler) getOrigin(r *http.Request) (string, *url.URL) {
+func (adminHandler) getOrigin(r *http.Request) (string, *url.URL) {
 	origin := r.Header.Get("Origin")
 	if origin == "" {
 		origin = r.Header.Get("Referer")
@@ -961,9 +961,11 @@ func handleConfig(w http.ResponseWriter, r *http.Request) error {
 		// Set the ETag as a trailer header.
 		// The alternative is to write the config to a buffer, and
 		// then hash that.
+
 		w.Header().Set("Trailer", "ETag")
 
 		hash := etagHasher()
+
 		configWriter := io.MultiWriter(w, hash)
 		err := readConfig(r.URL.Path, configWriter)
 		if err != nil {
@@ -1022,7 +1024,7 @@ func handleConfig(w http.ResponseWriter, r *http.Request) error {
 	return nil
 }
 
-func handleConfigID(w http.ResponseWriter, r *http.Request) error {
+func handleConfigID(_ http.ResponseWriter, r *http.Request) error {
 	idPath := r.URL.Path
 
 	parts := strings.Split(idPath, "/")

admin_test.go

@@ -194,6 +194,9 @@ func TestETags(t *testing.T) {
 
 func BenchmarkLoad(b *testing.B) {
 	for i := 0; i < b.N; i++ {
-		Load(testCfg, true)
+		err := Load(testCfg, true)
+		if err != nil {
+			b.Fatal(err)
+		}
 	}
 }

caddy.go

@@ -884,11 +884,11 @@ func Version() (simple, full string) {
 		if CustomVersion != "" {
 			full = CustomVersion
 			simple = CustomVersion
-			return
+			return full, simple
 		}
 		full = "unknown"
 		simple = "unknown"
-		return
+		return full, simple
 	}
 	// find the Caddy module in the dependency list
 	for _, dep := range bi.Deps {
@@ -968,7 +968,7 @@ func Version() (simple, full string) {
 		}
 	}
 
-	return
+	return simple, full
 }
 
 // ActiveContext returns the currently-active context.

caddyconfig/caddyfile/dispenser.go

@@ -236,8 +236,8 @@ func (d *Dispenser) ScalarVal() any {
 	if num, err := strconv.ParseFloat(text, 64); err == nil {
 		return num
 	}
-	if bool, err := strconv.ParseBool(text); err == nil {
-		return bool
+	if boolean, err := strconv.ParseBool(text); err == nil {
+		return boolean
 	}
 	return text
 }

caddyconfig/caddyfile/dispenser_test.go

@@ -305,7 +305,7 @@ func TestDispenser_ArgErr_Err(t *testing.T) {
 		t.Errorf("Expected error message with custom message in it ('foobar'); got '%v'", err)
 	}
 
-	var ErrBarIsFull = errors.New("bar is full")
+	ErrBarIsFull := errors.New("bar is full")
 	bookingError := d.Errf("unable to reserve: %w", ErrBarIsFull)
 	if !errors.Is(bookingError, ErrBarIsFull) {
 		t.Errorf("Errf(): should be able to unwrap the error chain")

caddyconfig/caddyfile/formatter.go

@@ -81,10 +81,10 @@ func Format(input []byte) []byte {
 				space = true
 				nextLine()
 				continue
-			} else {
-				write(ch)
-				continue
 			}
+			write(ch)
+			continue
+
 		}
 
 		if !escaped && ch == '\\' {
@@ -125,13 +125,13 @@ func Format(input []byte) []byte {
 		spacePrior := space
 		space = false
 
-		//////////////////////////////////////////////////////////
+		// ////////////////////////////////////////////////////////
 		// I find it helpful to think of the formatting loop in two
 		// main sections; by the time we reach this point, we
 		// know we are in a "regular" part of the file: we know
 		// the character is not a space, not in a literal segment
 		// like a comment or quoted, it's not escaped, etc.
-		//////////////////////////////////////////////////////////
+		// ////////////////////////////////////////////////////////
 
 		if ch == '#' {
 			comment = true

caddyconfig/caddyfile/formatter_test.go

@@ -375,7 +375,7 @@ block {
 
 		if string(actual) != tc.expect {
 			t.Errorf("\n[TEST %d: %s]\n====== EXPECTED ======\n%s\n====== ACTUAL ======\n%s^^^^^^^^^^^^^^^^^^^^^",
-				i, tc.description, string(tc.expect), string(actual))
+				i, tc.description, tc.expect, string(actual))
 		}
 	}
 }

caddyconfig/caddyfile/lexer_test.go

@@ -468,6 +468,7 @@ EOF same-line-arg
 }
 
 func lexerCompare(t *testing.T, n int, expected, actual []Token) {
+	t.Helper()
 	if len(expected) != len(actual) {
 		t.Fatalf("Test case %d: expected %d token(s) but got %d", n, len(expected), len(actual))
 	}

caddyconfig/caddyfile/parse.go

@@ -446,7 +446,7 @@ func (p *parser) doImport(nesting int) error {
 
 	var (
 		maybeSnippet   bool
-		maybeSnippetId bool
+		maybeSnippetID bool
 		index          int
 	)
 
@@ -472,16 +472,16 @@ func (p *parser) doImport(nesting int) error {
 			}
 
 			if index == 0 && len(token.Text) >= 3 && strings.HasPrefix(token.Text, "(") && strings.HasSuffix(token.Text, ")") {
-				maybeSnippetId = true
+				maybeSnippetID = true
 			}
 		}
 
 		switch token.Text {
 		case "{":
 			nesting++
-			if index == 1 && maybeSnippetId && nesting == 1 {
+			if index == 1 && maybeSnippetID && nesting == 1 {
 				maybeSnippet = true
-				maybeSnippetId = false
+				maybeSnippetID = false
 			}
 		case "}":
 			nesting--

caddyconfig/caddyfile/parse_test.go

@@ -22,7 +22,7 @@ import (
 )
 
 func TestParseVariadic(t *testing.T) {
-	var args = make([]string, 10)
+	args := make([]string, 10)
 	for i, tc := range []struct {
 		input  string
 		result bool
@@ -107,7 +107,6 @@ func TestAllTokens(t *testing.T) {
 	input := []byte("a b c\nd e")
 	expected := []string{"a", "b", "c", "d", "e"}
 	tokens, err := allTokens("TestAllTokens", input)
-
 	if err != nil {
 		t.Fatalf("Expected no error, got %v", err)
 	}
@@ -145,10 +144,11 @@ func TestParseOneAndImport(t *testing.T) {
 			"localhost",
 		}, []int{1}},
 
-		{`localhost:1234
+		{
+			`localhost:1234
 		  dir1 foo bar`, false, []string{
-			"localhost:1234",
-		}, []int{3},
+				"localhost:1234",
+			}, []int{3},
 		},
 
 		{`localhost {
@@ -403,13 +403,13 @@ func TestRecursiveImport(t *testing.T) {
 	err = os.WriteFile(recursiveFile1, []byte(
 		`localhost
 		dir1
-		import recursive_import_test2`), 0644)
+		import recursive_import_test2`), 0o600)
 	if err != nil {
 		t.Fatal(err)
 	}
 	defer os.Remove(recursiveFile1)
 
-	err = os.WriteFile(recursiveFile2, []byte("dir2 1"), 0644)
+	err = os.WriteFile(recursiveFile2, []byte("dir2 1"), 0o600)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -437,7 +437,7 @@ func TestRecursiveImport(t *testing.T) {
 	err = os.WriteFile(recursiveFile1, []byte(
 		`localhost
 		dir1
-		import `+recursiveFile2), 0644)
+		import `+recursiveFile2), 0o600)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -491,7 +491,7 @@ func TestDirectiveImport(t *testing.T) {
 	}
 
 	err = os.WriteFile(directiveFile, []byte(`prop1 1
-	prop2 2`), 0644)
+	prop2 2`), 0o600)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -780,6 +780,7 @@ func TestSnippets(t *testing.T) {
 }
 
 func writeStringToTempFileOrDie(t *testing.T, str string) (pathToFile string) {
+	t.Helper()
 	file, err := os.CreateTemp("", t.Name())
 	if err != nil {
 		panic(err) // get a stack trace so we know where this was called from.

caddyconfig/httpcaddyfile/addresses.go

@@ -150,7 +150,7 @@ func (st *ServerType) mapAddressToServerBlocks(originalServerBlocks []serverBloc
 // entries are deleted from the addrToServerBlocks map. Essentially, each pairing (each
 // association from multiple addresses to multiple server blocks; i.e. each element of
 // the returned slice) becomes a server definition in the output JSON.
-func (st *ServerType) consolidateAddrMappings(addrToServerBlocks map[string][]serverBlock) []sbAddrAssociation {
+func (*ServerType) consolidateAddrMappings(addrToServerBlocks map[string][]serverBlock) []sbAddrAssociation {
 	sbaddrs := make([]sbAddrAssociation, 0, len(addrToServerBlocks))
 	for addr, sblocks := range addrToServerBlocks {
 		// we start with knowing that at least this address
@@ -188,7 +188,7 @@ func (st *ServerType) consolidateAddrMappings(addrToServerBlocks map[string][]se
 
 // listenerAddrsForServerBlockKey essentially converts the Caddyfile
 // site addresses to Caddy listener addresses for each server block.
-func (st *ServerType) listenerAddrsForServerBlockKey(sblock serverBlock, key string,
+func (*ServerType) listenerAddrsForServerBlockKey(sblock serverBlock, key string,
 	options map[string]any,
 ) ([]string, error) {
 	addr, err := ParseAddress(key)

caddyconfig/httpcaddyfile/directives.go

@@ -272,7 +272,7 @@ func (h Helper) GroupRoutes(vals []ConfigValue) {
 
 // NewBindAddresses returns config values relevant to adding
 // listener bind addresses to the config.
-func (h Helper) NewBindAddresses(addrs []string) []ConfigValue {
+func (Helper) NewBindAddresses(addrs []string) []ConfigValue {
 	return []ConfigValue{{Class: "bind", Value: addrs}}
 }
 

caddyconfig/httpcaddyfile/directives_test.go

@@ -31,20 +31,23 @@ func TestHostsFromKeys(t *testing.T) {
 			[]Address{
 				{Original: ":2015", Port: "2015"},
 			},
-			[]string{}, []string{},
+			[]string{},
+			[]string{},
 		},
 		{
 			[]Address{
 				{Original: ":443", Port: "443"},
 			},
-			[]string{}, []string{},
+			[]string{},
+			[]string{},
 		},
 		{
 			[]Address{
 				{Original: "foo", Host: "foo"},
 				{Original: ":2015", Port: "2015"},
 			},
-			[]string{}, []string{"foo"},
+			[]string{},
+			[]string{"foo"},
 		},
 		{
 			[]Address{