Limit nilaway to the main module by default

README.md

@@ -143,15 +143,22 @@ 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:
+
+NilAway reads every package that it loads by default, which costs seconds and gigabytes and reports flows that the code under analysis cannot fix.
+constable limits it to the main modules of the packages that it analyzes.
+Two flags change the choice,
 
 ```sh
-constable -nilaway_config.include-pkgs=$(go list -m) ./...
+# Analyze the packages in this module, and no other package.
+constable -nilaway_config.include-pkgs=example.com/my/module ./...
+
+# Analyze every package, including dependencies and the standard library.
+constable -nilaway.include-all-pkgs ./...
 ```
 
+An explicit `-nilaway_config.include-pkgs` wins over `-nilaway.include-all-pkgs`, and both win over the default.
 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.
+The driver registers the flags of a required analyzer, so that flag reaches the analyzer that reads it.
 
 ## Development
 

cmd/constable/main.go

@@ -21,6 +21,7 @@ var version string
 func main() {
 	driver.Main(
 		version,
+		driver.Hooks{AfterLoad: nilaway.UseMainModules},
 		nonmutating.Analyzer,
 		methodical.Analyzer,
 		predicates.Analyzer,

cmd/constable/main_test.go

@@ -318,13 +318,14 @@ func TestCLIEmbeddedAnalyzerFlags(t *testing.T) {
 		"testifylint",
 		"testifylint.disable-all",
 		"nilaway",
+		"nilaway.include-all-pkgs",
 		"nilaway_config.include-pkgs",
 	} {
 		assert.Contains(t, string(output), `"`+name+`"`)
 	}
 }
 
-func TestCLIEmbeddedNilawayReadsRequiredAnalyzerFlag(t *testing.T) {
+func TestCLIEmbeddedNilawayPackages(t *testing.T) {
 	binary := buildBinary(t)
 	moduleDir := writeModule(t, module{
 		GoMod: []string{
@@ -350,9 +351,12 @@ func TestCLIEmbeddedNilawayReadsRequiredAnalyzerFlag(t *testing.T) {
 	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")
+	// The default covers the main module, and every package keeps the report.
+	for _, args := range [][]string{{}, {"-nilaway.include-all-pkgs"}} {
+		output, err = runConstable(t, binary, moduleDir, args...)
+		require.Error(t, err, "args=%v output=%s", args, output)
+		assert.Contains(t, output, "Potential nil panic detected", "args=%v", args)
+	}
 }
 
 func TestCLIEmbeddedAnalyzersSkipGeneratedFiles(t *testing.T) {

hk.pkl

@@ -16,12 +16,9 @@ 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 -nilaway_config.include-pkgs=$(go list -m) ./..."
-  fix = "go run ./cmd/constable -nilaway_config.include-pkgs=$(go list -m) -fix ./..."
+  check = "go run ./cmd/constable ./..."
+  fix = "go run ./cmd/constable -fix ./..."
 }
 
 // The templates build the check, fix, pre-commit, and commit-msg hooks, and

internal/analysis/complexity/complexity.go

@@ -42,7 +42,7 @@ func isLogicalOr(op token.Token) bool {
 
 func run(pass *analysis.Pass) (any, error) {
 	for _, file := range pass.Files {
-		if generated.IsGenerated(file) {
+		if generated.Skipped(file) {
 			continue
 		}
 

internal/analysis/methodical/fix.go

@@ -85,7 +85,7 @@ func readFixFile(pass *analysis.Pass, file *ast.File) *fixFile {
 		return nil
 	}
 	src, err := pass.ReadFile(tf.Name())
-	if err != nil || generated.IsGenerated(file) || hasStandaloneComments(pass.Fset, file) {
+	if err != nil || generated.Skipped(file) || hasStandaloneComments(pass.Fset, file) {
 		return nil
 	}
 	return &fixFile{file: file, token: tf, source: src}

internal/analysis/methodical/methodical.go

@@ -64,7 +64,7 @@ func isNilVar(v *types.Var) bool {
 func collectTypeFiles(pass *analysis.Pass) map[types.Object]string {
 	typeFiles := make(map[types.Object]string)
 	for _, file := range pass.Files {
-		if generated.IsGenerated(file) {
+		if generated.Skipped(file) {
 			continue
 		}
 
@@ -107,7 +107,7 @@ type groupKey struct {
 func collectMethods(pass *analysis.Pass, typeFiles map[types.Object]string) []methodInfo {
 	var methods []methodInfo
 	for _, file := range pass.Files {
-		if generated.IsGenerated(file) {
+		if generated.Skipped(file) {
 			continue
 		}
 
@@ -242,7 +242,7 @@ func checkGrouped(reporter *diagnosticReporter, typeFiles map[types.Object]strin
 }
 
 func checkFileGrouped(reporter *diagnosticReporter, file *ast.File, typeFiles map[types.Object]string) {
-	if generated.IsGenerated(file) {
+	if generated.Skipped(file) {
 		return
 	}
 

internal/analysis/nilaway/nilaway.go

@@ -6,21 +6,101 @@
 package nilaway
 
 import (
+	"flag"
+	"sort"
+	"strings"
+
 	"go.uber.org/nilaway"
 	nilawayconfig "go.uber.org/nilaway/config"
+	"golang.org/x/tools/go/packages"
 
 	"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.
+// analyzer with constable's generated-file policy applied and with the
+// analyzed packages limited to the module that constable runs on.
 var Analyzer = generated.Skip(nilaway.Analyzer)
 
+// includeAllPkgs keeps nilaway's own default, every package that it loads.
+var includeAllPkgs bool
+
 func init() {
+	Analyzer.Flags.BoolVar(&includeAllPkgs, "include-all-pkgs", false,
+		"analyze every package that nilaway loads, including dependencies and the standard library")
+
 	// 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")
 	}
 }
+
+// UseMainModules limits nilaway to the main modules of the packages that the
+// driver loaded, unless the command line chose the packages itself.
+//
+// NilAway reads every package that it loads by default, which costs seconds
+// and gigabytes and reports flows that the analyzed repository cannot fix. Its
+// -include-pkgs flag takes package path prefixes, and the module path of a
+// root covers every package of that module.
+//
+// UseMainModules is a driver.Hooks.AfterLoad step: the packages and their
+// modules exist only after the driver loads them.
+func UseMainModules(roots []*packages.Package) {
+	if includeAllPkgs {
+		return
+	}
+
+	chosen := nilawayconfig.Analyzer.Flags.Lookup(nilawayconfig.IncludePkgsFlag)
+	if chosen == nil {
+		return
+	}
+	if isChosen(chosen) {
+		return
+	}
+
+	modules := mainModules(roots)
+	if len(modules) == 0 {
+		return
+	}
+	_ = chosen.Value.Set(strings.Join(modules, ","))
+}
+
+// mainModules returns the sorted paths of the main modules of roots.
+//
+// A root outside a module, such as a package of the standard library that a
+// pattern names, contributes nothing.
+func mainModules(roots []*packages.Package) []string {
+	var paths []string
+	seen := make(map[string]bool)
+	for _, root := range roots {
+		if isNilRoot(root) || isNilModule(root.Module) || isDependencyModule(root.Module) || isSeen(seen, root.Module.Path) {
+			continue
+		}
+		seen[root.Module.Path] = true
+		paths = append(paths, root.Module.Path)
+	}
+	sort.Strings(paths)
+	return paths
+}
+
+func isChosen(f *flag.Flag) bool {
+	return f.Value.String() != ""
+}
+
+func isNilRoot(root *packages.Package) bool {
+	return root == nil
+}
+
+func isNilModule(module *packages.Module) bool {
+	return module == nil
+}
+
+func isDependencyModule(module *packages.Module) bool {
+	return !module.Main
+}
+
+func isSeen(seen map[string]bool, path string) bool {
+	return seen[path]
+}

internal/analysis/nilaway/nilaway_test.go

@@ -3,11 +3,101 @@ package nilaway_test
 import (
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"go.uber.org/nilaway/config"
 	"golang.org/x/tools/go/analysis/analysistest"
+	"golang.org/x/tools/go/packages"
 
 	"github.com/nnutter/constable/internal/analysis/nilaway"
 )
 
 func TestAnalyzer(t *testing.T) {
 	analysistest.Run(t, analysistest.TestData(), nilaway.Analyzer, "na")
 }
+
+// includePkgs returns the package list that the configuration analyzer read.
+func includePkgs(t *testing.T) string {
+	t.Helper()
+
+	flag := config.Analyzer.Flags.Lookup(config.IncludePkgsFlag)
+	require.NotNil(t, flag)
+	return flag.Value.String()
+}
+
+// setIncludePkgs records the value that a test gives the flag, and clears it
+// afterwards so that the flag does not leak into another test.
+func setIncludePkgs(t *testing.T, value string) {
+	t.Helper()
+
+	flag := config.Analyzer.Flags.Lookup(config.IncludePkgsFlag)
+	require.NotNil(t, flag)
+	require.NoError(t, flag.Value.Set(value))
+	t.Cleanup(func() {
+		require.NoError(t, flag.Value.Set(""))
+	})
+}
+
+// setIncludeAllPkgs sets the adapter flag, and restores it afterwards.
+func setIncludeAllPkgs(t *testing.T, value string) {
+	t.Helper()
+
+	require.NoError(t, nilaway.Analyzer.Flags.Set("include-all-pkgs", value))
+	t.Cleanup(func() {
+		require.NoError(t, nilaway.Analyzer.Flags.Set("include-all-pkgs", "false"))
+	})
+}
+
+func moduleRoot(path string, main bool) *packages.Package {
+	return &packages.Package{PkgPath: path + "/pkg", Module: &packages.Module{Path: path, Main: main}}
+}
+
+func TestUseMainModules(t *testing.T) {
+	setIncludePkgs(t, "")
+
+	nilaway.UseMainModules([]*packages.Package{
+		moduleRoot("example.com/m", true),
+		moduleRoot("example.com/m", true),
+		moduleRoot("example.com/dep", false),
+		{PkgPath: "fmt", Module: nil},
+		nil,
+	})
+
+	assert.Equal(t, "example.com/m", includePkgs(t))
+}
+
+func TestUseMainModulesCoversEveryMainModule(t *testing.T) {
+	setIncludePkgs(t, "")
+
+	nilaway.UseMainModules([]*packages.Package{
+		moduleRoot("example.com/one", true),
+		moduleRoot("example.com/two", true),
+	})
+
+	assert.Equal(t, "example.com/one,example.com/two", includePkgs(t))
+}
+
+func TestUseMainModulesKeepsChosenPackages(t *testing.T) {
+	setIncludePkgs(t, "example.com/chosen")
+
+	nilaway.UseMainModules([]*packages.Package{moduleRoot("example.com/m", true)})
+
+	assert.Equal(t, "example.com/chosen", includePkgs(t))
+}
+
+func TestUseMainModulesWithAllPackages(t *testing.T) {
+	setIncludePkgs(t, "")
+	setIncludeAllPkgs(t, "true")
+
+	nilaway.UseMainModules([]*packages.Package{moduleRoot("example.com/m", true)})
+
+	assert.Empty(t, includePkgs(t))
+}
+
+func TestUseMainModulesWithoutModule(t *testing.T) {
+	setIncludePkgs(t, "")
+
+	nilaway.UseMainModules([]*packages.Package{{PkgPath: "fmt", Module: nil}})
+
+	assert.Empty(t, includePkgs(t))
+}

internal/analysis/nonmutating/nonmutating.go

@@ -60,7 +60,7 @@ func hasNoArgs(call *ast.CallExpr) bool {
 
 func run(pass *analysis.Pass) (any, error) {
 	for _, file := range pass.Files {
-		if generated.IsGenerated(file) {
+		if generated.Skipped(file) {
 			continue
 		}
 

internal/analysis/predicates/predicates.go

@@ -55,7 +55,7 @@ func isBoolName(ident *ast.Ident) bool {
 
 func run(pass *analysis.Pass) (any, error) {
 	for _, file := range pass.Files {
-		if generated.IsGenerated(file) {
+		if generated.Skipped(file) {
 			continue
 		}
 

internal/analysis/testify/testify.go

@@ -47,7 +47,7 @@ var testifyPackages = map[string]string{
 
 func run(pass *analysis.Pass) (any, error) {
 	for _, file := range pass.Files {
-		if generated.IsGenerated(file) {
+		if generated.Skipped(file) {
 			continue
 		}