*: enable all default linters (#5504)

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

.circleci/config.yml

@@ -15,7 +15,11 @@ jobs:
     steps:
     - checkout
     - run: make promu
-    - run: make check_license style unused lint build check_assets
+    - run:
+        command: make check_license style unused lint build check_assets
+        environment:
+          # Run garbage collection more aggresively to avoid getting OOMed during the lint phase.
+          GOGC: "20"
     - run:
        command: |
          curl -s -L https://github.com/protocolbuffers/protobuf/releases/download/v3.5.1/protoc-3.5.1-linux-x86_64.zip > /tmp/protoc.zip

.golangci.yml

@@ -1,8 +1,13 @@
 run:
   modules-download-mode: vendor
+  deadline: 5m
 
-# Run only staticcheck for now. Additional linters will be enabled one-by-one.
+issues:
-linters:
+  exclude-rules:
-  enable:
+  - path: _test.go
-  - staticcheck
+    linters:
-  disable-all: true
+    - errcheck
+
+linters-settings:
+  errcheck:
+    exclude: scripts/errcheck_excludes.txt

Makefile.common

@@ -73,6 +73,7 @@ PROMU_VERSION ?= 0.3.0
 PROMU_URL     := https://github.com/prometheus/promu/releases/download/v$(PROMU_VERSION)/promu-$(PROMU_VERSION).$(GO_BUILD_PLATFORM).tar.gz
 
 GOLANGCI_LINT :=
+GOLANGCI_LINT_OPTS ?=
 GOLANGCI_LINT_VERSION ?= v1.16.0
 # golangci-lint only supports linux, darwin and windows platforms on i386/amd64.
 # windows isn't included here because of the path separator being different.
@@ -166,7 +167,7 @@ ifdef GO111MODULE
 # 'go list' needs to be executed before staticcheck to prepopulate the modules cache.
 # Otherwise staticcheck might fail randomly for some reason not yet explained.
 	GO111MODULE=$(GO111MODULE) $(GO) list -e -compiled -test=true -export=false -deps=true -find=false -tags= -- ./... > /dev/null
-	GO111MODULE=$(GO111MODULE) $(GOLANGCI_LINT) run $(pkgs)
+	GO111MODULE=$(GO111MODULE) $(GOLANGCI_LINT) run $(GOLANGCI_LINT_OPTS) $(pkgs)
 else
 	$(GOLANGCI_LINT) run $(pkgs)
 endif

cmd/promtool/http.go

@@ -1,45 +0,0 @@
-// Copyright 2015 The Prometheus Authors
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package main
-
-import (
-	"time"
-
-	"github.com/pkg/errors"
-	"github.com/prometheus/client_golang/api"
-)
-
-const defaultTimeout = 2 * time.Minute
-
-type prometheusHTTPClient struct {
-	requestTimeout time.Duration
-	httpClient     api.Client
-}
-
-func newPrometheusHTTPClient(serverURL string) (*prometheusHTTPClient, error) {
-	hc, err := api.NewClient(api.Config{
-		Address: serverURL,
-	})
-	if err != nil {
-		return nil, errors.Wrapf(err, "error creating HTTP client")
-	}
-	return &prometheusHTTPClient{
-		requestTimeout: defaultTimeout,
-		httpClient:     hc,
-	}, nil
-}
-
-func (c *prometheusHTTPClient) urlJoin(path string) string {
-	return c.httpClient.URL(path, nil).String()
-}

cmd/promtool/http_test.go

@@ -1,56 +0,0 @@
-// Copyright 2015 The Prometheus Authors
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package main
-
-import "testing"
-
-func TestURLJoin(t *testing.T) {
-
-	testCases := []struct {
-		inputHost string
-		inputPath string
-		expected  string
-	}{
-		{"http://host", "path", "http://host/path"},
-		{"http://host", "path/", "http://host/path"},
-		{"http://host", "/path", "http://host/path"},
-		{"http://host", "/path/", "http://host/path"},
-
-		{"http://host/", "path", "http://host/path"},
-		{"http://host/", "path/", "http://host/path"},
-		{"http://host/", "/path", "http://host/path"},
-		{"http://host/", "/path/", "http://host/path"},
-
-		{"https://host", "path", "https://host/path"},
-		{"https://host", "path/", "https://host/path"},
-		{"https://host", "/path", "https://host/path"},
-		{"https://host", "/path/", "https://host/path"},
-
-		{"https://host/", "path", "https://host/path"},
-		{"https://host/", "path/", "https://host/path"},
-		{"https://host/", "/path", "https://host/path"},
-		{"https://host/", "/path/", "https://host/path"},
-	}
-	for i, c := range testCases {
-		client, err := newPrometheusHTTPClient(c.inputHost)
-		if err != nil {
-			panic(err)
-		}
-		actual := client.urlJoin(c.inputPath)
-		if actual != c.expected {
-			t.Errorf("Error on case %d: %v(actual) != %v(expected)", i, actual, c.expected)
-		}
-		t.Logf("Case %d: %v(actual) == %v(expected)", i, actual, c.expected)
-	}
-}

cmd/promtool/main.go

@@ -619,11 +619,14 @@ func (p *promqlPrinter) printLabelValues(val model.LabelValues) {
 type jsonPrinter struct{}
 
 func (j *jsonPrinter) printValue(v model.Value) {
+	//nolint:errcheck
 	json.NewEncoder(os.Stdout).Encode(v)
 }
 func (j *jsonPrinter) printSeries(v []model.LabelSet) {
+	//nolint:errcheck
 	json.NewEncoder(os.Stdout).Encode(v)
 }
 func (j *jsonPrinter) printLabelValues(v model.LabelValues) {
+	//nolint:errcheck
 	json.NewEncoder(os.Stdout).Encode(v)
 }

discovery/consul/consul.go

@@ -341,7 +341,7 @@ func (d *Discovery) Run(ctx context.Context, ch chan<- []*targetgroup.Group) {
 // Watch the catalog for new services we would like to watch. This is called only
 // when we don't know yet the names of the services and need to ask Consul the
 // entire list of services.
-func (d *Discovery) watchServices(ctx context.Context, ch chan<- []*targetgroup.Group, lastIndex *uint64, services map[string]func()) error {
+func (d *Discovery) watchServices(ctx context.Context, ch chan<- []*targetgroup.Group, lastIndex *uint64, services map[string]func()) {
 	catalog := d.client.Catalog()
 	level.Debug(d.logger).Log("msg", "Watching services", "tags", d.watchedTags)
 
@@ -360,11 +360,11 @@ func (d *Discovery) watchServices(ctx context.Context, ch chan<- []*targetgroup.
 		level.Error(d.logger).Log("msg", "Error refreshing service list", "err", err)
 		rpcFailuresCount.Inc()
 		time.Sleep(retryInterval)
-		return err
+		return
 	}
 	// If the index equals the previous one, the watch timed out with no update.
 	if meta.LastIndex == *lastIndex {
-		return nil
+		return
 	}
 	*lastIndex = meta.LastIndex
 
@@ -396,12 +396,11 @@ func (d *Discovery) watchServices(ctx context.Context, ch chan<- []*targetgroup.
 			// Send clearing target group.
 			select {
 			case <-ctx.Done():
-				return ctx.Err()
+				return
 			case ch <- []*targetgroup.Group{{Source: name}}:
 			}
 		}
 	}
-	return nil
 }
 
 // consulService contains data belonging to the same service.
@@ -441,14 +440,17 @@ func (d *Discovery) watchService(ctx context.Context, ch chan<- []*targetgroup.G
 				return
 			default:
 				srv.watch(ctx, ch, catalog, &lastIndex)
-				<-ticker.C
+				select {
+				case <-ticker.C:
+				case <-ctx.Done():
+				}
 			}
 		}
 	}()
 }
 
 // Get updates for a service.
