Enable default revive rules (#13068)

Signed-off-by: Oleksandr Redko <Oleksandr_Redko@epam.com>

.golangci.yml

@@ -81,8 +81,39 @@ linters-settings:
   gofumpt:
     extra-rules: true
   revive:
+    # By default, revive will enable only the linting rules that are named in the configuration file.
+    # So, it's needed to explicitly set in configuration all required rules.
+    # The following configuration enables all the rules from the defaults.toml
+    # https://github.com/mgechev/revive/blob/master/defaults.toml
     rules:
-      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#unused-parameter
-      - name: unused-parameter
-        severity: warning
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md
+      - name: blank-imports
+      - name: context-as-argument
+        arguments:
+          # allow functions with test or bench signatures
+          - allowTypesBefore: "*testing.T,testing.TB"
+      - name: context-keys-type
+      - name: dot-imports
+      # A lot of false positives: incorrectly identifies channel draining as "empty code block".
+      # See https://github.com/mgechev/revive/issues/386
+      - name: empty-block
         disabled: true
+      - name: error-naming
+      - name: error-return
+      - name: error-strings
+      - name: errorf
+      - name: exported
+      - name: increment-decrement
+      - name: indent-error-flow
+      - name: package-comments
+      - name: range
+      - name: receiver-naming
+      - name: redefines-builtin-id
+      - name: superfluous-else
+      - name: time-naming
+      - name: unexported-return
+      - name: unreachable-code
+      - name: unused-parameter
+        disabled: true
+      - name: var-declaration
+      - name: var-naming

cmd/promtool/main.go

@@ -411,7 +411,7 @@ func checkExperimental(f bool) {
 	}
 }
 
