Skip to content

Commit 9badb60

Browse files
authored
Skip analyzers that don't emit facts when ignoring diagnostics (#4402)
**What type of PR is this?** Performance improvement **What does this PR do? Why is it needed?** Avoid unnecessary work by skipping analyzers that don't emit facts when reported diagnostics are known to be ignored. **Which issues(s) does this PR fix?** Work towards #4327 **Other notes for review**
1 parent ad65a6b commit 9badb60

File tree

3 files changed

+20
-5
lines changed

3 files changed

+20
-5
lines changed

go/private/actions/compilepkg.bzl

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,9 @@ def _run_nogo(
249249
nogo_args.add_all([cgo_go_srcs], before_each = "-ignore_src")
250250

251251
nogo_args.add_all(archives, before_each = "-facts", map_each = _facts)
252+
if not out_validation:
253+
# Since diagnostics are ignored, all analyzers that don't generated facts can be skipped.
254+
nogo_args.add("-facts_only")
252255
nogo_args.add("-out_facts", out_facts)
253256
nogo_args.add_all("-out", [out_diagnostics], expand_directories = False)
254257
nogo_args.add("-nogo", nogo.executable)

go/tools/builders/nogo.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,12 @@ func nogo(args []string) error {
2626
var testFilter string
2727
var outFactsPath, outPath string
2828
var coverMode string
29+
var factsOnly bool
2930
fs.Var(&unfilteredSrcs, "src", ".go, .c, .cc, .m, .mm, .s, or .S file to be filtered and checked")
3031
fs.Var(&ignoreSrcs, "ignore_src", ".go, .c, .cc, .m, .mm, .s, or .S file to be filtered and checked, but with its diagnostics ignored")
3132
fs.Var(&deps, "arc", "Import path, package path, and file name of a direct dependency, separated by '='")
3233
fs.Var(&facts, "facts", "Import path, package path, and file name of a direct dependency's nogo facts file, separated by '='")
34+
fs.BoolVar(&factsOnly, "facts_only", false, "If true, only nogo facts are emitted, no nogo checks are run")
3335
fs.StringVar(&importPath, "importpath", "", "The import path of the package being compiled. Not passed to the compiler, but may be displayed in debug data.")
3436
fs.StringVar(&packagePath, "p", "", "The package path (importmap) of the package being compiled")
3537
fs.StringVar(&packageListPath, "package_list", "", "The file containing the list of standard library packages")
@@ -83,10 +85,10 @@ func nogo(args []string) error {
8385
return err
8486
}
8587

86-
return runNogo(workDir, nogoPath, goSrcs, ignoreSrcs, facts, importPath, importcfgPath, outFactsPath, outPath)
88+
return runNogo(workDir, nogoPath, goSrcs, ignoreSrcs, facts, factsOnly, importPath, importcfgPath, outFactsPath, outPath)
8789
}
8890

89-
func runNogo(workDir string, nogoPath string, srcs, ignores []string, facts []archive, packagePath, importcfgPath, outFactsPath, outDirPath string) error {
91+
func runNogo(workDir string, nogoPath string, srcs, ignores []string, facts []archive, factsOnly bool, packagePath, importcfgPath, outFactsPath, outDirPath string) error {
9092
if len(srcs) == 0 {
9193
// emit_compilepkg expects a nogo facts file, even if it's empty.
9294
err := os.WriteFile(outFactsPath, nil, 0o666)
@@ -103,6 +105,9 @@ func runNogo(workDir string, nogoPath string, srcs, ignores []string, facts []ar
103105
for _, fact := range facts {
104106
args = append(args, "-fact", fmt.Sprintf("%s=%s", fact.importPath, fact.file))
105107
}
108+
if factsOnly {
109+
args = append(args, "-facts_only")
110+
}
106111
args = append(args, "-x", outFactsPath)
107112
for _, ignore := range ignores {
108113
args = append(args, "-ignore", ignore)

go/tools/builders/nogo_main.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ func run(args []string) (error, int) {
7474
factMap := factMultiFlag{}
7575
flags := flag.NewFlagSet("nogo", flag.ExitOnError)
7676
flags.Var(&factMap, "fact", "Import path and file containing facts for that library, separated by '=' (may be repeated)'")
77+
factsOnly := flags.Bool("facts_only", false, "If true, only facts are emitted, no analyzers are run")
7778
importcfg := flags.String("importcfg", "", "The import configuration file")
7879
packagePath := flags.String("p", "", "The package path (importmap) of the package being compiled")
7980
xPath := flags.String("x", "", "The archive file where serialized facts should be written")
@@ -88,7 +89,7 @@ func run(args []string) (error, int) {
8889
return fmt.Errorf("error parsing importcfg: %v", err), nogoError
8990
}
9091

91-
diagnostics, pkg, err := checkPackage(analyzers, *packagePath, packageFile, importMap, factMap, srcs, ignores)
92+
diagnostics, pkg, err := checkPackage(analyzers, *packagePath, packageFile, importMap, factMap, *factsOnly, srcs, ignores)
9293
if err != nil {
9394
return fmt.Errorf("error running analyzers: %v", err), nogoError
9495
}
@@ -214,7 +215,7 @@ func setAnalyzerFlags(a *analysis.Analyzer, flags map[string]string) error {
214215
// It returns an empty string if no source code diagnostics need to be printed.
215216
//
216217
// This implementation was adapted from that of golang.org/x/tools/go/checker/internal/checker.
217-
func checkPackage(analyzers []*analysis.Analyzer, packagePath string, packageFile, importMap, factMap map[string]string, filenames, ignoreFiles []string) ([]diagnosticEntry, *goPackage, error) {
218+
func checkPackage(analyzers []*analysis.Analyzer, packagePath string, packageFile, importMap, factMap map[string]string, factsOnly bool, filenames, ignoreFiles []string) ([]diagnosticEntry, *goPackage, error) {
218219
// Register fact types and establish dependencies between analyzers.
219220
actions := make(map[*analysis.Analyzer]*action)
220221
var visit func(a *analysis.Analyzer) *action
@@ -258,7 +259,13 @@ func checkPackage(analyzers []*analysis.Analyzer, packagePath string, packageFil
258259

259260
roots := make([]*action, 0, len(analyzers))
260261
for _, a := range analyzers {
261-
roots = append(roots, visit(a))
262+
if !factsOnly || len(a.FactTypes) > 0 {
263+
roots = append(roots, visit(a))
264+
}
265+
}
266+
if len(roots) == 0 {
267+
// No analyzers to run, return early.
268+
return nil, nil, nil
262269
}
263270

264271
// Load the package, including AST, types, and facts.

0 commit comments

Comments
 (0)