From 2a75604f8e6dfb9e8da7e4fec4b9f4b71457519f Mon Sep 17 00:00:00 2001 From: Oleksandr Redko Date: Wed, 29 Nov 2023 19:23:34 +0200 Subject: [PATCH] Enable default revive rules (#13068) Signed-off-by: Oleksandr Redko --- .golangci.yml | 37 +++++- cmd/promtool/main.go | 12 +- discovery/openstack/instance.go | 8 +- model/histogram/float_histogram.go | 108 +++++++++--------- promql/parser/lex.go | 3 +- promql/parser/parse.go | 2 +- storage/remote/azuread/azuread.go | 2 +- storage/remote/azuread/azuread_test.go | 2 +- .../prometheus/normalize_label.go | 1 - tsdb/errors/errors.go | 2 +- tsdb/wlog/wlog.go | 7 +- util/annotations/annotations.go | 13 ++- 12 files changed, 113 insertions(+), 84 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 4df572c19..158bb3579 100644 --- a/.golangci.yml +++ b/.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 diff --git a/cmd/promtool/main.go b/cmd/promtool/main.go index 973795a86..cb7e9d03d 100644 --- a/cmd/promtool/main.go +++ b/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)} } } diff --git a/discovery/openstack/instance.go b/discovery/openstack/instance.go index b2fe1e787..9b28c1d6e 100644 --- a/discovery/openstack/instance.go +++ b/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 { diff --git a/model/histogram/float_histogram.go b/model/histogram/float_histogram.go index 6fa221354..1c2f3ebec 100644 --- a/model/histogram/float_histogram.go +++ b/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 } } diff --git a/promql/parser/lex.go b/promql/parser/lex.go index 4f602433d..641bedcfc 100644 --- a/promql/parser/lex.go +++ b/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) } diff --git a/promql/parser/parse.go b/promql/parser/parse.go index 09d84fe6b..122286c55 100644 --- a/promql/parser/parse.go +++ b/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 diff --git a/storage/remote/azuread/azuread.go b/storage/remote/azuread/azuread.go index 393886286..20d48d008 100644 --- a/storage/remote/azuread/azuread.go +++ b/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"` diff --git a/storage/remote/azuread/azuread_test.go b/storage/remote/azuread/azuread_test.go index 1143b71da..ad7ba63a0 100644 --- a/storage/remote/azuread/azuread_test.go +++ b/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) diff --git a/storage/remote/otlptranslator/prometheus/normalize_label.go b/storage/remote/otlptranslator/prometheus/normalize_label.go index af0960e86..b44ba869f 100644 --- a/storage/remote/otlptranslator/prometheus/normalize_label.go +++ b/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 diff --git a/tsdb/errors/errors.go b/tsdb/errors/errors.go index 6a8e72f04..ff230c44b 100644 --- a/tsdb/errors/errors.go +++ b/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 diff --git a/tsdb/wlog/wlog.go b/tsdb/wlog/wlog.go index c4305bcbf..c3ae001d9 100644 --- a/tsdb/wlog/wlog.go +++ b/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) } diff --git a/util/annotations/annotations.go b/util/annotations/annotations.go index 2041d0217..180d40843 100644 --- a/util/annotations/annotations.go +++ b/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),