Implementing linters

.github/workflows/golangci-lint.yml

@@ -0,0 +1,28 @@
+name: golangci-lint
+on:
+  push:
+    tags:
+      - v*
+    branches:
+      - master
+  pull_request:
+jobs:
+  golangci:
+    name: lint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v1
+        with:
+          # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version.
+          version: v1.30
+
+          # Optional: working directory, useful for monorepos
+          # working-directory: somedir
+
+          # Optional: golangci-lint command line arguments.
+          args: --enable gofmt,goimports,gocritic --disable errcheck
+
+          # Optional: show only new issues if it's a pull request. The default value is `false`.
+          # only-new-issues: true

Makefile

@@ -25,7 +25,7 @@ run-test:
 test:
 	make run-test COMMAND="test"
 	make run-test COMMAND="vet"
-	# make run-test COMMAND="test -race"
+	make run-test COMMAND="test -race"
 
 gox-build:
 	rm -rf build

api/api.go

@@ -72,7 +72,7 @@ func (api *Api) Listen(addr *net.TCPAddr) error {
 		api.Go(func(exit chan struct{}) {
 			defer s.Stop()
 
-			if err := s.Serve(tcpListener); err != nil {
+			if err := s.Serve(tcpListener); err != nil { //nolint:staticcheck
 				// may be stopped - not error
 				// zapwriter.Logger("api").Fatal("failed to serve", zap.Error(err))
 			}

api/sample/cache-query/cache-query.go

@@ -19,7 +19,8 @@ func main() {
 	timeout := flag.Duration("timeout", time.Second, "connect and read timeout")
 	flag.Parse()
 
-	conn, err := grpc.Dial(*server, grpc.WithInsecure(), grpc.WithTimeout(*timeout))
+	// TODO: grpc.WithTimeout deprecated
+	conn, err := grpc.Dial(*server, grpc.WithInsecure(), grpc.WithTimeout(*timeout)) //nolint:staticcheck
 	if err != nil {
 		log.Fatalf("did not connect: %v", err)
 	}

cache/cache.go

@@ -61,8 +61,8 @@ type Cache struct {
 type Shard struct {
 	sync.RWMutex     // Read Write mutex, guards access to internal map.
 	items            map[string]*points.Points
-	notConfirmed     []*points.Points // linear search for value/slot
-	notConfirmedUsed int              // search value in notConfirmed[:notConfirmedUsed]
+	notConfirmed     []*points.Points    // linear search for value/slot
+	notConfirmedUsed int                 // search value in notConfirmed[:notConfirmedUsed]
 	adds             map[string]struct{} // map to maintain all the new metric names
 }
 
@@ -328,13 +328,13 @@ func (c *Cache) WriteoutQueue() *WriteoutQueue {
 	return c.writeoutQueue
 }
 
-// called at every scan-frequency by fileListUpdater in carbonserver. 
+// called at every scan-frequency by fileListUpdater in carbonserver.
 // Iterates over every shard to:
 // - collect the new metric names (append shard.adds map to slice)
 // - replace shard.adds with new empty map
 func (c *Cache) GetRecentNewMetrics() []map[string]struct{} {
-	metricNames := make([]map[string]struct{},shardCount)
-	for i:=0; i<shardCount ; i++ {
+	metricNames := make([]map[string]struct{}, shardCount)
+	for i := 0; i < shardCount; i++ {
 		shard, newAdds := c.data[i], make(map[string]struct{})
 		shard.Lock()
 		currNames := shard.adds
@@ -343,4 +343,4 @@ func (c *Cache) GetRecentNewMetrics() []map[string]struct{} {
 		metricNames[i] = currNames
 	}
 	return metricNames
-}
+}

cache/cache_test.go

@@ -67,8 +67,6 @@ func createCacheAndPopulate(metricsCount int, maxPointsPerMetric int) *Cache {
 	return cache
 }
 
-var gp *points.Points
-
 func benchmarkStrategy(b *testing.B, strategy string) {
 	cache := createCacheAndPopulate(1000*1000, 100)
 	if err := cache.SetWriteStrategy(strategy); err != nil {

cache/carbonlink.go

@@ -45,23 +45,24 @@ func pickleGetStr(buf *[]byte) (string, bool) {
 	}
 	b := *buf
 
-	if b[0] == 'U' { // short string
+	switch {
+	case b[0] == 'U': // short string
 		if len(b) >= 2 {
 			sLen := int(uint8(b[1]))
 			if len(b) >= 2+sLen {
 				*buf = b[2+sLen:]
 				return string(b[2 : 2+sLen]), true
 			}
 		}
-	} else if b[0] == 'T' || b[0] == 'X' { //long string or utf8 string
+	case b[0] == 'T' || b[0] == 'X': //long string or utf8 string
 		if len(b) >= 5 {
 			sLen := int(binary.LittleEndian.Uint32(b[1:]))
 			if len(b) >= 5+sLen {
 				*buf = b[5+sLen:]
 				return string(b[5 : 5+sLen]), true
 			}
 		}
-	} else if bytes.Index(b, []byte("\x8c")) == 0 { // Pickle protocol 4 or 5
+	case bytes.Index(b, []byte("\x8c")) == 0: // Pickle protocol 4 or 5
 		if len(b) >= 2 {
 			sLen := int(uint8(b[1]))
 			if len(b) >= 2+sLen {
@@ -102,17 +103,18 @@ func ParseCarbonlinkRequest(d []byte) (*CarbonlinkRequest, error) {
 	asciiPklTypeBytes := []byte("U\x04type")
 
 	var unicodePklMetricBytes, unicodePklTypeBytes []byte
-	if (expectBytes(&d, []byte("\x80\x02}")) ||
-		expectBytes(&d, []byte("\x80\x03}"))) && pickleMaybeMemo(&d) && expectBytes(&d, []byte("(")) {
+	switch {
+	case (expectBytes(&d, []byte("\x80\x02}")) ||
+		expectBytes(&d, []byte("\x80\x03}"))) && pickleMaybeMemo(&d) && expectBytes(&d, []byte("(")):
 		// message is using pickle protocol 2 or 3.
 		// unicode bytes if Pickle request came from Python 3.0+
 		unicodePklMetricBytes = []byte("X\x06\x00\x00\x00metric")
 		unicodePklTypeBytes = []byte("X\x04\x00\x00\x00type")
-	} else if protocolFourOrFiveFirstBytes(&d) && pickleMaybeMemo(&d) && expectBytes(&d, []byte("(")) {
+	case protocolFourOrFiveFirstBytes(&d) && pickleMaybeMemo(&d) && expectBytes(&d, []byte("(")):
 		// message is using pickle protocol 4, or 5
 		unicodePklMetricBytes = []byte("\x8c\x06metric")
 		unicodePklTypeBytes = []byte("\x8c\x04type")
-	} else {
+	default:
 		return nil, fmt.Errorf("Bad pickle message, unknown pickle protocol")
 	}
 
@@ -121,7 +123,8 @@ func ParseCarbonlinkRequest(d []byte) (*CarbonlinkRequest, error) {
 	var Metric, Type string
 	var ok bool
 
-	if expectBytes(&d, asciiPklMetricBytes) || expectBytes(&d, unicodePklMetricBytes) {
+	switch {
+	case expectBytes(&d, asciiPklMetricBytes) || expectBytes(&d, unicodePklMetricBytes):
 		if !pickleMaybeMemo(&d) {
 			return nil, badErr
 		}
@@ -145,8 +148,7 @@ func ParseCarbonlinkRequest(d []byte) (*CarbonlinkRequest, error) {
 
 		req.Metric = Metric
 		req.Type = Type
-
-	} else if expectBytes(&d, asciiPklTypeBytes) || expectBytes(&d, unicodePklTypeBytes) {
+	case expectBytes(&d, asciiPklTypeBytes) || expectBytes(&d, unicodePklTypeBytes):
 		if !pickleMaybeMemo(&d) {
 			return nil, badErr
 		}
@@ -171,8 +173,7 @@ func ParseCarbonlinkRequest(d []byte) (*CarbonlinkRequest, error) {
 
 		req.Metric = Metric
 		req.Type = Type
-
-	} else {
+	default:
 		return nil, badErr
 	}
 
@@ -200,20 +201,6 @@ func (listener *CarbonlinkListener) SetReadTimeout(timeout time.Duration) {
 	listener.readTimeout = timeout
 }
 
-func pickleWriteMemo(b *bytes.Buffer, memo *uint32) {
-	if *memo < 256 {
-		b.WriteByte('q')
-		b.WriteByte(uint8(*memo))
-	} else {
-		b.WriteByte('r')
-		var buf [4]byte
-		s := buf[:]
-		binary.LittleEndian.PutUint32(s, *memo)
-		b.Write(s)
-	}
-	*memo += 1
-}
-
 func picklePoint(b *bytes.Buffer, p points.Point) {
 	var buf [8]byte
 	s := buf[:]
@@ -239,17 +226,16 @@ func packReply(data []points.Point) []byte {
 		buf.WriteByte('(')
 	}
 
-	if data != nil {
-		for _, point := range data {
-			picklePoint(buf, point)
-		}
+	for _, point := range data {
+		picklePoint(buf, point)
 	}
 
-	if numPoints == 0 {
+	switch {
+	case numPoints == 0:
 		buf.Write([]byte{'s', '.'})
-	} else if numPoints == 1 {
+	case numPoints == 1:
 		buf.Write([]byte{'a', 's', '.'})
-	} else if numPoints > 1 {
+	case numPoints > 1:
 		buf.Write([]byte{'e', 's', '.'})
 	}
 
@@ -319,10 +305,8 @@ func (listener *CarbonlinkListener) Listen(addr *net.TCPAddr) error {
 		listener.tcpListener = tcpListener
 
 		listener.Go(func(exit chan bool) {
-			select {
-			case <-exit:
-				tcpListener.Close()
-			}
+			<-exit
+			tcpListener.Close()
 		})
 
 		listener.Go(func(exit chan bool) {

carbon/collector.go

@@ -27,7 +27,7 @@ type Collector struct {
 	endpoint       string
 	data           chan *points.Points
 	stats          []statFunc
-	logger         *zap.Logger
+	logger         *zap.Logger //nolint:unused,structcheck
 }
 
 func RuntimeStat(send helper.StatCallback) {

carbon/config.go

@@ -86,10 +86,6 @@ type grpcConfig struct {
 	Enabled bool   `toml:"enabled"`
 }
 
-type receiverConfig struct {
-	DSN string `toml:"dsn"`
-}
-
 type tagsConfig struct {
 	Enabled             bool      `toml:"enabled"`
 	TagDB               string    `toml:"tagdb-url"`

carbon/restore_test.go

@@ -28,46 +28,46 @@ func TestRestore(t *testing.T) {
 		w("cache.33.1470687188677488570", "")
 
 		expected := []*points.Points{
-			&points.Points{
+			{
 				Metric: "m2",
 				Data: []points.Point{
-					points.Point{
+					{
 						Value:     2.000000,
 						Timestamp: 1470687039,
 					},
 				},
 			},
-			&points.Points{
+			{
 				Metric: "m1",
 				Data: []points.Point{
-					points.Point{
+					{
 						Value:     1.000000,
 						Timestamp: 1470687039,
 					},
 				},
 			},
-			&points.Points{
+			{
 				Metric: "m5",
 				Data: []points.Point{
-					points.Point{
+					{
 						Value:     5.000000,
 						Timestamp: 1470687217,
 					},
 				},
 			},
-			&points.Points{
+			{
 				Metric: "m4",
 				Data: []points.Point{
-					points.Point{
+					{
 						Value:     4.000000,
 						Timestamp: 1470687217,
 					},
 				},
 			},
-			&points.Points{
+			{
 				Metric: "m3",
 				Data: []points.Point{
-					points.Point{
+					{
 						Value:     3.000000,
 						Timestamp: 1470687217,
 					},

carbonserver/cache_index_test.go

@@ -126,11 +126,9 @@ func (f *testInfo) checkExpandGlobs(t *testing.T, query string, shdExist bool) {
 			t.Errorf("files: '%v', epxected: '%s'\n", file, query)
 			return
 		}
-	} else {
-		if len(expandedGlobs[0].Files) != 0 {
-			t.Errorf("expected no files but found - '%v'\n", expandedGlobs[0].Files)
-			return
-		}
+	} else if len(expandedGlobs[0].Files) != 0 {
+		t.Errorf("expected no files but found - '%v'\n", expandedGlobs[0].Files)
+		return
 	}
 }
 

carbonserver/capability.go

@@ -70,8 +70,8 @@ func (listener *CarbonserverListener) capabilityHandler(wr http.ResponseWriter,
 			hostname = "(unknown)"
 		}
 		pvResponse := protov3.CapabilityResponse{
-			SupportedProtocols: []string{"carbonapi_v3_pb", "carbonapi_v2_pb", "graphite-web-pickle", "graphite-web-pickle-1.1", "carbonapi_v2_json"},
-			Name:               hostname,
+			SupportedProtocols:        []string{"carbonapi_v3_pb", "carbonapi_v2_pb", "graphite-web-pickle", "graphite-web-pickle-1.1", "carbonapi_v2_json"},
+			Name:                      hostname,
 			HighPrecisionTimestamps:   false,
 			SupportFilteringFunctions: false,
 			LikeSplittedRequests:      true,
@@ -83,7 +83,7 @@ func (listener *CarbonserverListener) capabilityHandler(wr http.ResponseWriter,
 		switch formatCode {
 		case jsonFormat:
 			contentType = httpHeaders.ContentTypeJSON
-			data, err = json.Marshal(pvResponse)
+			data, _ = json.Marshal(pvResponse)
 		case protoV3Format:
 			contentType = httpHeaders.ContentTypeCarbonAPIv3PB
 			data, err = pvResponse.Marshal()
@@ -116,5 +116,4 @@ func (listener *CarbonserverListener) capabilityHandler(wr http.ResponseWriter,
 		zap.Duration("runtime_seconds", time.Since(t0)),
 		zap.Int("http_code", http.StatusOK),
 	)
-	return
 }