From 2fa25afa3d948a730dc5b4874308fb4f12b36065 Mon Sep 17 00:00:00 2001 From: DolceTriade Date: Sat, 3 Dec 2022 08:02:32 -0800 Subject: [PATCH] nogo: Add a _base key to be a default config for all Analyzers. (#3351) This is useful for situations where you want to exclude/include the same directories for all the analyzers. Specifying a config for a particular analyzer will cause the _base config, if it exists, to be underlayed for that analyzer (ie, if the analyzer doesn't set an option, it will use the options specified by the _base config). You can use it like so: { "_base": { "exclude_files": { "external/": "third_party", "proto/": "generated protobuf" } }, "override": "description": "oops, I actually don't want to exclude any files here...", "exclude_files": {} } } --- go/nogo.rst | 9 +++++++ go/tools/builders/nogo_main.go | 34 ++++++++++++++++++++------- tests/core/nogo/custom/custom_test.go | 30 +++++++++++++++++++++++ 3 files changed, 65 insertions(+), 8 deletions(-) diff --git a/go/nogo.rst b/go/nogo.rst index 212d55ee1e..222d967b0b 100644 --- a/go/nogo.rst +++ b/go/nogo.rst @@ -204,6 +204,9 @@ contain the following key-value pairs: | ``flag.Value`` specified by the analyzer. | +----------------------------+---------------------------------------------------------------------+ +``nogo`` also supports a special key to specify the same config for all analyzers, even if they are +not explicitly specified called ``_base``. See below for an example of its usage. + Example ^^^^^^^ @@ -216,6 +219,12 @@ on a command line driver. .. code:: json { + "_base": { + "description": "Base config that all subsequent analyzers, even unspecified will inherit.", + "exclude_files": { + "third_party/": "exclude all third_party code for all analyzers" + } + }, "importunsafe": { "exclude_files": { "src/foo\\.go": "manually verified that behavior is working-as-intended", diff --git a/go/tools/builders/nogo_main.go b/go/tools/builders/nogo_main.go index e33b490277..5d488ffdca 100644 --- a/go/tools/builders/nogo_main.go +++ b/go/tools/builders/nogo_main.go @@ -43,6 +43,8 @@ import ( "golang.org/x/tools/go/gcexportdata" ) +const nogoBaseConfigName = "_base" + func init() { if err := analysis.Validate(analyzers); err != nil { log.Fatal(err) @@ -89,7 +91,7 @@ func run(args []string) error { return fmt.Errorf("errors found by nogo during build-time code analysis:\n%s\n", diagnostics) } if *xPath != "" { - if err := ioutil.WriteFile(abs(*xPath), facts, 0666); err != nil { + if err := ioutil.WriteFile(abs(*xPath), facts, 0o666); err != nil { return fmt.Errorf("error writing facts: %v", err) } } @@ -399,10 +401,26 @@ func checkAnalysisResults(actions []*action, pkg *goPackage) string { if len(act.diagnostics) == 0 { continue } - config, ok := configs[act.a.Name] - if !ok { - // If the analyzer is not explicitly configured, it emits diagnostics for - // all files. + var currentConfig config + // Use the base config if it exists. + if baseConfig, ok := configs[nogoBaseConfigName]; ok { + currentConfig = baseConfig + } + // Overwrite the config with the desired config. Any unset fields + // in the config will default to the base config. + if actionConfig, ok := configs[act.a.Name]; ok { + if actionConfig.analyzerFlags != nil { + currentConfig.analyzerFlags = actionConfig.analyzerFlags + } + if actionConfig.onlyFiles != nil { + currentConfig.onlyFiles = actionConfig.onlyFiles + } + if actionConfig.excludeFiles != nil { + currentConfig.excludeFiles = actionConfig.excludeFiles + } + } + + if currentConfig.onlyFiles == nil && currentConfig.excludeFiles == nil { for _, diag := range act.diagnostics { diagnostics = append(diagnostics, entry{Diagnostic: diag, Analyzer: act.a}) } @@ -418,10 +436,10 @@ func checkAnalysisResults(actions []*action, pkg *goPackage) string { filename = p.Filename } include := true - if len(config.onlyFiles) > 0 { + if len(currentConfig.onlyFiles) > 0 { // This analyzer emits diagnostics for only a set of files. include = false - for _, pattern := range config.onlyFiles { + for _, pattern := range currentConfig.onlyFiles { if pattern.MatchString(filename) { include = true break @@ -429,7 +447,7 @@ func checkAnalysisResults(actions []*action, pkg *goPackage) string { } } if include { - for _, pattern := range config.excludeFiles { + for _, pattern := range currentConfig.excludeFiles { if pattern.MatchString(filename) { include = false break diff --git a/tests/core/nogo/custom/custom_test.go b/tests/core/nogo/custom/custom_test.go index 72f3a37313..2762443114 100644 --- a/tests/core/nogo/custom/custom_test.go +++ b/tests/core/nogo/custom/custom_test.go @@ -297,6 +297,24 @@ func run(pass *analysis.Pass) (interface{}, error) { } } +-- baseconfig.json -- +{ + "_base": { + "exclude_files": { + "has_.*\\.go": "Visibility analyzer not specified. Still inherits this special exception." + } + }, + "importfmt": { + "only_files": { + "has_errors\\.go": "" + } + }, + "foofuncname": { + "description": "no exemptions since we know this check is 100% accurate, so override base config", + "exclude_files": {} + } +} + -- has_errors.go -- package haserrors @@ -418,6 +436,18 @@ func Test(t *testing.T) { excludes: []string{ `importfmt`, }, + }, { + desc: "custom_config_with_base_linedirective", + config: "baseconfig.json", + target: "//:has_errors_linedirective", + wantSuccess: false, + includes: []string{ + `linedirective_foofuncname.go:.*function must not be named Foo \(foofuncname\)`, + `linedirective_visibility.go:.*function D is not visible in this package \(visibility\)`, + }, + excludes: []string{ + `importfmt`, + }, }, { desc: "uses_cgo_with_errors", config: "config.json",