Skip to content

Commit a520323

Browse files
committed
all: more linters (#24783)
This enables the following linters - typecheck - unused - staticcheck - bidichk - durationcheck - exportloopref - gosec With a few exceptions. - We use a deprecated protobuf in trezor. I didn't want to mess with that, since I cannot meaningfully test any changes there. - The deprecated TypeMux is used in a few places still, so the warning for it is silenced for now. - Using string type in context.WithValue is apparently wrong, one should use a custom type, to prevent collisions between different places in the hierarchy of callers. That should be fixed at some point, but may require some attention. - The warnings for using weak random generator are squashed, since we use a lot of random without need for cryptographic guarantees. Some amendments from ethereum/go-ethereum@a907d7e
1 parent bd57f69 commit a520323

File tree

198 files changed

+144
-818
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

198 files changed

+144
-818
lines changed

.golangci.yml

+31-15
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# This file configures github.com/golangci/golangci-lint.
22

33
run:
4-
timeout: 5m
4+
timeout: 20m
55
tests: true
66
# default is true. Enables skipping of directories:
77
# vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
@@ -27,6 +27,15 @@ linters:
2727
- exportloopref
2828
- whitespace
2929

30+
# - structcheck # lots of false positives
31+
# - errcheck #lot of false positives
32+
# - contextcheck
33+
# - errchkjson # lots of false positives
34+
# - errorlint # this check crashes
35+
# - exhaustive # silly check
36+
# - makezero # false positives
37+
# - nilerr # several intentional
38+
3039
linters-settings:
3140
gofmt:
3241
simplify: true
@@ -35,20 +44,27 @@ linters-settings:
3544
min-occurrences: 6 # minimum number of occurrences
3645

3746
issues:
38-
new-from-rev: c4e9657
3947
exclude-rules:
40-
- path: crypto/blake2b/
41-
linters:
42-
- deadcode
43-
- path: crypto/bn256/cloudflare
44-
linters:
45-
- deadcode
46-
- path: p2p/discv5/
47-
linters:
48-
- deadcode
49-
- path: core/vm/instructions_test.go
50-
linters:
51-
- goconst
52-
- path: cmd/faucet/
48+
- path: crypto/bn256/cloudflare/optate.go
5349
linters:
5450
- deadcode
51+
- staticcheck
52+
- path: internal/build/pgp.go
53+
text: 'SA1019: "golang.org/x/crypto/openpgp" is deprecated: this package is unmaintained except for security fixes.'
54+
- path: core/vm/contracts.go
55+
text: 'SA1019: "golang.org/x/crypto/ripemd160" is deprecated: RIPEMD-160 is a legacy hash and should not be used for new applications.'
56+
- path: accounts/usbwallet/trezor.go
57+
text: 'SA1019: "github.com/golang/protobuf/proto" is deprecated: Use the "google.golang.org/protobuf/proto" package instead.'
58+
- path: accounts/usbwallet/trezor/
59+
text: 'SA1019: "github.com/golang/protobuf/proto" is deprecated: Use the "google.golang.org/protobuf/proto" package instead.'
60+
- path: plugin/account/internal/testutils/matchers.go
61+
text: 'SA1019: "github.com/golang/protobuf/proto" is deprecated: Use the "google.golang.org/protobuf/proto" package instead.'
62+
- path: rpc/
63+
text: 'SA1019: "github.com/golang/protobuf/proto" is deprecated: Use the "google.golang.org/protobuf/proto" package instead.'
64+
- path: rpc/
65+
text: 'SA1019: "github.com/golang/protobuf/ptypes" is deprecated: Well-known types have specialized functionality directly injected into the generated packages for each message type. See the deprecation notice for each function for the suggested alternative'
66+
exclude:
67+
- 'SA1019: event.TypeMux is deprecated: use Feed'
68+
- 'SA1019: strings.Title is deprecated'
69+
- 'SA1019: strings.Title has been deprecated since Go 1.18 and an alternative has been available since Go 1.0: The rule Title uses for word boundaries does not handle Unicode punctuation properly. Use golang.org/x/text/cases instead.'
70+
- 'SA1029: should not use built-in type string as key for value'

accounts/abi/abi_test.go

+1-3
Original file line numberDiff line numberDiff line change
@@ -1028,9 +1028,7 @@ func TestABI_EventById(t *testing.T) {
10281028
}
10291029
if event == nil {
10301030
t.Errorf("We should find a event for topic %s, test #%d", topicID.Hex(), testnum)
1031-
}
1032-
1033-
if event.ID != topicID {
1031+
} else if event.ID != topicID {
10341032
t.Errorf("Event id %s does not match topic %s, test #%d", event.ID.Hex(), topicID.Hex(), testnum)
10351033
}
10361034

accounts/abi/bind/backends/simulated.go

+8-8
Original file line numberDiff line numberDiff line change
@@ -165,11 +165,11 @@ func (b *SimulatedBackend) CodeAt(ctx context.Context, contract common.Address,
165165
b.mu.Lock()
166166
defer b.mu.Unlock()
167167

168-
stateDB, err := b.stateByBlockNumber(ctx, blockNumber)
168+
_, err := b.stateByBlockNumber(ctx, blockNumber)
169169
if err != nil {
170170
return nil, err
171171
}
172-
stateDB, _, _ = b.blockchain.State()
172+
stateDB, _, _ := b.blockchain.State()
173173
return stateDB.GetCode(contract), nil
174174
}
175175

@@ -178,11 +178,11 @@ func (b *SimulatedBackend) BalanceAt(ctx context.Context, contract common.Addres
178178
b.mu.Lock()
179179
defer b.mu.Unlock()
180180

181-
stateDB, err := b.stateByBlockNumber(ctx, blockNumber)
181+
_, err := b.stateByBlockNumber(ctx, blockNumber)
182182
if err != nil {
183183
return nil, err
184184
}
185-
stateDB, _, _ = b.blockchain.State()
185+
stateDB, _, _ := b.blockchain.State()
186186
return stateDB.GetBalance(contract), nil
187187
}
188188

@@ -191,11 +191,11 @@ func (b *SimulatedBackend) NonceAt(ctx context.Context, contract common.Address,
191191
b.mu.Lock()
192192
defer b.mu.Unlock()
193193

194-
stateDB, err := b.stateByBlockNumber(ctx, blockNumber)
194+
_, err := b.stateByBlockNumber(ctx, blockNumber)
195195
if err != nil {
196196
return 0, err
197197
}
198-
stateDB, _, _ = b.blockchain.State()
198+
stateDB, _, _ := b.blockchain.State()
199199
return stateDB.GetNonce(contract), nil
200200
}
201201

@@ -204,11 +204,11 @@ func (b *SimulatedBackend) StorageAt(ctx context.Context, contract common.Addres
204204
b.mu.Lock()
205205
defer b.mu.Unlock()
206206

207-
stateDB, err := b.stateByBlockNumber(ctx, blockNumber)
207+
_, err := b.stateByBlockNumber(ctx, blockNumber)
208208
if err != nil {
209209
return nil, err
210210
}
211-
stateDB, _, _ = b.blockchain.State()
211+
stateDB, _, _ := b.blockchain.State()
212212
val := stateDB.GetState(contract, key)
213213
return val[:], nil
214214
}

accounts/abi/bind/backends/simulated_test.go

+2-3
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ func TestSimulatedBackend(t *testing.T) {
7878
}
7979

8080
sim.Commit()
81-
tx, isPending, err = sim.TransactionByHash(context.Background(), txHash)
81+
_, isPending, err = sim.TransactionByHash(context.Background(), txHash)
8282
if err != nil {
8383
t.Fatalf("error getting transaction with hash: %v", txHash.String())
8484
}
@@ -621,8 +621,7 @@ func TestSimulatedBackend_HeaderByNumber(t *testing.T) {
621621
}
622622
if latestBlockHeader == nil {
623623
t.Errorf("received a nil block header")
624-
}
625-
if latestBlockHeader.Number.Uint64() != uint64(0) {
624+
} else if latestBlockHeader.Number.Uint64() != uint64(0) {
626625
t.Errorf("expected block header number 0, instead got %v", latestBlockHeader.Number.Uint64())
627626
}
628627

accounts/abi/bind/base_test.go

-1
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ func (mc *mockCaller) PendingCallContract(ctx context.Context, call ethereum.Cal
6060
return nil, nil
6161
}
6262
func TestPassingBlockNumber(t *testing.T) {
63-
6463
mc := &mockCaller{}
6564

6665
bc := bind.NewBoundContract(common.HexToAddress("0x0"), abi.ABI{

accounts/abi/error.go

-1
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,6 @@ func typeCheck(t Type, value reflect.Value) error {
7373
} else {
7474
return nil
7575
}
76-
7776
}
7877

7978
// typeErr returns a formatted type casting error.

accounts/abi/event_test.go

-1
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,6 @@ func TestEventMultiValueWithArrayUnpack(t *testing.T) {
161161
}
162162

163163
func TestEventTupleUnpack(t *testing.T) {
164-
165164
type EventTransfer struct {
166165
Value *big.Int
167166
}

accounts/abi/reflect.go

-1
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,6 @@ func mapArgNamesToStructFields(argNames []string, value reflect.Value) (map[stri
233233

234234
// second round ~~~
235235
for _, argName := range argNames {
236-
237236
structFieldName := ToCamelCase(argName)
238237

239238
if structFieldName == "" {

accounts/abi/unpack.go

-1
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,6 @@ func ReadFixedBytes(t Type, word []byte) (interface{}, error) {
115115

116116
reflect.Copy(array, reflect.ValueOf(word[0:t.Size]))
117117
return array.Interface(), nil
118-
119118
}
120119

121120
// forEachUnpack iteratively unpack elements.

accounts/external/backend.go

-5
Original file line numberDiff line numberDiff line change
@@ -152,11 +152,6 @@ func (api *ExternalSigner) SelfDerive(bases []accounts.DerivationPath, chain eth
152152
log.Error("operation SelfDerive not supported on external signers")
153153
}
154154

155-
func (api *ExternalSigner) signHash(account accounts.Account, hash []byte) ([]byte, error) {
156-
return []byte{}, fmt.Errorf("operation not supported on external signers")
157-
}
158-
159-
// SignData signs keccak256(data). The mimetype parameter describes the type of data being signed
160155
func (api *ExternalSigner) SignData(account accounts.Account, mimeType string, data []byte) ([]byte, error) {
161156
var res hexutil.Bytes
162157
var signAddress = common.NewMixedcaseAddress(account.Address)

accounts/keystore/keystore_test.go

-2
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,6 @@ func TestImportExport(t *testing.T) {
389389
if _, err = ks2.Import(json, "new", "new"); err == nil {
390390
t.Errorf("importing a key twice succeeded")
391391
}
392-
393392
}
394393

395394
// TestImportRace tests the keystore on races.
@@ -416,7 +415,6 @@ func TestImportRace(t *testing.T) {
416415
if _, err := ks2.Import(json, "new", "new"); err != nil {
417416
atomic.AddUint32(&atom, 1)
418417
}
419-
420418
}()
421419
}
422420
wg.Wait()

accounts/keystore/passphrase.go

-2
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,6 @@ func (ks keyStorePassphrase) JoinPath(filename string) string {
139139

140140
// Encryptdata encrypts the data given as 'data' with the password 'auth'.
141141
func EncryptDataV3(data, auth []byte, scryptN, scryptP int) (CryptoJSON, error) {
142-
143142
salt := make([]byte, 32)
144143
if _, err := io.ReadFull(rand.Reader, salt); err != nil {
145144
panic("reading from crypto/rand failed: " + err.Error())
@@ -342,7 +341,6 @@ func getKDFKey(cryptoJSON CryptoJSON, auth string) ([]byte, error) {
342341
r := ensureInt(cryptoJSON.KDFParams["r"])
343342
p := ensureInt(cryptoJSON.KDFParams["p"])
344343
return scrypt.Key(authArray, salt, n, r, p, dkLen)
345-
346344
} else if cryptoJSON.KDF == "pbkdf2" {
347345
c := ensureInt(cryptoJSON.KDFParams["c"])
348346
prf := cryptoJSON.KDFParams["prf"].(string)

accounts/usbwallet/wallet.go

-1
Original file line numberDiff line numberDiff line change
@@ -527,7 +527,6 @@ func (w *wallet) signHash(account accounts.Account, hash []byte) ([]byte, error)
527527

528528
// SignData signs keccak256(data). The mimetype parameter describes the type of data being signed
529529
func (w *wallet) SignData(account accounts.Account, mimeType string, data []byte) ([]byte, error) {
530-
531530
// Unless we are doing 712 signing, simply dispatch to signHash
532531
if !(mimeType == accounts.MimetypeTypedData && len(data) == 66 && data[0] == 0x19 && data[1] == 0x01) {
533532
return w.signHash(account, crypto.Keccak256(data))

cmd/clef/main.go

-5
Original file line numberDiff line numberDiff line change
@@ -897,7 +897,6 @@ func confirm(text string) bool {
897897
}
898898

899899
func testExternalUI(api *core.SignerAPI) {
900-
901900
ctx := context.WithValue(context.Background(), "remote", "clef binary")
902901
ctx = context.WithValue(ctx, "scheme", "in-proc")
903902
ctx = context.WithValue(ctx, "local", "main")
@@ -997,7 +996,6 @@ func testExternalUI(api *core.SignerAPI) {
997996
expectDeny("signdata - text", err)
998997
}
999998
{ // Sign transaction
1000-
1001999
api.UI.ShowInfo("Please reject next transaction")
10021000
time.Sleep(delay)
10031001
data := hexutil.Bytes([]byte{})
@@ -1040,7 +1038,6 @@ func testExternalUI(api *core.SignerAPI) {
10401038
}
10411039
result := fmt.Sprintf("Tests completed. %d errors:\n%s\n", len(errs), strings.Join(errs, "\n"))
10421040
api.UI.ShowInfo(result)
1043-
10441041
}
10451042

10461043
type encryptedSeedStorage struct {
@@ -1077,7 +1074,6 @@ func decryptSeed(keyjson []byte, auth string) ([]byte, error) {
10771074

10781075
// GenDoc outputs examples of all structures used in json-rpc communication
10791076
func GenDoc(ctx *cli.Context) {
1080-
10811077
var (
10821078
a = common.HexToAddress("0xdeadbeef000000000000000000000000deadbeef")
10831079
b = common.HexToAddress("0x1111111122222222222233333333334444444444")
@@ -1187,7 +1183,6 @@ func GenDoc(ctx *cli.Context) {
11871183
var tx types.Transaction
11881184
tx.UnmarshalBinary(rlpdata)
11891185
add("OnApproved - SignTransactionResult", desc, &ethapi.SignTransactionResult{Raw: rlpdata, Tx: &tx})
1190-
11911186
}
11921187
{ // User input
11931188
add("UserInputRequest", "Sent when clef needs the user to provide data. If 'password' is true, the input field should be treated accordingly (echo-free)",

cmd/devp2p/internal/ethtest/chain.go

-1
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,6 @@ func (c *Chain) GetHeaders(req GetBlockHeaders) (BlockHeaders, error) {
108108
for i := 1; i < int(req.Amount); i++ {
109109
blockNumber -= (1 - req.Skip)
110110
headers[i] = c.blocks[blockNumber].Header()
111-
112111
}
113112

114113
return headers, nil

cmd/devp2p/internal/ethtest/eth66_suite.go

-1
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,6 @@ func (s *Suite) TestMaliciousTx_66(t *utesting.T) {
365365
if err := sendConn.Write(&Transactions{tx}); err != nil {
366366
t.Fatalf("could not write to connection: %v", err)
367367
}
368-
369368
}
370369
// check to make sure bad txs aren't propagated
371370
waitForTxPropagation(t, s, badTxs, recvConn)

cmd/devp2p/internal/ethtest/suite.go

-1
Original file line numberDiff line numberDiff line change
@@ -543,7 +543,6 @@ func (s *Suite) TestMaliciousTx(t *utesting.T) {
543543
if err := sendConn.Write(&Transactions{tx}); err != nil {
544544
t.Fatalf("could not write to connection: %v", err)
545545
}
546-
547546
}
548547
// check to make sure bad txs aren't propagated
549548
waitForTxPropagation(t, s, badTxs, recvConn)

cmd/devp2p/internal/v5test/framework.go

+3-5
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,9 @@ type conn struct {
6060
remoteAddr *net.UDPAddr
6161
listeners []net.PacketConn
6262

63-
log logger
64-
codec *v5wire.Codec
65-
lastRequest v5wire.Packet
66-
lastChallenge *v5wire.Whoareyou
67-
idCounter uint32
63+
log logger
64+
codec *v5wire.Codec
65+
idCounter uint32
6866
}
6967

7068
type logger interface {

cmd/evm/internal/t8ntool/execution.go

-1
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@ type stEnvMarshaling struct {
8383
func (pre *Prestate) Apply(vmConfig vm.Config, chainConfig *params.ChainConfig,
8484
txs types.Transactions, miningReward int64,
8585
getTracerFn func(txIndex int, txHash common.Hash) (tracer vm.Tracer, err error)) (*state.StateDB, *ExecutionResult, error) {
86-
8786
// Capture errors for BLOCKHASH operation, if we haven't been supplied the
8887
// required blockhashes
8988
var hashError error

cmd/evm/internal/t8ntool/transition.go

-1
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,6 @@ func Main(ctx *cli.Context) error {
221221
collector := make(Alloc)
222222
state.DumpToCollector(collector, false, false, false, nil, -1)
223223
return dispatchOutput(ctx, baseDir, result, collector, body)
224-
225224
}
226225

227226
// txWithKey is a helper-struct, to allow us to use the types.Transaction along with

cmd/geth/accountcmd.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -240,14 +240,15 @@ func ambiguousAddrRecovery(ks *keystore.KeyStore, err *keystore.AmbiguousAddrErr
240240
}
241241
fmt.Println("Testing your password against all of them...")
242242
var match *accounts.Account
243-
for _, a := range err.Matches {
244-
if err := ks.Unlock(a, auth); err == nil {
245-
match = &a
243+
for i, a := range err.Matches {
244+
if e := ks.Unlock(a, auth); e == nil {
245+
match = &err.Matches[i]
246246
break
247247
}
248248
}
249249
if match == nil {
250250
utils.Fatalf("None of the listed files could be unlocked.")
251+
return accounts.Account{}
251252
}
252253
fmt.Printf("Your password unlocked %s\n", match.URL)
253254
fmt.Println("In order to avoid this warning, you need to remove the following duplicate key files:")

cmd/geth/config_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ func TestFlagsConfig(t *testing.T) {
252252
case cli.UintFlag:
253253
set.Uint(f.Name, f.Value+10, f.Usage)
254254
default:
255-
t.Log(fmt.Sprintf("unknown %t", f))
255+
t.Logf("unknown %t", f)
256256
t.Fail()
257257
}
258258
}

0 commit comments

Comments
 (0)