Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add spancheck linter #4290

Merged
merged 24 commits into from
Jan 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .golangci.reference.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -2442,6 +2460,7 @@ linters:
- rowserrcheck
- scopelint
- sloglint
- spancheck
- sqlclosecheck
- staticcheck
- structcheck
Expand Down Expand Up @@ -2562,6 +2581,7 @@ linters:
- rowserrcheck
- scopelint
- sloglint
- spancheck
- sqlclosecheck
- staticcheck
- structcheck
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 7 additions & 1 deletion pkg/config/linters_settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"`
Expand Down
29 changes: 29 additions & 0 deletions pkg/golinters/spancheck.go
Original file line number Diff line number Diff line change
@@ -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)
}
8 changes: 8 additions & 0 deletions pkg/lint/lintersdb/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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").
Expand Down
1 change: 1 addition & 0 deletions test/linters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ func TestSourcesFromTestdataSubDir(t *testing.T) {
"ginkgolinter",
"zerologlint",
"protogetter",
"spancheck",
}

for _, dir := range subDirs {
Expand Down
8 changes: 8 additions & 0 deletions test/testdata/spancheck/configs/enable_all.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
linters-settings:
spancheck:
checks:
- "end"
- "record-error"
- "set-status"
ignore-check-signatures:
- "recordErr"
14 changes: 14 additions & 0 deletions test/testdata/spancheck/go.mod
Original file line number Diff line number Diff line change
@@ -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
)
16 changes: 16 additions & 0 deletions test/testdata/spancheck/go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

90 changes: 90 additions & 0 deletions test/testdata/spancheck/spancheck_default.go
Original file line number Diff line number Diff line change
@@ -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()
}
Loading
Loading