switch to golangci-lint linter (#539)

* switch to golangci-lint linter

* fixes for review comments

.circleci/config.yml

@@ -41,7 +41,7 @@ workflows:
 # https://circleci.com/blog/circleci-hacks-reuse-yaml-in-your-circleci-config-with-yaml/
 defaults: &defaults
   docker:
-    - image: grafana/loki-build-image:0.1.0
+    - image: grafana/loki-build-image:0.2.0
   working_directory: /go/src/github.com/grafana/loki
 
 jobs:

.golangci.yml

@@ -0,0 +1,74 @@
+# This file contains all available configuration options
+# with their default values.
+
+# options for analysis running
+run:
+  # default concurrency is a available CPU number
+  concurrency: 2
+
+  # timeout for analysis, e.g. 30s, 5m, default is 1m
+  deadline: 5m
+
+  # exit code when at least one issue was found, default is 1
+  issues-exit-code: 1
+
+  # include test files or not, default is true
+  tests: true
+
+  # list of build tags, all linters use it. Default is empty list.
+  build-tags:
+
+  # which dirs to skip: they won't be analyzed;
+  # can use regexp here: generated.*, regexp is applied on full path;
+  # default value is empty list, but next dirs are always skipped independently
+  # from this option's value:
+  #   	vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
+  skip-dirs:
+
+  # which files to skip: they will be analyzed, but issues from them
+  # won't be reported. Default value is empty list, but there is
+  # no need to include all autogenerated files, we confidently recognize
+  # autogenerated files. If it's not please let us know.
+  skip-files:
+    - ".*\\.y$"
+    - ".*yaccpar$"
+
+# output configuration options
+output:
+  # colored-line-number|line-number|json|tab|checkstyle, default is "colored-line-number"
+  format: colored-line-number
+
+  # print lines of code with issue, default is true
+  print-issued-lines: true
+
+  # print linter name in the end of issue text, default is true
+  print-linter-name: true
+
+linters:
+  enable:
+    - deadcode
+    - errcheck
+    - goconst
+    - gofmt
+    - goimports
+    - golint
+    - gosimple
+    - ineffassign
+    - megacheck
+    - misspell
+    - structcheck
+    - unconvert
+    - unparam
+    - varcheck
+    - govet
+    - unused # new from here.
+    - interfacer
+    - typecheck
+    - dupl
+    - gocyclo
+    - nakedret
+
+issues:
+  exclude-use-default: false
+  exclude:
+    - Error return value of .*log\.Logger\)\.Log\x60 is not checked 

Makefile

@@ -171,7 +171,7 @@ $(EXES): loki-build-image/$(UPTODATE)
 	goyacc -p $(basename $(notdir $<)) -o $@ $<
 
 lint: loki-build-image/$(UPTODATE)
-	gometalinter ./...
+	golangci-lint run
 
 check-generated-files: loki-build-image/$(UPTODATE) yacc protos
 	@git diff-files || (echo "changed files; failing check" && exit 1)

loki-build-image/Dockerfile

@@ -18,11 +18,9 @@ RUN go get \
 		github.com/go-delve/delve/cmd/dlv \
 		golang.org/x/tools/cmd/goyacc && \
 		rm -rf /go/pkg /go/src
