From d6e03d2368e9d1fa11ed19f9a4d54e4c578d727c Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Fri, 22 Sep 2017 11:42:29 -0400 Subject: [PATCH] linting: add to Makefile & do some fixes --- Makefile | 43 ++++++++++++++++++++++++++++++++++++- autofile/autofile_test.go | 4 +++- autofile/group.go | 4 +--- autofile/group_test.go | 3 +-- autofile/sighup_watcher.go | 2 +- cli/flags/log_level_test.go | 2 -- clist/clist_test.go | 6 ++++-- common/cmap.go | 4 ++-- common/errors.go | 2 +- common/http_test.go | 2 +- common/os.go | 10 +++------ common/string.go | 5 +---- events/events_test.go | 15 ++++++------- flowrate/io_test.go | 5 +---- log/filter_test.go | 6 ++---- log/tmfmt_logger.go | 7 +++--- log/tracing_logger_test.go | 3 +-- merkle/simple_tree.go | 3 ++- process/util.go | 4 ++-- pubsub/query/query.peg.go | 3 +++ 20 files changed, 82 insertions(+), 51 deletions(-) diff --git a/Makefile b/Makefile index 8e43dd11..ba164ec1 100644 --- a/Makefile +++ b/Makefile @@ -1,11 +1,15 @@ .PHONY: all test get_vendor_deps ensure_tools GOTOOLS = \ - github.com/Masterminds/glide + github.com/Masterminds/glide \ + github.com/alecthomas/gometalinter + REPO:=github.com/tendermint/tmlibs all: test +NOVENDOR = go list github.com/tendermint/tmlibs/... | grep -v /vendor/ + test: go test `glide novendor` @@ -16,3 +20,40 @@ get_vendor_deps: ensure_tools ensure_tools: go get $(GOTOOLS) + +metalinter: ensure_tools + @gometalinter --install + gometalinter --vendor --deadline=600s --enable-all --disable=lll ./... + +metalinter_test: ensure_tools + @gometalinter --install + gometalinter --vendor --deadline=600s --disable-all \ + --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=interfacer \ + --enable=unparam \ + --enable=vet \ + ./... + + #--enable=aligncheck \ + #--enable=dupl \ + #--enable=errcheck \ + #--enable=gocyclo \ + #--enable=golint \ <== comments on anything exported + #--enable=interfacer \ + #--enable=unparam \ + #--enable=vet \ diff --git a/autofile/autofile_test.go b/autofile/autofile_test.go index 8f8017e1..c7aa93be 100644 --- a/autofile/autofile_test.go +++ b/autofile/autofile_test.go @@ -1,12 +1,14 @@ +// nolint: goimports package autofile import ( - . "github.com/tendermint/tmlibs/common" "os" "sync/atomic" "syscall" "testing" "time" + + . "github.com/tendermint/tmlibs/common" ) func TestSIGHUP(t *testing.T) { diff --git a/autofile/group.go b/autofile/group.go index ce3e3000..689b1cb9 100644 --- a/autofile/group.go +++ b/autofile/group.go @@ -107,7 +107,6 @@ func (g *Group) OnStart() error { func (g *Group) OnStop() { g.BaseService.OnStop() g.ticker.Stop() - return } func (g *Group) SetHeadSizeLimit(limit int64) { @@ -568,8 +567,7 @@ func (gr *GroupReader) ReadLine() (string, error) { bytesRead, err := gr.curReader.ReadBytes('\n') if err == io.EOF { // Open the next file - err := gr.openFile(gr.curIndex + 1) - if err != nil { + if err := gr.openFile(gr.curIndex + 1); err != nil { return "", err } if len(bytesRead) > 0 && bytesRead[len(bytesRead)-1] == byte('\n') { diff --git a/autofile/group_test.go b/autofile/group_test.go index 92e25970..91c6a0bb 100644 --- a/autofile/group_test.go +++ b/autofile/group_test.go @@ -100,8 +100,7 @@ func TestCheckHeadSizeLimit(t *testing.T) { // Write 1000 bytes 999 times. for i := 0; i < 999; i++ { - err := g.WriteLine(RandStr(999)) - if err != nil { + if err := g.WriteLine(RandStr(999)); err != nil { t.Fatal("Error appending to head", err) } } diff --git a/autofile/sighup_watcher.go b/autofile/sighup_watcher.go index facc238d..56fbd4d8 100644 --- a/autofile/sighup_watcher.go +++ b/autofile/sighup_watcher.go @@ -22,7 +22,7 @@ func initSighupWatcher() { signal.Notify(c, syscall.SIGHUP) go func() { - for _ = range c { + for range c { sighupWatchers.closeAll() atomic.AddInt32(&sighupCounter, 1) } diff --git a/cli/flags/log_level_test.go b/cli/flags/log_level_test.go index 458a9e24..faf9b19d 100644 --- a/cli/flags/log_level_test.go +++ b/cli/flags/log_level_test.go @@ -49,8 +49,6 @@ func TestParseLogLevel(t *testing.T) { t.Fatal(err) } - logger = logger - buf.Reset() logger.With("module", "wire").Debug("Kingpin") diff --git a/clist/clist_test.go b/clist/clist_test.go index ab5cf4b2..2063cf46 100644 --- a/clist/clist_test.go +++ b/clist/clist_test.go @@ -55,6 +55,7 @@ func TestSmall(t *testing.T) { This test is quite hacky because it relies on SetFinalizer which isn't guaranteed to run at all. */ +// nolint: megacheck func _TestGCFifo(t *testing.T) { const numElements = 1000000 @@ -102,6 +103,7 @@ func _TestGCFifo(t *testing.T) { This test is quite hacky because it relies on SetFinalizer which isn't guaranteed to run at all. */ +// nolint: megacheck func _TestGCRandom(t *testing.T) { const numElements = 1000000 @@ -132,7 +134,7 @@ func _TestGCRandom(t *testing.T) { for _, i := range rand.Perm(numElements) { el := els[i] l.Remove(el) - el = el.Next() + _ = el.Next() } runtime.GC() @@ -153,7 +155,7 @@ func TestScanRightDeleteRandom(t *testing.T) { l := New() stop := make(chan struct{}) - els := make([]*CElement, numElements, numElements) + els := make([]*CElement, numElements) for i := 0; i < numElements; i++ { el := l.PushBack(i) els[i] = el diff --git a/common/cmap.go b/common/cmap.go index 5de6fa2f..e2a140dd 100644 --- a/common/cmap.go +++ b/common/cmap.go @@ -10,7 +10,7 @@ type CMap struct { func NewCMap() *CMap { return &CMap{ - m: make(map[string]interface{}, 0), + m: make(map[string]interface{}), } } @@ -48,7 +48,7 @@ func (cm *CMap) Size() int { func (cm *CMap) Clear() { cm.l.Lock() defer cm.l.Unlock() - cm.m = make(map[string]interface{}, 0) + cm.m = make(map[string]interface{}) } func (cm *CMap) Values() []interface{} { diff --git a/common/errors.go b/common/errors.go index 3a1b0954..039342a6 100644 --- a/common/errors.go +++ b/common/errors.go @@ -21,7 +21,7 @@ func (se StackError) Error() string { // panic wrappers // A panic resulting from a sanity check means there is a programmer error -// and some gaurantee is not satisfied. +// and some guarantee is not satisfied. func PanicSanity(v interface{}) { panic(Fmt("Panicked on a Sanity Check: %v", v)) } diff --git a/common/http_test.go b/common/http_test.go index 73761fb1..4272f606 100644 --- a/common/http_test.go +++ b/common/http_test.go @@ -95,7 +95,7 @@ func TestWriteCode(t *testing.T) { common.WriteCode(w, &marshalFailer{}, code) wantCode := http.StatusBadRequest assert.Equal(t, w.Code, wantCode, "#%d", i) - assert.True(t, strings.Contains(string(w.Body.Bytes()), errFooFailed.Error()), + assert.True(t, strings.Contains(w.Body.String(), errFooFailed.Error()), "#%d: expected %q in the error message", i, errFooFailed) } } diff --git a/common/os.go b/common/os.go index 9c2bda50..8b7143f5 100644 --- a/common/os.go +++ b/common/os.go @@ -8,6 +8,7 @@ import ( "os" "os/signal" "strings" + "syscall" ) var ( @@ -17,7 +18,7 @@ var ( func TrapSignal(cb func()) { c := make(chan os.Signal, 1) signal.Notify(c, os.Interrupt) - signal.Notify(c, os.Kill) + signal.Notify(c, syscall.SIGTERM) go func() { for sig := range c { fmt.Printf("captured %v, exiting...\n", sig) @@ -83,12 +84,7 @@ func MustReadFile(filePath string) []byte { } func WriteFile(filePath string, contents []byte, mode os.FileMode) error { - err := ioutil.WriteFile(filePath, contents, mode) - if err != nil { - return err - } - // fmt.Printf("File written to %v.\n", filePath) - return nil + return ioutil.WriteFile(filePath, contents, mode) } func MustWriteFile(filePath string, contents []byte, mode os.FileMode) { diff --git a/common/string.go b/common/string.go index 2818f5ed..1ab91f15 100644 --- a/common/string.go +++ b/common/string.go @@ -31,10 +31,7 @@ func LeftPadString(s string, totalLength int) string { func IsHex(s string) bool { if len(s) > 2 && s[:2] == "0x" { _, err := hex.DecodeString(s[2:]) - if err != nil { - return false - } - return true + return err == nil } return false } diff --git a/events/events_test.go b/events/events_test.go index c1b48b16..dee50e5b 100644 --- a/events/events_test.go +++ b/events/events_test.go @@ -14,7 +14,7 @@ import ( func TestAddListenerForEventFireOnce(t *testing.T) { evsw := NewEventSwitch() started, err := evsw.Start() - if started == false || err != nil { + if !started || err != nil { t.Errorf("Failed to start EventSwitch, error: %v", err) } messages := make(chan EventData) @@ -34,7 +34,7 @@ func TestAddListenerForEventFireOnce(t *testing.T) { func TestAddListenerForEventFireMany(t *testing.T) { evsw := NewEventSwitch() started, err := evsw.Start() - if started == false || err != nil { + if !started || err != nil { t.Errorf("Failed to start EventSwitch, error: %v", err) } doneSum := make(chan uint64) @@ -63,7 +63,7 @@ func TestAddListenerForEventFireMany(t *testing.T) { func TestAddListenerForDifferentEvents(t *testing.T) { evsw := NewEventSwitch() started, err := evsw.Start() - if started == false || err != nil { + if !started || err != nil { t.Errorf("Failed to start EventSwitch, error: %v", err) } doneSum := make(chan uint64) @@ -108,7 +108,7 @@ func TestAddListenerForDifferentEvents(t *testing.T) { func TestAddDifferentListenerForDifferentEvents(t *testing.T) { evsw := NewEventSwitch() started, err := evsw.Start() - if started == false || err != nil { + if !started || err != nil { t.Errorf("Failed to start EventSwitch, error: %v", err) } doneSum1 := make(chan uint64) @@ -168,7 +168,7 @@ func TestAddDifferentListenerForDifferentEvents(t *testing.T) { func TestAddAndRemoveListener(t *testing.T) { evsw := NewEventSwitch() started, err := evsw.Start() - if started == false || err != nil { + if !started || err != nil { t.Errorf("Failed to start EventSwitch, error: %v", err) } doneSum1 := make(chan uint64) @@ -213,7 +213,7 @@ func TestAddAndRemoveListener(t *testing.T) { func TestRemoveListener(t *testing.T) { evsw := NewEventSwitch() started, err := evsw.Start() - if started == false || err != nil { + if !started || err != nil { t.Errorf("Failed to start EventSwitch, error: %v", err) } count := 10 @@ -266,7 +266,7 @@ func TestRemoveListener(t *testing.T) { func TestRemoveListenersAsync(t *testing.T) { evsw := NewEventSwitch() started, err := evsw.Start() - if started == false || err != nil { + if !started || err != nil { t.Errorf("Failed to start EventSwitch, error: %v", err) } doneSum1 := make(chan uint64) @@ -377,5 +377,4 @@ func fireEvents(evsw EventSwitch, event string, doneChan chan uint64, } doneChan <- sentSum close(doneChan) - return } diff --git a/flowrate/io_test.go b/flowrate/io_test.go index 6d4934a8..db40337c 100644 --- a/flowrate/io_test.go +++ b/flowrate/io_test.go @@ -171,10 +171,7 @@ func statusesAreEqual(s1 *Status, s2 *Status) bool { } func durationsAreEqual(d1 time.Duration, d2 time.Duration, maxDeviation time.Duration) bool { - if d2-d1 <= maxDeviation { - return true - } - return false + return d2-d1 <= maxDeviation } func ratesAreEqual(r1 int64, r2 int64, maxDeviation int64) bool { diff --git a/log/filter_test.go b/log/filter_test.go index fafafacb..8d8b3b27 100644 --- a/log/filter_test.go +++ b/log/filter_test.go @@ -73,8 +73,7 @@ func TestVariousLevels(t *testing.T) { func TestLevelContext(t *testing.T) { var buf bytes.Buffer - var logger log.Logger - logger = log.NewTMJSONLogger(&buf) + logger := log.NewTMJSONLogger(&buf) logger = log.NewFilter(logger, log.AllowError()) logger = logger.With("context", "value") @@ -93,8 +92,7 @@ func TestLevelContext(t *testing.T) { func TestVariousAllowWith(t *testing.T) { var buf bytes.Buffer - var logger log.Logger - logger = log.NewTMJSONLogger(&buf) + logger := log.NewTMJSONLogger(&buf) logger1 := log.NewFilter(logger, log.AllowError(), log.AllowInfoWith("context", "value")) logger1.With("context", "value").Info("foo", "bar", "baz") diff --git a/log/tmfmt_logger.go b/log/tmfmt_logger.go index 14028d75..2b464a6b 100644 --- a/log/tmfmt_logger.go +++ b/log/tmfmt_logger.go @@ -49,9 +49,10 @@ func (l tmfmtLogger) Log(keyvals ...interface{}) error { enc.Reset() defer tmfmtEncoderPool.Put(enc) + const unknown = "unknown" lvl := "none" - msg := "unknown" - module := "unknown" + msg := unknown + module := unknown // indexes of keys to skip while encoding later excludeIndexes := make([]int, 0) @@ -90,7 +91,7 @@ func (l tmfmtLogger) Log(keyvals ...interface{}) error { // Stopping ... - message enc.buf.WriteString(fmt.Sprintf("%c[%s] %-44s ", lvl[0]-32, time.Now().UTC().Format("01-02|15:04:05.000"), msg)) - if module != "unknown" { + if module != unknown { enc.buf.WriteString("module=" + module + " ") } diff --git a/log/tracing_logger_test.go b/log/tracing_logger_test.go index 584b34be..6b0838ca 100644 --- a/log/tracing_logger_test.go +++ b/log/tracing_logger_test.go @@ -14,8 +14,7 @@ import ( func TestTracingLogger(t *testing.T) { var buf bytes.Buffer - var logger log.Logger - logger = log.NewTMJSONLogger(&buf) + logger := log.NewTMJSONLogger(&buf) logger1 := log.NewTracingLogger(logger) err1 := errors.New("Courage is grace under pressure.") diff --git a/merkle/simple_tree.go b/merkle/simple_tree.go index b5520f72..b373743f 100644 --- a/merkle/simple_tree.go +++ b/merkle/simple_tree.go @@ -22,6 +22,7 @@ For larger datasets, use IAVLTree. */ +// nolint: goimports package merkle import ( @@ -31,8 +32,8 @@ import ( "golang.org/x/crypto/ripemd160" - . "github.com/tendermint/tmlibs/common" "github.com/tendermint/go-wire" + . "github.com/tendermint/tmlibs/common" ) func SimpleHashFromTwoHashes(left []byte, right []byte) []byte { diff --git a/process/util.go b/process/util.go index b3e0aef1..24cf3528 100644 --- a/process/util.go +++ b/process/util.go @@ -15,8 +15,8 @@ func Run(dir string, command string, args []string) (string, bool, error) { <-proc.WaitCh if proc.ExitState.Success() { - return string(outFile.Bytes()), true, nil + return outFile.String(), true, nil } else { - return string(outFile.Bytes()), false, nil + return outFile.String(), false, nil } } diff --git a/pubsub/query/query.peg.go b/pubsub/query/query.peg.go index 37ce75cd..8c3e83ef 100644 --- a/pubsub/query/query.peg.go +++ b/pubsub/query/query.peg.go @@ -1,3 +1,6 @@ +// nolint: megacheck +// nolint: varcheck +// nolint: deadcode package query import (