Skip to content

Commit

Permalink
nogo: Add a _base key to be a default config for all Analyzers. (#3351)
Browse files Browse the repository at this point in the history
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": {}
	}
}
  • Loading branch information
DolceTriade authored Dec 3, 2022
1 parent 60cd1e4 commit fe4715a
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 8 deletions.
9 changes: 9 additions & 0 deletions go/nogo.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
^^^^^^^

Expand All @@ -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",
Expand Down
34 changes: 26 additions & 8 deletions go/tools/builders/nogo_main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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})
}
Expand All @@ -418,18 +436,18 @@ 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
}
}
}
if include {
for _, pattern := range config.excludeFiles {
for _, pattern := range currentConfig.excludeFiles {
if pattern.MatchString(filename) {
include = false
break
Expand Down
30 changes: 30 additions & 0 deletions tests/core/nogo/custom/custom_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down

0 comments on commit fe4715a

Please sign in to comment.