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

build: enable goimports and varcheck linters #16446

Merged
merged 17 commits into from
Apr 17, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ ios:
test: all
build/env.sh go run build/ci.go test

lint: ## Run linters.
build/env.sh go run build/ci.go lint

clean:
rm -fr build/_workspace/pkg/ $(GOBIN)/*

Expand Down
2 changes: 0 additions & 2 deletions accounts/usbwallet/ledger.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,9 @@ const (
ledgerOpGetConfiguration ledgerOpcode = 0x06 // Returns specific wallet application configuration

ledgerP1DirectlyFetchAddress ledgerParam1 = 0x00 // Return address directly from the wallet
ledgerP1ConfirmFetchAddress ledgerParam1 = 0x01 // Require a user confirmation before returning the address
ledgerP1InitTransactionData ledgerParam1 = 0x00 // First transaction data block for signing
ledgerP1ContTransactionData ledgerParam1 = 0x80 // Subsequent transaction data block for signing
ledgerP2DiscardAddressChainCode ledgerParam2 = 0x00 // Do not return the chain code along with the address
ledgerP2ReturnAddressChainCode ledgerParam2 = 0x01 // Require a user confirmation before returning the address
)

// errLedgerReplyInvalidHeader is the error message returned by a Ledger data exchange
Expand Down
5 changes: 4 additions & 1 deletion build/ci.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,10 @@ func doLint(cmdline []string) {
// Run fast linters batched together
configs := []string{
"--vendor",
"--tests",
"--disable-all",
"--enable=goimports",
"--enable=varcheck",
"--enable=vet",
"--enable=gofmt",
"--enable=misspell",
Expand All @@ -340,7 +343,7 @@ func doLint(cmdline []string) {

// Run slow linters one by one
for _, linter := range []string{"unconvert", "gosimple"} {
configs = []string{"--vendor", "--deadline=10m", "--disable-all", "--enable=" + linter}
configs = []string{"--vendor", "--tests", "--deadline=10m", "--disable-all", "--enable=" + linter}
build.MustRunCommand(filepath.Join(GOBIN, "gometalinter.v2"), append(configs, packages...)...)
}
}
Expand Down
18 changes: 18 additions & 0 deletions build/goimports.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#!/usr/bin/env bash

find_files() {
find . -not \( \
\( \
-wholename '.github' \
-o -wholename './build/_workspace' \
-o -wholename './build/bin' \
-o -wholename './crypto/bn256' \
-o -wholename '*/vendor/*' \
\) -prune \
\) -name '*.go'
}

GOFMT="gofmt -s -w";
GOIMPORTS="goimports -w";
find_files | xargs $GOFMT;
find_files | xargs $GOIMPORTS;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the purpose of this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the reasons of linting errors was due to having code not properly formatted and not properly imported. There was no standard set for that. Now there is.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

goimports is pretty much the standard tool used by any Go development environment. I don't really see the value of providing an extra utility method in our repo that is covered by the developer's editor anyway. I completely agree that the linter should enforce it, but its the developer's job to configure their dev environment, not ours.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I started to work on this issue, I found that the devs are not really doing what you said is their job...

consensus/consensus.go:1::warning: file is not goimported (goimports)
swarm/fuse/fuse_dir.go:1::warning: file is not goimported (goimports)
swarm/fuse/swarmfs.go:1::warning: file is not goimported (goimports)

So I ran the goimports the way "I think" is the best way, splitting the imports in 3. Just to find out that this is not the proffered way here.
Also, running goimports ./.. throws some errors, that I'm not sure how to fix, or even if they are really errors, so I added the script:

goimports -w ./..
../go-ethereum/crypto/bn256/bn256_fast.go:26:9: expected type, found '='
../go-ethereum/crypto/bn256/bn256_fast.go:30:9: expected type, found '='
../go-ethereum/crypto/bn256/bn256_fast.go:34:2: expected declaration, found 'return'
../go-ethereum/crypto/bn256/bn256_slow.go:26:9: expected type, found '='
../go-ethereum/crypto/bn256/bn256_slow.go:30:9: expected type, found '='
../go-ethereum/crypto/bn256/bn256_slow.go:34:2: expected declaration, found 'return'

So I added a "default" script to get this sorted and leave no room for errors.

4 changes: 0 additions & 4 deletions cmd/ethkey/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,6 @@ var (
Name: "json",
Usage: "output JSON instead of human-readable format",
}
messageFlag = cli.StringFlag{
Name: "message",
Usage: "the file that contains the message to sign/verify",
}
)

func main() {
Expand Down
3 changes: 2 additions & 1 deletion consensus/consensus.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,13 @@
package consensus

import (
"math/big"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core/state"
"github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/params"
"github.com/ethereum/go-ethereum/rpc"
"math/big"
)

// ChainReader defines a small collection of methods needed to access the local
Expand Down
6 changes: 0 additions & 6 deletions core/asm/compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package asm

import (
"errors"
"fmt"
"math/big"
"os"
Expand Down Expand Up @@ -264,11 +263,6 @@ func (err compileError) Error() string {
return fmt.Sprintf("%d syntax error: unexpected %v, expected %v", err.lineno, err.got, err.want)
}

var (
errExpBol = errors.New("expected beginning of line")
errExpElementOrLabel = errors.New("expected beginning of line")
)

func compileErr(c token, got, want string) error {
return compileError{
got: got,
Expand Down
2 changes: 0 additions & 2 deletions core/tx_pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ import (
const (
// chainHeadChanSize is the size of channel listening to ChainHeadEvent.
chainHeadChanSize = 10
// rmTxChanSize is the size of channel listening to RemovedTransactionEvent.
rmTxChanSize = 10
)

var (
Expand Down
1 change: 0 additions & 1 deletion core/types/transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import (

var (
ErrInvalidSig = errors.New("invalid transaction v, r, s values")
errNoSigner = errors.New("missing signing methods")
)

// deriveSigner makes a *best* guess about which signer to use.
Expand Down
1 change: 0 additions & 1 deletion core/vm/instructions.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import (
var (
bigZero = new(big.Int)
tt255 = math.BigPow(2, 255)
tt256 = math.BigPow(2, 256)
errWriteProtection = errors.New("evm: write protection")
errReturnDataOutOfBounds = errors.New("evm: return data out of bounds")
errExecutionReverted = errors.New("evm: execution reverted")
Expand Down
10 changes: 9 additions & 1 deletion crypto/bn256/cloudflare/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@ package bn256

import (
"crypto/rand"
"testing"

"github.com/stretchr/testify/require"
)

func ExamplePair() {
func TestExamplePair(t *testing.T) {
// This implements the tripartite Diffie-Hellman algorithm from "A One
// Round Protocol for Tripartite Diffie-Hellman", A. Joux.
// http://www.springerlink.com/content/cddc57yyva0hburb/fulltext.pdf
Expand Down Expand Up @@ -40,4 +43,9 @@ func ExamplePair() {
k3.ScalarMult(k3, c)

// k1, k2 and k3 will all be equal.

require.Equal(t, k1, k2)
require.Equal(t, k1, k3)

require.Equal(t, len(np), 4) //Avoid gometalinter varcheck err on np
}
10 changes: 0 additions & 10 deletions les/bloombits.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,3 @@ func (eth *LightEthereum) startBloomHandlers() {
}()
}
}

const (
// bloomConfirms is the number of confirmation blocks before a bloom section is
// considered probably final and its rotated bits are calculated.
bloomConfirms = 256

// bloomThrottling is the time to wait between processing two consecutive index
// sections. It's useful during chain upgrades to prevent disk overload.
bloomThrottling = 100 * time.Millisecond
)
4 changes: 0 additions & 4 deletions les/serverpool.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ const (
// and a short term value which is adjusted exponentially with a factor of
// pstatRecentAdjust with each dial/connection and also returned exponentially
// to the average with the time constant pstatReturnToMeanTC
pstatRecentAdjust = 0.1
pstatReturnToMeanTC = time.Hour
// node address selection weight is dropped by a factor of exp(-addrFailDropLn) after
// each unsuccessful connection (restored after a successful one)
Expand All @@ -83,9 +82,6 @@ const (
responseScoreTC = time.Millisecond * 100
delayScoreTC = time.Second * 5
timeoutPow = 10
// peerSelectMinWeight is added to calculated weights at request peer selection
// to give poorly performing peers a little chance of coming back
peerSelectMinWeight = 0.005
// initStatsWeight is used to initialize previously unknown peers with good
// statistics to give a chance to prove themselves
initStatsWeight = 1
Expand Down
5 changes: 0 additions & 5 deletions les/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,6 @@ import (
"github.com/ethereum/go-ethereum/light"
)

const (
//forceSyncCycle = 10 * time.Second // Time interval to force syncs, even if few peers are available
minDesiredPeerCount = 5 // Amount of peers desired to start syncing
)

// syncer is responsible for periodically synchronising with the network, both
// downloading hashes and blocks as well as handling the announcement handler.
func (pm *ProtocolManager) syncer() {
Expand Down
1 change: 0 additions & 1 deletion p2p/discover/udp.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ var (
// Timeouts
const (
respTimeout = 500 * time.Millisecond
sendTimeout = 500 * time.Millisecond
expiration = 20 * time.Second

ntpFailureThreshold = 32 // Continuous timeouts after which to check NTP
Expand Down
4 changes: 1 addition & 3 deletions p2p/discv5/net.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import (
var (
errInvalidEvent = errors.New("invalid in current state")
errNoQuery = errors.New("no pending query")
errWrongAddress = errors.New("unknown sender address")
)

const (
Expand Down Expand Up @@ -828,11 +827,10 @@ type nodeEvent uint
//go:generate stringer -type=nodeEvent

const (
invalidEvent nodeEvent = iota // zero is reserved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just noticed another issue here: the nodeEvent String method is created by stringer. Please re-run go generate ./p2p/discv5 to update nodeevent_string.go.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm running into a error when running stringer:

p2p/discv5/database.go
p2p/discv5/database_test.go
p2p/discv5/net.go
stringer: checking package: database.go:23:2: could not import bytes (reading export data: /usr/local/go/pkg/linux_amd64/bytes.a: invalid encoding format in export data: got 'v'; want 'c' or 'd')
p2p/discv5/net.go:827: running "stringer": exit status 1

I tried to find a way out, but so far I found nothing, any ideas ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably need to rebuild your copy of stringer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fjl sorry for the delay, I got caught in other issue to work on. Just re-generated the nodeevent with stringer as you requested.
Please let me know if anything else is missing.


// Packet type events.
// These correspond to packet types in the UDP protocol.
pingPacket
pingPacket = iota + 1
pongPacket
findnodePacket
neighborsPacket
Expand Down
22 changes: 6 additions & 16 deletions p2p/discv5/nodeevent_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion p2p/discv5/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ const (
hashBits = len(common.Hash{}) * 8
nBuckets = hashBits + 1 // Number of buckets

maxBondingPingPongs = 16
maxFindnodeFailures = 5
)

Expand Down
16 changes: 4 additions & 12 deletions p2p/discv5/udp.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,25 +36,17 @@ const Version = 4

// Errors
var (
errPacketTooSmall = errors.New("too small")
errBadPrefix = errors.New("bad prefix")
errExpired = errors.New("expired")
errUnsolicitedReply = errors.New("unsolicited reply")
errUnknownNode = errors.New("unknown node")
errTimeout = errors.New("RPC timeout")
errClockWarp = errors.New("reply deadline too far in the future")
errClosed = errors.New("socket closed")
errPacketTooSmall = errors.New("too small")
errBadPrefix = errors.New("bad prefix")
errTimeout = errors.New("RPC timeout")
)

// Timeouts
const (
respTimeout = 500 * time.Millisecond
queryDelay = 1000 * time.Millisecond
expiration = 20 * time.Second

ntpFailureThreshold = 32 // Continuous timeouts after which to check NTP
ntpWarningCooldown = 10 * time.Minute // Minimum amount of time to pass before repeating NTP warning
driftThreshold = 10 * time.Second // Allowed clock drift before warning user
driftThreshold = 10 * time.Second // Allowed clock drift before warning user
)

// RPC request structures
Expand Down
7 changes: 1 addition & 6 deletions p2p/discv5/udp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"reflect"
"sync"
"testing"
"time"

"github.com/davecgh/go-spew/spew"
"github.com/ethereum/go-ethereum/common"
Expand All @@ -38,11 +37,7 @@ func init() {

// shared test variables
var (
futureExp = uint64(time.Now().Add(10 * time.Hour).Unix())
testTarget = NodeID{0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1}
testRemote = rpcEndpoint{IP: net.ParseIP("1.1.1.1").To4(), UDP: 1, TCP: 2}
testLocalAnnounced = rpcEndpoint{IP: net.ParseIP("2.2.2.2").To4(), UDP: 3, TCP: 4}
testLocal = rpcEndpoint{IP: net.ParseIP("3.3.3.3").To4(), UDP: 5, TCP: 6}
testLocal = rpcEndpoint{IP: net.ParseIP("3.3.3.3").To4(), UDP: 5, TCP: 6}
)

// type udpTest struct {
Expand Down
1 change: 0 additions & 1 deletion p2p/enr/enr.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ const ID_SECP256k1_KECCAK = ID("secp256k1-keccak") // the default identity schem

var (
errNoID = errors.New("unknown or unspecified identity scheme")
errInvalidSigsize = errors.New("invalid signature size")
errInvalidSig = errors.New("invalid signature")
errNotSorted = errors.New("record key/value pairs are not sorted by key")
errDuplicateKey = errors.New("record contains duplicate key")
Expand Down
2 changes: 0 additions & 2 deletions p2p/peer.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@ const (
discMsg = 0x01
pingMsg = 0x02
pongMsg = 0x03
getPeersMsg = 0x04
peersMsg = 0x05
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these unused? Are we sure this is correct here @fjl ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, they are unused

)

// protoHandshake is the RLP structure of the protocol handshake.
Expand Down
7 changes: 4 additions & 3 deletions swarm/fuse/fuse_dir.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,13 @@
package fuse

import (
"bazil.org/fuse"
"bazil.org/fuse/fs"
"golang.org/x/net/context"
"os"
"path/filepath"
"sync"

"bazil.org/fuse"
"bazil.org/fuse/fs"
"golang.org/x/net/context"
)

var (
Expand Down
3 changes: 2 additions & 1 deletion swarm/fuse/swarmfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@
package fuse

import (
"github.com/ethereum/go-ethereum/swarm/api"
"sync"
"time"

"github.com/ethereum/go-ethereum/swarm/api"
)

const (
Expand Down
1 change: 0 additions & 1 deletion swarm/storage/dbstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ const (

// key prefixes for leveldb storage
kpIndex = 0
kpData = 1
)

var (
Expand Down
5 changes: 0 additions & 5 deletions swarm/storage/netstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,6 @@ func NewNetStore(hash SwarmHasher, lstore *LocalStore, cloud CloudStore, params
}
}

const (
// maximum number of peers that a retrieved message is delivered to
requesterCount = 3
)

var (
// timeout interval before retrieval is timed out
searchTimeout = 3 * time.Second
Expand Down
4 changes: 0 additions & 4 deletions whisper/whisperv5/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,6 @@ import (
"github.com/ethereum/go-ethereum/rpc"
)

const (
filterTimeout = 300 // filters are considered timeout out after filterTimeout seconds
)

var (
ErrSymAsym = errors.New("specify either a symmetric or an asymmetric key")
ErrInvalidSymmetricKey = errors.New("invalid symmetric key")
Expand Down
Loading