CI: enable error-handling linters and fix a few bugs (#6479)

cce · web-flow · commit f381e2763aec · 2025-10-31T16:36:15.000-04:00
diff --git a/.github/workflows/reviewdog.yml b/.github/workflows/reviewdog.yml
@@ -1,6 +1,6 @@
 name: "ReviewDog workflow"
 env:
-  GOLANGCI_LINT_VERSION: "v2.5.1-0.20251021235302-b99da877a247"
+  GOLANGCI_LINT_VERSION: "v2.6.0"
 on:
   push:
     branches:
diff --git a/.golangci.yml b/.golangci.yml
@@ -5,17 +5,24 @@ run:
 linters:
   default: none
   enable:
+    - asciicheck
+    - bidichk
     - copyloopvar
     - dupword
     - errcheck
     - govet
     - ineffassign
+    - iotamixing
     - misspell
     - nilerr
+    - nilnesserr
     - nolintlint
     - paralleltest
+    - reassign
     - revive
+    #- sqlclosecheck
     - staticcheck
+    - testifylint
     - unused
   
   settings:
@@ -32,13 +39,16 @@ linters:
     govet:
       # Enables these linters in addition to the default ones.
       enable:
+        - nilness
+        - reflectvaluecompare
         - shadow
+        - sortslice
+        - waitgroup
       disable:
         - buildtag
       settings:
         printf:
           # Comma-separated list of print function names to check (in addition to default, see `go tool vet help printf`).
-          # Default: []
           funcs:
             - (github.com/algorand/go-algorand/logging.Logger).Debugf
             - (github.com/algorand/go-algorand/logging.Logger).Infof
@@ -87,6 +97,10 @@ linters:
         - '-ST1005' # ignore "error strings should not be capitalized"
         - '-QF1001' # ignore De Morgan's law suggestions
         - '-QF1003' # ignore suggestions to replace if/else chain with switch
+    testifylint:
+      enable:
+        - error-is-as
+      disable-all: true
 
   exclusions:
     generated: lax
@@ -132,15 +146,6 @@ linters:
         text: '^empty-block: this block is empty, you can remove it'
       - linters: revive
         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
diff --git a/agreement/fuzzer/networkFacade_test.go b/agreement/fuzzer/networkFacade_test.go
@@ -332,7 +332,7 @@ func (n *NetworkFacade) pushPendingReceivedMessage() bool {
 	case network.Broadcast:
 		n.broadcast(storedMsg.tag, storedMsg.data, -1, "NetworkFacade service-%v Broadcast-Action %v %v\n")
 	default:
-		panic(nil) // not handled; agreement doesn't currently use this one.
+		panic(fmt.Sprintf("unhandled network action %v", outMsg.Action))
 	}
 
 	if n.debugMessages {
diff --git a/agreement/pseudonode_test.go b/agreement/pseudonode_test.go
@@ -19,7 +19,6 @@ package agreement
 import (
 	"context"
 	"crypto/sha256"
-	"errors"
 	"fmt"
 	"strings"
 	"testing"
@@ -520,21 +519,21 @@ func TestPseudonodeNonEnqueuedTasks(t *testing.T) {
 	for i := 0; i < pseudonodeVerificationBacklog*2; i++ {
 		ch, err = pb.MakeProposals(context.Background(), startRound, period(i))
 		if err != nil {
-			require.ErrorAs(t, errPseudonodeBacklogFull, &err)
+			require.ErrorIs(t, err, errPseudonodeBacklogFull)
 			break
 		}
 		channels = append(channels, ch)
 	}
 	enqueuedProposals := len(channels)
 	require.Error(t, err, "MakeProposals did not returned an error when being overflowed with requests")
-	require.True(t, errors.Is(err, errPseudonodeBacklogFull))
+	require.ErrorIs(t, err, errPseudonodeBacklogFull)
 
 	persist := make(chan error)
 	close(persist)
 	for i := 0; i < pseudonodeVerificationBacklog*2; i++ {
 		ch, err = pb.MakeVotes(context.Background(), startRound, period(i), step(i%5), makeProposalValue(period(i), accounts[0].Address()), persist)
 		if err != nil {
-			require.ErrorAs(t, errPseudonodeBacklogFull, &err)
+			require.ErrorIs(t, err, errPseudonodeBacklogFull)
 			break
 		}
 		channels = append(channels, ch)
diff --git a/catchup/universalFetcher_test.go b/catchup/universalFetcher_test.go
@@ -74,7 +74,8 @@ func TestUGetBlockWs(t *testing.T) {
 	block, cert, duration, err = fetcher.fetchBlock(context.Background(), next+1, up)
 
 	require.Error(t, err)
-	require.Error(t, noBlockForRoundError{}, err)
+	var noBlockErr noBlockForRoundError
+	require.ErrorAs(t, err, &noBlockErr)
 	require.Equal(t, next+1, err.(noBlockForRoundError).round)
 	require.Equal(t, next, err.(noBlockForRoundError).latest)
 	require.Nil(t, block)
@@ -120,7 +121,8 @@ func TestUGetBlockHTTP(t *testing.T) {
 
 	block, cert, duration, err = fetcher.fetchBlock(context.Background(), next+1, net.GetPeers()[0])
 
-	require.Error(t, noBlockForRoundError{}, err)
+	var noBlockErr noBlockForRoundError
+	require.ErrorAs(t, err, &noBlockErr)
 	require.Equal(t, next+1, err.(noBlockForRoundError).round)
 	require.Equal(t, next, err.(noBlockForRoundError).latest)
 	require.Contains(t, err.Error(), "no block available for given round")
diff --git a/cmd/algorelay/relayCmd.go b/cmd/algorelay/relayCmd.go
@@ -524,9 +524,6 @@ func ensureRelayStatus(checkOnly bool, relay eb.Relay, nameDomain string, srvDom
 // Returns an array of names starting with the target ip/name and ending with the outermost reference
 func getTargetDNSChain(nameEntries map[string]string, target string) (names []string, err error) {
 	target = strings.ToLower(target)
-	if err != nil {
-		return
-	}
 
 	names = append(names, target)
 	for {
diff --git a/cmd/dispenser/server.go b/cmd/dispenser/server.go
@@ -136,7 +136,7 @@ func dispense(w http.ResponseWriter, r *http.Request) {
 	targets := r.Form["target"]
 	if len(targets) != 1 {
 		log.Printf("Corrupted target argument\n")
-		http.Error(w, err.Error(), http.StatusInternalServerError)
+		http.Error(w, "Corrupted target argument", http.StatusBadRequest)
 		return
 	}
 
diff --git a/cmd/tealdbg/local.go b/cmd/tealdbg/local.go
@@ -90,12 +90,8 @@ func txnGroupFromParams(dp *DebugParams) (txnGroup []transactions.SignedTxn, err
 
 	// if conversion failed report all intermediate decoding errors
 	if err != nil {
-		if err1 != nil {
-			log.Printf("Decoding as JSON txn failed: %s", err1.Error())
-		}
-		if err2 != nil {
-			log.Printf("Decoding as JSON txn group failed: %s", err2.Error())
-		}
+		log.Printf("Decoding as JSON txn failed: %v", err1)
+		log.Printf("Decoding as JSON txn group failed: %v", err2)
 	}
 
 	return
@@ -141,12 +137,8 @@ func balanceRecordsFromParams(dp *DebugParams) (records []basics.BalanceRecord,
 
 	// if conversion failed report all intermediate decoding errors
 	if err != nil {
-		if err1 != nil {
-			log.Printf("Decoding as JSON record failed: %s", err1.Error())
-		}
-		if err2 != nil {
-			log.Printf("Decoding as JSON array of records failed: %s", err2.Error())
-		}
+		log.Printf("Decoding as JSON record failed: %v", err1)
+		log.Printf("Decoding as JSON array of records failed: %v", err2)
 	}
 
 	return
diff --git a/crypto/merklesignature/merkleSignatureScheme_test.go b/crypto/merklesignature/merkleSignatureScheme_test.go
@@ -18,7 +18,6 @@ package merklesignature
 
 import (
 	"crypto/rand"
-	"errors"
 	"math"
 	"testing"
 
@@ -329,7 +328,7 @@ func TestBadRound(t *testing.T) {
 	err = signer.GetVerifier().VerifyBytes(start+2, msg, &sig)
 	a.Error(err)
 	a.ErrorIs(err, ErrSignatureSchemeVerificationFailed)
-	a.True(errors.Is(err, ErrSignatureSchemeVerificationFailed))
+	a.ErrorIs(err, ErrSignatureSchemeVerificationFailed)
 }
 
 func TestBadMerkleProofInSignature(t *testing.T) {
diff --git a/daemon/algod/api/server/common/test/handlers_test.go b/daemon/algod/api/server/common/test/handlers_test.go
@@ -17,7 +17,6 @@
 package test
 
 import (
-	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -34,11 +33,11 @@ import (
 
 func mockNodeStatusInRangeHelper(
 	t *testing.T, statusCode MockNodeCatchupStatus,
-	expectedErr error, expectedStatus node.StatusReport) {
+	expectedErr string, expectedStatus node.StatusReport) {
 	mockNodeInstance := makeMockNode(statusCode)
 	status, err := mockNodeInstance.Status()
-	if expectedErr != nil {
-		require.Error(t, err, expectedErr)
+	if expectedErr != "" {
+		require.EqualError(t, err, expectedErr)
 	} else {
 		require.Equal(t, expectedStatus, status)
 	}
@@ -48,13 +47,13 @@ func TestMockNodeStatus(t *testing.T) {
 	partitiontest.PartitionTest(t)
 
 	mockNodeStatusInRangeHelper(
-		t, CaughtUpAndReady, nil, cannedStatusReportCaughtUpAndReadyGolden)
+		t, CaughtUpAndReady, "", cannedStatusReportCaughtUpAndReadyGolden)
 	mockNodeStatusInRangeHelper(
-		t, CatchingUpFast, nil, cannedStatusReportCatchingUpFastGolden)
+		t, CatchingUpFast, "", cannedStatusReportCatchingUpFastGolden)
 	mockNodeStatusInRangeHelper(
-		t, StoppedAtUnsupported, nil, cannedStatusReportStoppedAtUnsupportedGolden)
+		t, StoppedAtUnsupported, "", cannedStatusReportStoppedAtUnsupportedGolden)
 	mockNodeStatusInRangeHelper(
-		t, 399, fmt.Errorf("catchup status out of scope error"), node.StatusReport{})
+		t, 399, "catchup status out of scope error", node.StatusReport{})
 }
 
 func readyEndpointTestHelper(
diff --git a/data/txHandler_test.go b/data/txHandler_test.go
@@ -2345,13 +2345,13 @@ func TestMakeTxHandlerErrors(t *testing.T) {
 		nil, nil, nil, &mocks.MockNetwork{}, config.Local{},
 	}
 	_, err := MakeTxHandler(opts)
-	require.Error(t, err, ErrInvalidTxPool)
+	require.ErrorIs(t, err, ErrInvalidTxPool)
 
 	opts = TxHandlerOpts{
 		&pools.TransactionPool{}, nil, nil, &mocks.MockNetwork{}, config.Local{},
 	}
 	_, err = MakeTxHandler(opts)
-	require.Error(t, err, ErrInvalidLedger)
+	require.ErrorIs(t, err, ErrInvalidLedger)
 
 	// it is not possible to test MakeStreamVerifier returning an error, because it is not possible to
 	// get the ledger to return an error for returining the header of its latest round
diff --git a/ledger/catchupaccessor.go b/ledger/catchupaccessor.go
@@ -253,11 +253,11 @@ const (
 	CatchpointCatchupStateBlocksDownload
 	// CatchpointCatchupStateSwitch indicates that we're switching to use the downloaded ledger/blocks content
 	CatchpointCatchupStateSwitch
-
-	// catchpointCatchupStateLast is the last entry in the CatchpointCatchupState enumeration.
-	catchpointCatchupStateLast = CatchpointCatchupStateSwitch
 )
 
+// catchpointCatchupStateLast is the last entry in the CatchpointCatchupState enumeration.
+const catchpointCatchupStateLast = CatchpointCatchupStateSwitch
+
 // CatchupAccessorClientLedger represents ledger interface needed for catchpoint accessor clients
 type CatchupAccessorClientLedger interface {
 	BlockCert(rnd basics.Round) (blk bookkeeping.Block, cert agreement.Certificate, err error)
diff --git a/ledger/eval/prefetcher/prefetcher_test.go b/ledger/eval/prefetcher/prefetcher_test.go
@@ -18,7 +18,6 @@ package prefetcher_test
 
 import (
 	"context"
-	"errors"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -637,7 +636,7 @@ func TestAssetLookupError(t *testing.T) {
 		if loadedTxnGroup.Err != nil {
 			errorReceived = true
 			require.Equal(t, int64(2), loadedTxnGroup.Err.GroupIdx)
-			require.True(t, errors.Is(loadedTxnGroup.Err, assetLookupError{}))
+			require.ErrorIs(t, loadedTxnGroup.Err, assetLookupError{})
 			require.Equal(t, makeAddress(2), *loadedTxnGroup.Err.Address)
 			require.Equal(t, errorTriggerAssetIndex, int(loadedTxnGroup.Err.CreatableIndex))
 			require.Equal(t, basics.AssetCreatable, loadedTxnGroup.Err.CreatableType)
@@ -693,7 +692,7 @@ func TestGetCreatorForRoundError(t *testing.T) {
 		receivedNumGroups++
 		if loadedTxnGroup.Err != nil {
 			errorReceived = true
-			require.True(t, errors.Is(loadedTxnGroup.Err, getCreatorError{}))
+			require.ErrorIs(t, loadedTxnGroup.Err, getCreatorError{})
 			require.Nil(t, loadedTxnGroup.Err.Address)
 			require.Equal(t, errorTriggerCreatableIndex, int(loadedTxnGroup.Err.CreatableIndex))
 			require.Equal(t, basics.AssetCreatable, loadedTxnGroup.Err.CreatableType)
@@ -750,7 +749,7 @@ func TestLookupWithoutRewards(t *testing.T) {
 		receivedNumGroups++
 		if loadedTxnGroup.Err != nil {
 			errorReceived = true
-			require.True(t, errors.Is(loadedTxnGroup.Err, lookupError{}))
+			require.ErrorIs(t, loadedTxnGroup.Err, lookupError{})
 			require.Equal(t, makeAddress(10), *loadedTxnGroup.Err.Address)
 			require.Equal(t, 0, int(loadedTxnGroup.Err.CreatableIndex))
 			require.Equal(t, basics.AssetCreatable, loadedTxnGroup.Err.CreatableType)
diff --git a/ledger/ledger_test.go b/ledger/ledger_test.go
@@ -19,7 +19,6 @@ package ledger
 import (
 	"bytes"
 	"context"
-	"errors"
 	"fmt"
 	"math/rand"
 	"os"
@@ -1761,7 +1760,7 @@ func TestLedgerVerifiesOldStateProofs(t *testing.T) {
 	_, err = l.BlockHdr(basics.Round(proto.StateProofInterval))
 	require.Error(t, err)
 	expectedErr := &ledgercore.ErrNoEntry{}
-	require.True(t, errors.As(err, expectedErr), fmt.Sprintf("got error %s", err))
+	require.ErrorAs(t, err, expectedErr, fmt.Sprintf("got error %s", err))
 
 	l.acctsOnline.voters.votersMu.Lock()
 	for k := range l.acctsOnline.voters.votersForRoundCache {
@@ -2294,7 +2293,7 @@ func resetAccountDBToV6(t *testing.T, l *Ledger) {
 			return err0
 		}
 
-		if err0 := tx.Testing().AccountsUpdateSchemaTest(ctx); err != nil {
+		if err0 := tx.Testing().AccountsUpdateSchemaTest(ctx); err0 != nil {
 			return err0
 		}
 
diff --git a/ledger/store/trackerdb/sqlitedriver/sql_test.go b/ledger/store/trackerdb/sqlitedriver/sql_test.go
@@ -19,7 +19,6 @@ package sqlitedriver
 import (
 	"context"
 	"database/sql"
-	"errors"
 	"testing"
 
 	"github.com/algorand/go-algorand/data/basics"
@@ -102,7 +101,7 @@ func TestWrapIOError(t *testing.T) {
 	require.ErrorAs(t, wrapIOError(err), &trackerIOErr)
 
 	err = sqlite3.Error{Code: sqlite3.ErrSchema}
-	require.False(t, errors.As(wrapIOError(err), &trackerIOErr))
+	require.NotErrorAs(t, wrapIOError(err), &trackerIOErr)
 
 	// confirm that double wrapping only applies once
 	err = sqlite3.Error{Code: sqlite3.ErrIoErr}
diff --git a/ledger/txtail_test.go b/ledger/txtail_test.go
@@ -87,7 +87,7 @@ func TestTxTailCheckdup(t *testing.T) {
 		require.Errorf(t, err, "round %d", rnd)
 		if rnd < lastRound-lookback-txvalidity-1 {
 			var missingRoundErr *errTxTailMissingRound
-			require.Truef(t, errors.As(err, &missingRoundErr), "error a errTxTailMissingRound(%d) : %v ", rnd, err)
+			require.ErrorAsf(t, err, &missingRoundErr, "error a errTxTailMissingRound(%d) : %v ", rnd, err)
 		} else {
 			var txInLedgerErr *ledgercore.TransactionInLedgerError
 			require.Truef(t, errors.As(err, &txInLedgerErr), "error a TransactionInLedgerError(%d) : %v ", rnd, err)
@@ -101,7 +101,7 @@ func TestTxTailCheckdup(t *testing.T) {
 		require.Errorf(t, err, "round %d", rnd)
 		if rnd < lastRound-lookback-1 {
 			var missingRoundErr *errTxTailMissingRound
-			require.Truef(t, errors.As(err, &missingRoundErr), "error a errTxTailMissingRound(%d) : %v ", rnd, err)
+			require.ErrorAsf(t, err, &missingRoundErr, "error a errTxTailMissingRound(%d) : %v ", rnd, err)
 		} else {
 			var leaseInLedgerErr *ledgercore.LeaseInLedgerError
 			require.Truef(t, errors.As(err, &leaseInLedgerErr), "error a LeaseInLedgerError(%d) : %v ", rnd, err)
diff --git a/netdeploy/remote/nodecfg/nodeConfigurator.go b/netdeploy/remote/nodecfg/nodeConfigurator.go
@@ -123,10 +123,6 @@ func (nc *nodeConfigurator) apply(rootConfigDir, rootNodeDir string) (err error)
 
 func (nc *nodeConfigurator) prepareNodeDirs(configs []remote.NodeConfig, rootConfigDir, rootNodeDir string) (nodeDirs []nodeDir, err error) {
 	rootHostDir := filepath.Join(rootConfigDir, "hosts", nc.config.Name)
-	if err != nil {
-		err = fmt.Errorf("error loading genesis data: %v", err)
-		return
-	}
 	genesisDir := nc.genesisData.ID()
 
 	// Importing root keys is complicated - just use goal's support for it
diff --git a/network/p2pNetwork.go b/network/p2pNetwork.go
@@ -347,8 +347,8 @@ func NewP2PNetwork(log logging.Logger, cfg config.Local, datadir string, phonebo
 	if cfg.EnableDHTProviders {
 		disc, err0 := p2p.MakeCapabilitiesDiscovery(net.ctx, cfg, h, net.genesisInfo.NetworkID, net.log, bootstrapper.BootstrapFunc)
 		if err0 != nil {
-			log.Errorf("Failed to create dht node capabilities discovery: %v", err)
-			return nil, err
+			log.Errorf("Failed to create dht node capabilities discovery: %v", err0)
+			return nil, err0
 		}
 		net.capabilitiesDiscovery = disc
 	}
diff --git a/scripts/buildtools/versions b/scripts/buildtools/versions
diff --git a/test/e2e-go/features/catchup/catchpointCatchup_test.go b/test/e2e-go/features/catchup/catchpointCatchup_test.go
diff --git a/test/e2e-go/features/participation/overlappingParticipationKeys_test.go b/test/e2e-go/features/participation/overlappingParticipationKeys_test.go
diff --git a/test/linttest/lintissues.go b/test/linttest/lintissues.go
diff --git a/test/netperf-go/puppeteer/puppeteer.go b/test/netperf-go/puppeteer/puppeteer.go
diff --git a/util/db/dbutil_test.go b/util/db/dbutil_test.go

Original file line number	Diff line number	Diff line change
`@@ -332,7 +332,7 @@ func (n *NetworkFacade) pushPendingReceivedMessage() bool {`
`332`	`332`	`case network.Broadcast:`
`333`	`333`	`n.broadcast(storedMsg.tag, storedMsg.data, -1, "NetworkFacade service-%v Broadcast-Action %v %v\n")`
`334`	`334`	`default:`
`335`		`- panic(nil) // not handled; agreement doesn't currently use this one.`
	`335`	`+ panic(fmt.Sprintf("unhandled network action %v", outMsg.Action))`
`336`	`336`	`}`
`337`	`337`
`338`	`338`	`if n.debugMessages {`
Original file line number	Diff line number	Diff line change
`@@ -136,7 +136,7 @@ func dispense(w http.ResponseWriter, r *http.Request) {`
`136`	`136`	`targets := r.Form["target"]`
`137`	`137`	`if len(targets) != 1 {`
`138`	`138`	`log.Printf("Corrupted target argument\n")`
`139`		`- http.Error(w, err.Error(), http.StatusInternalServerError)`
	`139`	`+ http.Error(w, "Corrupted target argument", http.StatusBadRequest)`
`140`	`140`	`return`
`141`	`141`	`}`
`142`	`142`