diff --git a/cmd/constable/main.go b/cmd/constable/main.go index 992a88b..1ea8f60 100644 --- a/cmd/constable/main.go +++ b/cmd/constable/main.go @@ -2,6 +2,7 @@ package main import ( "github.com/nnutter/constable/internal/analysis/complexity" + "github.com/nnutter/constable/internal/analysis/errcheck" "github.com/nnutter/constable/internal/analysis/methodical" "github.com/nnutter/constable/internal/analysis/nonmutating" "github.com/nnutter/constable/internal/analysis/predicates" @@ -15,5 +16,13 @@ import ( var version string func main() { - driver.Main(version, nonmutating.Analyzer, methodical.Analyzer, predicates.Analyzer, testify.Analyzer, complexity.Analyzer) + driver.Main( + version, + nonmutating.Analyzer, + methodical.Analyzer, + predicates.Analyzer, + testify.Analyzer, + complexity.Analyzer, + errcheck.Analyzer, + ) } diff --git a/go.mod b/go.mod index 77ea8c2..0b51c97 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/nnutter/constable go 1.27.3 require ( + github.com/kisielk/errcheck v1.30.0 github.com/stretchr/testify v1.12.1 golang.org/x/tools v0.52.0 ) diff --git a/go.sum b/go.sum index 47a4388..0ecfdac 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,7 @@ github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= +github.com/kisielk/errcheck v1.30.0 h1:ZIxNDAfAd4larXHTceoZP/dYPAB90dHg1OYhA2sVh5k= +github.com/kisielk/errcheck v1.30.0/go.mod h1:uiCAInmFuyOje4VDdbQSP9mgqf5gORYSXrDf19XilsI= github.com/stretchr/testify v1.12.1 h1:EuwCh5fleGS7H32xRwO3wRGT7DxrDhLAT6FF8MpWDWE= github.com/stretchr/testify v1.12.1/go.mod h1:MDEgiDPPsNp5cuIrHPPCyornHKgEVbtFUmoNlxoYthg= go.yaml.in/yaml/v3 v3.0.5 h1:N6y/pJk8buWs9NY5ERU2HSMfm+IuD/OtfdAnq6kESPw= diff --git a/internal/analysis/errcheck/errcheck.go b/internal/analysis/errcheck/errcheck.go new file mode 100644 index 0000000..f0be609 --- /dev/null +++ b/internal/analysis/errcheck/errcheck.go @@ -0,0 +1,16 @@ +// Package errcheck adapts the errcheck analyzer to constable. +// +// constable embeds the analyzer from another module instead of reimplementing +// it, so this package holds the places where constable's own policy differs +// from the upstream default. +package errcheck + +import ( + upstream "github.com/kisielk/errcheck/errcheck" + + "github.com/nnutter/constable/internal/generated" +) + +// Analyzer reports unchecked errors. It is the upstream analyzer with +// constable's generated-file policy applied. +var Analyzer = generated.Skip(upstream.Analyzer) diff --git a/internal/analysis/errcheck/errcheck_test.go b/internal/analysis/errcheck/errcheck_test.go new file mode 100644 index 0000000..53dbfe5 --- /dev/null +++ b/internal/analysis/errcheck/errcheck_test.go @@ -0,0 +1,13 @@ +package errcheck_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + + "github.com/nnutter/constable/internal/analysis/errcheck" +) + +func TestAnalyzer(t *testing.T) { + analysistest.Run(t, analysistest.TestData(), errcheck.Analyzer, "er") +} diff --git a/internal/analysis/errcheck/testdata/src/er/er.go b/internal/analysis/errcheck/testdata/src/er/er.go new file mode 100644 index 0000000..b5d9b0f --- /dev/null +++ b/internal/analysis/errcheck/testdata/src/er/er.go @@ -0,0 +1,7 @@ +package er + +import "os" + +func F() { + os.Remove("x") // want `unchecked error` +} diff --git a/internal/analysis/errcheck/testdata/src/er/zz_generated.go b/internal/analysis/errcheck/testdata/src/er/zz_generated.go new file mode 100644 index 0000000..c8fd150 --- /dev/null +++ b/internal/analysis/errcheck/testdata/src/er/zz_generated.go @@ -0,0 +1,9 @@ +// Code generated by hand. DO NOT EDIT. + +package er + +import "os" + +func Generated() { + os.Remove("x") +} diff --git a/internal/generated/skip.go b/internal/generated/skip.go new file mode 100644 index 0000000..875e4fd --- /dev/null +++ b/internal/generated/skip.go @@ -0,0 +1,50 @@ +package generated + +import ( + "go/ast" + + "golang.org/x/tools/go/analysis" +) + +// Skip returns a copy of a that reports no diagnostic in a generated file. +// +// constable skips generated files because a diagnostic there is not +// actionable: the next run of the generator overwrites the file. An analyzer +// that constable embeds from another module does not know this policy, so +// Skip applies it. +func Skip(a *analysis.Analyzer) *analysis.Analyzer { + skipped := *a + run := a.Run + skipped.Run = func(pass *analysis.Pass) (any, error) { + generated := generatedFiles(pass, pass.Files) + if len(generated) == 0 { + return run(pass) + } + + filtered := *pass + report := pass.Report + filtered.Report = func(diag analysis.Diagnostic) { + if generated[pass.Fset.Position(diag.Pos).Filename] { + return + } + report(diag) + } + return run(&filtered) + } + return &skipped +} + +// generatedFiles returns the names of the generated files in files. +func generatedFiles(pass *analysis.Pass, files []*ast.File) map[string]bool { + var names map[string]bool + for _, file := range files { + if !IsGenerated(file) { + continue + } + if names == nil { + names = make(map[string]bool) + } + names[pass.Fset.Position(file.Pos()).Filename] = true + } + return names +} diff --git a/internal/generated/skip_test.go b/internal/generated/skip_test.go new file mode 100644 index 0000000..f08eb77 --- /dev/null +++ b/internal/generated/skip_test.go @@ -0,0 +1,34 @@ +package generated_test + +import ( + "go/ast" + "testing" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/analysistest" + + "github.com/nnutter/constable/internal/generated" +) + +// reportFuncs reports every function declaration, so a test can observe which +// files the analyzer reaches. +var reportFuncs = &analysis.Analyzer{ + Name: "reportfuncs", + Doc: "reports every function declaration", + Run: func(pass *analysis.Pass) (any, error) { + for _, file := range pass.Files { + for _, decl := range file.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok { + continue + } + pass.Reportf(fn.Name.Pos(), "func %s", fn.Name.Name) + } + } + return nil, nil + }, +} + +func TestSkip(t *testing.T) { + analysistest.Run(t, analysistest.TestData(), generated.Skip(reportFuncs), "g") +} diff --git a/internal/generated/testdata/src/g/g.go b/internal/generated/testdata/src/g/g.go new file mode 100644 index 0000000..ba9c948 --- /dev/null +++ b/internal/generated/testdata/src/g/g.go @@ -0,0 +1,3 @@ +package g + +func Gen() {} // want `func Gen` diff --git a/internal/generated/testdata/src/g/zz_generated.go b/internal/generated/testdata/src/g/zz_generated.go new file mode 100644 index 0000000..cc2e573 --- /dev/null +++ b/internal/generated/testdata/src/g/zz_generated.go @@ -0,0 +1,5 @@ +// Code generated by hand. DO NOT EDIT. + +package g + +func Generated() {}