lint: enable iface and revive, drop maintidx

.golangci.yml

@@ -13,6 +13,7 @@ linters:
     - fatcontext # Detects nested contexts in loops and function literals. [auto-fix]
     - ginkgolinter # Enforces standards of using ginkgo and gomega. [auto-fix]
     - goheader # Check if file header matches to pattern. [fast, auto-fix]
+    - iface # Detects incorrect use of interfaces.
     - gosec # Inspects source code for security problems.
     - govet # Vet examines Go source code and reports suspicious constructs. It is roughly the same as 'go vet' and uses its passes. [auto-fix]
     - importas # Enforces consistent import aliases. [auto-fix]
@@ -29,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.
@@ -74,10 +76,20 @@ linters:
       # Enable all analyzers.
       enable-all: false
     iface:
+      # Only "identical" is enabled. The other two checks do not fit this codebase:
+      #
+      # "unused" only looks inside the declaring package, and the driver pattern puts SPI
+      # interfaces in dedicated driver/api packages that are implemented and consumed from
+      # other modules, so they are unused at home by construction. It reported 66 of these;
+      # 61 had between 1 and 79 referencing files elsewhere in the tree. The 5 that had none
+      # were genuinely dead and have been deleted.
+      #
+      # "opaque" wants constructors to return their concrete type, but in 14 of its 17 reports
+      # that type is unexported, so callers could not name the result and revive's
+      # unexported-return rule below would reject it. One is impossible outright: the concrete
+      # type is an unexported generic in fabric-smart-client.
       enable:
         - identical # Identifies interfaces in the same package that have identical method sets.
-        - unused # Identifies interfaces that are not used anywhere in the same package where the interface is defined.
-        - opaque # Identifies functions that return interfaces, but the actual returned value is always a single concrete implementation.
     ireturn:
       # By default, it allows using errors, empty interfaces, anonymous interfaces,
       # and interfaces provided by the standard library.
@@ -115,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/token/generators/crypto/fabtokenv1/gen.go

@@ -103,10 +103,6 @@ func (l *Layout) PeersInOrg(orgName string) []Peer {
 	return peers
 }
 
-type FSCPlatform interface {
-	PeerOrgs() []*node.Organization
-}
-
 type CryptoMaterialGenerator struct {
 	TokenPlatform     generators.TokenPlatform
 	EventuallyTimeout time.Duration

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(),
 	}
 }

integration/nwo/txgen/service/metrics/types.go

@@ -6,20 +6,8 @@ SPDX-License-Identifier: Apache-2.0
 
 package metrics
 
-import (
-	"time"
-
-	c "github.com/LFDT-Panurus/panurus/integration/nwo/txgen/model/constants"
-)
-
 type Reporter interface {
 	GetTotalRequests() string
 	GetActiveRequests() string
 	Summary() string
 }
-
-type Collector interface {
-	IncrementRequests()
-	DecrementRequests()
-	AddDuration(millisDuration time.Duration, requestType c.ApiRequestType, success bool)
-}

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")

token/driver/action.go

@@ -41,6 +41,12 @@ type IssueAction interface {
 }
 
 // Input represents a specific token that is being spent in a transaction.
+//
+// It shares a method set with LedgerToken by coincidence, not by design: an Input is a token
+// being spent by an action, a LedgerToken is a token read back from the ledger. They are
+// mocked separately and evolve independently.
+//
+//nolint:iface // distinct concept from LedgerToken, see above
 type Input interface {
 	// GetOwner returns the cryptographic owner of the token.
 	GetOwner() []byte

token/driver/validator.go

@@ -38,13 +38,6 @@ type SignatureProvider interface {
 	Signatures() [][]byte
 }
 
-//go:generate counterfeiter -o mock/validator.go -fake-name Validator . Validator
-//go:generate counterfeiter -o mock/validator_ledger.go -fake-name ValidatorLedger . ValidatorLedger
-
-type ValidatorLedger interface {
-	GetState(id token.ID) ([]byte, error)
-}
-
 // Validator provides stateless methods for validating token transaction requests.
 // It ensures that requests are well-formed and consistent with the cryptographic,
 // authorization, and policy rules defined by the token driver. Ledger existence

token/driver/vault.go

@@ -139,6 +139,9 @@ type TokenVault interface {
 
 //go:generate counterfeiter -o mock/ledger_token.go -fake-name LedgerToken . LedgerToken
 
+// LedgerToken is a token as read back from the ledger.
+//
+//nolint:iface // See the note on Input: same method set, different concept, separate mocks.
 type LedgerToken interface {
 	GetOwner() []byte
 }

token/services/interop/htlc/wallet.go

@@ -20,10 +20,6 @@ import (
 	"github.com/hyperledger-labs/fabric-smart-client/platform/view/view"
 )
 
-type Vault interface {
-	DeleteTokens(ctx context.Context, toDelete ...*token2.ID) error
-}
-
 type QueryEngine interface {
 	// UnspentTokensIteratorBy returns an iterator over all unspent tokens by type and id. Type can be empty
 	UnspentTokensIteratorBy(ctx context.Context, id string, tokenType token2.Type) (driver.UnspentTokensIterator, error)