-var lintError = fmt.Errorf("lint error")
+var errLint = fmt.Errorf("lint error")
 
 type lintConfig struct {
 	all            bool
@@ -763,7 +763,7 @@ func checkRulesFromStdin(ls lintConfig) (bool, bool) {
 		fmt.Fprintln(os.Stderr, "  FAILED:")
 		for _, e := range errs {
 			fmt.Fprintln(os.Stderr, e.Error())
-			hasErrors = hasErrors || !errors.Is(e, lintError)
+			hasErrors = hasErrors || !errors.Is(e, errLint)
 		}
 		if hasErrors {
 			return failed, hasErrors
@@ -776,7 +776,7 @@ func checkRulesFromStdin(ls lintConfig) (bool, bool) {
 		}
 		failed = true
 		for _, err := range errs {
-			hasErrors = hasErrors || !errors.Is(err, lintError)
+			hasErrors = hasErrors || !errors.Is(err, errLint)
 		}
 	} else {
 		fmt.Printf("  SUCCESS: %d rules found\n", n)
@@ -797,7 +797,7 @@ func checkRules(files []string, ls lintConfig) (bool, bool) {
 			fmt.Fprintln(os.Stderr, "  FAILED:")
 			for _, e := range errs {
 				fmt.Fprintln(os.Stderr, e.Error())
-				hasErrors = hasErrors || !errors.Is(e, lintError)
+				hasErrors = hasErrors || !errors.Is(e, errLint)
 			}
 			if hasErrors {
 				continue
@@ -810,7 +810,7 @@ func checkRules(files []string, ls lintConfig) (bool, bool) {
 			}
 			failed = true
 			for _, err := range errs {
-				hasErrors = hasErrors || !errors.Is(err, lintError)
+				hasErrors = hasErrors || !errors.Is(err, errLint)
 			}
 		} else {
 			fmt.Printf("  SUCCESS: %d rules found\n", n)
@@ -837,7 +837,7 @@ func checkRuleGroups(rgs *rulefmt.RuleGroups, lintSettings lintConfig) (int, []e
 				})
 			}
 			errMessage += "Might cause inconsistency while recording expressions"
-			return 0, []error{fmt.Errorf("%w %s", lintError, errMessage)}
+			return 0, []error{fmt.Errorf("%w %s", errLint, errMessage)}
 		}
 	}
 

discovery/openstack/instance.go

@@ -145,16 +145,16 @@ func (i *InstanceDiscovery) refresh(ctx context.Context) ([]*targetgroup.Group,
 				openstackLabelUserID:         model.LabelValue(s.UserID),
 			}
 
-			flavorId, ok := s.Flavor["id"].(string)
+			flavorID, ok := s.Flavor["id"].(string)
 			if !ok {
 				level.Warn(i.logger).Log("msg", "Invalid type for flavor id, expected string")
 				continue
 			}
-			labels[openstackLabelInstanceFlavor] = model.LabelValue(flavorId)
+			labels[openstackLabelInstanceFlavor] = model.LabelValue(flavorID)
 
-			imageId, ok := s.Image["id"].(string)
+			imageID, ok := s.Image["id"].(string)
 			if ok {
-				labels[openstackLabelInstanceImage] = model.LabelValue(imageId)
+				labels[openstackLabelInstanceImage] = model.LabelValue(imageID)
 			}
 
 			for k, v := range s.Metadata {

model/histogram/float_histogram.go

@@ -353,12 +353,12 @@ func (h *FloatHistogram) Equals(h2 *FloatHistogram) bool {
 // Size returns the total size of the FloatHistogram, which includes the size of the pointer
 // to FloatHistogram, all its fields, and all elements contained in slices.
 // NOTE: this is only valid for 64 bit architectures.
-func (fh *FloatHistogram) Size() int {
+func (h *FloatHistogram) Size() int {
 	// Size of each slice separately.
-	posSpanSize := len(fh.PositiveSpans) * 8     // 8 bytes (int32 + uint32).
-	negSpanSize := len(fh.NegativeSpans) * 8     // 8 bytes (int32 + uint32).
-	posBucketSize := len(fh.PositiveBuckets) * 8 // 8 bytes (float64).
-	negBucketSize := len(fh.NegativeBuckets) * 8 // 8 bytes (float64).
+	posSpanSize := len(h.PositiveSpans) * 8     // 8 bytes (int32 + uint32).
+	negSpanSize := len(h.NegativeSpans) * 8     // 8 bytes (int32 + uint32).
+	posBucketSize := len(h.PositiveBuckets) * 8 // 8 bytes (float64).
+	negBucketSize := len(h.NegativeBuckets) * 8 // 8 bytes (float64).
 
 	// Total size of the struct.
 
@@ -1000,8 +1000,8 @@ func addBuckets(
 	spansB []Span, bucketsB []float64,
 ) ([]Span, []float64) {
 	var (
-		iSpan              int = -1
-		iBucket            int = -1
+		iSpan              = -1
+		iBucket            = -1
 		iInSpan            int32
 		indexA             int32
 		indexB             int32
@@ -1034,17 +1034,16 @@ func addBuckets(
 						spansA[0].Length++
 						spansA[0].Offset--
 						goto nextLoop
-					} else {
-						spansA = append(spansA, Span{})
-						copy(spansA[1:], spansA)
-						spansA[0] = Span{Offset: indexB, Length: 1}
-						if len(spansA) > 1 {
-							// Convert the absolute offset in the formerly
-							// first span to a relative offset.
-							spansA[1].Offset -= indexB + 1
-						}
-						goto nextLoop
 					}
+					spansA = append(spansA, Span{})
+					copy(spansA[1:], spansA)
+					spansA[0] = Span{Offset: indexB, Length: 1}
+					if len(spansA) > 1 {
+						// Convert the absolute offset in the formerly
+						// first span to a relative offset.
+						spansA[1].Offset -= indexB + 1
+					}
+					goto nextLoop
 				} else if spansA[0].Offset == indexB {
 					// Just add to first bucket.
 					bucketsA[0] += bucketB
@@ -1062,48 +1061,47 @@ func addBuckets(
 					iInSpan += deltaIndex
 					bucketsA[iBucket] += bucketB
 					break
-				} else {
-					deltaIndex -= remainingInSpan
-					iBucket += int(remainingInSpan)
-					iSpan++
-					if iSpan == len(spansA) || deltaIndex < spansA[iSpan].Offset {
-						// Bucket is in gap behind previous span (or there are no further spans).
-						bucketsA = append(bucketsA, 0)
-						copy(bucketsA[iBucket+1:], bucketsA[iBucket:])
-						bucketsA[iBucket] = bucketB
-						switch {
-						case deltaIndex == 0:
-							// Directly after previous span, extend previous span.
-							if iSpan < len(spansA) {
-								spansA[iSpan].Offset--
-							}
-							iSpan--
-							iInSpan = int32(spansA[iSpan].Length)
-							spansA[iSpan].Length++
-							goto nextLoop
-						case iSpan < len(spansA) && deltaIndex == spansA[iSpan].Offset-1:
-							// Directly before next span, extend next span.
-							iInSpan = 0
+				}
+				deltaIndex -= remainingInSpan
+				iBucket += int(remainingInSpan)
+				iSpan++
+				if iSpan == len(spansA) || deltaIndex < spansA[iSpan].Offset {
+					// Bucket is in gap behind previous span (or there are no further spans).
+					bucketsA = append(bucketsA, 0)
+					copy(bucketsA[iBucket+1:], bucketsA[iBucket:])
+					bucketsA[iBucket] = bucketB
+					switch {
+					case deltaIndex == 0:
+						// Directly after previous span, extend previous span.
+						if iSpan < len(spansA) {
 							spansA[iSpan].Offset--
-							spansA[iSpan].Length++
-							goto nextLoop
-						default:
-							// No next span, or next span is not directly adjacent to new bucket.
-							// Add new span.
-							iInSpan = 0
-							if iSpan < len(spansA) {
-								spansA[iSpan].Offset -= deltaIndex + 1
-							}
-							spansA = append(spansA, Span{})
-							copy(spansA[iSpan+1:], spansA[iSpan:])
-							spansA[iSpan] = Span{Length: 1, Offset: deltaIndex}
-							goto nextLoop
 						}
-					} else {
-						// Try start of next span.
-						deltaIndex -= spansA[iSpan].Offset
+						iSpan--
+						iInSpan = int32(spansA[iSpan].Length)
+						spansA[iSpan].Length++
+						goto nextLoop
+					case iSpan < len(spansA) && deltaIndex == spansA[iSpan].Offset-1:
+						// Directly before next span, extend next span.
 						iInSpan = 0
+						spansA[iSpan].Offset--
+						spansA[iSpan].Length++
+						goto nextLoop
+					default:
+						// No next span, or next span is not directly adjacent to new bucket.
+						// Add new span.
+						iInSpan = 0
+						if iSpan < len(spansA) {
+							spansA[iSpan].Offset -= deltaIndex + 1
+						}
+						spansA = append(spansA, Span{})
+						copy(spansA[iSpan+1:], spansA[iSpan:])
+						spansA[iSpan] = Span{Length: 1, Offset: deltaIndex}
+						goto nextLoop
 					}
+				} else {
+					// Try start of next span.
+					deltaIndex -= spansA[iSpan].Offset
+					iInSpan = 0
 				}
 			}
 

promql/parser/lex.go

@@ -566,9 +566,8 @@ Loop:
 				if l.peek() == ':' {
 					l.emit(desc)
 					return lexHistogram
-				} else {
-					l.errorf("missing `:` for histogram descriptor")
 				}
+				l.errorf("missing `:` for histogram descriptor")
 			} else {
 				l.errorf("bad histogram descriptor found: %q", word)
 			}

promql/parser/parse.go

@@ -72,7 +72,7 @@ func WithFunctions(functions map[string]*Function) Opt {
 }
 
 // NewParser returns a new parser.
-func NewParser(input string, opts ...Opt) *parser {
+func NewParser(input string, opts ...Opt) *parser { //nolint:revive // unexported-return.
 	p := parserPool.Get().(*parser)
 
 	p.functions = Functions

storage/remote/azuread/azuread.go

@@ -62,7 +62,7 @@ type OAuthConfig struct {
 }
 
 // AzureADConfig is used to store the config values.
-type AzureADConfig struct {
+type AzureADConfig struct { //nolint:revive // exported.
 	// ManagedIdentity is the managed identity that is being used to authenticate.
 	ManagedIdentity *ManagedIdentityConfig `yaml:"managed_identity,omitempty"`
 

storage/remote/azuread/azuread_test.go

@@ -283,7 +283,7 @@ func (s *TokenProviderTestSuite) TestNewTokenProvider() {
 			s.Assert().NotNil(actualTokenProvider.getAccessToken(context.Background()))
 
 			s.mockCredential.AssertNumberOfCalls(s.T(), "GetToken", 2*mockGetTokenCallCounter)
-			mockGetTokenCallCounter += 1
+			mockGetTokenCallCounter++
 			accessToken, err := actualTokenProvider.getAccessToken(context.Background())
 			s.Assert().Nil(err)
 			s.Assert().NotEqual(accessToken, testTokenString)

storage/remote/otlptranslator/prometheus/normalize_label.go

@@ -25,7 +25,6 @@ var dropSanitizationGate = featuregate.GlobalRegistry().MustRegister(
 //
 // Exception is made for double-underscores which are allowed
 func NormalizeLabel(label string) string {
-
 	// Trivial case
 	if len(label) == 0 {
 		return label

tsdb/errors/errors.go

@@ -25,7 +25,7 @@ import (
 type multiError []error
 
 // NewMulti returns multiError with provided errors added if not nil.
-func NewMulti(errs ...error) multiError {
+func NewMulti(errs ...error) multiError { //nolint:revive // unexported-return.
 	m := multiError{}
 	m.Add(errs...)
 	return m

tsdb/wlog/wlog.go

@@ -970,7 +970,7 @@ type segmentBufReader struct {
 	off  int // Offset of read data into current segment.
 }
 
-func NewSegmentBufReader(segs ...*Segment) *segmentBufReader {
+func NewSegmentBufReader(segs ...*Segment) io.ReadCloser {
 	if len(segs) == 0 {
 		return &segmentBufReader{}
 	}
@@ -981,15 +981,16 @@ func NewSegmentBufReader(segs ...*Segment) *segmentBufReader {
 	}
 }
 
-func NewSegmentBufReaderWithOffset(offset int, segs ...*Segment) (sbr *segmentBufReader, err error) {
+func NewSegmentBufReaderWithOffset(offset int, segs ...*Segment) (io.ReadCloser, error) {
 	if offset == 0 || len(segs) == 0 {
 		return NewSegmentBufReader(segs...), nil
 	}
 
-	sbr = &segmentBufReader{
+	sbr := &segmentBufReader{
 		buf:  bufio.NewReaderSize(segs[0], 16*pageSize),
 		segs: segs,
 	}
+	var err error
 	if offset > 0 {
 		_, err = sbr.buf.Discard(offset)
 	}

util/annotations/annotations.go

@@ -94,6 +94,7 @@ func (a Annotations) AsStrings(query string, maxAnnos int) []string {
 	return arr
 }
 
+//nolint:revive // error-naming.
 var (
 	// Currently there are only 2 types, warnings and info.
 	// For now, info are visually identical with warnings as we have not updated
@@ -130,7 +131,7 @@ func (e annoErr) Unwrap() error {
 
 // NewInvalidQuantileWarning is used when the user specifies an invalid quantile
 // value, i.e. a float that is outside the range [0, 1] or NaN.
-func NewInvalidQuantileWarning(q float64, pos posrange.PositionRange) annoErr {
+func NewInvalidQuantileWarning(q float64, pos posrange.PositionRange) error {
 	return annoErr{
 		PositionRange: pos,
 		Err:           fmt.Errorf("%w, got %g", InvalidQuantileWarning, q),
@@ -139,7 +140,7 @@ func NewInvalidQuantileWarning(q float64, pos posrange.PositionRange) annoErr {
 
 // NewBadBucketLabelWarning is used when there is an error parsing the bucket label
 // of a classic histogram.
-func NewBadBucketLabelWarning(metricName, label string, pos posrange.PositionRange) annoErr {
+func NewBadBucketLabelWarning(metricName, label string, pos posrange.PositionRange) error {
 	return annoErr{
 		PositionRange: pos,
 		Err:           fmt.Errorf("%w of %q for metric name %q", BadBucketLabelWarning, label, metricName),
@@ -149,7 +150,7 @@ func NewBadBucketLabelWarning(metricName, label string, pos posrange.PositionRan
 // NewMixedFloatsHistogramsWarning is used when the queried series includes both
 // float samples and histogram samples for functions that do not support mixed
 // samples.
-func NewMixedFloatsHistogramsWarning(metricName string, pos posrange.PositionRange) annoErr {
+func NewMixedFloatsHistogramsWarning(metricName string, pos posrange.PositionRange) error {
 	return annoErr{
 		PositionRange: pos,
 		Err:           fmt.Errorf("%w %q", MixedFloatsHistogramsWarning, metricName),
@@ -158,7 +159,7 @@ func NewMixedFloatsHistogramsWarning(metricName string, pos posrange.PositionRan
 
 // NewMixedClassicNativeHistogramsWarning is used when the queried series includes
 // both classic and native histograms.
-func NewMixedClassicNativeHistogramsWarning(metricName string, pos posrange.PositionRange) annoErr {
+func NewMixedClassicNativeHistogramsWarning(metricName string, pos posrange.PositionRange) error {
 	return annoErr{
 		PositionRange: pos,
 		Err:           fmt.Errorf("%w %q", MixedClassicNativeHistogramsWarning, metricName),
@@ -167,7 +168,7 @@ func NewMixedClassicNativeHistogramsWarning(metricName string, pos posrange.Posi
 
 // NewPossibleNonCounterInfo is used when a named counter metric with only float samples does not
 // have the suffixes _total, _sum, _count, or _bucket.
-func NewPossibleNonCounterInfo(metricName string, pos posrange.PositionRange) annoErr {
+func NewPossibleNonCounterInfo(metricName string, pos posrange.PositionRange) error {
 	return annoErr{
 		PositionRange: pos,
 		Err:           fmt.Errorf("%w %q", PossibleNonCounterInfo, metricName),
@@ -176,7 +177,7 @@ func NewPossibleNonCounterInfo(metricName string, pos posrange.PositionRange) an
 
 // NewHistogramQuantileForcedMonotonicityInfo is used when the input (classic histograms) to
 // histogram_quantile needs to be forced to be monotonic.
-func NewHistogramQuantileForcedMonotonicityInfo(metricName string, pos posrange.PositionRange) annoErr {
+func NewHistogramQuantileForcedMonotonicityInfo(metricName string, pos posrange.PositionRange) error {
 	return annoErr{
 		PositionRange: pos,
 		Err:           fmt.Errorf("%w %q", HistogramQuantileForcedMonotonicityInfo, metricName),