lint: enable revive with an explicit rule list, drop maintidx

Second half of #1991. These are the two you and Angelo were asked about and I have gone
ahead and made the call on both, happy to change either.

revive: enabled, but not with enable-all-rules. That reports 3452 issues over 688 files,
which is not something anyone can review in one go. The config now lists 38 rules
explicitly, all of which are already clean tree-wide, so revive lands as a ratchet against
new code rather than as a backlog. Four fixes were needed to get there: one comment
spacing in a zkatdlog test and three in txgen's error.go.

The rules left out are named in a comment in the config with their current counts, so the
next step does not have to re-measure. The big ones are exported (1568), unused-parameter
(362), unused-receiver (334) and import-shadowing (328). Three more are left out because
another enabled linter already covers them, line-length-limit by lll, imports-blocklist by
depguard, file-header by goheader, and six because they are no-ops at their default
settings.

Dropping enable-all-rules also removes the nine "disabled: true" entries, which only
existed to switch rules back off, and argument-limit and function-result-limit, which
were tuned but have 103 and 49 violations and so belong with the deferred set.

maintidx: settings block deleted rather than enabled. It reports 19 functions and every
one of them would have to be suppressed: 12 are in _test.go, 4 are in the shared testutils
and dbtest helpers, 1 is Prove in the CSP range proof, and 2 are the fungible suite entry
point and its topology. The score is driven by Halstead volume, so a long table-driven
test or a flat block of declarative topology wiring scores badly for being long rather
than for being tangled, and table-driven tests are the style AGENTS.md asks for. A linter
that starts at a 100% suppression rate is measuring the wrong thing here. gocognit is
still configured and unenabled, and at min-complexity 15 it targets branching directly,
which is the check actually wanted.

golangci-lint is clean on all nine modules and make checks passes.

That leaves gocognit and wrapcheck as the only settings blocks for linters that are not
enabled.

Signed-off-by: atharrva01 <atharvaborade568@gmail.com>

Author: atharrva01

.golangci.yml

@@ -30,6 +30,7 @@ linters:
     - nolintlint # Reports ill-formed or insufficient nolint directives. [fast, auto-fix]
     - perfsprint # Checks that fmt.Sprintf can be replaced with a faster alternative. [auto-fix]
     - reassign # Checks that package variables are not reassigned.
+    - revive # Fast, configurable, extensible, flexible, and beautiful linter for Go. [auto-fix]
     - rowserrcheck # Checks whether Rows.Err of rows is checked successfully.
     - sloglint # Ensure consistent code style when using log/slog. [auto-fix]
     - spancheck # Checks for mistakes with OpenTelemetry/Census spans.
@@ -126,47 +127,67 @@ linters:
     lll:
       # Max line length, lines longer will be reported.
       line-length: 240
-    maintidx:
-      under: 20
     nolintlint:
       require-specific: true
     revive:
-      enable-all-rules: true
+      # enable-all-rules was never on, because revive itself was never enabled. Turning it on
+      # as-is reports 3452 issues over 688 files, so instead of one unreviewable change this
+      # lists the rules explicitly. Everything below is already clean tree-wide, which makes
+      # revive a ratchet against new code rather than a backlog.
+      #
+      # Deliberately not listed yet, with their current counts, each wants its own change:
+      # exported (1568), unused-parameter (362), unused-receiver (334), import-shadowing (328),
+      # unchecked-type-assertion (125), argument-limit (103), unexported-naming (62),
+      # var-naming (59), defer (59), redundant-import-alias (51), function-result-limit (49).
+      #
+      # Not listed because another enabled linter already covers them: line-length-limit (lll),
+      # imports-blocklist (depguard), file-header (goheader). Omitted as no-ops at their default
+      # settings: comments-density, file-length-limit, enforce-map-style, enforce-slice-style,
+      # enforce-repeated-arg-type-style, string-format.
       rules:
-        - name: argument-limit
-          arguments:
-            - 5
-        - name: line-length-limit
-          arguments:
-            - 240
-        - name: file-header
-          disabled: true
-        - name: package-comments
-          disabled: true
-        - name: max-public-structs
-          disabled: true
-        - name: banned-characters
-          disabled: true
-        - name: cognitive-complexity
-          disabled: true
-        - name: cyclomatic
-          disabled: true
-        - name: function-length
-          disabled: true
-        - name: function-result-limit
+        - name: atomic
+        - name: bool-literal-in-expr
+        - name: comment-spacings
           arguments:
