feat: add spancheck linter (#4290)

jjti · ldez · web-flow · commit d23c35470b89 · 2024-01-03T04:24:30.000+01:00
Co-authored-by: Fernandez Ludovic &lt;ldez@users.noreply.github.com&gt;
diff --git a/.golangci.reference.yml b/.golangci.reference.yml
@@ -1912,6 +1912,24 @@ linters-settings:
     # Default: false
     args-on-sep-lines: true
 
+  spancheck:
+    # Checks to enable.
+    # Options include:
+    # - `end`: check that `span.End()` is called
+    # - `record-error`: check that `span.RecordError(err)` is called when an error is returned
+    # - `set-status`: check that `span.SetStatus(codes.Error, msg)` is called when an error is returned
+    # Default: ["end"]
+    checks:
+      - end
+      - record-error
+      - set-status
+    # A list of regexes for function signatures that silence `record-error` and `set-status` reports
+    # if found in the call path to a returned error.
+    # https://github.com/jjti/go-spancheck#ignore-check-signatures
+    # Default: []
+    ignore-check-signatures:
+      - "telemetry.RecordError"
+
   staticcheck:
     # Deprecated: use the global `run.go` instead.
     go: "1.15"
@@ -2442,6 +2460,7 @@ linters:
     - rowserrcheck
     - scopelint
     - sloglint
+    - spancheck
     - sqlclosecheck
     - staticcheck
     - structcheck
@@ -2562,6 +2581,7 @@ linters:
     - rowserrcheck
     - scopelint
     - sloglint
+    - spancheck
     - sqlclosecheck
     - staticcheck
     - structcheck
diff --git a/go.mod b/go.mod
@@ -57,6 +57,7 @@ require (
 	github.com/jgautheron/goconst v1.7.0
 	github.com/jingyugao/rowserrcheck v1.1.1
 	github.com/jirfag/go-printf-func-name v0.0.0-20200119135958-7558a9eaa5af
+	github.com/jjti/go-spancheck v0.4.2
 	github.com/julz/importas v0.1.0
 	github.com/kisielk/errcheck v1.6.3
 	github.com/kkHAIKE/contextcheck v1.1.4
diff --git a/go.sum b/go.sum
diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go
@@ -245,13 +245,14 @@ type LintersSettings struct {
 	Revive           ReviveSettings
 	RowsErrCheck     RowsErrCheckSettings
 	SlogLint         SlogLintSettings
+	Spancheck        SpancheckSettings
 	Staticcheck      StaticCheckSettings
 	Structcheck      StructCheckSettings
 	Stylecheck       StaticCheckSettings
 	TagAlign         TagAlignSettings
 	Tagliatelle      TagliatelleSettings
-	Testifylint      TestifylintSettings
 	Tenv             TenvSettings
+	Testifylint      TestifylintSettings
 	Testpackage      TestpackageSettings
 	Thelper          ThelperSettings
 	Unparam          UnparamSettings
@@ -773,6 +774,11 @@ type SlogLintSettings struct {
 	ArgsOnSepLines bool   `mapstructure:"args-on-sep-lines"`
 }
 
+type SpancheckSettings struct {
+	Checks                []string `mapstructure:"checks"`
+	IgnoreCheckSignatures []string `mapstructure:"ignore-check-signatures"`
+}
+
 type StaticCheckSettings struct {
 	// Deprecated: use the global `run.go` instead.
 	GoVersion string `mapstructure:"go"`
diff --git a/pkg/golinters/spancheck.go b/pkg/golinters/spancheck.go
@@ -0,0 +1,29 @@
+package golinters
+
+import (
+	"github.com/jjti/go-spancheck"
+	"golang.org/x/tools/go/analysis"
+
+	"github.com/golangci/golangci-lint/pkg/config"
+	"github.com/golangci/golangci-lint/pkg/golinters/goanalysis"
+)
+
+func NewSpancheck(settings *config.SpancheckSettings) *goanalysis.Linter {
+	cfg := spancheck.NewDefaultConfig()
+
+	if settings != nil {
+		if settings.Checks != nil {
+			cfg.EnabledChecks = settings.Checks
+		}
+
+		if settings.IgnoreCheckSignatures != nil {
+			cfg.IgnoreChecksSignaturesSlice = settings.IgnoreCheckSignatures
+		}
+	}
+
+	a := spancheck.NewAnalyzerWithConfig(cfg)
+
+	return goanalysis.
+		NewLinter(a.Name, a.Doc, []*analysis.Analyzer{a}, nil).
+		WithLoadMode(goanalysis.LoadModeTypesInfo)
+}
diff --git a/pkg/lint/lintersdb/manager.go b/pkg/lint/lintersdb/manager.go
@@ -131,6 +131,7 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config {
 		reviveCfg           *config.ReviveSettings
 		rowserrcheckCfg     *config.RowsErrCheckSettings
 		sloglintCfg         *config.SlogLintSettings
+		spancheckCfg        *config.SpancheckSettings
 		staticcheckCfg      *config.StaticCheckSettings
 		structcheckCfg      *config.StructCheckSettings
 		stylecheckCfg       *config.StaticCheckSettings
@@ -216,6 +217,7 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config {
 		reviveCfg = &m.cfg.LintersSettings.Revive
 		rowserrcheckCfg = &m.cfg.LintersSettings.RowsErrCheck
 		sloglintCfg = &m.cfg.LintersSettings.SlogLint
+		spancheckCfg = &m.cfg.LintersSettings.Spancheck
 		staticcheckCfg = &m.cfg.LintersSettings.Staticcheck
 		structcheckCfg = &m.cfg.LintersSettings.Structcheck
 		stylecheckCfg = &m.cfg.LintersSettings.Stylecheck
@@ -782,6 +784,12 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config {
 			WithLoadForGoAnalysis().
 			WithURL("https://github.com/ryanrolds/sqlclosecheck"),
 
+		linter.NewConfig(golinters.NewSpancheck(spancheckCfg)).
+			WithSince("v1.56.0").
+			WithLoadForGoAnalysis().
+			WithPresets(linter.PresetBugs).
+			WithURL("https://github.com/jjti/go-spancheck"),
+
 		linter.NewConfig(golinters.NewStaticcheck(staticcheckCfg)).
 			WithEnabledByDefault().
 			WithSince("v1.0.0").
diff --git a/test/linters_test.go b/test/linters_test.go
@@ -33,6 +33,7 @@ func TestSourcesFromTestdataSubDir(t *testing.T) {
 		"ginkgolinter",
 		"zerologlint",
 		"protogetter",
+		"spancheck",
 	}
 
 	for _, dir := range subDirs {
diff --git a/test/testdata/spancheck/configs/enable_all.yml b/test/testdata/spancheck/configs/enable_all.yml
@@ -0,0 +1,8 @@
+linters-settings:
+  spancheck:
+    checks:
+      - "end"
+      - "record-error"
+      - "set-status"
+    ignore-check-signatures:
+      - "recordErr"
diff --git a/test/testdata/spancheck/go.mod b/test/testdata/spancheck/go.mod
@@ -0,0 +1,14 @@
+module spancheck
+
+go 1.20
+
+require (
+	go.opentelemetry.io/otel v1.21.0
+	go.opentelemetry.io/otel/trace v1.21.0
+)
+
+require (
+	github.com/go-logr/logr v1.4.1 // indirect
+	github.com/go-logr/stdr v1.2.2 // indirect
+	go.opentelemetry.io/otel/metric v1.21.0 // indirect
+)
diff --git a/test/testdata/spancheck/go.sum b/test/testdata/spancheck/go.sum
diff --git a/test/testdata/spancheck/spancheck_default.go b/test/testdata/spancheck/spancheck_default.go
@@ -0,0 +1,90 @@
+//golangcitest:args -Espancheck
+package spancheck
+
+import (
+	"context"
+	"errors"
+	"fmt"
+
+	"go.opentelemetry.io/otel"
+	"go.opentelemetry.io/otel/codes"
+)
+
+type testDefaultError struct{}
+
+func (e *testDefaultError) Error() string {
+	return "foo"
+}
+
+// incorrect
+
+func _() {
+	otel.Tracer("foo").Start(context.Background(), "bar")           // want "span is unassigned, probable memory leak"
+	ctx, _ := otel.Tracer("foo").Start(context.Background(), "bar") // want "span is unassigned, probable memory leak"
+	fmt.Print(ctx)
+}
+
+func _() {
+	ctx, span := otel.Tracer("foo").Start(context.Background(), "bar") // want "span.End is not called on all paths, possible memory leak"
+	print(ctx.Done(), span.IsRecording())
+} // want "return can be reached without calling span.End"
+
+func _() {
+	var ctx, span = otel.Tracer("foo").Start(context.Background(), "bar") // want "span.End is not called on all paths, possible memory leak"
+	print(ctx.Done(), span.IsRecording())
+} // want "return can be reached without calling span.End"
+
+func _() {
+	_, span := otel.Tracer("foo").Start(context.Background(), "bar") // want "span.End is not called on all paths, possible memory leak"
+	_, span = otel.Tracer("foo").Start(context.Background(), "bar")
+	fmt.Print(span)
+	defer span.End()
+} // want "return can be reached without calling span.End"
+
+// correct
+
+func _() error {
+	_, span := otel.Tracer("foo").Start(context.Background(), "bar")
+	defer span.End()
+
+	return nil
+}
+
+func _() error {
+	_, span := otel.Tracer("foo").Start(context.Background(), "bar")
+	defer span.End()
+
+	if true {
+		return nil
+	}
+
+	return nil
+}
+
+func _() error {
+	_, span := otel.Tracer("foo").Start(context.Background(), "bar")
+	defer span.End()
+
+	if false {
+		err := errors.New("foo")
+		span.SetStatus(codes.Error, err.Error())
+		span.RecordError(err)
+		return err
+	}
+
+	if true {
+		span.SetStatus(codes.Error, "foo")
+		span.RecordError(errors.New("foo"))
+		return errors.New("bar")
+	}
+
+	return nil
+}
+
+func _() {
+	_, span := otel.Tracer("foo").Start(context.Background(), "bar")
+	defer span.End()
+
+	_, span = otel.Tracer("foo").Start(context.Background(), "bar")
+	defer span.End()
+}
diff --git a/test/testdata/spancheck/spancheck_enable_all.go b/test/testdata/spancheck/spancheck_enable_all.go

-Original file line number
+Diff line change
 	github.com/jgautheron/goconst v1.7.0
 	github.com/jingyugao/rowserrcheck v1.1.1
 	github.com/jirfag/go-printf-func-name v0.0.0-20200119135958-7558a9eaa5af
 +	github.com/jjti/go-spancheck v0.4.2
 	github.com/julz/importas v0.1.0
 	github.com/kisielk/errcheck v1.6.3
 	github.com/kkHAIKE/contextcheck v1.1.4
Original file line number	Diff line number	Diff line change
`@@ -33,6 +33,7 @@ func TestSourcesFromTestdataSubDir(t *testing.T) {`
`33`	`33`	`"ginkgolinter",`
`34`	`34`	`"zerologlint",`
`35`	`35`	`"protogetter",`
	`36`	`+ "spancheck",`
`36`	`37`	`}`
`37`	`38`
`38`	`39`	`for _, dir := range subDirs {`