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 @@ -38,7 +38,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 @@ -118,6 +123,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 for this data directory. Would you like to overwrite it? (Y/n)")
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 @@ -484,14 +484,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 489 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L487-L489

Added lines #L487 - L489 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 494 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L492-L494

Added lines #L492 - L494 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 @@ -559,23 +559,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 564 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L562-L564

Added lines #L562 - L564 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 569 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L567-L569

Added lines #L567 - L569 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 578 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L576-L578

Added lines #L576 - L578 were not covered by tests
}
}
} else {
Expand Down Expand Up @@ -629,23 +629,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 634 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L632-L634

Added lines #L632 - L634 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 639 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L637-L639

Added lines #L637 - L639 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 648 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L646-L648

Added lines #L646 - L648 were not covered by tests
}
}
} else {
Expand Down Expand Up @@ -699,23 +699,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 704 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L702-L704

Added lines #L702 - L704 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 709 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L707-L709

Added lines #L707 - L709 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 718 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L716-L718

Added lines #L716 - L718 were not covered by tests
}
}
} else {
Expand Down Expand Up @@ -769,23 +769,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 774 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L772-L774

Added lines #L772 - L774 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 779 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L777-L779

Added lines #L777 - L779 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 788 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L786-L788

Added lines #L786 - L788 were not covered by tests
}
}
} else {
Expand Down Expand Up @@ -839,23 +839,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 844 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L842-L844

Added lines #L842 - L844 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 849 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L847-L849

Added lines #L847 - L849 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 858 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L856-L858

Added lines #L856 - L858 were not covered by tests
}
}
} else {
Expand Down Expand Up @@ -909,23 +909,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 914 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L912-L914

Added lines #L912 - L914 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 919 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L917-L919

Added lines #L917 - L919 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 928 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L926-L928

Added lines #L926 - L928 were not covered by tests
}
}
} else {
Expand Down Expand Up @@ -1314,9 +1314,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 1319 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L1317-L1319

Added lines #L1317 - L1319 were not covered by tests
}
retType = &theRetType
}
Expand Down Expand Up @@ -1405,9 +1405,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 1410 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L1408-L1410

Added lines #L1408 - L1410 were not covered by tests
}
for i := range txnGroup {
txnGroup[i].Group = groupID
Expand All @@ -1432,9 +1432,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 1437 in cmd/goal/application.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/application.go#L1435-L1437

Added lines #L1435 - L1437 were not covered by tests
}

signedTxnGroup = append(signedTxnGroup, signedTxn)
Expand Down