clean up linter exclusions list

Author: cce

.golangci.yml

@@ -1,6 +1,7 @@
 version: "2"
 run:
   tests: true
+
 linters:
   default: none
   enable:
@@ -15,6 +16,7 @@ linters:
     - revive
     - staticcheck
     - unused
+  
   settings:
     errcheck:
       exclude-functions:
@@ -66,24 +68,10 @@ linters:
       require-specific: true
       # require comments like "//nolint:errcheck // Explanation of why we are ignoring linter here..."
       require-explanation: true
+
   exclusions:
     generated: lax
     rules:
-      - linters:
-          - unused
-        path: cmd/algofix/
-      - linters:
-          - unused
-        path: cmd/algocfg/
-      - linters:
-          - unused
-        path: cmd/catchpointdump/
-      - linters:
-          - unused
-        path: tools/
-      - linters:
-          - unused
-        path: daemon/kmd/lib/kmdapi/
       - linters:
           - errcheck
           - ineffassign
@@ -94,98 +82,72 @@ linters:
         path: _test\.go
       - linters:
           - staticcheck
-        path: _test\.go
-        text: 'SA4006: this value'
-      - linters:
-          - staticcheck
-        text: '(SA3001|SA1019):'
+        text: 'SA3001:' # we assign to b.N in several benchmarks
       - linters:
           - revive
         path: _test\.go
-        text: 'dot-imports: should not use dot imports'
+        text: 'dot-imports: should not use dot imports' # dot imports OK for tests
       - linters:
           - staticcheck
-        text: 'SA1019: rand*'
-      # QF1008: suggests removing embedded field names from selectors (e.g., txn.SignedTxn.Txn -> txn.Txn)
-      # We prefer explicit embedded field access for clarity with transaction types and other embedded structs
+        text: 'SA1019: rand*' # XXX should fix, in Go 1.24 rand.Seed() is a no-op
       - linters:
           - staticcheck
-        text: 'QF1008:'
-      # ST1005: error strings should not be capitalized
+        text: 'QF1008:' # ignore suggestions to remove embedded fields (e.g., txn.SignedTxn.Txn -> txn.Txn)
       - linters:
           - staticcheck
-        text: 'ST1005:'
-      # QF1001: De Morgan's law suggestions
+        text: 'ST1005:' # ignore "error strings should not be capitalized"
       - linters:
           - staticcheck
-        text: 'QF1001:'
-      # QF1003: tagged switch suggestions
+        text: 'QF1001:' # ignore De Morgan's law suggestions
       - linters:
           - staticcheck
-        text: 'QF1003:'
-      # revive: util is an acceptable package name in this codebase
+        text: 'QF1003:' # ignore suggestions to replace if/else chain with switch
       - linters:
           - revive
         path: util/
-        text: 'var-naming: avoid meaningless package names'
-      # allow shadowing in test code
+        text: 'var-naming: avoid meaningless package names' # util package is OK
       - linters:
           - govet
         path: _test\.go
-        text: shadows declaration at line
-      # Ignore missing parallel tests in existing packages
+        text: shadows declaration at line # allow shadowing in test code
       - linters:
-          - paralleltest
+          - paralleltest # Ignore missing t.Parallel calls in the following packages
         path: ^(agreement|catchup|config|crypto|daemon|data|gen|ledger|logging|network|node|protocol|rpcs|stateproof|test|tools|util).*_test\.go
-      # Add all linters here -- Comment this block out for testing linters
       - linters:
-          - errcheck
-          - govet
-          - ineffassign
-          - misspell
-          - revive
-          - unused
-        path: test/linttest/lintissues\.go
-      # ignore govet false positive fixed in https://github.com/golang/go/issues/45043
-      - path: (.+)\.go$
-        text: 'sigchanyzer: misuse of unbuffered os.Signal channel as argument to signal.Notify'
-      # ignore golint false positive fixed in https://github.com/golang/lint/pull/487
-      - path: (.+)\.go$
-        text: exported method (.*).Unwrap` should have comment or be unexported
-      # ignore issues about the way we use _struct fields to define encoding settings
-      - path: (.+)\.go$
-        text: 'field _struct is unused'
-        linters:
           - unused
+        text: 'field _struct is unused' # we use _struct field tags for msgp/json encoding settings
+
       # Enable some golangci-lint default exception rules:
       # "EXC0001 errcheck: Almost all programs ignore errors on these functions and in most cases it's ok"
       - path: (.+)\.go$
         text: Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked
-      # "EXC0005 staticcheck: Developers tend to write in C-style with an explicit 'break' in a 'switch', so it's ok to ignore"
-      - path: (.+)\.go$
-        text: ineffective break statement. Did you mean to break out of the outer loop
-      # revive: irrelevant error about naming
-      - path: (.+)\.go$
-        text: '^var-naming: don''t use leading k in Go names'
       # revive: ignore unused-paramter, package-comments, unexported-return, redefines-builtin-id, var-declaration, empty-block, superfluous-else
       - path: (.+)\.go$
         text: '^unused-parameter: parameter'
       - path: (.+)\.go$
         text: '^package-comments: should have a package comment'
       - path: (.+)\.go$
-        text: '^unexported-return: '
+        text: '^unexported-return: exported func .* returns unexported type .*, which can be annoying'
       - path: (.+)\.go$
-        text: '^redefines-builtin-id: redefinition of'
+        text: '^redefines-builtin-id: redefinition of the built-in (func|type)'
       - path: (.+)\.go$
-        text: '^var-declaration: should'
+        text: '^var-declaration: should omit type .* from declaration of var .*; it will be inferred from the right-hand side'
+      - path: (.+)\.go$
+        text: '^var-declaration: should drop .* from declaration of var .*; it is the zero value'
       - path: (.+)\.go$
         text: '^empty-block: this block is empty, you can remove it'
       - path: (.+)\.go$
-        text: '^superfluous-else: if block ends with'
-    paths:
-      - third_party$
-      - builtin$
-      - examples$
+        text: '^superfluous-else: if block ends with .* so drop this else and outdent its block'
+      # Comment this block out for testing linters
+      - linters:
+          - errcheck
+          - govet
+          - ineffassign
+          - misspell
+          - revive
+          - unused
+        path: test/linttest/lintissues\.go
+
 issues:
   # Work our way back over time to be clean against all these
   # checkers.  If you'd like to contribute, raise the number after ~,
@@ -199,13 +161,11 @@ issues:
   max-same-issues: 0
 severity:
   default: error
+
 formatters:
   enable:
     - gofmt
   exclusions:
     generated: lax
     paths:
-      - third_party$
-      - builtin$
-      - examples$
       - test/linttest/lintissues\.go

tools/debug/transplanter/main.go

@@ -14,6 +14,7 @@
 // You should have received a copy of the GNU Affero General Public License
 // along with go-algorand.  If not, see <https://www.gnu.org/licenses/>.
 
+//nolint:unused // old debug program
 package main
 
 import (

util/cmdUtils.go

@@ -42,7 +42,6 @@ func RunFuncWithSpinningCursor(asyncFunc func()) {
 		case <-doneChan:
 			finished = true
 			ticker.Stop()
-			break
 		case <-ticker.C:
 			fmt.Print(progressStrings[i])
 			fmt.Print("\b")