diff --git a/.golangci.yml b/.golangci.yml index 90011ed3a3a..03322ddd264 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -86,6 +86,9 @@ linters: # Finds sending HTTP request without context.Context. - noctx + # Fast, configurable, extensible, flexible, and beautiful linter for Go. Drop-in replacement of golint. + - revive + # Checks usage of github.com/stretchr/testify. - testifylint @@ -112,7 +115,7 @@ linters-settings: desc: "Do not refer to crossdock from other packages" files: - "!**/crossdock/**" - + disallow-uber/goleak: deny: - pkg: go.uber.org/goleak @@ -143,6 +146,118 @@ linters-settings: - fieldalignment # Disable shadow - shadow + revive: + ignore-generated-header: true + severity: error + enable-all-rules: true + # See https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md + rules: + # not a completely bad linter, but needs clean-up and sensible width (80 is too small) + - name: line-length-limit + disabled: true + arguments: [80] + # would be ok if we could exclude the test files, but otherwise too noisy + - name: add-constant + disabled: true + # maybe enable in the future, needs more investigation + - name: cognitive-complexity + disabled: true + # not sure how different from previous one + - name: cyclomatic + disabled: true + # do a clean-up and enable + - name: use-any + disabled: true + # do a clean-up and enable + - name: unused-parameter + disabled: true + # do a clean-up and enable + - name: unused-receiver + disabled: true + # we use storage_v2, so... + - name: var-naming + disabled: true + # could be useful to catch issues, but needs a clean-up and some ignores + - name: unchecked-type-assertion + disabled: true + # wtf: "you have exceeded the maximum number of public struct declarations" + - name: max-public-structs + disabled: true + # probably a good one to enable after cleanup + - name: import-shadowing + disabled: true + # TBD - often triggered in tests + - name: unhandled-error + disabled: true + # this one looks like it's catching real errors, need to enable it + - name: modifies-value-receiver + disabled: true + # often looks like a red herring, needs investigation + - name: flag-parameter + disabled: true + # looks like a good linter, needs cleanup + - name: confusing-naming + disabled: true + # too pendantic + - name: function-length + disabled: true + # definitely a good one, needs cleanup first + - name: argument-limit + disabled: true + # another good one, needs cleanup first + - name: unexported-naming + disabled: true + # maybe enable, needs invesitgation of the impact + - name: import-alias-naming + disabled: true + # maybe enable, needs invesitgation of the impact + - name: get-return + disabled: true + # enable after cleanup + - name: early-return + disabled: true + # enable after cleanup + - name: bare-return + disabled: true + # enable after cleanup + - name: empty-lines + disabled: true + # investigate, could be real bugs. But didn't recent Go version changed loop variables semantics? + - name: range-val-address + disabled: true + # enable after cleanup + - name: confusing-results + disabled: true + # we do use dot imports, but not a bad idea to make it explicit + - name: dot-imports + disabled: true + # enable after cleanup: "tag on not-exported field" + - name: struct-tag + disabled: true + # enable after cleanup + - name: receiver-naming + disabled: true + # this is idiocy, promotes less readable code. Don't enable. + - name: var-declaration + disabled: true + # enable after cleanup: warns of stutter names + - name: exported + disabled: true + # enable after cleanup + - name: redefines-builtin-id + disabled: true + # "no nested structs are allowed" - don't enable, doesn't make sense + - name: nested-structs + disabled: true + # enable after cleanup + - name: indent-error-flow + disabled: true + # enable after cleanup + - name: unexported-return + disabled: true + # looks useful, but requires refactoring: "calls to log.Fatal only in main() or init() functions" + - name: deep-exit + disabled: true testifylint: disable: - float-compare diff --git a/cmd/agent/app/processors/thrift_processor_test.go b/cmd/agent/app/processors/thrift_processor_test.go index f3315f26430..ce824a13fff 100644 --- a/cmd/agent/app/processors/thrift_processor_test.go +++ b/cmd/agent/app/processors/thrift_processor_test.go @@ -79,6 +79,7 @@ func createProcessor(t *testing.T, mFactory metrics.Factory, tFactory thrift.TPr return transport.Addr().String(), processor } +// revive:disable-next-line function-result-limit func initCollectorAndReporter(t *testing.T) (*metricstest.Factory, *testutils.GrpcCollector, reporter.Reporter, *grpc.ClientConn) { grpcCollector := testutils.StartGRPCCollector(t) conn, err := grpc.NewClient(grpcCollector.Listener().Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) diff --git a/cmd/agent/app/reporter/metrics_test.go b/cmd/agent/app/reporter/metrics_test.go index 6795781b28f..39806d462e6 100644 --- a/cmd/agent/app/reporter/metrics_test.go +++ b/cmd/agent/app/reporter/metrics_test.go @@ -101,11 +101,13 @@ func TestMetricsReporter(t *testing.T) { } for _, test := range tests { - metricsFactory := metricstest.NewFactory(time.Microsecond) - defer metricsFactory.Stop() - r := WrapWithMetrics(test.rep, metricsFactory) - test.action(r) - metricsFactory.AssertCounterMetrics(t, test.expectedCounters...) - metricsFactory.AssertGaugeMetrics(t, test.expectedGauges...) + t.Run("", func(t *testing.T) { + metricsFactory := metricstest.NewFactory(time.Microsecond) + defer metricsFactory.Stop() + r := WrapWithMetrics(test.rep, metricsFactory) + test.action(r) + metricsFactory.AssertCounterMetrics(t, test.expectedCounters...) + metricsFactory.AssertGaugeMetrics(t, test.expectedGauges...) + }) } } diff --git a/cmd/collector/app/span_processor_test.go b/cmd/collector/app/span_processor_test.go index 9d7b5f21bb9..eff4f6d63bb 100644 --- a/cmd/collector/app/span_processor_test.go +++ b/cmd/collector/app/span_processor_test.go @@ -81,7 +81,7 @@ func TestBySvcMetrics(t *testing.T) { } } - for _, test := range tests { + testFn := func(t *testing.T, test TestCase) { mb := metricstest.NewFactory(time.Hour) defer mb.Backend.Stop() logger := zap.NewNop() @@ -160,6 +160,11 @@ func TestBySvcMetrics(t *testing.T) { } mb.AssertCounterMetrics(t, expected...) } + for _, test := range tests { + t.Run(fmt.Sprintf("%v", test), func(t *testing.T) { + testFn(t, test) + }) + } } func isSpanAllowed(span *model.Span) bool { diff --git a/cmd/ingester/app/consumer/consumer.go b/cmd/ingester/app/consumer/consumer.go index 93b77d04f2b..65d90f8ff9a 100644 --- a/cmd/ingester/app/consumer/consumer.go +++ b/cmd/ingester/app/consumer/consumer.go @@ -145,6 +145,7 @@ func (c *Consumer) handleMessages(pc sc.PartitionConsumer) { if msgProcessor == nil { msgProcessor = c.processorFactory.new(pc.Topic(), pc.Partition(), msg.Offset-1) + // revive:disable-next-line defer defer msgProcessor.Close() } diff --git a/cmd/internal/flags/service_test.go b/cmd/internal/flags/service_test.go index fbdd13fb3cf..9c4ec76fd37 100644 --- a/cmd/internal/flags/service_test.go +++ b/cmd/internal/flags/service_test.go @@ -74,7 +74,7 @@ func TestStartErrors(t *testing.T) { } for _, test := range scenarios { t.Run(test.name, func(t *testing.T) { - s := NewService( /*default port=*/ 0) + s := NewService( /* default port= */ 0) v, cmd := config.Viperize(s.AddFlags) err := cmd.ParseFlags(test.flags) require.NoError(t, err) diff --git a/cmd/jaeger/internal/extension/jaegerstorage/extension.go b/cmd/jaeger/internal/extension/jaegerstorage/extension.go index 23709c00d3b..5c43e23f29b 100644 --- a/cmd/jaeger/internal/extension/jaegerstorage/extension.go +++ b/cmd/jaeger/internal/extension/jaegerstorage/extension.go @@ -17,7 +17,6 @@ import ( memoryCfg "github.com/jaegertracing/jaeger/pkg/memory/config" "github.com/jaegertracing/jaeger/pkg/metrics" "github.com/jaegertracing/jaeger/plugin/storage/badger" - badgerCfg "github.com/jaegertracing/jaeger/plugin/storage/badger" "github.com/jaegertracing/jaeger/plugin/storage/cassandra" "github.com/jaegertracing/jaeger/plugin/storage/es" "github.com/jaegertracing/jaeger/plugin/storage/grpc" @@ -79,7 +78,7 @@ type starter[Config any, Factory storage.Factory] struct { builder func(Config, metrics.Factory, *zap.Logger) (Factory, error) } -func (s *starter[Config, Factory]) build(ctx context.Context, host component.Host) error { +func (s *starter[Config, Factory]) build(_ context.Context, _ component.Host) error { for name, cfg := range s.cfg { if _, ok := s.ext.factories[name]; ok { return fmt.Errorf("duplicate %s storage name %s", s.storageKind, name) @@ -111,7 +110,7 @@ func (s *storageExt) Start(ctx context.Context, host component.Host) error { return memory.NewFactoryWithConfig(cfg, metricsFactory, logger), nil }, } - badgerStarter := &starter[badgerCfg.NamespaceConfig, *badger.Factory]{ + badgerStarter := &starter[badger.NamespaceConfig, *badger.Factory]{ ext: s, storageKind: "badger", cfg: s.config.Badger, diff --git a/cmd/query/app/grpc_handler_test.go b/cmd/query/app/grpc_handler_test.go index bec65d371b3..2254bd2de12 100644 --- a/cmd/query/app/grpc_handler_test.go +++ b/cmd/query/app/grpc_handler_test.go @@ -948,6 +948,7 @@ func withTenantedServerAndClient(t *testing.T, tm *tenancy.Manager, actualTest f } // withOutgoingMetadata returns a Context with metadata for a server to receive +// revive:disable-next-line context-as-argument func withOutgoingMetadata(t *testing.T, ctx context.Context, headerName, headerValue string) context.Context { t.Helper() diff --git a/cmd/query/app/internal/api_v3/traces.go b/cmd/query/app/internal/api_v3/traces.go index c38841c7785..7090f099689 100644 --- a/cmd/query/app/internal/api_v3/traces.go +++ b/cmd/query/app/internal/api_v3/traces.go @@ -5,7 +5,7 @@ package api_v3 import ( "github.com/gogo/protobuf/jsonpb" - proto "github.com/gogo/protobuf/proto" + "github.com/gogo/protobuf/proto" "go.opentelemetry.io/collector/pdata/ptrace" "github.com/jaegertracing/jaeger/pkg/gogocodec"