-            - 3
-        - name: add-constant
-          disabled: true
+            - 'nolint:'
+        - name: constant-logical-expr
+        - name: context-keys-type
+        - name: datarace
+        - name: deep-exit
+        - name: duplicated-imports
+        - name: epoch-naming
+        - name: error-return
+        - name: errorf
+        - name: filename-format
+        - name: forbidden-call-in-wg-go
+        - name: get-return
+        - name: identical-ifelseif-conditions
+        - name: identical-switch-conditions
+        - name: inefficient-map-lookup
+        - name: modifies-parameter
+        - name: modifies-value-receiver
+        - name: optimize-operands-order
+        - name: range
+        - name: range-val-address
+        - name: range-val-in-closure
+        - name: redundant-build-tag
+        - name: redundant-test-main-exit
+        - name: string-of-int
+        - name: time-date
+        - name: time-equal
+        - name: time-naming
+        - name: unconditional-recursion
         - name: unhandled-error
           arguments:
             - fmt.Printf
             - fmt.Println
-        - name: confusing-naming
-          disabled: true
-        - name: comment-spacings
-          arguments:
-            - 'nolint:'
+        - name: unnecessary-if
+        - name: unreachable-code
+        - name: unsecure-url-scheme
+        - name: use-any
+        - name: use-errors-new
+        - name: useless-break
+        - name: waitgroup-by-value
     rowserrcheck:
       packages:
         - github.com/jackc/pgx/v5

integration/nwo/txgen/model/api/error.go

@@ -69,7 +69,7 @@ func NewBadRequestError(err error, message string) *AppError {
 		Code:    http.StatusBadRequest,
 		Message: message,
 		Cause:   err,
-		//Location: utils.FileWithLineNum(),
+		// Location: utils.FileWithLineNum(),
 
 	}
 }
@@ -83,7 +83,7 @@ func NewNotFoundError(err error, message string) *AppError {
 		Code:    http.StatusNotFound,
 		Message: message,
 		Cause:   err,
-		//Location: utils.FileWithLineNum(),
+		// Location: utils.FileWithLineNum(),
 	}
 }
 
@@ -96,7 +96,7 @@ func NewInternalServerError(err error, message string) *AppError {
 		Code:    http.StatusInternalServerError,
 		Message: message,
 		Cause:   err,
-		//Location: utils.FileWithLineNum(),
+		// Location: utils.FileWithLineNum(),
 	}
 }
 
@@ -109,7 +109,7 @@ func NewDBError(err error, message string) *AppError {
 		Code:    http.StatusInternalServerError,
 		Message: message,
 		Cause:   err,
-		//Location: utils.FileWithLineNum(),
+		// Location: utils.FileWithLineNum(),
 	}
 }
 
@@ -122,7 +122,7 @@ func NewAuthorizationError(err error, message string) *AppError {
 		Code:    http.StatusUnauthorized,
 		Message: message,
 		Cause:   err,
-		//Location: utils.FileWithLineNum(),
+		// Location: utils.FileWithLineNum(),
 	}
 }
 
@@ -135,6 +135,6 @@ func NewForbiddenError(err error, message string) *AppError {
 		Code:    http.StatusForbidden,
 		Message: message,
 		Cause:   err,
-		//Location: utils.FileWithLineNum(),
+		// Location: utils.FileWithLineNum(),
 	}
 }

token/core/zkatdlog/nogh/v1/validator/validator_security_test.go

@@ -70,7 +70,7 @@ func newSecurityTestEnv(t *testing.T) *testing2.Env {
 // ctx.Ledger.GetState() will cause a nil-function-pointer panic on the chaincode,
 // constituting a denial-of-service vector. The fix adds an explicit nil guard.
 func TestSecurityPanicNilBackendLedger(t *testing.T) {
-	backend := common.NewBackend(logging.MustGetLogger(), nil /*nil ledger*/, nil, nil)
+	backend := common.NewBackend(logging.MustGetLogger(), nil /* nil ledger */, nil, nil)
 
 	_, err := backend.GetState(token2.ID{TxId: "tx1", Index: 0})
 	require.Error(t, err, "GetState with nil ledger must return an error, not panic")