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": {}
	}
}

3 files changed, 65 insertions, 8 deletions

diff --git a/go/nogo.rst b/go/nogo.rst
index 212d55ee..222d967b 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 e33b4902..5d488ffd 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 72f3a373..27624431 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
 
@@ -419,6 +437,18 @@ func Test(t *testing.T) {
 				`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",
 			target:      "//:uses_cgo_with_errors",