-func (srv *consulService) watch(ctx context.Context, ch chan<- []*targetgroup.Group, catalog *consul.Catalog, lastIndex *uint64) error {
+func (srv *consulService) watch(ctx context.Context, ch chan<- []*targetgroup.Group, catalog *consul.Catalog, lastIndex *uint64) {
 	level.Debug(srv.logger).Log("msg", "Watching service", "service", srv.name, "tags", srv.tags)
 
 	t0 := time.Now()
@@ -465,7 +467,7 @@ func (srv *consulService) watch(ctx context.Context, ch chan<- []*targetgroup.Gr
 	// Check the context before in order to exit early.
 	select {
 	case <-ctx.Done():
-		return ctx.Err()
+		return
 	default:
 		// Continue.
 	}
@@ -474,11 +476,11 @@ func (srv *consulService) watch(ctx context.Context, ch chan<- []*targetgroup.Gr
 		level.Error(srv.logger).Log("msg", "Error refreshing service", "service", srv.name, "tags", srv.tags, "err", err)
 		rpcFailuresCount.Inc()
 		time.Sleep(retryInterval)
-		return err
+		return
 	}
 	// If the index equals the previous one, the watch timed out with no update.
 	if meta.LastIndex == *lastIndex {
-		return nil
+		return
 	}
 	*lastIndex = meta.LastIndex
 
@@ -536,8 +538,6 @@ func (srv *consulService) watch(ctx context.Context, ch chan<- []*targetgroup.Gr
 
 	select {
 	case <-ctx.Done():
-		return ctx.Err()
 	case ch <- []*targetgroup.Group{&tgroup}:
 	}
-	return nil
 }

discovery/kubernetes/endpoints.go

@@ -325,11 +325,12 @@ func (e *Endpoints) resolvePodRef(ref *apiv1.ObjectReference) *apiv1.Pod {
 	p.Name = ref.Name
 
 	obj, exists, err := e.podStore.Get(p)
-	if err != nil || !exists {
-		return nil
-	}
 	if err != nil {
 		level.Error(e.logger).Log("msg", "resolving pod ref failed", "err", err)
+		return nil
+	}
+	if !exists {
+		return nil
 	}
 	return obj.(*apiv1.Pod)
 }
@@ -340,11 +341,12 @@ func (e *Endpoints) addServiceLabels(ns, name string, tg *targetgroup.Group) {
 	svc.Name = name
 
 	obj, exists, err := e.serviceStore.Get(svc)
-	if !exists || err != nil {
-		return
-	}
 	if err != nil {
 		level.Error(e.logger).Log("msg", "retrieving service failed", "err", err)
+		return
+	}
+	if !exists {
+		return
 	}
 	svc = obj.(*apiv1.Service)
 

promql/ast.go

@@ -323,5 +323,6 @@ func (f inspector) Visit(node Node, path []Node) (Visitor, error) {
 // f(node, path); node must not be nil. If f returns a nil error, Inspect invokes f
 // for all the non-nil children of node, recursively.
 func Inspect(node Node, f inspector) {
+	//nolint: errcheck
 	Walk(inspector(f), node, nil)
 }

promql/engine_test.go

@@ -791,6 +791,7 @@ func TestRecoverEvaluatorRuntime(t *testing.T) {
 
 	// Cause a runtime panic.
 	var a []int
+	//nolint:govet
 	a[123] = 1
 
 	if err.Error() != "unexpected error" {

promql/parse_test.go

@@ -1789,6 +1789,7 @@ func TestRecoverParserRuntime(t *testing.T) {
 	defer p.recover(&err)
 	// Cause a runtime panic.
 	var a []int
+	//nolint:govet
 	a[123] = 1
 }
 

rules/manager.go

@@ -355,8 +355,8 @@ func (g *Group) stop() {
 
 func (g *Group) hash() uint64 {
 	l := labels.New(
-		labels.Label{"name", g.name},
+		labels.Label{Name: "name", Value: g.name},
-		labels.Label{"file", g.file},
+		labels.Label{Name: "file", Value: g.file},
 	)
 	return l.Hash()
 }

rules/manager_test.go

@@ -552,7 +552,7 @@ func TestStaleness(t *testing.T) {
 	metricSample[2].V = 42 // reflect.DeepEqual cannot handle NaN.
 
 	want := map[string][]promql.Point{
-		metric: {{0, 2}, {1000, 3}, {2000, 42}},
+		metric: {{T: 0, V: 2}, {T: 1000, V: 3}, {T: 2000, V: 42}},
 	}
 
 	testutil.Equals(t, want, samples)

scrape/target.go

@@ -118,7 +118,9 @@ func (t *Target) setMetadataStore(s metricMetadataStore) {
 // hash returns an identifying hash for the target.
 func (t *Target) hash() uint64 {
 	h := fnv.New64a()
+	//nolint: errcheck
 	h.Write([]byte(fmt.Sprintf("%016d", t.labels.Hash())))
+	//nolint: errcheck
 	h.Write([]byte(t.URL().String()))
 
 	return h.Sum64()

scripts/errcheck_excludes.txt

@@ -0,0 +1,13 @@
+// Don't flag lines such as "io.Copy(ioutil.Discard, resp.Body)".
+io.Copy
+// The next two are used in HTTP handlers, any error is handled by the server itself.
+io.WriteString
+(net/http.ResponseWriter).Write
+// No need to check for errors on server's shutdown.
+(*net/http.Server).Shutdown
+
+// Never check for logger errors.
+(github.com/go-kit/kit/log.Logger).Log
+
+// Never check for rollback errors as Rollback() is called when a previous error was detected.
+(github.com/prometheus/prometheus/storage.Appender).Rollback

storage/remote/wal_watcher_test.go

@@ -112,7 +112,7 @@ func TestTailSamples(t *testing.T) {
 		series := enc.Series([]tsdb.RefSeries{
 			tsdb.RefSeries{
 				Ref:    uint64(ref),
-				Labels: labels.Labels{labels.Label{"__name__", fmt.Sprintf("metric_%d", i)}},
+				Labels: labels.Labels{labels.Label{Name: "__name__", Value: fmt.Sprintf("metric_%d", i)}},
 			},
 		}, nil)
 		testutil.Ok(t, w.Log(series))
@@ -182,7 +182,7 @@ func TestReadToEndNoCheckpoint(t *testing.T) {
 		series := enc.Series([]tsdb.RefSeries{
 			tsdb.RefSeries{
 				Ref:    uint64(i),
-				Labels: labels.Labels{labels.Label{"__name__", fmt.Sprintf("metric_%d", i)}},
+				Labels: labels.Labels{labels.Label{Name: "__name__", Value: fmt.Sprintf("metric_%d", i)}},
 			},
 		}, nil)
 		recs = append(recs, series)
@@ -246,7 +246,7 @@ func TestReadToEndWithCheckpoint(t *testing.T) {
 		series := enc.Series([]tsdb.RefSeries{
 			tsdb.RefSeries{
 				Ref:    uint64(ref),
-				Labels: labels.Labels{labels.Label{"__name__", fmt.Sprintf("metric_%d", i)}},
+				Labels: labels.Labels{labels.Label{Name: "__name__", Value: fmt.Sprintf("metric_%d", i)}},
 			},
 		}, nil)
 		testutil.Ok(t, w.Log(series))
@@ -272,7 +272,7 @@ func TestReadToEndWithCheckpoint(t *testing.T) {
 		series := enc.Series([]tsdb.RefSeries{
 			tsdb.RefSeries{
 				Ref:    uint64(i),
-				Labels: labels.Labels{labels.Label{"__name__", fmt.Sprintf("metric_%d", i)}},
+				Labels: labels.Labels{labels.Label{Name: "__name__", Value: fmt.Sprintf("metric_%d", i)}},
 			},
 		}, nil)
 		testutil.Ok(t, w.Log(series))
@@ -328,7 +328,7 @@ func TestReadCheckpoint(t *testing.T) {
 		series := enc.Series([]tsdb.RefSeries{
 			tsdb.RefSeries{
 				Ref:    uint64(ref),
-				Labels: labels.Labels{labels.Label{"__name__", fmt.Sprintf("metric_%d", i)}},
+				Labels: labels.Labels{labels.Label{Name: "__name__", Value: fmt.Sprintf("metric_%d", i)}},
 			},
 		}, nil)
 		testutil.Ok(t, w.Log(series))
@@ -391,7 +391,7 @@ func TestReadCheckpointMultipleSegments(t *testing.T) {
 			series := enc.Series([]tsdb.RefSeries{
 				tsdb.RefSeries{
 					Ref:    uint64(ref),
-					Labels: labels.Labels{labels.Label{"__name__", fmt.Sprintf("metric_%d", j)}},
+					Labels: labels.Labels{labels.Label{Name: "__name__", Value: fmt.Sprintf("metric_%d", j)}},
 				},
 			}, nil)
 			testutil.Ok(t, w.Log(series))
@@ -458,7 +458,7 @@ func TestCheckpointSeriesReset(t *testing.T) {
 		series := enc.Series([]tsdb.RefSeries{
 			tsdb.RefSeries{
 				Ref:    uint64(ref),
-				Labels: labels.Labels{labels.Label{"__name__", fmt.Sprintf("metric_%d", i)}},
+				Labels: labels.Labels{labels.Label{Name: "__name__", Value: fmt.Sprintf("metric_%d", i)}},
 			},
 		}, nil)
 		testutil.Ok(t, w.Log(series))

template/template_test.go

@@ -25,18 +25,16 @@ import (
 	"github.com/prometheus/prometheus/util/testutil"
 )
 
-type testTemplatesScenario struct {
-	text        string
-	output      string
-	input       interface{}
-	queryResult promql.Vector
-	shouldFail  bool
-	html        bool
-	errorMsg    string
-}
-
 func TestTemplateExpansion(t *testing.T) {
-	scenarios := []testTemplatesScenario{
+	scenarios := []struct {
+		text        string
+		output      string
+		input       interface{}
+		queryResult promql.Vector
+		shouldFail  bool
+		html        bool
+		errorMsg    string
+	}{
 		{
 			// No template.
 			text:   "plain text",

util/strutil/quote_test.go

@@ -17,13 +17,11 @@ import (
 	"testing"
 )
 
-type quoteTest struct {
+var quotetests = []struct {
 	in    string
 	out   string
 	ascii string
-}
+}{
-
-var quotetests = []quoteTest{
 	{"\a\b\f\r\n\t\v", `"\a\b\f\r\n\t\v"`, `"\a\b\f\r\n\t\v"`},
 	{"\\", `"\\"`, `"\\"`},
 	{"abc\xffdef", `"abc\xffdef"`, `"abc\xffdef"`},
@@ -32,12 +30,10 @@ var quotetests = []quoteTest{
 	{"\x04", `"\x04"`, `"\x04"`},
 }
 
-type unQuoteTest struct {
+var unquotetests = []struct {
 	in  string
 	out string
-}
+}{
-
-var unquotetests = []unQuoteTest{
 	{`""`, ""},
 	{`"a"`, "a"},
 	{`"abc"`, "abc"},

web/api/v1/api.go

@@ -838,7 +838,10 @@ func (api *API) serveFlags(r *http.Request) apiFuncResult {
 }
 
 func (api *API) remoteRead(w http.ResponseWriter, r *http.Request) {
-	api.remoteReadGate.Start(r.Context())
+	if err := api.remoteReadGate.Start(r.Context()); err != nil {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
 	remoteReadQueries.Inc()
 
 	defer api.remoteReadGate.Done()

web/api/v1/api_test.go

@@ -884,7 +884,7 @@ func assertAPIError(t *testing.T, got *apiError, exp errorType) {
 		}
 		return
 	}
-	if got == nil && exp != errorNone {
+	if exp != errorNone {
 		t.Fatalf("Expected error of type %q but got none", exp)
 	}
 }

web/federate.go

@@ -136,7 +136,7 @@ func (h *Handler) federation(w http.ResponseWriter, req *http.Request) {
 	}
 	if set.Err() != nil {
 		federationErrors.Inc()
-		http.Error(w, err.Error(), http.StatusInternalServerError)
+		http.Error(w, set.Err().Error(), http.StatusInternalServerError)
 		return
 	}