-ENV GOMETALINTER_VER="2.0.11"
-RUN curl -L -o /tmp/gometalinter-$GOMETALINTER_VER.tgz https://github.com/alecthomas/gometalinter/releases/download/v$GOMETALINTER_VER/gometalinter-$GOMETALINTER_VER-linux-amd64.tar.gz && \
-		tar -xz -C /tmp -f /tmp/gometalinter-$GOMETALINTER_VER.tgz && \
-		mv /tmp/gometalinter-$GOMETALINTER_VER-linux-amd64/* /usr/bin && \
-		rm /tmp/gometalinter-$GOMETALINTER_VER.tgz
+ENV GOLANGCI_LINT_VER="1.16.0"
+RUN curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b $(go env GOPATH)/bin v${GOLANGCI_LINT_VER} && \
+	golangci-lint --version
 COPY build.sh /
 ENV GOCACHE=/go/cache
 ENTRYPOINT ["/build.sh"]

pkg/chunkenc/gzip.go

@@ -454,10 +454,6 @@ type listIterator struct {
 	cur entry
 }
 
-func (li *listIterator) Seek(int64) bool {
-	return false
-}
-
 func (li *listIterator) Next() bool {
 	if len(li.entries) > 0 {
 		li.cur = li.entries[0]
@@ -500,10 +496,6 @@ func newBufferedIterator(s *bufio.Reader) *bufferedIterator {
 	}
 }
 
-func (si *bufferedIterator) Seek(int64) bool {
-	return false
-}
-
 func (si *bufferedIterator) Next() bool {
 	ts, err := binary.ReadVarint(si.s)
 	if err != nil {

pkg/distributor/distributor.go

@@ -248,8 +248,8 @@ func (d *Distributor) sendSamplesErr(ctx context.Context, ingester ring.Ingester
 
 func tokenFor(userID, labels string) uint32 {
 	h := fnv.New32()
-	h.Write([]byte(userID))
-	h.Write([]byte(labels))
+	_, _ = h.Write([]byte(userID))
+	_, _ = h.Write([]byte(labels))
 	return h.Sum32()
 }
 

pkg/ingester/chunk_test.go

@@ -63,7 +63,7 @@ func TestIterator(t *testing.T) {
 				iter, err := chunk.Iterator(time.Unix(int64(from), 0), time.Unix(int64(from+len), 0), logproto.FORWARD)
 				require.NoError(t, err)
 				testIteratorForward(t, iter, int64(from), int64(from+len))
-				iter.Close()
+				_ = iter.Close()
 			}
 
 			for i := 0; i < entries; i++ {
@@ -72,7 +72,7 @@ func TestIterator(t *testing.T) {
 				iter, err := chunk.Iterator(time.Unix(int64(from), 0), time.Unix(int64(from+len), 0), logproto.BACKWARD)
 				require.NoError(t, err)
 				testIteratorBackward(t, iter, int64(from), int64(from+len))
-				iter.Close()
+				_ = iter.Close()
 			}
 		})
 	}

pkg/ingester/client/client.go

@@ -8,7 +8,7 @@ import (
 	cortex_client "github.com/cortexproject/cortex/pkg/ingester/client"
 	"github.com/grafana/loki/pkg/logproto"
 	"github.com/grpc-ecosystem/grpc-opentracing/go/otgrpc"
-	"github.com/mwitkow/go-grpc-middleware"
+	grpc_middleware "github.com/mwitkow/go-grpc-middleware"
 	opentracing "github.com/opentracing/opentracing-go"
 	"github.com/weaveworks/common/middleware"
 	"google.golang.org/grpc"

pkg/ingester/flush_test.go

@@ -68,10 +68,6 @@ func newTestStore(t require.TestingT, cfg Config) (*testStore, *Ingester) {
 	return store, ing
 }
 
-func newDefaultTestStore(t require.TestingT) (*testStore, *Ingester) {
-	return newTestStore(t, defaultIngesterTestConfig())
-}
-
 func defaultIngesterTestConfig() Config {
 	consul := ring.NewInMemoryKVClient()
 	cfg := Config{}
@@ -108,7 +104,7 @@ func (s *testStore) Put(ctx context.Context, chunks []chunk.Chunk) error {
 
 func (s *testStore) Stop() {}
 
-func pushTestSamples(t *testing.T, ing *Ingester) ([]string, map[string][]*logproto.Stream) {
+func pushTestSamples(t *testing.T, ing logproto.PusherServer) ([]string, map[string][]*logproto.Stream) {
 	userIDs := []string{"1", "2", "3"}
 
 	// Create test samples.

pkg/ingester/stream_test.go

@@ -44,7 +44,7 @@ func TestStreamIterator(t *testing.T) {
 				require.NotNil(t, iter)
 				require.NoError(t, err)
 				testIteratorForward(t, iter, int64(from), int64(from+len))
-				iter.Close()
+				_ = iter.Close()
 			}
 
 			for i := 0; i < 100; i++ {
@@ -54,7 +54,7 @@ func TestStreamIterator(t *testing.T) {
 				require.NotNil(t, iter)
 				require.NoError(t, err)
 				testIteratorBackward(t, iter, int64(from), int64(from+len))
-				iter.Close()
+				_ = iter.Close()
 			}
 		})
 	}

pkg/iter/iterator_test.go

@@ -23,15 +23,15 @@ func TestIterator(t *testing.T) {
 	}{
 		// Test basic identity.
 		{
-			iterator:  mkStreamIterator(testSize, identity, defaultLabels),
+			iterator:  mkStreamIterator(identity, defaultLabels),
 			generator: identity,
 			length:    testSize,
 			labels:    defaultLabels,
 		},
 
 		// Test basic identity (backwards).
 		{
-			iterator:  mkStreamIterator(testSize, inverse(identity), defaultLabels),
+			iterator:  mkStreamIterator(inverse(identity), defaultLabels),
 			generator: inverse(identity),
 			length:    testSize,
 			labels:    defaultLabels,
@@ -40,9 +40,9 @@ func TestIterator(t *testing.T) {
 		// Test dedupe of overlapping iterators with the heap iterator.
 		{
 			iterator: NewHeapIterator([]EntryIterator{
-				mkStreamIterator(testSize, offset(0, identity), defaultLabels),
-				mkStreamIterator(testSize, offset(testSize/2, identity), defaultLabels),
-				mkStreamIterator(testSize, offset(testSize, identity), defaultLabels),
+				mkStreamIterator(offset(0, identity), defaultLabels),
+				mkStreamIterator(offset(testSize/2, identity), defaultLabels),
+				mkStreamIterator(offset(testSize, identity), defaultLabels),
 			}, logproto.FORWARD),
 			generator: identity,
 			length:    2 * testSize,
@@ -52,9 +52,9 @@ func TestIterator(t *testing.T) {
 		// Test dedupe of overlapping iterators with the heap iterator (backward).
 		{
 			iterator: NewHeapIterator([]EntryIterator{
-				mkStreamIterator(testSize, inverse(offset(0, identity)), defaultLabels),
-				mkStreamIterator(testSize, inverse(offset(-testSize/2, identity)), defaultLabels),
-				mkStreamIterator(testSize, inverse(offset(-testSize, identity)), defaultLabels),
+				mkStreamIterator(inverse(offset(0, identity)), defaultLabels),
+				mkStreamIterator(inverse(offset(-testSize/2, identity)), defaultLabels),
+				mkStreamIterator(inverse(offset(-testSize, identity)), defaultLabels),
 			}, logproto.BACKWARD),
 			generator: inverse(identity),
 			length:    2 * testSize,
@@ -64,9 +64,9 @@ func TestIterator(t *testing.T) {
 		// Test dedupe of entries with the same timestamp but different entries.
 		{
 			iterator: NewHeapIterator([]EntryIterator{
-				mkStreamIterator(testSize, offset(0, constant(0)), defaultLabels),
-				mkStreamIterator(testSize, offset(0, constant(0)), defaultLabels),
-				mkStreamIterator(testSize, offset(testSize, constant(0)), defaultLabels),
+				mkStreamIterator(offset(0, constant(0)), defaultLabels),
+				mkStreamIterator(offset(0, constant(0)), defaultLabels),
+				mkStreamIterator(offset(testSize, constant(0)), defaultLabels),
 			}, logproto.FORWARD),
 			generator: constant(0),
 			length:    2 * testSize,
@@ -75,7 +75,7 @@ func TestIterator(t *testing.T) {
 
 		// Test basic identity with non-default labels.
 		{
-			iterator:  mkStreamIterator(testSize, identity, "{foobar: \"bazbar\"}"),
+			iterator:  mkStreamIterator(identity, "{foobar: \"bazbar\"}"),
 			generator: identity,
 			length:    testSize,
 			labels:    "{foobar: \"bazbar\"}",
@@ -105,8 +105,8 @@ func TestIteratorMultipleLabels(t *testing.T) {
 		// Test merging with differing labels but same timestamps and values.
 		{
 			iterator: NewHeapIterator([]EntryIterator{
-				mkStreamIterator(testSize, identity, "{foobar: \"baz1\"}"),
-				mkStreamIterator(testSize, identity, "{foobar: \"baz2\"}"),
+				mkStreamIterator(identity, "{foobar: \"baz1\"}"),
+				mkStreamIterator(identity, "{foobar: \"baz2\"}"),
 			}, logproto.FORWARD),
 			generator: func(i int64) logproto.Entry {
 				return identity(i / 2)
@@ -123,8 +123,8 @@ func TestIteratorMultipleLabels(t *testing.T) {
 		// Test merging with differing labels but all the same timestamps and different values.
 		{
 			iterator: NewHeapIterator([]EntryIterator{
-				mkStreamIterator(testSize, constant(0), "{foobar: \"baz1\"}"),
-				mkStreamIterator(testSize, constant(0), "{foobar: \"baz2\"}"),
+				mkStreamIterator(constant(0), "{foobar: \"baz1\"}"),
+				mkStreamIterator(constant(0), "{foobar: \"baz2\"}"),
 			}, logproto.FORWARD),
 			generator: func(i int64) logproto.Entry {
 				return constant(0)(i % testSize)
@@ -154,9 +154,9 @@ func TestIteratorMultipleLabels(t *testing.T) {
 
 type generator func(i int64) logproto.Entry
 
-func mkStreamIterator(numEntries int64, f generator, labels string) EntryIterator {
+func mkStreamIterator(f generator, labels string) EntryIterator {
 	entries := []logproto.Entry{}
-	for i := int64(0); i < numEntries; i++ {
+	for i := int64(0); i < testSize; i++ {
 		entries = append(entries, f(i))
 	}
 	return newStreamIterator(&logproto.Stream{
@@ -178,6 +178,7 @@ func offset(j int64, g generator) generator {
 	}
 }
 
+// nolint
 func constant(t int64) generator {
 	return func(i int64) logproto.Entry {
 		return logproto.Entry{