From b6a510a3e7cefca56af4bdb009413e3950c6d59e Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Fri, 8 Mar 2019 09:46:09 +0400 Subject: [PATCH] make ineffassign linter pass (#3386) Refs #3262 This fixes two small bugs: 1) lite/dbprovider: return `ok` instead of true in parse* functions. It's weird that we're ignoring `ok` value before. 2) consensus/state: previously because of the shadowing we almost never output "Error with msg". Now we declare both `added` and `err` in the beginning of the function, so there's no shadowing. --- .golangci.yml | 1 - consensus/state.go | 12 ++++++++---- lite/dbprovider.go | 7 ++++--- lite/dynamic_verifier_test.go | 1 + lite/proxy/query_test.go | 2 ++ p2p/conn/connection_test.go | 8 +++++--- rpc/core/status.go | 2 +- state/execution_test.go | 1 + state/state_test.go | 6 ++++++ 9 files changed, 28 insertions(+), 12 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index cf8bf165d..a051e1a45 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -9,7 +9,6 @@ linters: - maligned - errcheck - staticcheck - - ineffassign - interfacer - unconvert - goconst diff --git a/consensus/state.go b/consensus/state.go index cf32afe7b..d4a12a0c3 100644 --- a/consensus/state.go +++ b/consensus/state.go @@ -670,7 +670,10 @@ func (cs *ConsensusState) handleMsg(mi msgInfo) { cs.mtx.Lock() defer cs.mtx.Unlock() - var err error + var ( + err error + added bool + ) msg, peerID := mi.Msg, mi.PeerID switch msg := msg.(type) { case *ProposalMessage: @@ -679,7 +682,7 @@ func (cs *ConsensusState) handleMsg(mi msgInfo) { err = cs.setProposal(msg.Proposal) case *BlockPartMessage: // if the proposal is complete, we'll enterPrevote or tryFinalizeCommit - added, err := cs.addProposalBlockPart(msg, peerID) + added, err = cs.addProposalBlockPart(msg, peerID) if added { cs.statsMsgQueue <- mi } @@ -691,7 +694,7 @@ func (cs *ConsensusState) handleMsg(mi msgInfo) { case *VoteMessage: // attempt to add the vote and dupeout the validator if its a duplicate signature // if the vote gives us a 2/3-any or 2/3-one, we transition - added, err := cs.tryAddVote(msg.Vote, peerID) + added, err = cs.tryAddVote(msg.Vote, peerID) if added { cs.statsMsgQueue <- mi } @@ -714,7 +717,8 @@ func (cs *ConsensusState) handleMsg(mi msgInfo) { cs.Logger.Error("Unknown msg type", reflect.TypeOf(msg)) } if err != nil { - cs.Logger.Error("Error with msg", "height", cs.Height, "round", cs.Round, "type", reflect.TypeOf(msg), "peer", peerID, "err", err, "msg", msg) + cs.Logger.Error("Error with msg", "height", cs.Height, "round", cs.Round, + "peer", peerID, "err", err, "msg", msg) } } diff --git a/lite/dbprovider.go b/lite/dbprovider.go index ef1b2a598..5582a9632 100644 --- a/lite/dbprovider.go +++ b/lite/dbprovider.go @@ -258,14 +258,15 @@ func parseKey(key []byte) (chainID string, height int64, part string, ok bool) { } func parseSignedHeaderKey(key []byte) (chainID string, height int64, ok bool) { - chainID, height, part, ok := parseKey(key) + var part string + chainID, height, part, ok = parseKey(key) if part != "sh" { return "", 0, false } - return chainID, height, true + return } func parseChainKeyPrefix(key []byte) (chainID string, height int64, ok bool) { chainID, height, _, ok = parseKey(key) - return chainID, height, true + return } diff --git a/lite/dynamic_verifier_test.go b/lite/dynamic_verifier_test.go index 386de513c..e85cb7de0 100644 --- a/lite/dynamic_verifier_test.go +++ b/lite/dynamic_verifier_test.go @@ -255,6 +255,7 @@ func TestConcurrencyInquirerVerify(t *testing.T) { cert.SetLogger(log.TestingLogger()) err = source.SaveFullCommit(fcz[7]) + require.Nil(err, "%+v", err) err = source.SaveFullCommit(fcz[8]) require.Nil(err, "%+v", err) sh := fcz[8].SignedHeader diff --git a/lite/proxy/query_test.go b/lite/proxy/query_test.go index c1450a5e6..db2b6e46c 100644 --- a/lite/proxy/query_test.go +++ b/lite/proxy/query_test.go @@ -93,6 +93,8 @@ func _TestAppProofs(t *testing.T) { // verify a query before the tx block has no data (and valid non-exist proof) bs, height, proof, err := GetWithProof(prt, k, brh-1, cl, cert) require.NoError(err, "%#v", err) + require.NotNil(proof) + require.Equal(height, brh-1) // require.NotNil(proof) // TODO: Ensure that *some* keys will be there, ensuring that proof is nil, // (currently there's a race condition) diff --git a/p2p/conn/connection_test.go b/p2p/conn/connection_test.go index afad69d1d..283b00ebe 100644 --- a/p2p/conn/connection_test.go +++ b/p2p/conn/connection_test.go @@ -223,7 +223,10 @@ func TestMConnectionMultiplePongsInTheBeginning(t *testing.T) { serverGotPing := make(chan struct{}) go func() { // read ping (one byte) - var packet, err = Packet(nil), error(nil) + var ( + packet Packet + err error + ) _, err = cdc.UnmarshalBinaryLengthPrefixedReader(server, &packet, maxPingPongPacketSize) require.Nil(t, err) serverGotPing <- struct{}{} @@ -492,8 +495,7 @@ func TestMConnectionReadErrorUnknownMsgType(t *testing.T) { defer mconnServer.Stop() // send msg with unknown msg type - err := error(nil) - err = amino.EncodeUvarint(mconnClient.conn, 4) + err := amino.EncodeUvarint(mconnClient.conn, 4) assert.Nil(t, err) _, err = mconnClient.conn.Write([]byte{0xFF, 0xFF, 0xFF, 0xFF}) assert.Nil(t, err) diff --git a/rpc/core/status.go b/rpc/core/status.go index 224857d00..ae22ecd35 100644 --- a/rpc/core/status.go +++ b/rpc/core/status.go @@ -71,7 +71,7 @@ import ( // } // ``` func Status() (*ctypes.ResultStatus, error) { - var latestHeight int64 = -1 + var latestHeight int64 if consensusReactor.FastSync() { latestHeight = blockStore.Height() } else { diff --git a/state/execution_test.go b/state/execution_test.go index 94336851c..a9fdfe270 100644 --- a/state/execution_test.go +++ b/state/execution_test.go @@ -43,6 +43,7 @@ func TestApplyBlock(t *testing.T) { block := makeBlock(state, 1) blockID := types.BlockID{block.Hash(), block.MakePartSet(testPartSize).Header()} + //nolint:ineffassign state, err = blockExec.ApplyBlock(state, blockID, block) require.Nil(t, err) diff --git a/state/state_test.go b/state/state_test.go index 4566d93ea..ff8eed027 100644 --- a/state/state_test.go +++ b/state/state_test.go @@ -668,6 +668,7 @@ func TestLargeGenesisValidator(t *testing.T) { blockID := types.BlockID{block.Hash(), block.MakePartSet(testPartSize).Header()} updatedState, err := updateState(oldState, blockID, &block.Header, abciResponses, validatorUpdates) + require.NoError(t, err) // no changes in voting power (ProposerPrio += VotingPower == Voting in 1st round; than shiftByAvg == 0, // than -Total == -Voting) // -> no change in ProposerPrio (stays zero): @@ -692,6 +693,7 @@ func TestLargeGenesisValidator(t *testing.T) { block := makeBlock(oldState, oldState.LastBlockHeight+1) blockID := types.BlockID{block.Hash(), block.MakePartSet(testPartSize).Header()} updatedState, err := updateState(oldState, blockID, &block.Header, abciResponses, validatorUpdates) + require.NoError(t, err) lastState := updatedState for i := 0; i < 200; i++ { @@ -706,6 +708,7 @@ func TestLargeGenesisValidator(t *testing.T) { blockID := types.BlockID{block.Hash(), block.MakePartSet(testPartSize).Header()} updatedStateInner, err := updateState(lastState, blockID, &block.Header, abciResponses, validatorUpdates) + require.NoError(t, err) lastState = updatedStateInner } // set state to last state of above iteration @@ -735,6 +738,7 @@ func TestLargeGenesisValidator(t *testing.T) { block := makeBlock(oldState, oldState.LastBlockHeight+1) blockID := types.BlockID{block.Hash(), block.MakePartSet(testPartSize).Header()} state, err = updateState(state, blockID, &block.Header, abciResponses, validatorUpdates) + require.NoError(t, err) } require.Equal(t, 10+2, len(state.NextValidators.Validators)) @@ -766,6 +770,7 @@ func TestLargeGenesisValidator(t *testing.T) { block = makeBlock(curState, curState.LastBlockHeight+1) blockID = types.BlockID{block.Hash(), block.MakePartSet(testPartSize).Header()} curState, err = updateState(curState, blockID, &block.Header, abciResponses, validatorUpdates) + require.NoError(t, err) if !bytes.Equal(curState.Validators.Proposer.Address, curState.NextValidators.Proposer.Address) { isProposerUnchanged = false } @@ -790,6 +795,7 @@ func TestLargeGenesisValidator(t *testing.T) { blockID := types.BlockID{block.Hash(), block.MakePartSet(testPartSize).Header()} updatedState, err = updateState(updatedState, blockID, &block.Header, abciResponses, validatorUpdates) + require.NoError(t, err) if i > numVals { // expect proposers to cycle through after the first iteration (of numVals blocks): if proposers[i%numVals] == nil { proposers[i%numVals] = updatedState.NextValidators.Proposer