diff --git a/Makefile b/Makefile index 8e43dd11a..25773ed36 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,37 @@ 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=gosimple \ + --enable=ineffassign \ + --enable=interfacer \ + --enable=megacheck \ + --enable=misspell \ + --enable=staticcheck \ + --enable=safesql \ + --enable=structcheck \ + --enable=unconvert \ + --enable=unused \ + --enable=varcheck \ + --enable=vetshadow \ + --enable=vet \ + ./... + + #--enable=aligncheck \ + #--enable=dupl \ + #--enable=errcheck \ + #--enable=gocyclo \ + #--enable=goimports \ + #--enable=golint \ <== comments on anything exported + #--enable=gotype \ + #--enable=unparam \ diff --git a/autofile/autofile_test.go b/autofile/autofile_test.go index 8f8017e1b..8f453dd07 100644 --- a/autofile/autofile_test.go +++ b/autofile/autofile_test.go @@ -1,20 +1,20 @@ package autofile import ( - . "github.com/tendermint/tmlibs/common" "os" "sync/atomic" "syscall" "testing" "time" + + cmn "github.com/tendermint/tmlibs/common" ) func TestSIGHUP(t *testing.T) { // First, create an AutoFile writing to a tempfile dir - file, name := Tempfile("sighup_test") - err := file.Close() - if err != nil { + file, name := cmn.Tempfile("sighup_test") + if err := file.Close(); err != nil { t.Fatalf("Error creating tempfile: %v", err) } // Here is the actual AutoFile @@ -57,17 +57,15 @@ func TestSIGHUP(t *testing.T) { if err != nil { t.Fatalf("Error writing to autofile: %v", err) } - err = af.Close() - if err != nil { + if err := af.Close(); err != nil { t.Fatalf("Error closing autofile") } // Both files should exist - if body := MustReadFile(name + "_old"); string(body) != "Line 1\nLine 2\n" { + if body := cmn.MustReadFile(name + "_old"); string(body) != "Line 1\nLine 2\n" { t.Errorf("Unexpected body %s", body) } - if body := MustReadFile(name); string(body) != "Line 3\nLine 4\n" { + if body := cmn.MustReadFile(name); string(body) != "Line 3\nLine 4\n" { t.Errorf("Unexpected body %s", body) } - } diff --git a/autofile/group.go b/autofile/group.go index ce3e30009..eedb67b50 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,9 +567,8 @@ 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 { - return "", err + if err1 := gr.openFile(gr.curIndex + 1); err1 != nil { + return "", err1 } if len(bytesRead) > 0 && bytesRead[len(bytesRead)-1] == byte('\n') { return linePrefix + string(bytesRead[:len(bytesRead)-1]), nil diff --git a/autofile/group_test.go b/autofile/group_test.go index 92e259701..0cfcef72f 100644 --- a/autofile/group_test.go +++ b/autofile/group_test.go @@ -77,8 +77,7 @@ func TestCheckHeadSizeLimit(t *testing.T) { assertGroupInfo(t, g.ReadGroupInfo(), 0, 0, 999000, 999000) // Write 1000 more bytes. - err := g.WriteLine(RandStr(999)) - if err != nil { + if err := g.WriteLine(RandStr(999)); err != nil { t.Fatal("Error appending to head", err) } g.Flush() @@ -88,8 +87,7 @@ func TestCheckHeadSizeLimit(t *testing.T) { assertGroupInfo(t, g.ReadGroupInfo(), 0, 1, 1000000, 0) // Write 1000 more bytes. - err = g.WriteLine(RandStr(999)) - if err != nil { + if err := g.WriteLine(RandStr(999)); err != nil { t.Fatal("Error appending to head", err) } g.Flush() @@ -100,8 +98,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) } } @@ -113,7 +110,7 @@ func TestCheckHeadSizeLimit(t *testing.T) { assertGroupInfo(t, g.ReadGroupInfo(), 0, 2, 2000000, 0) // Write 1000 more bytes. - _, err = g.Head.Write([]byte(RandStr(999) + "\n")) + _, err := g.Head.Write([]byte(RandStr(999) + "\n")) if err != nil { t.Fatal("Error appending to head", err) } diff --git a/autofile/sighup_watcher.go b/autofile/sighup_watcher.go index facc238d5..56fbd4d86 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/circle.yml b/circle.yml index 23ac4bd9f..8e3ad168b 100644 --- a/circle.yml +++ b/circle.yml @@ -18,4 +18,4 @@ dependencies: test: override: - "go version" - - "cd $PROJECT_PATH && make get_vendor_deps && make test" + - "cd $PROJECT_PATH && make get_vendor_deps && make metalinter_test && make test" diff --git a/cli/flags/log_level_test.go b/cli/flags/log_level_test.go index 458a9e24d..faf9b19db 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 ab5cf4b26..2063cf465 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 5de6fa2fa..e2a140dd0 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 3a1b09542..039342a67 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 73761fb1e..4272f6062 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 71ee88422..e0a009264 100644 --- a/common/os.go +++ b/common/os.go @@ -9,6 +9,7 @@ import ( "os/signal" "path/filepath" "strings" + "syscall" ) var ( @@ -17,8 +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, os.Interrupt, syscall.SIGTERM) go func() { for sig := range c { fmt.Printf("captured %v, exiting...\n", sig) @@ -84,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/service.go b/common/service.go index 71fc03cb9..8d4de30a8 100644 --- a/common/service.go +++ b/common/service.go @@ -140,18 +140,16 @@ func (bs *BaseService) OnStop() {} // Implements Service func (bs *BaseService) Reset() (bool, error) { - if atomic.CompareAndSwapUint32(&bs.stopped, 1, 0) { - // whether or not we've started, we can reset - atomic.CompareAndSwapUint32(&bs.started, 1, 0) - - bs.Quit = make(chan struct{}) - return true, bs.impl.OnReset() - } else { + if !atomic.CompareAndSwapUint32(&bs.stopped, 1, 0) { bs.Logger.Debug(Fmt("Can't reset %v. Not stopped", bs.name), "impl", bs.impl) return false, nil } - // never happens - return false, nil + + // whether or not we've started, we can reset + atomic.CompareAndSwapUint32(&bs.started, 1, 0) + + bs.Quit = make(chan struct{}) + return true, bs.impl.OnReset() } // Implements Service diff --git a/common/string.go b/common/string.go index 2818f5ed5..1ab91f15a 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/db/c_level_db_test.go b/db/c_level_db_test.go index 0ee6d6414..e7336cc5f 100644 --- a/db/c_level_db_test.go +++ b/db/c_level_db_test.go @@ -50,7 +50,7 @@ func BenchmarkRandomReadsWrites2(b *testing.B) { //fmt.Printf("Get %X -> %X\n", idxBytes, valBytes) if val == 0 { if !bytes.Equal(valBytes, nil) { - b.Errorf("Expected %X for %v, got %X", + b.Errorf("Expected %v for %v, got %X", nil, idx, valBytes) break } diff --git a/db/go_level_db_test.go b/db/go_level_db_test.go index 0603b2d4f..2cd3192c3 100644 --- a/db/go_level_db_test.go +++ b/db/go_level_db_test.go @@ -49,7 +49,7 @@ func BenchmarkRandomReadsWrites(b *testing.B) { //fmt.Printf("Get %X -> %X\n", idxBytes, valBytes) if val == 0 { if !bytes.Equal(valBytes, nil) { - b.Errorf("Expected %X for %v, got %X", + b.Errorf("Expected %v for %v, got %X", nil, idx, valBytes) break } diff --git a/events/events_test.go b/events/events_test.go index c1b48b16f..dee50e5bd 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 6d4934a8a..db40337c9 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 fafafacb0..8d8b3b27c 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 14028d756..2b464a6b0 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 584b34bef..6b0838ca8 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 b5520f723..8106246d6 100644 --- a/merkle/simple_tree.go +++ b/merkle/simple_tree.go @@ -31,8 +31,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 b3e0aef11..24cf35280 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 37ce75cd9..c86e4a47f 100644 --- a/pubsub/query/query.peg.go +++ b/pubsub/query/query.peg.go @@ -1,3 +1,4 @@ +// nolint package query import (