Merge pull request #53 from tendermint/metalinter

add metalinter to CI and address some lint warnings

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 \

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")
+	file, name := cmn.Tempfile("sighup_test")
-	err := file.Close()
+	if err := file.Close(); err != nil {
-	if 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 := af.Close(); err != nil {
-	if 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)
 	}
-
 }

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 err1 := gr.openFile(gr.curIndex + 1); err1 != nil {
-			if err != nil {
+				return "", err1
-				return "", err
 			}
 			if len(bytesRead) > 0 && bytesRead[len(bytesRead)-1] == byte('\n') {
 				return linePrefix + string(bytesRead[:len(bytesRead)-1]), nil

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 := g.WriteLine(RandStr(999)); err != nil {
-	if 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 := g.WriteLine(RandStr(999)); err != nil {
-	if 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 := g.WriteLine(RandStr(999)); err != nil {
-		if 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)
 	}

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)
 		}

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"

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")

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

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{} {

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))
 }

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)
 	}
 }

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.Interrupt, syscall.SIGTERM)
-	signal.Notify(c, os.Kill)
 	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)
+	return ioutil.WriteFile(filePath, contents, mode)
-	if err != nil {
-		return err
-	}
-	// fmt.Printf("File written to %v.\n", filePath)
-	return nil
 }
 
 func MustWriteFile(filePath string, contents []byte, mode os.FileMode) {

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) {
+	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
+	}
+
 	// 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 {
-		bs.Logger.Debug(Fmt("Can't reset %v. Not stopped", bs.name), "impl", bs.impl)
-		return false, nil
-	}
-	// never happens
-	return false, nil
 }
 
 // Implements Service

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 err == nil
-			return false
-		}
-		return true
 	}
 	return false
 }

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
 				}

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
 				}

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
 }

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 d2-d1 <= maxDeviation
-		return true
-	}
-	return false
 }
 
 func ratesAreEqual(r1 int64, r2 int64, maxDeviation int64) bool {

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")

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"
+	msg := unknown
-	module := "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 + " ")
 	}
 

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.")

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 {

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
 	}
 }

pubsub/query/query.peg.go

@@ -1,3 +1,4 @@
+// nolint
 package query
 
 import (