Skip to content
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
33 changes: 33 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,39 @@ A move is conservative, because the analyzer must be sure that the result compil
A fix writes only files that Git tracks and that have no changes, so `git checkout` recovers a fix that went wrong.
Diagnostics that remain after the fixes are printed, and the command exits unsuccessfully.

## Embedded analyzers

constable runs four analyzers from other modules: `errcheck`, `sumtype`, `testifylint`, and `nilaway`.
Each keeps its upstream name, so its flags keep the upstream prefix, such as `-errcheck.blank`, `-sumtype.include-shared-interfaces`, and `-testifylint.disable-all`.
Run `constable -flags` for the complete list.
Each embedded analyzer skips files that carry the generated-code comment, the same as the constable analyzers.

### errcheck

Reports an error that the code discards.

### sumtype

Reports a type switch over a sum type that omits a variant.
Mark the interface with `//sumtype:decl` and the switch must cover every variant.

### testifylint

Reports a test that misuses testify.

### nilaway

Reports a nil value that flows to a dereference.
NilAway reads every dependency by default, which costs seconds and gigabytes.
Give it the module of the code that constable analyzes:

```sh
constable -nilaway_config.include-pkgs=$(go list -m) ./...
```

The `nilaway_config` prefix is the name of the analyzer that holds the configuration, not the name of the analyzer that reports the diagnostic.
The driver registers the flags of a required analyzer, so this flag reaches the analyzer that reads it.

## Development

Run all fixes,
Expand Down
2 changes: 2 additions & 0 deletions cmd/constable/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ 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/nilaway"
"github.com/nnutter/constable/internal/analysis/nonmutating"
"github.com/nnutter/constable/internal/analysis/predicates"
"github.com/nnutter/constable/internal/analysis/sumtype"
Expand All @@ -28,5 +29,6 @@ func main() {
errcheck.Analyzer,
sumtype.Analyzer,
testifylint.Analyzer,
nilaway.Analyzer,
)
}
95 changes: 95 additions & 0 deletions cmd/constable/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,101 @@ func TestCLIComplexityPass(t *testing.T) {
assert.Empty(t, output)
}

func TestCLIEmbeddedAnalyzerFlags(t *testing.T) {
binary := buildBinary(t)

command := exec.Command(binary, "-flags")
output, err := command.CombinedOutput()
require.NoError(t, err, string(output))

// A flag of a required analyzer must reach the command line, because the
// analyzer that constable embeds reads its configuration from it.
for _, name := range []string{
"errcheck",
"errcheck.blank",
"sumtype",
"sumtype.include-shared-interfaces",
"testifylint",
"testifylint.disable-all",
"nilaway",
"nilaway_config.include-pkgs",
} {
assert.Contains(t, string(output), `"`+name+`"`)
}
}

func TestCLIEmbeddedNilawayReadsRequiredAnalyzerFlag(t *testing.T) {
binary := buildBinary(t)
moduleDir := writeModule(t, module{
GoMod: []string{
"module example.com/nil",
"",
"go 1.26",
},
Files: map[string][]string{"a.go": {
"package nil",
"",
"type T struct{ X int }",
"",
"func F() {",
" var t *T",
" _ = t.X",
"}",
}},
})

// The package is out of scope, so nilaway must stay silent. This holds
// only when the flag of the required configuration analyzer is parsed.
output, err := runConstable(t, binary, moduleDir, "-nilaway_config.include-pkgs=example.com/other")
require.NoError(t, err, output)
assert.Empty(t, output)

output, err = runConstable(t, binary, moduleDir)
require.Error(t, err)
assert.Contains(t, output, "Potential nil panic detected")
}

func TestCLIEmbeddedAnalyzersSkipGeneratedFiles(t *testing.T) {
binary := buildBinary(t)
moduleDir := writeModule(t, module{
GoMod: []string{
"module example.com/generated",
"",
"go 1.26",
},
Files: map[string][]string{"gen.go": {
"// Code generated by hand. DO NOT EDIT.",
"",
"package generated",
"",
"import \"os\"",
"",
"//sumtype:decl",
"type Shape interface{ shape() }",
"",
"type Circle struct{}",
"",
"func (*Circle) shape() {}",
"",
"type Square struct{}",
"",
"func (*Square) shape() {}",
"",
"func F(s Shape) {",
"\t_ = os.Remove(\"x\")",
"\tswitch s.(type) {",
"\tcase *Circle:",
"\t}",
"}",
}},
})

output, err := runConstable(t, binary, moduleDir)

require.NoError(t, err, output)
assert.Empty(t, output)
}

