summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
-rw-r--r--nogo.yaml2
-rw-r--r--tools/nogo/BUILD8
-rw-r--r--tools/nogo/config.go18
-rw-r--r--tools/nogo/config_test.go301
-rw-r--r--tools/nogo/findings.go2
5 files changed, 324 insertions, 7 deletions
diff --git a/nogo.yaml b/nogo.yaml
index 9b7fc5c8f..d7a3ad956 100644
--- a/nogo.yaml
+++ b/nogo.yaml
@@ -130,6 +130,8 @@ analyzers:
external: # Enabled.
unreachable:
external: # Enabled.
+ exclude:
+ - ".*protobuf/.*.go"
unsafeptr:
internal:
exclude:
diff --git a/tools/nogo/BUILD b/tools/nogo/BUILD
index a7e280b32..27fe48680 100644
--- a/tools/nogo/BUILD
+++ b/tools/nogo/BUILD
@@ -1,4 +1,4 @@
-load("//tools:defs.bzl", "bzl_library", "go_library", "select_goarch", "select_goos")
+load("//tools:defs.bzl", "bzl_library", "go_library", "go_test", "select_goarch", "select_goos")
load("//tools/nogo:defs.bzl", "nogo_objdump_tool", "nogo_stdlib", "nogo_target")
package(licenses = ["notice"])
@@ -73,6 +73,12 @@ go_library(
],
)
+go_test(
+ name = "nogo_test",
+ srcs = ["config_test.go"],
+ library = ":nogo",
+)
+
bzl_library(
name = "defs_bzl",
srcs = ["defs.bzl"],
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
}
diff --git a/tools/nogo/config_test.go b/tools/nogo/config_test.go
new file mode 100644
index 000000000..685cffbec
--- /dev/null
+++ b/tools/nogo/config_test.go
@@ -0,0 +1,301 @@
+// Copyright 2021 The gVisor Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.package nogo
+package nogo
+
+import (
+ "go/token"
+ "testing"
+)
+
+// TestShouldReport validates the suppression behavior of Config.ShouldReport.
+func TestShouldReport(t *testing.T) {
+ config := &Config{
+ Groups: []Group{
+ {
+ Name: "default-enabled",
+ Regex: "^default-enabled/",
+ Default: true,
+ },
+ {
+ Name: "default-disabled",
+ Regex: "^default-disabled/",
+ Default: false,
+ },
+ {
+ Name: "default-disabled-omitted-from-global",
+ Regex: "^default-disabled-omitted-from-global/",
+ Default: false,
+ },
+ },
+ Global: AnalyzerConfig{
+ "default-enabled": &ItemConfig{
+ Exclude: []string{"excluded.go"},
+ Suppress: []string{"suppressed"},
+ },
+ "default-disabled": &ItemConfig{
+ Exclude: []string{"excluded.go"},
+ Suppress: []string{"suppressed"},
+ },
+ // Omitting default-disabled-omitted-from-global here
+ // has no effect on configuration below.
+ },
+ Analyzers: map[AnalyzerName]AnalyzerConfig{
+ "analyzer-suppressions": AnalyzerConfig{
+ // Suppress some.
+ "default-enabled": &ItemConfig{
+ Exclude: []string{"limited-exclude.go"},
+ Suppress: []string{"limited suppress"},
+ },
+ // Enable all.
+ "default-disabled": nil,
+ },
+ "enabled-for-default-disabled": AnalyzerConfig{
+ "default-disabled": nil,
+ "default-disabled-omitted-from-global": nil,
+ },
+ },
+ }
+
+ if err := config.Compile(); err != nil {
+ t.Fatalf("Compile(%+v) = %v, want nil", config, err)
+ }
+
+ cases := []struct {
+ name string
+ finding Finding
+ want bool
+ }{
+ {
+ name: "enabled",
+ finding: Finding{
+ Category: "foo",
+ Position: token.Position{
+ Filename: "default-enabled/file.go",
+ Offset: 0,
+ Line: 1,
+ Column: 1,
+ },
+ Message: "message",
+ },
+ want: true,
+ },
+ {
+ name: "ungrouped",
+ finding: Finding{
+ Category: "foo",
+ Position: token.Position{
+ Filename: "ungrouped/file.go",
+ Offset: 0,
+ Line: 1,
+ Column: 1,
+ },
+ Message: "message",
+ },
+ want: true,
+ },
+ {
+ name: "suppressed",
+ finding: Finding{
+ Category: "foo",
+ Position: token.Position{
+ Filename: "default-enabled/file.go",
+ Offset: 0,
+ Line: 1,
+ Column: 1,
+ },
+ Message: "message suppressed",
+ },
+ want: false,
+ },
+ {
+ name: "excluded",
+ finding: Finding{
+ Category: "foo",
+ Position: token.Position{
+ Filename: "default-enabled/excluded.go",
+ Offset: 0,
+ Line: 1,
+ Column: 1,
+ },
+ Message: "message",
+ },
+ want: false,
+ },
+ {
+ name: "disabled",
+ finding: Finding{
+ Category: "foo",
+ Position: token.Position{
+ Filename: "default-disabled/file.go",
+ Offset: 0,
+ Line: 1,
+ Column: 1,
+ },
+ Message: "message",
+ },
+ want: false,
+ },
+ {
+ name: "analyzer suppressed",
+ finding: Finding{
+ Category: "analyzer-suppressions",
+ Position: token.Position{
+ Filename: "default-enabled/file.go",
+ Offset: 0,
+ Line: 1,
+ Column: 1,
+ },
+ Message: "message limited suppress",
+ },
+ want: false,
+ },
+ {
+ name: "analyzer suppressed not global",
+ finding: Finding{
+ // Doesn't apply outside of analyzer-suppressions.
+ Category: "foo",
+ Position: token.Position{
+ Filename: "default-enabled/file.go",
+ Offset: 0,
+ Line: 1,
+ Column: 1,
+ },
+ Message: "message limited suppress",
+ },
+ want: true,
+ },
+ {
+ name: "analyzer suppressed grouped",
+ finding: Finding{
+ Category: "analyzer-suppressions",
+ Position: token.Position{
+ // Doesn't apply outside of default-enabled.
+ Filename: "default-disabled/file.go",
+ Offset: 0,
+ Line: 1,
+ Column: 1,
+ },
+ Message: "message limited suppress",
+ },
+ want: true,
+ },
+ {
+ name: "analyzer excluded",
+ finding: Finding{
+ Category: "analyzer-suppressions",
+ Position: token.Position{
+ Filename: "default-enabled/limited-exclude.go",
+ Offset: 0,
+ Line: 1,
+ Column: 1,
+ },
+ Message: "message",
+ },
+ want: false,
+ },
+ {
+ name: "analyzer excluded not global",
+ finding: Finding{
+ // Doesn't apply outside of analyzer-suppressions.
+ Category: "foo",
+ Position: token.Position{
+ Filename: "default-enabled/limited-exclude.go",
+ Offset: 0,
+ Line: 1,
+ Column: 1,
+ },
+ Message: "message",
+ },
+ want: true,
+ },
+ {
+ name: "analyzer excluded grouped",
+ finding: Finding{
+ Category: "analyzer-suppressions",
+ Position: token.Position{
+ // Doesn't apply outside of default-enabled.
+ Filename: "default-disabled/limited-exclude.go",
+ Offset: 0,
+ Line: 1,
+ Column: 1,
+ },
+ Message: "message",
+ },
+ want: true,
+ },
+ {
+ name: "disabled-omitted",
+ finding: Finding{
+ Category: "foo",
+ Position: token.Position{
+ Filename: "default-disabled-omitted-from-global/file.go",
+ Offset: 0,
+ Line: 1,
+ Column: 1,
+ },
+ Message: "message",
+ },
+ want: false,
+ },
+ {
+ name: "default enabled applies to customized analyzer",
+ finding: Finding{
+ Category: "enabled-for-default-disabled",
+ Position: token.Position{
+ Filename: "default-enabled/file.go",
+ Offset: 0,
+ Line: 1,
+ Column: 1,
+ },
+ Message: "message",
+ },
+ want: true,
+ },
+ {
+ name: "default overridden in customized analyzer",
+ finding: Finding{
+ Category: "enabled-for-default-disabled",
+ Position: token.Position{
+ Filename: "default-disabled/file.go",
+ Offset: 0,
+ Line: 1,
+ Column: 1,
+ },
+ Message: "message",
+ },
+ want: true,
+ },
+ {
+ name: "default overridden in customized analyzer even when omitted from global",
+ finding: Finding{
+ Category: "enabled-for-default-disabled",
+ Position: token.Position{
+ Filename: "default-disabled-omitted-from-global/file.go",
+ Offset: 0,
+ Line: 1,
+ Column: 1,
+ },
+ Message: "message",
+ },
+ want: true,
+ },
+ }
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ if got := config.ShouldReport(tc.finding); got != tc.want {
+ t.Errorf("ShouldReport(%+v) = %v, want %v", tc.finding, got, tc.want)
+ }
+ })
+ }
+}
diff --git a/tools/nogo/findings.go b/tools/nogo/findings.go
index 329a7062e..a73bf1a09 100644
--- a/tools/nogo/findings.go
+++ b/tools/nogo/findings.go
@@ -109,7 +109,7 @@ func ExtractFindingsFromFile(filename string, asJSON bool) (FindingSet, error) {
return ExtractFindingsFrom(r, asJSON)
}
-// ExtractFindingsFromBytes loads findings from bytes.
+// ExtractFindingsFrom loads findings from an io.Reader.
func ExtractFindingsFrom(r io.Reader, asJSON bool) (findings FindingSet, err error) {
if asJSON {
dec := json.NewDecoder(r)