diff options
author | Michael Pratt <mpratt@google.com> | 2021-07-26 10:24:11 -0700 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2021-07-26 10:26:43 -0700 |
commit | a42d3fd0aeb6c67c3fd2fb851845a1f88a298972 (patch) | |
tree | 4cac50f3a73482ba3c82aa2ac1bcfcac9a9a46b6 /tools/nogo/config.go | |
parent | 9ba8c40a3a3c7fed40d9137fed8a87fa9d536a22 (diff) |
Fix per-analyzer overrides of default-disabled groups
Currently behavior of config groups with `default: false` is buggy. The
intention is that adding an empty suppression section for that group to a
specific analyzer config should enable reporting for that analyzer. i.e.,
```
groups:
- name: foo
regex: "^foo/"
default: false
global:
...
analyzers:
asmdecl:
foo: # Enabled.
```
This should enable the foo group only for asmdecl. Unfortunately, today the
actual behavior depends on the contents of the `global:` section. If `global:`
contains an entry for foo, then it will work as described. If `global:` does
_not_ contain an entry for foo, then the group default (disabled) always
applies and the individual analyzer options have no effect.
The cause of this is confusion in `AnalyzerConfig.shouldReport`, which doesn't
distinguish between explicit suppression via a global suppression/exclude and
simply having no configuration at all. Make this more explicit, so that the no
configuration case can continue to per-analyzer configuration before falling
back to the group default.
The last test case in the added test fails without this change.
This re-enables several opted-in analyzers for external dependencies, which
have gained a few more false positives to suppress.
PiperOrigin-RevId: 386904725
Diffstat (limited to 'tools/nogo/config.go')
-rw-r--r-- | tools/nogo/config.go | 18 |
1 files changed, 13 insertions, 5 deletions
diff --git a/tools/nogo/config.go b/tools/nogo/config.go index 6436f9d34..ee2533610 100644 --- a/tools/nogo/config.go +++ b/tools/nogo/config.go @@ -186,16 +186,19 @@ func (a AnalyzerConfig) merge(other AnalyzerConfig) { } } -func (a AnalyzerConfig) shouldReport(groupConfig *Group, fullPos, msg string) bool { +// shouldReport returns whether the finding should be reported or suppressed. +// It returns !ok if there is no configuration sufficient to decide one way or +// another. +func (a AnalyzerConfig) shouldReport(groupConfig *Group, fullPos, msg string) (report, ok bool) { gc, ok := a[groupConfig.Name] if !ok { - return groupConfig.Default + return false, false } // Note that if a section appears for a particular group // for a particular analyzer, then it will now be enabled, // and the group default no longer applies. - return gc.shouldReport(fullPos, msg) + return gc.shouldReport(fullPos, msg), true } // Config is a nogo configuration. @@ -298,7 +301,8 @@ func (c *Config) ShouldReport(finding Finding) bool { } // Suppress via global rule? - if !c.Global.shouldReport(groupConfig, fullPos, finding.Message) { + report, ok := c.Global.shouldReport(groupConfig, fullPos, finding.Message) + if ok && !report { return false } @@ -307,5 +311,9 @@ func (c *Config) ShouldReport(finding Finding) bool { if !ok { return groupConfig.Default } - return ac.shouldReport(groupConfig, fullPos, finding.Message) + report, ok = ac.shouldReport(groupConfig, fullPos, finding.Message) + if !ok { + return groupConfig.Default + } + return report } |