func buildBinary(t *testing.T) string {
t.Helper()

Expand Down
4 changes: 3 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@ require (
github.com/kisielk/errcheck v1.30.0
github.com/nnutter/testifylint v0.0.1
github.com/stretchr/testify v1.12.1
go.uber.org/nilaway v0.0.0-20260918162853-acb8859b9031
golang.org/x/tools v0.52.0
)

require (
github.com/google/go-cmp v0.7.0 // indirect
github.com/klauspost/compress v1.20.0 // indirect
go.yaml.in/yaml/v3 v3.0.5 // indirect
golang.org/x/exp/typeparams v0.0.0-20260908205506-85c1c2202aba // indirect
golang.org/x/mod v0.42.0 // indirect
golang.org/x/sync v0.24.0 // indirect
)
10 changes: 10 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,22 @@ 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/klauspost/compress v1.20.0 h1:a3C1ke2ohxFymNlb2HWAHjDeKCI90scRskErZkR0ezA=
github.com/klauspost/compress v1.20.0/go.mod h1:LUdAzn7YLVvxLpc7y3V1m40wESHTgc1422pwwBSKYuI=
github.com/nnutter/testifylint v0.0.1 h1:QhFq2HgC7LxnaG5Z1Gi4aYN42y+fywfMY+e9xTYe0WM=
github.com/nnutter/testifylint v0.0.1/go.mod h1:DvNLVJQrA3YfswHJxaGzitfYt2gxFzlIs29buaH8sP4=
github.com/stretchr/objx v0.5.3 h1:jmXUvGomnU1o3W/V5h2VEradbpJDwGrzugQQvL0POH4=
github.com/stretchr/objx v0.5.3/go.mod h1:rDQraq+vQZU7Fde9LOZLr8Tax6zZvy4kuNKF+QYS+U0=
github.com/stretchr/testify v1.12.1 h1:EuwCh5fleGS7H32xRwO3wRGT7DxrDhLAT6FF8MpWDWE=
github.com/stretchr/testify v1.12.1/go.mod h1:MDEgiDPPsNp5cuIrHPPCyornHKgEVbtFUmoNlxoYthg=
go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto=
go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE=
go.uber.org/nilaway v0.0.0-20260918162853-acb8859b9031 h1:WRbdrL7q1+PV8w97QZDQUlCnyM0aKHrpMizqHM+reWc=
go.uber.org/nilaway v0.0.0-20260918162853-acb8859b9031/go.mod h1:h/Z4MnNYOaqAjgJglu/yah/YzvCdcJDvDth5TWUqIJc=
go.yaml.in/yaml/v3 v3.0.5 h1:N6y/pJk8buWs9NY5ERU2HSMfm+IuD/OtfdAnq6kESPw=
go.yaml.in/yaml/v3 v3.0.5/go.mod h1:HVTZu1O7/Vkt2N+BFy8Zza+lnLsABggaTM2ZpNIGuKg=
golang.org/x/exp/typeparams v0.0.0-20260908205506-85c1c2202aba h1:ZoZmFkcuQ289xJilBm9Ck3W+/okrhXc4LUpbtKFQE4k=
golang.org/x/exp/typeparams v0.0.0-20260908205506-85c1c2202aba/go.mod h1:qkMnvgP8C7+xkci04txL2SJFc/0PGqnkf6gs5JUC43E=
golang.org/x/mod v0.42.0 h1:ICeL3t2L30ljSPHHxDnR4fqVcBt/zR2zJAb9juavmQg=
golang.org/x/mod v0.42.0/go.mod h1:u5fzIrRInkL8MaO1cbRkUHT64v303YIT5KEqpNr7jRs=
golang.org/x/sync v0.24.0 h1:iqiDrGoDgyuvAKrhLQthUEzU/R378N78gXbBjxed7oA=
Expand Down
7 changes: 5 additions & 2 deletions hk.pkl
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,12 @@ exclude = List("**/testdata/**")

// This repository develops constable, so every hook runs the working tree's
// build instead of the installed release.
// nilaway is part of constable now, and it reads every dependency unless it is
// told which module to analyze. Without the flag, a hook spends seconds and
// gigabytes on the standard library.
local const constableDev = (Extra.constable) {
check = "go run ./cmd/constable ./..."
fix = "go run ./cmd/constable -fix ./..."
check = "go run ./cmd/constable -nilaway_config.include-pkgs=$(go list -m) ./..."
fix = "go run ./cmd/constable -nilaway_config.include-pkgs=$(go list -m) -fix ./..."
}

// The templates build the check, fix, pre-commit, and commit-msg hooks, and
Expand Down
26 changes: 26 additions & 0 deletions internal/analysis/nilaway/nilaway.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Package nilaway adapts the nilaway 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 nilaway

import (
"go.uber.org/nilaway"
nilawayconfig "go.uber.org/nilaway/config"

"github.com/nnutter/constable/internal/generated"
)

// Analyzer reports a nil value that flows to a dereference. It is the upstream
// analyzer with constable's generated-file policy applied.
var Analyzer = generated.Skip(nilaway.Analyzer)

func init() {
// nilaway pretty-prints a multi-line explanation by default, with color
// when the terminal supports it. constable prints one diagnostic per line
// for editors and hooks, so the default is off. A flag still turns it on.
if flag := nilawayconfig.Analyzer.Flags.Lookup(nilawayconfig.PrettyPrintFlag); flag != nil {
_ = flag.Value.Set("false")
}
}
13 changes: 13 additions & 0 deletions internal/analysis/nilaway/nilaway_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package nilaway_test

import (
"testing"

"golang.org/x/tools/go/analysis/analysistest"

"github.com/nnutter/constable/internal/analysis/nilaway"
)

func TestAnalyzer(t *testing.T) {
analysistest.Run(t, analysistest.TestData(), nilaway.Analyzer, "na")
}
8 changes: 8 additions & 0 deletions internal/analysis/nilaway/testdata/src/na/na.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package na

type T struct{ X int }

func F() {
var t *T
_ = t.X // want `Potential nil panic detected`
}
8 changes: 8 additions & 0 deletions internal/analysis/nilaway/testdata/src/na/zz_generated.go

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

62 changes: 52 additions & 10 deletions internal/driver/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,15 +124,10 @@ func Main(version string, analyzers ...*analysis.Analyzer) {
enable := new(triState)
flag.Var(enable, a.Name, fmt.Sprintf("enable %q analysis", a.Name))
enabled[a] = enable

a.Flags.VisitAll(func(f *flag.Flag) {
name := a.Name + "." + f.Name
if flag.Lookup(name) == nil {
flag.Var(f.Value, name, f.Usage)
}
})
}

registerFlags(flag.CommandLine, analyzers)

printFlags := flag.Bool("flags", false, "print analyzer flags in JSON")
var showVersion versionValue
flag.Var(&showVersion, "V", "print version and exit")
Expand Down Expand Up @@ -174,6 +169,38 @@ func Main(version string, analyzers ...*analysis.Analyzer) {
os.Exit(Run(args, analyzers, opts))
}

// registerFlags registers the flags of each analyzer in the requirement graph
// of analyzers under the "<analyzer>.<flag>" names. A required analyzer
// carries configuration that its dependents read, so the flags of a required
// analyzer must reach the command line even though only the roots have an
// enable flag.
func registerFlags(fs *flag.FlagSet, analyzers []*analysis.Analyzer) {
seen := make(map[*analysis.Analyzer]bool)
for _, a := range analyzers {
registerAnalyzerFlags(fs, a, seen)
}
}

// registerAnalyzerFlags registers the flags of a and of the analyzers it
// requires.
func registerAnalyzerFlags(fs *flag.FlagSet, a *analysis.Analyzer, seen map[*analysis.Analyzer]bool) {
if seen[a] {
return
}
seen[a] = true

a.Flags.VisitAll(func(f *flag.Flag) {
name := a.Name + "." + f.Name
if fs.Lookup(name) == nil {
fs.Var(f.Value, name, f.Usage)
}
})

for _, req := range a.Requires {
registerAnalyzerFlags(fs, req, seen)
}
}

func succeeded(ok bool) bool {
return ok
}
Expand Down Expand Up @@ -389,16 +416,31 @@ func writeHelp(progname string, analyzers []*analysis.Analyzer, args []string) {
for _, a := range analyzers {
if a.Name == args[0] {
_, _ = fmt.Fprintf(os.Stdout, "%s: %s\n", a.Name, a.Doc)
a.Flags.VisitAll(func(f *flag.Flag) {
_, _ = fmt.Fprintf(os.Stdout, "-%s.%s: %s\n", a.Name, f.Name, f.Usage)
})
writeAnalyzerFlags(a, make(map[*analysis.Analyzer]bool))
return
}
}
fmt.Fprintf(os.Stderr, "unknown analyzer %q\n", args[0])
os.Exit(1)
}

// writeAnalyzerFlags writes the flags of a and of the analyzers it requires,
// because a required analyzer carries configuration that a reads.
func writeAnalyzerFlags(a *analysis.Analyzer, seen map[*analysis.Analyzer]bool) {
if seen[a] {
return
}
seen[a] = true

a.Flags.VisitAll(func(f *flag.Flag) {
_, _ = fmt.Fprintf(os.Stdout, "-%s.%s: %s\n", a.Name, f.Name, f.Usage)
})

for _, req := range a.Requires {
writeAnalyzerFlags(req, seen)
}
}

// Run loads the packages and applies the analyzers, printing
// diagnostics with paths relative to the current working directory.
// It returns the appropriate exit code.
Expand Down
Loading
Loading