Enable ineffassign linter and resolve its errors (algorand#2574)

The [ineffassign](https://github.com/gordonklaus/ineffassign) linter is in the default set of linters enabled by golangci-lint but wasn't added in algorand#2523 because it was reporting issues. It's also one of the least objectionable linters; ineffectual assignments are rarely intentional and can point out bugs related to assumptions that a variable assignment had some effect. There were various fixes here but I'm not sure if all the changes to remove or fix ineffectual assignments were the right ones.
danyreyna · Jul 22, 2021 · 07d3c1a · 07d3c1a
1 parent 58fd804
commit 07d3c1a
Show file tree

Hide file tree

Showing 16 changed files with 40 additions and 34 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -7,12 +7,12 @@ linters:
     - golint
     - misspell
     - govet
+    - ineffassign
 
   disable:
     - deadcode
     - errcheck
     - gosimple
-    - ineffassign
     - staticcheck
     - structcheck
     - unused
@@ -26,6 +26,12 @@ issues:
   # don't use default exclude rules listed in `golangci-lint run --help`
   exclude-use-default: false
 
+  # Maximum issues count per one linter. Set to 0 to disable. Default is 50.
+  max-issues-per-linter: 0
+
+  # Maximum count of issues with the same text. Set to 0 to disable. Default is 3.
+  max-same-issues: 0
+
   exclude-rules:
     # ignore govet false positive fixed in https://github.com/golang/go/issues/45043
     - linters:

diff --git a/agreement/proposal.go b/agreement/proposal.go
@@ -197,13 +197,13 @@ func verifyNewSeed(p unauthenticatedProposal, ledger LedgerReader) error {
 
 	if value.OriginalPeriod == 0 {
 		verifier := proposerRecord.SelectionID
-		ok, vrfOut := verifier.Verify(p.SeedProof, prevSeed)
+		ok, _ := verifier.Verify(p.SeedProof, prevSeed) // ignoring VrfOutput returned by Verify
 		if !ok {
 			return fmt.Errorf("payload seed proof malformed (%v, %v)", prevSeed, p.SeedProof)
 		}
 		// TODO remove the following Hash() call,
 		// redundant with the Verify() call above.
-		vrfOut, ok = p.SeedProof.Hash()
+		vrfOut, ok := p.SeedProof.Hash()
 		if !ok {
 			// If proof2hash fails on a proof we produced with VRF Prove, this indicates our VRF code has a dangerous bug.
 			// Panicking is the only safe thing to do.

diff --git a/catchup/service.go b/catchup/service.go
@@ -307,7 +307,8 @@ func (s *Service) fetchAndWrite(r basics.Round, prevFetchCompleteChan chan bool,
 				}
 
 				if s.cfg.CatchupVerifyTransactionSignatures() || s.cfg.CatchupVerifyApplyData() {
-					vb, err := s.ledger.Validate(s.ctx, *block, s.blockValidationPool)
+					var vb *ledger.ValidatedBlock
+					vb, err = s.ledger.Validate(s.ctx, *block, s.blockValidationPool)
 					if err != nil {
 						if s.ctx.Err() != nil {
 							// if the context expired, just exit.

diff --git a/cmd/goal/account.go b/cmd/goal/account.go
@@ -31,7 +31,7 @@ import (
 	"github.com/algorand/go-algorand/config"
 	"github.com/algorand/go-algorand/crypto"
 	"github.com/algorand/go-algorand/crypto/passphrase"
-	"github.com/algorand/go-algorand/daemon/algod/api/spec/v1"
+	v1 "github.com/algorand/go-algorand/daemon/algod/api/spec/v1"
 	algodAcct "github.com/algorand/go-algorand/data/account"
 	"github.com/algorand/go-algorand/data/basics"
 	"github.com/algorand/go-algorand/data/transactions"
@@ -1195,7 +1195,6 @@ var importRootKeysCmd = &cobra.Command{
 			handle, err = db.MakeErasableAccessor(filepath.Join(keyDir, filename))
 			if err != nil {
 				// Couldn't open it, skip it
-				err = nil
 				continue
 			}
 
@@ -1204,7 +1203,6 @@ var importRootKeysCmd = &cobra.Command{
 			handle.Close()
 			if err != nil {
 				// Couldn't read it, skip it
-				err = nil
 				continue
 			}
 

diff --git a/cmd/tealdbg/cdtState.go b/cmd/tealdbg/cdtState.go
@@ -368,7 +368,7 @@ func prepareTxn(txn *transactions.Transaction, groupIndex int) []fieldDesc {
 			continue
 		}
 		var value string
-		var valType string = "string"
+		var valType string
 		tv, err := logic.TxnFieldToTealValue(txn, groupIndex, logic.TxnField(field), 0)
 		if err != nil {
 			value = err.Error()

diff --git a/cmd/tealdbg/localLedger.go b/cmd/tealdbg/localLedger.go
@@ -187,10 +187,13 @@ func getAppCreatorFromIndexer(indexerURL string, indexerToken string, app basics
 	queryString := fmt.Sprintf("%s/v2/applications/%d", indexerURL, app)
 	client := &http.Client{}
 	request, err := http.NewRequest("GET", queryString, nil)
+	if err != nil {
+		return basics.Address{}, fmt.Errorf("application request error: %w", err)
+	}
 	request.Header.Set("X-Indexer-API-Token", indexerToken)
 	resp, err := client.Do(request)
 	if err != nil {
-		return basics.Address{}, fmt.Errorf("application request error: %s", err)
+		return basics.Address{}, fmt.Errorf("application request error: %w", err)
 	}
 	defer resp.Body.Close()
 	if resp.StatusCode != 200 {
@@ -200,13 +203,13 @@ func getAppCreatorFromIndexer(indexerURL string, indexerToken string, app basics
 	var appResp ApplicationIndexerResponse
 	err = json.NewDecoder(resp.Body).Decode(&appResp)
 	if err != nil {
-		return basics.Address{}, fmt.Errorf("application response decode error: %s", err)
+		return basics.Address{}, fmt.Errorf("application response decode error: %w", err)
 	}
 
 	creator, err := basics.UnmarshalChecksumAddress(appResp.Application.Params.Creator)
 
 	if err != nil {
-		return basics.Address{}, fmt.Errorf("UnmarshalChecksumAddress error: %s", err)
+		return basics.Address{}, fmt.Errorf("UnmarshalChecksumAddress error: %w", err)
 	}
 	return creator, nil
 }
@@ -215,10 +218,13 @@ func getBalanceFromIndexer(indexerURL string, indexerToken string, account basic
 	queryString := fmt.Sprintf("%s/v2/accounts/%s?round=%d", indexerURL, account, round)
 	client := &http.Client{}
 	request, err := http.NewRequest("GET", queryString, nil)
+	if err != nil {
+		return basics.AccountData{}, fmt.Errorf("account request error: %w", err)
+	}
 	request.Header.Set("X-Indexer-API-Token", indexerToken)
 	resp, err := client.Do(request)
 	if err != nil {
-		return basics.AccountData{}, fmt.Errorf("account request error: %s", err)
+		return basics.AccountData{}, fmt.Errorf("account request error: %w", err)
 	}
 	defer resp.Body.Close()
 	if resp.StatusCode != 200 {
@@ -228,11 +234,11 @@ func getBalanceFromIndexer(indexerURL string, indexerToken string, account basic
 	var accountResp AccountIndexerResponse
 	err = json.NewDecoder(resp.Body).Decode(&accountResp)
 	if err != nil {
-		return basics.AccountData{}, fmt.Errorf("account response decode error: %s", err)
+		return basics.AccountData{}, fmt.Errorf("account response decode error: %w", err)
 	}
 	balance, err := v2.AccountToAccountData(&accountResp.Account)
 	if err != nil {
-		return basics.AccountData{}, fmt.Errorf("AccountToAccountData error: %s", err)
+		return basics.AccountData{}, fmt.Errorf("AccountToAccountData error: %w", err)
 	}
 	return balance, nil
 }

diff --git a/debug/doberman/main.go b/debug/doberman/main.go
@@ -58,6 +58,9 @@ func main() {
 
 	// write logo
 	tf, err := ioutil.TempFile("", "algorand-logo.png")
+	if err != nil {
+		panic(err)
+	}
 	tfname = tf.Name()
 	defer func() {
 		time.Sleep(retireIn)

diff --git a/ledger/acctupdates.go b/ledger/acctupdates.go
@@ -1602,7 +1602,6 @@ func (au *accountUpdates) deleteStoredCatchpoints(ctx context.Context, dbQueries
 			err = os.Remove(absCatchpointFileName)
 			if err == nil || os.IsNotExist(err) {
 				// it's ok if the file doesn't exist. just remove it from the database and we'll be good to go.
-				err = nil
 			} else {
 				// we can't delete the file, abort -
 				return fmt.Errorf("unable to delete old catchpoint file '%s' : %v", absCatchpointFileName, err)

diff --git a/logging/log.go b/logging/log.go
@@ -320,6 +320,10 @@ func (l logger) source() *logrus.Entry {
 	if !ok {
 		file = "<???>"
 		line = 1
+		event = event.WithFields(logrus.Fields{
+			"file": file,
+			"line": line,
+		})
 	} else {
 		// Add file name and number
 		slash := strings.LastIndex(file, "/")

diff --git a/logging/telemetry.go b/logging/telemetry.go
@@ -24,7 +24,7 @@ import (
 	"strings"
 	"time"
 
-	"github.com/satori/go.uuid"
+	uuid "github.com/satori/go.uuid"
 	"github.com/sirupsen/logrus"
 
 	"github.com/algorand/go-algorand/config"
@@ -186,7 +186,6 @@ func EnsureTelemetryConfigCreated(dataDir *string, genesisID string) (TelemetryC
 	}
 	created := false
 	if err != nil {
-		err = nil
 		created = true
 		cfg = createTelemetryConfig()
 

diff --git a/rpcs/blockService.go b/rpcs/blockService.go
@@ -188,8 +188,6 @@ func (bs *BlockService) ServeHTTP(response http.ResponseWriter, request *http.Re
 				response.WriteHeader(http.StatusBadRequest)
 				return
 			}
-		} else {
-			versionStr = "1"
 		}
 	}
 	round, err := strconv.ParseUint(roundStr, 36, 64)

diff --git a/rpcs/ledgerService.go b/rpcs/ledgerService.go
@@ -163,8 +163,6 @@ func (ls *LedgerService) ServeHTTP(response http.ResponseWriter, request *http.R
 				response.Write([]byte(fmt.Sprintf("invalid number of version specified %d", len(versionStrs))))
 				return
 			}
-		} else {
-			versionStr = "1"
 		}
 	}
 	round, err := strconv.ParseUint(roundStr, 36, 64)

diff --git a/shared/pingpong/accounts.go b/shared/pingpong/accounts.go
@@ -80,7 +80,6 @@ func ensureAccounts(ac libgoal.Client, initCfg PpConfig) (accounts map[string]ui
 		}
 
 		if richestBalance >= cfg.MinAccountFunds {
-			srcAcctPresent = true
 			cfg.SrcAccount = richestAccount
 
 			fmt.Printf("Identified richest account to use for Source Account: %s -> %v\n", richestAccount, richestBalance)

diff --git a/shared/pingpong/pingpong.go b/shared/pingpong/pingpong.go
@@ -153,10 +153,7 @@ func computeAccountMinBalance(client libgoal.Client, cfg PpConfig) (requiredBala
 		fmt.Printf("required min balance for app accounts: %d\n", requiredBalance)
 		return
 	}
-	var fee uint64 = 1000
-	if cfg.MinFee > fee {
-		fee = cfg.MinFee
-	}
+	var fee uint64
 	if cfg.MaxFee != 0 {
 		fee = cfg.MaxFee
 	} else {

diff --git a/test/e2e-go/cli/tealdbg/cdtmock/main.go b/test/e2e-go/cli/tealdbg/cdtmock/main.go
@@ -87,9 +87,9 @@ func main() {
 		os.Exit(1)
 	}
 
-	var counter int64 = 1
-	req := cdt.ChromeRequest{ID: counter, Method: "Debugger.Enable"}
+	var counter int64
 	counter++
+	req := cdt.ChromeRequest{ID: counter, Method: "Debugger.Enable"}
 
 	if err = client.SendJSON(req); err != nil {
 		fmt.Printf("Send error: %v", err)
@@ -101,8 +101,8 @@ func main() {
 	}
 	fmt.Printf("%s\n", string(data))
 
-	req = cdt.ChromeRequest{ID: counter, Method: "Runtime.runIfWaitingForDebugger"}
 	counter++
+	req = cdt.ChromeRequest{ID: counter, Method: "Runtime.runIfWaitingForDebugger"}
 
 	if err = client.SendJSON(req); err != nil {
 		fmt.Printf("Send error: %v", err)
@@ -114,8 +114,8 @@ func main() {
 	}
 	fmt.Printf("%s\n", string(data))
 
-	req = cdt.ChromeRequest{ID: counter, Method: "Debugger.resume"}
 	counter++
+	req = cdt.ChromeRequest{ID: counter, Method: "Debugger.resume"}
 
 	if err = client.SendJSON(req); err != nil {
 		fmt.Printf("Send error: %v", err)

diff --git a/test/framework/fixtures/libgoalFixture.go b/test/framework/fixtures/libgoalFixture.go
@@ -152,15 +152,13 @@ func (f *LibGoalFixture) importRootKeys(lg *libgoal.Client, dataDir string) {
 			handle, err = db.MakeAccessor(filepath.Join(keyDir, filename), false, false)
 			if err != nil {
 				// Couldn't open it, skip it
-				err = nil
 				continue
 			}
 
 			// Fetch an account.Root from the database
 			root, err := account.RestoreRoot(handle)
 			if err != nil {
 				// Couldn't read it, skip it
-				err = nil
 				continue
 			}
 
@@ -177,15 +175,13 @@ func (f *LibGoalFixture) importRootKeys(lg *libgoal.Client, dataDir string) {
 			handle, err = db.MakeErasableAccessor(filepath.Join(keyDir, filename))
 			if err != nil {
 				// Couldn't open it, skip it
-				err = nil
 				continue
 			}
 
 			// Fetch an account.Participation from the database
 			participation, err := account.RestoreParticipation(handle)
 			if err != nil {
 				// Couldn't read it, skip it
-				err = nil
 				handle.Close()
 				continue
 			}
@@ -224,6 +220,7 @@ func (f *LibGoalFixture) GetLibGoalClientFromDataDir(dataDir string) libgoal.Cli
 // GetLibGoalClientForNamedNode returns the LibGoal Client for a given named node
 func (f *LibGoalFixture) GetLibGoalClientForNamedNode(nodeName string) libgoal.Client {
 	nodeDir, err := f.network.GetNodeDir(nodeName)
+	f.failOnError(err, "network.GetNodeDir failed: %v")
 	client, err := libgoal.MakeClientWithBinDir(f.binDir, nodeDir, nodeDir, libgoal.KmdClient)
 	f.failOnError(err, "make libgoal client failed: %v")
 	f.importRootKeys(&client, nodeDir)
@@ -245,6 +242,7 @@ func (f *LibGoalFixture) GetLibGoalClientFromDataDirNoKeys(dataDir string) libgo
 // GetLibGoalClientForNamedNodeNoKeys returns the LibGoal Client for a given named node
 func (f *LibGoalFixture) GetLibGoalClientForNamedNodeNoKeys(nodeName string) libgoal.Client {
 	nodeDir, err := f.network.GetNodeDir(nodeName)
+	f.failOnError(err, "network.GetNodeDir failed: %v")
 	client, err := libgoal.MakeClientWithBinDir(f.binDir, nodeDir, nodeDir, libgoal.AlgodClient)
 	f.failOnError(err, "make libgoal client failed: %v")
 	return client