From d6e03d2368e9d1fa11ed19f9a4d54e4c578d727c Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Fri, 22 Sep 2017 11:42:29 -0400 Subject: [PATCH 1/5] 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 ( From 3c57c24921f8197343cf6e592de31b5fd31509cc Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Fri, 22 Sep 2017 12:14:27 -0400 Subject: [PATCH 2/5] linting: next round of fixes --- Makefile | 1 - autofile/group.go | 4 ++-- autofile/group_test.go | 8 +++----- common/service.go | 2 +- db/c_level_db_test.go | 2 +- db/go_level_db_test.go | 2 +- pubsub/query/query.peg.go | 4 +--- 7 files changed, 9 insertions(+), 14 deletions(-) diff --git a/Makefile b/Makefile index ba164ec1..6b2c7463 100644 --- a/Makefile +++ b/Makefile @@ -45,7 +45,6 @@ metalinter_test: ensure_tools --enable=varcheck \ --enable=vetshadow \ --enable=interfacer \ - --enable=unparam \ --enable=vet \ ./... diff --git a/autofile/group.go b/autofile/group.go index 689b1cb9..eedb67b5 100644 --- a/autofile/group.go +++ b/autofile/group.go @@ -567,8 +567,8 @@ func (gr *GroupReader) ReadLine() (string, error) { bytesRead, err := gr.curReader.ReadBytes('\n') if err == io.EOF { // Open the next file - if err := gr.openFile(gr.curIndex + 1); 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 91c6a0bb..0cfcef72 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() @@ -112,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/common/service.go b/common/service.go index 71fc03cb..5ac38631 100644 --- a/common/service.go +++ b/common/service.go @@ -151,7 +151,7 @@ func (bs *BaseService) Reset() (bool, error) { return false, nil } // never happens - return false, nil + return false, nil // nolint: vet } // Implements Service diff --git a/db/c_level_db_test.go b/db/c_level_db_test.go index 0ee6d641..e7336cc5 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 0603b2d4..2cd3192c 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/pubsub/query/query.peg.go b/pubsub/query/query.peg.go index 8c3e83ef..c86e4a47 100644 --- a/pubsub/query/query.peg.go +++ b/pubsub/query/query.peg.go @@ -1,6 +1,4 @@ -// nolint: megacheck -// nolint: varcheck -// nolint: deadcode +// nolint package query import ( From 2681f32bddcc061e8ae062734404e73cbe6ea5d0 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Fri, 22 Sep 2017 12:35:52 -0400 Subject: [PATCH 3/5] circle: add metalinter to test --- Makefile | 4 +--- circle.yml | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 6b2c7463..902197e7 100644 --- a/Makefile +++ b/Makefile @@ -35,6 +35,7 @@ metalinter_test: ensure_tools --enable=gosimple \ --enable=gotype \ --enable=ineffassign \ + --enable=interfacer \ --enable=megacheck \ --enable=misspell \ --enable=staticcheck \ @@ -44,7 +45,6 @@ metalinter_test: ensure_tools --enable=unused \ --enable=varcheck \ --enable=vetshadow \ - --enable=interfacer \ --enable=vet \ ./... @@ -53,6 +53,4 @@ metalinter_test: ensure_tools #--enable=errcheck \ #--enable=gocyclo \ #--enable=golint \ <== comments on anything exported - #--enable=interfacer \ #--enable=unparam \ - #--enable=vet \ diff --git a/circle.yml b/circle.yml index 23ac4bd9..8e3ad168 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" From cf49ba876fe8e7734acccd4c29289b77ea5829a5 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Tue, 3 Oct 2017 12:18:21 -0400 Subject: [PATCH 4/5] linter: couple fixes --- Makefile | 4 ++-- autofile/autofile_test.go | 21 ++++++++------------- common/os.go | 3 +-- common/service.go | 16 +++++++--------- merkle/simple_tree.go | 1 - 5 files changed, 18 insertions(+), 27 deletions(-) diff --git a/Makefile b/Makefile index 902197e7..25773ed3 100644 --- a/Makefile +++ b/Makefile @@ -31,9 +31,7 @@ metalinter_test: ensure_tools --enable=deadcode \ --enable=gas \ --enable=goconst \ - --enable=goimports \ --enable=gosimple \ - --enable=gotype \ --enable=ineffassign \ --enable=interfacer \ --enable=megacheck \ @@ -52,5 +50,7 @@ metalinter_test: ensure_tools #--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 c7aa93be..05152219 100644 --- a/autofile/autofile_test.go +++ b/autofile/autofile_test.go @@ -1,4 +1,3 @@ -// nolint: goimports package autofile import ( @@ -8,19 +7,18 @@ import ( "testing" "time" - . "github.com/tendermint/tmlibs/common" + 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 - af, err := OpenAutoFile(name) + af, err := cmn.OpenAutoFile(name) if err != nil { t.Fatalf("Error creating autofile: %v", err) } @@ -36,8 +34,7 @@ func TestSIGHUP(t *testing.T) { } // Move the file over - err = os.Rename(name, name+"_old") - if err != nil { + if err := os.Rename(name, name+"_old"); err != nil { t.Fatalf("Error moving autofile: %v", err) } @@ -59,17 +56,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/common/os.go b/common/os.go index 8b7143f5..19aa479f 100644 --- a/common/os.go +++ b/common/os.go @@ -8,7 +8,6 @@ import ( "os" "os/signal" "strings" - "syscall" ) var ( @@ -18,7 +17,7 @@ var ( func TrapSignal(cb func()) { c := make(chan os.Signal, 1) signal.Notify(c, os.Interrupt) - signal.Notify(c, syscall.SIGTERM) + signal.Notify(c, os.Kill) // nolint: megacheck go func() { for sig := range c { fmt.Printf("captured %v, exiting...\n", sig) diff --git a/common/service.go b/common/service.go index 5ac38631..2d86baaf 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 stopped := atomic.CompareAndSwapUint32(&bs.stopped, 1, 0); !stopped { bs.Logger.Debug(Fmt("Can't reset %v. Not stopped", bs.name), "impl", bs.impl) return false, nil } - // never happens - return false, nil // nolint: vet + + // 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/merkle/simple_tree.go b/merkle/simple_tree.go index b373743f..8106246d 100644 --- a/merkle/simple_tree.go +++ b/merkle/simple_tree.go @@ -22,7 +22,6 @@ For larger datasets, use IAVLTree. */ -// nolint: goimports package merkle import ( From c8805fd7deb52c9565f1f0c5e465b480dcf13f2c Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Wed, 4 Oct 2017 00:13:58 -0400 Subject: [PATCH 5/5] metalinter fixes from review --- autofile/autofile_test.go | 5 +++-- common/os.go | 4 ++-- common/service.go | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/autofile/autofile_test.go b/autofile/autofile_test.go index 05152219..8f453dd0 100644 --- a/autofile/autofile_test.go +++ b/autofile/autofile_test.go @@ -18,7 +18,7 @@ func TestSIGHUP(t *testing.T) { t.Fatalf("Error creating tempfile: %v", err) } // Here is the actual AutoFile - af, err := cmn.OpenAutoFile(name) + af, err := OpenAutoFile(name) if err != nil { t.Fatalf("Error creating autofile: %v", err) } @@ -34,7 +34,8 @@ func TestSIGHUP(t *testing.T) { } // Move the file over - if err := os.Rename(name, name+"_old"); err != nil { + err = os.Rename(name, name+"_old") + if err != nil { t.Fatalf("Error moving autofile: %v", err) } diff --git a/common/os.go b/common/os.go index 19aa479f..625d6ae1 100644 --- a/common/os.go +++ b/common/os.go @@ -8,6 +8,7 @@ import ( "os" "os/signal" "strings" + "syscall" ) var ( @@ -16,8 +17,7 @@ var ( func TrapSignal(cb func()) { c := make(chan os.Signal, 1) - signal.Notify(c, os.Interrupt) - signal.Notify(c, os.Kill) // nolint: megacheck + signal.Notify(c, os.Interrupt, syscall.SIGTERM) go func() { for sig := range c { fmt.Printf("captured %v, exiting...\n", sig) diff --git a/common/service.go b/common/service.go index 2d86baaf..8d4de30a 100644 --- a/common/service.go +++ b/common/service.go @@ -140,7 +140,7 @@ func (bs *BaseService) OnStop() {} // Implements Service func (bs *BaseService) Reset() (bool, error) { - if stopped := atomic.CompareAndSwapUint32(&bs.stopped, 1, 0); !stopped { + 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 }