From bd9f1d0d4c9ae1ceb689581c8c10ea4995e4cfc7 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Thu, 21 Sep 2017 15:21:08 -0400 Subject: [PATCH 1/7] metalinter: add linter to Makefile like tendermint --- Makefile | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 9c9a7c575..cbdc66e1c 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,7 @@ GOTOOLS = \ github.com/mitchellh/gox \ - github.com/Masterminds/glide + github.com/Masterminds/glide \ + github.com/alecthomas/gometalinter all: protoc install test @@ -50,4 +51,38 @@ get_vendor_deps: @ go get github.com/Masterminds/glide @ glide install +metalinter: tools + @gometalinter --install + gometalinter --vendor --deadline=600s --enable-all --disable=lll ./... + +metalinter_test: tools + @gometalinter --install + gometalinter --vendor --deadline=600s --disable-all \ + --enable=deadcode \ + --enable=gas \ + --enable=goimports \ + --enable=gosimple \ + --enable=gotype \ + --enable=ineffassign \ + --enable=misspell \ + --enable=safesql \ + --enable=structcheck \ + --enable=varcheck \ + ./... + + #--enable=aligncheck \ + #--enable=dupl \ + #--enable=errcheck \ + #--enable=goconst \ + #--enable=gocyclo \ + #--enable=golint \ <== comments on anything exported + #--enable=interfacer \ + #--enable=megacheck \ + #--enable=staticcheck \ + #--enable=unconvert \ + #--enable=unparam \ + #--enable=unused \ + #--enable=vet \ + #--enable=vetshadow \ + .PHONY: all build test fmt lint get_deps tools From 6a378d30f3d1eb962144a5b585d177335b0f4868 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Thu, 21 Sep 2017 15:26:43 -0400 Subject: [PATCH 2/7] linting: cover the basics --- client/grpc_client.go | 2 +- client/socket_client.go | 16 +++++----------- cmd/abci-cli/abci-cli.go | 4 ++-- example/block_aware/block_aware_app.go | 1 - example/dummy/dummy_test.go | 2 +- example/dummy/persistent_dummy.go | 7 ++----- tests/test_app/app.go | 4 ++-- 7 files changed, 13 insertions(+), 23 deletions(-) diff --git a/client/grpc_client.go b/client/grpc_client.go index a990455f3..e9a74026c 100644 --- a/client/grpc_client.go +++ b/client/grpc_client.go @@ -113,7 +113,7 @@ func (cli *grpcClient) SetResponseCallback(resCb Callback) { //---------------------------------------- // GRPC calls are synchronous, but some callbacks expect to be called asynchronously // (eg. the mempool expects to be able to lock to remove bad txs from cache). -// To accomodate, we finish each call in its own go-routine, +// To accommodate, we finish each call in its own go-routine, // which is expensive, but easy - if you want something better, use the socket protocol! // maybe one day, if people really want it, we use grpc streams, // but hopefully not :D diff --git a/client/socket_client.go b/client/socket_client.go index 1da93494a..74df93269 100644 --- a/client/socket_client.go +++ b/client/socket_client.go @@ -19,9 +19,9 @@ const ( LOG = "" ) -const reqQueueSize = 256 // TODO make configurable -const maxResponseSize = 1048576 // 1MB TODO make configurable -const flushThrottleMS = 20 // Don't wait longer than... +const reqQueueSize = 256 // TODO make configurable +// const maxResponseSize = 1048576 // 1MB TODO make configurable +const flushThrottleMS = 20 // Don't wait longer than... // This is goroutine-safe, but users should beware that // the application in general is not meant to be interfaced @@ -355,19 +355,13 @@ func (cli *socketClient) CommitSync() (res types.Result) { func (cli *socketClient) InitChainSync(params types.RequestInitChain) (err error) { cli.queueRequest(types.ToRequestInitChain(params)) cli.FlushSync() - if err := cli.Error(); err != nil { - return err - } - return nil + return cli.Error() } func (cli *socketClient) BeginBlockSync(params types.RequestBeginBlock) (err error) { cli.queueRequest(types.ToRequestBeginBlock(params)) cli.FlushSync() - if err := cli.Error(); err != nil { - return err - } - return nil + return cli.Error() } func (cli *socketClient) EndBlockSync(height uint64) (resEndBlock types.ResponseEndBlock, err error) { diff --git a/cmd/abci-cli/abci-cli.go b/cmd/abci-cli/abci-cli.go index 0f1dd82d0..b96c0346e 100644 --- a/cmd/abci-cli/abci-cli.go +++ b/cmd/abci-cli/abci-cli.go @@ -183,12 +183,12 @@ func badCmd(c *cli.Context, cmd string) { //Generates new Args array based off of previous call args to maintain flag persistence func persistentArgs(line []byte) []string { - //generate the arguments to run from orginal os.Args + // generate the arguments to run from original os.Args // to maintain flag arguments args := os.Args args = args[:len(args)-1] // remove the previous command argument - if len(line) > 0 { //prevents introduction of extra space leading to argument parse errors + if len(line) > 0 { // prevents introduction of extra space leading to argument parse errors args = append(args, strings.Split(string(line), " ")...) } return args diff --git a/example/block_aware/block_aware_app.go b/example/block_aware/block_aware_app.go index c88f22896..8a031da2f 100644 --- a/example/block_aware/block_aware_app.go +++ b/example/block_aware/block_aware_app.go @@ -57,7 +57,6 @@ func (app *ChainAwareApplication) Query(reqQuery types.RequestQuery) (resQuery t func (app *ChainAwareApplication) BeginBlock(reqBeginBlock types.RequestBeginBlock) { app.beginCount++ - return } func (app *ChainAwareApplication) EndBlock(height uint64) (resEndBlock types.ResponseEndBlock) { diff --git a/example/dummy/dummy_test.go b/example/dummy/dummy_test.go index 495e7858e..a616025e1 100644 --- a/example/dummy/dummy_test.go +++ b/example/dummy/dummy_test.go @@ -166,7 +166,7 @@ func TestValSetChanges(t *testing.T) { makeApplyBlock(t, dummy, 3, diff, tx1) - vals1 = append([]*types.Validator{v1}, vals1[1:len(vals1)]...) + vals1 = append([]*types.Validator{v1}, vals1[1:]...) vals2 = dummy.Validators() valsEqual(t, vals1, vals2) diff --git a/example/dummy/persistent_dummy.go b/example/dummy/persistent_dummy.go index 303876e80..371cf4ce4 100644 --- a/example/dummy/persistent_dummy.go +++ b/example/dummy/persistent_dummy.go @@ -187,10 +187,7 @@ func MakeValSetChangeTx(pubkey []byte, power uint64) []byte { } func isValidatorTx(tx []byte) bool { - if strings.HasPrefix(string(tx), ValidatorSetChangePrefix) { - return true - } - return false + return strings.HasPrefix(string(tx), ValidatorSetChangePrefix) } // format is "val:pubkey1/power1,addr2/power2,addr3/power3"tx @@ -232,7 +229,7 @@ func (app *PersistentDummyApplication) updateValidator(v *types.Validator) types app.app.state.Set(key, value.Bytes()) } - // we only update the changes array if we succesfully updated the tree + // we only update the changes array if we successfully updated the tree app.changes = append(app.changes, v) return types.OK diff --git a/tests/test_app/app.go b/tests/test_app/app.go index 281c9dcb1..30202b6a1 100644 --- a/tests/test_app/app.go +++ b/tests/test_app/app.go @@ -80,7 +80,7 @@ func deliverTx(client abcicli.Client, txBytes []byte, codeExp types.CodeType, da } } -func checkTx(client abcicli.Client, txBytes []byte, codeExp types.CodeType, dataExp []byte) { +/*func checkTx(client abcicli.Client, txBytes []byte, codeExp types.CodeType, dataExp []byte) { res := client.CheckTxSync(txBytes) code, data, log := res.Code, res.Data, res.Log if res.IsErr() { @@ -94,4 +94,4 @@ func checkTx(client abcicli.Client, txBytes []byte, codeExp types.CodeType, data panic(fmt.Sprintf("CheckTx response data was unexpected. Got %X expected %X", data, dataExp)) } -} +}*/ From 36e96c5bf1b104650bd2a43c7e7d7995964df933 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Thu, 21 Sep 2017 15:32:06 -0400 Subject: [PATCH 3/7] linting: catch some errors --- Makefile | 2 +- client/grpc_client.go | 4 +++- client/socket_client.go | 4 +++- server/grpc_server.go | 4 +++- server/socket_server.go | 4 +++- 5 files changed, 13 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index cbdc66e1c..13f5dae74 100644 --- a/Makefile +++ b/Makefile @@ -58,6 +58,7 @@ metalinter: tools metalinter_test: tools @gometalinter --install gometalinter --vendor --deadline=600s --disable-all \ + --enable=aligncheck \ --enable=deadcode \ --enable=gas \ --enable=goimports \ @@ -70,7 +71,6 @@ metalinter_test: tools --enable=varcheck \ ./... - #--enable=aligncheck \ #--enable=dupl \ #--enable=errcheck \ #--enable=goconst \ diff --git a/client/grpc_client.go b/client/grpc_client.go index e9a74026c..31ca3db28 100644 --- a/client/grpc_client.go +++ b/client/grpc_client.go @@ -41,7 +41,9 @@ func dialerFunc(addr string, timeout time.Duration) (net.Conn, error) { } func (cli *grpcClient) OnStart() error { - cli.BaseService.OnStart() + if err := cli.BaseService.OnStart(); err != nil { + return err + } RETRY_LOOP: for { diff --git a/client/socket_client.go b/client/socket_client.go index 74df93269..4c7265cd0 100644 --- a/client/socket_client.go +++ b/client/socket_client.go @@ -57,7 +57,9 @@ func NewSocketClient(addr string, mustConnect bool) *socketClient { } func (cli *socketClient) OnStart() error { - cli.BaseService.OnStart() + if err := cli.BaseService.OnStart(); err != nil { + return err + } var err error var conn net.Conn diff --git a/server/grpc_server.go b/server/grpc_server.go index 90346d690..ac3d728ac 100644 --- a/server/grpc_server.go +++ b/server/grpc_server.go @@ -37,7 +37,9 @@ func NewGRPCServer(protoAddr string, app types.ABCIApplicationServer) cmn.Servic // OnStart starts the gRPC service func (s *GRPCServer) OnStart() error { - s.BaseService.OnStart() + if err := s.BaseService.OnStart(); err != nil { + return err + } ln, err := net.Listen(s.proto, s.addr) if err != nil { return err diff --git a/server/socket_server.go b/server/socket_server.go index 88011a056..4b64a5f0d 100644 --- a/server/socket_server.go +++ b/server/socket_server.go @@ -44,7 +44,9 @@ func NewSocketServer(protoAddr string, app types.Application) cmn.Service { } func (s *SocketServer) OnStart() error { - s.BaseService.OnStart() + if err := s.BaseService.OnStart(); err != nil { + return err + } ln, err := net.Listen(s.proto, s.addr) if err != nil { return err From 65eb7e89745205aa30c3bba38c377fea0f233056 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Thu, 21 Sep 2017 15:46:51 -0400 Subject: [PATCH 4/7] linted, somewhat --- Makefile | 12 ++++++------ circle.yml | 1 + tests/test_app/app.go | 2 +- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index 13f5dae74..418ba3ee0 100644 --- a/Makefile +++ b/Makefile @@ -61,28 +61,28 @@ metalinter_test: tools --enable=aligncheck \ --enable=deadcode \ --enable=gas \ + --enable=goconst \ --enable=goimports \ --enable=gosimple \ --enable=gotype \ --enable=ineffassign \ + --enable=megacheck \ --enable=misspell \ + --enable=staticcheck \ --enable=safesql \ --enable=structcheck \ + --enable=unconvert \ + --enable=unused \ --enable=varcheck \ + --enable=vetshadow \ ./... #--enable=dupl \ #--enable=errcheck \ - #--enable=goconst \ #--enable=gocyclo \ #--enable=golint \ <== comments on anything exported #--enable=interfacer \ - #--enable=megacheck \ - #--enable=staticcheck \ - #--enable=unconvert \ #--enable=unparam \ - #--enable=unused \ #--enable=vet \ - #--enable=vetshadow \ .PHONY: all build test fmt lint get_deps tools diff --git a/circle.yml b/circle.yml index 6355bc041..ab89e13db 100644 --- a/circle.yml +++ b/circle.yml @@ -18,4 +18,5 @@ checkout: test: override: - "go version" + - "cd $REPO && make metalinter_test" - "cd $REPO && make test_integrations" diff --git a/tests/test_app/app.go b/tests/test_app/app.go index 30202b6a1..11ade88cf 100644 --- a/tests/test_app/app.go +++ b/tests/test_app/app.go @@ -59,7 +59,7 @@ func commit(client abcicli.Client, hashExp []byte) { res := client.CommitSync() _, data, log := res.Code, res.Data, res.Log if res.IsErr() { - panic(fmt.Sprintf("committing %v\nlog: %v", log)) + panic(fmt.Sprintf("committing %v\nlog: %v", log, res.Log)) } if !bytes.Equal(res.Data, hashExp) { panic(fmt.Sprintf("Commit hash was unexpected. Got %X expected %X", From 0e7d974410cbb67eefa227de0fc3be8cc7ff604a Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Thu, 21 Sep 2017 16:02:01 -0400 Subject: [PATCH 5/7] install --- circle.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/circle.yml b/circle.yml index ab89e13db..daa4218e0 100644 --- a/circle.yml +++ b/circle.yml @@ -18,5 +18,5 @@ checkout: test: override: - "go version" - - "cd $REPO && make metalinter_test" + - "cd $REPO && make get_vendor_deps && make metalinter_test" - "cd $REPO && make test_integrations" From a3ac8254900d7ab7a36ae38f30b8d113e485f106 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Fri, 22 Sep 2017 09:14:20 -0400 Subject: [PATCH 6/7] small fix --- tests/test_app/app.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_app/app.go b/tests/test_app/app.go index 11ade88cf..ab7947690 100644 --- a/tests/test_app/app.go +++ b/tests/test_app/app.go @@ -59,7 +59,7 @@ func commit(client abcicli.Client, hashExp []byte) { res := client.CommitSync() _, data, log := res.Code, res.Data, res.Log if res.IsErr() { - panic(fmt.Sprintf("committing %v\nlog: %v", log, res.Log)) + panic(fmt.Sprintf("committing err %v\n", res)) } if !bytes.Equal(res.Data, hashExp) { panic(fmt.Sprintf("Commit hash was unexpected. Got %X expected %X", From fe426de5d47af1593448fded9f8e9ef07ead7762 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Fri, 22 Sep 2017 09:45:50 -0400 Subject: [PATCH 7/7] lint: couple more fixes --- tests/test_app/app.go | 2 +- types/application.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_app/app.go b/tests/test_app/app.go index ab7947690..7fc3ace0a 100644 --- a/tests/test_app/app.go +++ b/tests/test_app/app.go @@ -57,7 +57,7 @@ func setOption(client abcicli.Client, key, value string) { func commit(client abcicli.Client, hashExp []byte) { res := client.CommitSync() - _, data, log := res.Code, res.Data, res.Log + _, data, _ := res.Code, res.Data, res.Log if res.IsErr() { panic(fmt.Sprintf("committing err %v\n", res)) } diff --git a/types/application.go b/types/application.go index 8f17bf502..b596685c8 100644 --- a/types/application.go +++ b/types/application.go @@ -1,4 +1,4 @@ -package types +package types // nolint: goimports import ( context "golang.org/x/net/context" @@ -18,7 +18,7 @@ type Application interface { // Consensus Connection InitChain(RequestInitChain) // Initialize blockchain with validators and other info from TendermintCore BeginBlock(RequestBeginBlock) // Signals the beginning of a block - DeliverTx(tx []byte) Result // Deliver a tx for full processing + DeliverTx(tx []byte) Result // Deliver a tx for full processing EndBlock(height uint64) ResponseEndBlock // Signals the end of a block, returns changes to the validator set Commit() Result // Commit the state and return the application Merkle root hash }