Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lint: enable govet shadow linter and resolve linter warnings #5261

Merged
merged 10 commits into from
Jun 1, 2023
10 changes: 10 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,12 @@ linters-settings:
# We do this 121 times and never check the error.
- (*github.com/spf13/cobra.Command).MarkFlagRequired
govet:
check-shadowing: true
settings:
shadow:
# explanation of strict vs non-strict:
# https://github.com/golang/tools/blob/v0.7.0/go/analysis/passes/shadow/shadow.go#L104-L122
strict: false
printf:
# Comma-separated list of print function names to check (in addition to default, see `go tool vet help printf`).
# Default: []
Expand Down Expand Up @@ -120,6 +125,11 @@ issues:
# - revive
- staticcheck
- typecheck
# allow shadowing in test code
- path: _test\.go
linters:
- govet
text: "shadows declaration at line"
# Ignore missing parallel tests in existing packages
- path: ^agreement.*_test\.go
linters:
Expand Down
6 changes: 3 additions & 3 deletions agreement/demux.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,9 @@
if err != nil {
warnMsg := fmt.Sprintf("disconnecting from peer: error decoding message tagged %v: %v", tag, err)
// check protocol version
cv, err := d.ledger.ConsensusVersion(d.ledger.NextRound())
if err == nil {
if _, ok := config.Consensus[cv]; !ok {
cv, cvErr := d.ledger.ConsensusVersion(d.ledger.NextRound())
if cvErr == nil {
if _, found := config.Consensus[cv]; !found {

Check warning on line 130 in agreement/demux.go

View check run for this annotation

Codecov / codecov/patch

agreement/demux.go#L128-L130

Added lines #L128 - L130 were not covered by tests
warnMsg = fmt.Sprintf("received proposal message was ignored. The node binary doesn't support the next network consensus (%v) and would no longer be able to process agreement messages", cv)
}
}
Expand Down
12 changes: 6 additions & 6 deletions agreement/pseudonode.go
Original file line number Diff line number Diff line change
Expand Up @@ -295,17 +295,17 @@
votes := make([]unauthenticatedVote, 0, len(accounts))
proposals := make([]proposal, 0, len(accounts))
for _, acc := range accounts {
payload, proposal, err := proposalForBlock(acc.Account, acc.VRF, ve, period, n.ledger)
if err != nil {
n.log.Errorf("pseudonode.makeProposals: could not create proposal for block (address %v): %v", acc.Account, err)
payload, proposal, pErr := proposalForBlock(acc.Account, acc.VRF, ve, period, n.ledger)
if pErr != nil {
n.log.Errorf("pseudonode.makeProposals: could not create proposal for block (address %v): %v", acc.Account, pErr)

Check warning on line 300 in agreement/pseudonode.go

View check run for this annotation

Codecov / codecov/patch

agreement/pseudonode.go#L300

Added line #L300 was not covered by tests
continue
}

// attempt to make the vote
rv := rawVote{Sender: acc.Account, Round: round, Period: period, Step: propose, Proposal: proposal}
uv, err := makeVote(rv, acc.VotingSigner(), acc.VRF, n.ledger)
if err != nil {
n.log.Warnf("pseudonode.makeProposals: could not create vote: %v", err)
uv, vErr := makeVote(rv, acc.VotingSigner(), acc.VRF, n.ledger)
if vErr != nil {
n.log.Warnf("pseudonode.makeProposals: could not create vote: %v", vErr)

Check warning on line 308 in agreement/pseudonode.go

View check run for this annotation

Codecov / codecov/patch

agreement/pseudonode.go#L308

Added line #L308 was not covered by tests
continue
}

Expand Down
8 changes: 4 additions & 4 deletions cmd/algocfg/profileCommand.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,13 +142,13 @@
reportErrorf("%v", err)
}
file := filepath.Join(dataDir, config.ConfigFilename)
if _, err := os.Stat(file); !forceUpdate && err == nil {
if _, statErr := os.Stat(file); !forceUpdate && statErr == nil {

Check warning on line 145 in cmd/algocfg/profileCommand.go

View check run for this annotation

Codecov / codecov/patch

cmd/algocfg/profileCommand.go#L145

Added line #L145 was not covered by tests
fmt.Printf("A config.json file already exists at %s\nWould you like to overwrite it? (Y/n)", file)
reader := bufio.NewReader(os.Stdin)
resp, err := reader.ReadString('\n')
resp, readErr := reader.ReadString('\n')

Check warning on line 148 in cmd/algocfg/profileCommand.go

View check run for this annotation

Codecov / codecov/patch

cmd/algocfg/profileCommand.go#L148

Added line #L148 was not covered by tests
resp = strings.TrimSpace(resp)
if err != nil {
reportErrorf("Failed to read response: %v", err)
if readErr != nil {
reportErrorf("Failed to read response: %v", readErr)

Check warning on line 151 in cmd/algocfg/profileCommand.go

View check run for this annotation

Codecov / codecov/patch

cmd/algocfg/profileCommand.go#L150-L151

Added lines #L150 - L151 were not covered by tests
}
if strings.ToLower(resp) == "n" {
reportInfof("Exiting without overwriting existing config.")
Expand Down
4 changes: 2 additions & 2 deletions cmd/catchpointdump/net.go
Original file line number Diff line number Diff line change
Expand Up @@ -314,8 +314,8 @@ func loadAndDump(addr string, tarFile string, genesisInitState ledgercore.InitSt
}

defer func() {
if err := deleteLedgerFiles(!loadOnly); err != nil {
reportWarnf("Error deleting ledger files: %v", err)
if delErr := deleteLedgerFiles(!loadOnly); delErr != nil {
reportWarnf("Error deleting ledger files: %v", delErr)
}
}()
defer l.Close()
Expand Down
10 changes: 5 additions & 5 deletions cmd/goal/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -948,16 +948,16 @@
reportErrorf(errorRequestFail, err)
}
// In an abundance of caution, check for ourselves that the key has been installed.
if err := client.VerifyParticipationKey(time.Minute, addResponse.PartId); err != nil {
err = fmt.Errorf("unable to verify key installation. Verify key installation with 'goal account partkeyinfo' and delete '%s', or retry the command. Error: %w", partKeyFile, err)
reportErrorf(errorRequestFail, err)
if vErr := client.VerifyParticipationKey(time.Minute, addResponse.PartId); vErr != nil {
vErr = fmt.Errorf("unable to verify key installation. Verify key installation with 'goal account partkeyinfo' and delete '%s', or retry the command. Error: %w", partKeyFile, vErr)
reportErrorf(errorRequestFail, vErr)

Check warning on line 953 in cmd/goal/account.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/account.go#L951-L953

Added lines #L951 - L953 were not covered by tests
}

reportInfof("Participation key installed successfully, Participation ID: %s\n", addResponse.PartId)

// Delete partKeyFile
if nil != os.Remove(partKeyFile) {
reportErrorf("An error occurred while removing the partkey file, please delete it manually: %s", err)
if osErr := os.Remove(partKeyFile); osErr != nil {
jannotti marked this conversation as resolved.
Show resolved Hide resolved
reportErrorf("An error occurred while removing the partkey file, please delete it manually: %s", osErr)

Check warning on line 960 in cmd/goal/account.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/account.go#L959-L960

Added lines #L959 - L960 were not covered by tests
}
},
}
Expand Down
138 changes: 69 additions & 69 deletions cmd/goal/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -480,14 +480,14 @@
if outFilename == "" {
// Broadcast
wh, pw := ensureWalletHandleMaybePassword(dataDir, walletName, true)
signedTxn, err := client.SignTransactionWithWalletAndSigner(wh, pw, signerAddress, tx)
if err != nil {
reportErrorf(errorSigningTX, err)
signedTxn, err2 := client.SignTransactionWithWalletAndSigner(wh, pw, signerAddress, tx)
if err2 != nil {
reportErrorf(errorSigningTX, err2)

Check warning on line 485 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L483-L485

Added lines #L483 - L485 were not covered by tests
}

txid, err := client.BroadcastTransaction(signedTxn)
if err != nil {
reportErrorf(errorBroadcastingTX, err)
txid, err2 := client.BroadcastTransaction(signedTxn)
if err2 != nil {
reportErrorf(errorBroadcastingTX, err2)

Check warning on line 490 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L488-L490

Added lines #L488 - L490 were not covered by tests
}

reportInfof("Attempting to create app (approval size %d, hash %v; clear size %d, hash %v)", len(approvalProg), crypto.HashObj(logic.Program(approvalProg)), len(clearProg), crypto.HashObj(logic.Program(clearProg)))
Expand Down Expand Up @@ -555,23 +555,23 @@
// Broadcast or write transaction to file
if outFilename == "" {
wh, pw := ensureWalletHandleMaybePassword(dataDir, walletName, true)
signedTxn, err := client.SignTransactionWithWalletAndSigner(wh, pw, signerAddress, tx)
if err != nil {
reportErrorf(errorSigningTX, err)
signedTxn, err2 := client.SignTransactionWithWalletAndSigner(wh, pw, signerAddress, tx)
if err2 != nil {
reportErrorf(errorSigningTX, err2)

Check warning on line 560 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L558-L560

Added lines #L558 - L560 were not covered by tests
}

txid, err := client.BroadcastTransaction(signedTxn)
if err != nil {
reportErrorf(errorBroadcastingTX, err)
txid, err2 := client.BroadcastTransaction(signedTxn)
if err2 != nil {
reportErrorf(errorBroadcastingTX, err2)

Check warning on line 565 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L563-L565

Added lines #L563 - L565 were not covered by tests
}

reportInfof("Attempting to update app (approval size %d, hash %v; clear size %d, hash %v)", len(approvalProg), crypto.HashObj(logic.Program(approvalProg)), len(clearProg), crypto.HashObj(logic.Program(clearProg)))
reportInfof("Issued transaction from account %s, txid %s (fee %d)", tx.Sender, txid, tx.Fee.Raw)

if !noWaitAfterSend {
_, err = waitForCommit(client, txid, lv)
if err != nil {
reportErrorf(err.Error())
_, err2 = waitForCommit(client, txid, lv)
if err2 != nil {
reportErrorf(err2.Error())

Check warning on line 574 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L572-L574

Added lines #L572 - L574 were not covered by tests
}
}
} else {
Expand Down Expand Up @@ -625,23 +625,23 @@
// Broadcast or write transaction to file
if outFilename == "" {
wh, pw := ensureWalletHandleMaybePassword(dataDir, walletName, true)
signedTxn, err := client.SignTransactionWithWalletAndSigner(wh, pw, signerAddress, tx)
if err != nil {
reportErrorf(errorSigningTX, err)
signedTxn, err2 := client.SignTransactionWithWalletAndSigner(wh, pw, signerAddress, tx)
if err2 != nil {
reportErrorf(errorSigningTX, err2)

Check warning on line 630 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L628-L630

Added lines #L628 - L630 were not covered by tests
}

txid, err := client.BroadcastTransaction(signedTxn)
if err != nil {
reportErrorf(errorBroadcastingTX, err)
txid, err2 := client.BroadcastTransaction(signedTxn)
if err2 != nil {
reportErrorf(errorBroadcastingTX, err2)

Check warning on line 635 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L633-L635

Added lines #L633 - L635 were not covered by tests
}

// Report tx details to user
reportInfof("Issued transaction from account %s, txid %s (fee %d)", tx.Sender, txid, tx.Fee.Raw)

if !noWaitAfterSend {
_, err = waitForCommit(client, txid, lv)
if err != nil {
reportErrorf(err.Error())
_, err2 = waitForCommit(client, txid, lv)
if err2 != nil {
reportErrorf(err2.Error())

Check warning on line 644 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L642-L644

Added lines #L642 - L644 were not covered by tests
}
}
} else {
Expand Down Expand Up @@ -695,23 +695,23 @@
// Broadcast or write transaction to file
if outFilename == "" {
wh, pw := ensureWalletHandleMaybePassword(dataDir, walletName, true)
signedTxn, err := client.SignTransactionWithWalletAndSigner(wh, pw, signerAddress, tx)
if err != nil {
reportErrorf(errorSigningTX, err)
signedTxn, err2 := client.SignTransactionWithWalletAndSigner(wh, pw, signerAddress, tx)
if err2 != nil {
reportErrorf(errorSigningTX, err2)

Check warning on line 700 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L698-L700

Added lines #L698 - L700 were not covered by tests
}

txid, err := client.BroadcastTransaction(signedTxn)
if err != nil {
reportErrorf(errorBroadcastingTX, err)
txid, err2 := client.BroadcastTransaction(signedTxn)
if err2 != nil {
reportErrorf(errorBroadcastingTX, err2)

Check warning on line 705 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L703-L705

Added lines #L703 - L705 were not covered by tests
}

// Report tx details to user
reportInfof("Issued transaction from account %s, txid %s (fee %d)", tx.Sender, txid, tx.Fee.Raw)

if !noWaitAfterSend {
_, err = waitForCommit(client, txid, lv)
if err != nil {
reportErrorf(err.Error())
_, err2 = waitForCommit(client, txid, lv)
if err2 != nil {
reportErrorf(err2.Error())

Check warning on line 714 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L712-L714

Added lines #L712 - L714 were not covered by tests
}
}
} else {
Expand Down Expand Up @@ -765,23 +765,23 @@
// Broadcast or write transaction to file
if outFilename == "" {
wh, pw := ensureWalletHandleMaybePassword(dataDir, walletName, true)
signedTxn, err := client.SignTransactionWithWalletAndSigner(wh, pw, signerAddress, tx)
if err != nil {
reportErrorf(errorSigningTX, err)
signedTxn, err2 := client.SignTransactionWithWalletAndSigner(wh, pw, signerAddress, tx)
if err2 != nil {
reportErrorf(errorSigningTX, err2)

Check warning on line 770 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L768-L770

Added lines #L768 - L770 were not covered by tests
}

txid, err := client.BroadcastTransaction(signedTxn)
if err != nil {
reportErrorf(errorBroadcastingTX, err)
txid, err2 := client.BroadcastTransaction(signedTxn)
if err2 != nil {
reportErrorf(errorBroadcastingTX, err2)

Check warning on line 775 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L773-L775

Added lines #L773 - L775 were not covered by tests
}

// Report tx details to user
reportInfof("Issued transaction from account %s, txid %s (fee %d)", tx.Sender, txid, tx.Fee.Raw)

if !noWaitAfterSend {
_, err = waitForCommit(client, txid, lv)
if err != nil {
reportErrorf(err.Error())
_, err2 = waitForCommit(client, txid, lv)
if err2 != nil {
reportErrorf(err2.Error())

Check warning on line 784 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L782-L784

Added lines #L782 - L784 were not covered by tests
}
}
} else {
Expand Down Expand Up @@ -835,23 +835,23 @@
// Broadcast or write transaction to file
if outFilename == "" {
wh, pw := ensureWalletHandleMaybePassword(dataDir, walletName, true)
signedTxn, err := client.SignTransactionWithWalletAndSigner(wh, pw, signerAddress, tx)
if err != nil {
reportErrorf(errorSigningTX, err)
signedTxn, err2 := client.SignTransactionWithWalletAndSigner(wh, pw, signerAddress, tx)
if err2 != nil {
reportErrorf(errorSigningTX, err2)

Check warning on line 840 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L838-L840

Added lines #L838 - L840 were not covered by tests
}

txid, err := client.BroadcastTransaction(signedTxn)
if err != nil {
reportErrorf(errorBroadcastingTX, err)
txid, err2 := client.BroadcastTransaction(signedTxn)
if err2 != nil {
reportErrorf(errorBroadcastingTX, err2)

Check warning on line 845 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L843-L845

Added lines #L843 - L845 were not covered by tests
}

// Report tx details to user
reportInfof("Issued transaction from account %s, txid %s (fee %d)", tx.Sender, txid, tx.Fee.Raw)

if !noWaitAfterSend {
_, err = waitForCommit(client, txid, lv)
if err != nil {
reportErrorf(err.Error())
_, err2 = waitForCommit(client, txid, lv)
if err2 != nil {
reportErrorf(err2.Error())

Check warning on line 854 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L852-L854

Added lines #L852 - L854 were not covered by tests
}
}
} else {
Expand Down Expand Up @@ -905,23 +905,23 @@
// Broadcast or write transaction to file
if outFilename == "" {
wh, pw := ensureWalletHandleMaybePassword(dataDir, walletName, true)
signedTxn, err := client.SignTransactionWithWalletAndSigner(wh, pw, signerAddress, tx)
if err != nil {
reportErrorf(errorSigningTX, err)
signedTxn, err2 := client.SignTransactionWithWalletAndSigner(wh, pw, signerAddress, tx)
if err2 != nil {
reportErrorf(errorSigningTX, err2)

Check warning on line 910 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L908-L910

Added lines #L908 - L910 were not covered by tests
}

txid, err := client.BroadcastTransaction(signedTxn)
if err != nil {
reportErrorf(errorBroadcastingTX, err)
txid, err2 := client.BroadcastTransaction(signedTxn)
if err2 != nil {
reportErrorf(errorBroadcastingTX, err2)

Check warning on line 915 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L913-L915

Added lines #L913 - L915 were not covered by tests
}

// Report tx details to user
reportInfof("Issued transaction from account %s, txid %s (fee %d)", tx.Sender, txid, tx.Fee.Raw)

if !noWaitAfterSend {
_, err = waitForCommit(client, txid, lv)
if err != nil {
reportErrorf(err.Error())
_, err2 = waitForCommit(client, txid, lv)
if err2 != nil {
reportErrorf(err2.Error())

Check warning on line 924 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L922-L924

Added lines #L922 - L924 were not covered by tests
}
}
} else {
Expand Down Expand Up @@ -1310,9 +1310,9 @@

var retType *abi.Type
if retTypeStr != abi.VoidReturnType {
theRetType, err := abi.TypeOf(retTypeStr)
if err != nil {
reportErrorf("cannot cast %s to abi type: %v", retTypeStr, err)
theRetType, typeErr := abi.TypeOf(retTypeStr)
if typeErr != nil {
reportErrorf("cannot cast %s to abi type: %v", retTypeStr, typeErr)

Check warning on line 1315 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L1313-L1315

Added lines #L1313 - L1315 were not covered by tests
}
retType = &theRetType
}
Expand Down Expand Up @@ -1401,9 +1401,9 @@
txnGroup = append(txnGroup, appCallTxn)
if len(txnGroup) > 1 {
// Only if transaction arguments are present, assign group ID
groupID, err := client.GroupID(txnGroup)
if err != nil {
reportErrorf("Cannot assign transaction group ID: %s", err)
groupID, gidErr := client.GroupID(txnGroup)
if gidErr != nil {
reportErrorf("Cannot assign transaction group ID: %s", gidErr)

Check warning on line 1406 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L1404-L1406

Added lines #L1404 - L1406 were not covered by tests
}
for i := range txnGroup {
txnGroup[i].Group = groupID
Expand All @@ -1428,9 +1428,9 @@
continue
}

signedTxn, err := createSignedTransaction(client, shouldSign, dataDir, walletName, unsignedTxn, txnFromArgs.AuthAddr)
if err != nil {
reportErrorf(errorSigningTX, err)
signedTxn, signErr := createSignedTransaction(client, shouldSign, dataDir, walletName, unsignedTxn, txnFromArgs.AuthAddr)
if signErr != nil {
reportErrorf(errorSigningTX, signErr)

Check warning on line 1433 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L1431-L1433

Added lines #L1431 - L1433 were not covered by tests
}

signedTxnGroup = append(signedTxnGroup, signedTxn)
Expand Down