Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nogo: Add a _base key to be a default config for all Analyzers. #3351

Merged
merged 3 commits into from
Dec 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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