From 8fde4632faec33af22168a1cad789c676583b87f Mon Sep 17 00:00:00 2001
From: Patrick Ohly <patrick.ohly@intel.com>
Date: Wed, 31 May 2023 17:25:59 +0200
Subject: [PATCH] rules: support inverted path match (#3617)
---
.golangci.reference.yml | 5 +++++
docs/src/docs/usage/false-positives.mdx | 12 ++++++++++
pkg/config/issues.go | 23 +++++++++++++-------
pkg/lint/runner.go | 18 ++++++++-------
pkg/result/processors/base_rule.go | 23 ++++++++++++--------
pkg/result/processors/exclude_rules.go | 4 ++++
pkg/result/processors/exclude_rules_test.go | 11 ++++++++++
pkg/result/processors/severity_rules.go | 4 ++++
pkg/result/processors/severity_rules_test.go | 11 ++++++++++
test/testdata/configs/path-except.yml | 13 +++++++++++
test/testdata/path_except.go | 14 ++++++++++++
test/testdata/path_except_test.go | 14 ++++++++++++
12 files changed, 127 insertions(+), 25 deletions(-)
create mode 100644 test/testdata/configs/path-except.yml
create mode 100644 test/testdata/path_except.go
create mode 100644 test/testdata/path_except_test.go
diff --git a/.golangci.reference.yml b/.golangci.reference.yml
index 29d4414c..a23840f6 100644
--- a/.golangci.reference.yml
+++ b/.golangci.reference.yml
@@ -2323,6 +2323,11 @@ issues:
- dupl
- gosec
+ # Run some linter only for test files by excluding its issues for everything else.
+ - path-except: _test\.go
+ linters:
+ - forbidigo
+
# Exclude known linters from partially hard-vendored code,
# which is impossible to exclude via `nolint` comments.
# `/` will be replaced by current OS file path separator to properly work on Windows.
diff --git a/docs/src/docs/usage/false-positives.mdx b/docs/src/docs/usage/false-positives.mdx
index fe94197a..20b91a5b 100644
--- a/docs/src/docs/usage/false-positives.mdx
+++ b/docs/src/docs/usage/false-positives.mdx
@@ -81,6 +81,18 @@ issues:
- goconst
```
+The opposite, excluding reports **except** for specific paths, is also possible.
+In the following example, only test files get checked:
+
+```yml
+issues:
+ exclude-rules:
+ - path-except: '(.+)_test\.go'
+ linters:
+ - funlen
+ - goconst
+```
+
In the following example, all the reports related to the files (`skip-files`) are excluded:
```yml
diff --git a/pkg/config/issues.go b/pkg/config/issues.go
index b2437ec9..417b28bd 100644
--- a/pkg/config/issues.go
+++ b/pkg/config/issues.go
@@ -125,21 +125,25 @@ type ExcludeRule struct {
BaseRule `mapstructure:",squash"`
}
-func (e ExcludeRule) Validate() error {
+func (e *ExcludeRule) Validate() error {
return e.BaseRule.Validate(excludeRuleMinConditionsCount)
}
type BaseRule struct {
- Linters []string
- Path string
- Text string
- Source string
+ Linters []string
+ Path string
+ PathExcept string `mapstructure:"path-except"`
+ Text string
+ Source string
}
-func (b BaseRule) Validate(minConditionsCount int) error {
+func (b *BaseRule) Validate(minConditionsCount int) error {
if err := validateOptionalRegex(b.Path); err != nil {
return fmt.Errorf("invalid path regex: %v", err)
}
+ if err := validateOptionalRegex(b.PathExcept); err != nil {
+ return fmt.Errorf("invalid path-except regex: %v", err)
+ }
if err := validateOptionalRegex(b.Text); err != nil {
return fmt.Errorf("invalid text regex: %v", err)
}
@@ -150,7 +154,10 @@ func (b BaseRule) Validate(minConditionsCount int) error {
if len(b.Linters) > 0 {
nonBlank++
}
- if b.Path != "" {
+ // Filtering by path counts as one condition, regardless how it is done (one or both).
+ // Otherwise, a rule with Path and PathExcept set would pass validation
+ // whereas before the introduction of path-except that wouldn't have been precise enough.
+ if b.Path != "" || b.PathExcept != "" {
nonBlank++
}
if b.Text != "" {
@@ -160,7 +167,7 @@ func (b BaseRule) Validate(minConditionsCount int) error {
nonBlank++
}
if nonBlank < minConditionsCount {
- return fmt.Errorf("at least %d of (text, source, path, linters) should be set", minConditionsCount)
+ return fmt.Errorf("at least %d of (text, source, path[-except], linters) should be set", minConditionsCount)
}
return nil
}
diff --git a/pkg/lint/runner.go b/pkg/lint/runner.go
index b1180438..d270892d 100644
--- a/pkg/lint/runner.go
+++ b/pkg/lint/runner.go
@@ -276,10 +276,11 @@ func getExcludeRulesProcessor(cfg *config.Issues, log logutils.Log, files *fsuti
for _, r := range cfg.ExcludeRules {
excludeRules = append(excludeRules, processors.ExcludeRule{
BaseRule: processors.BaseRule{
- Text: r.Text,
- Source: r.Source,
- Path: r.Path,
- Linters: r.Linters,
+ Text: r.Text,
+ Source: r.Source,
+ Path: r.Path,
+ PathExcept: r.PathExcept,
+ Linters: r.Linters,
},
})
}
@@ -319,10 +320,11 @@ func getSeverityRulesProcessor(cfg *config.Severity, log logutils.Log, files *fs
severityRules = append(severityRules, processors.SeverityRule{
Severity: r.Severity,
BaseRule: processors.BaseRule{
- Text: r.Text,
- Source: r.Source,
- Path: r.Path,
- Linters: r.Linters,
+ Text: r.Text,
+ Source: r.Source,
+ Path: r.Path,
+ PathExcept: r.PathExcept,
+ Linters: r.Linters,
},
})
}
diff --git a/pkg/result/processors/base_rule.go b/pkg/result/processors/base_rule.go
index eaed829a..b5e13880 100644
--- a/pkg/result/processors/base_rule.go
+++ b/pkg/result/processors/base_rule.go
@@ -9,21 +9,23 @@ import (
)
type BaseRule struct {
- Text string
- Source string
- Path string
- Linters []string
+ Text string
+ Source string
+ Path string
+ PathExcept string
+ Linters []string
}
type baseRule struct {
- text *regexp.Regexp
- source *regexp.Regexp
- path *regexp.Regexp
- linters []string
+ text *regexp.Regexp
+ source *regexp.Regexp
+ path *regexp.Regexp
+ pathExcept *regexp.Regexp
+ linters []string
}
func (r *baseRule) isEmpty() bool {
- return r.text == nil && r.source == nil && r.path == nil && len(r.linters) == 0
+ return r.text == nil && r.source == nil && r.path == nil && r.pathExcept == nil && len(r.linters) == 0
}
func (r *baseRule) match(issue *result.Issue, files *fsutils.Files, log logutils.Log) bool {
@@ -36,6 +38,9 @@ func (r *baseRule) match(issue *result.Issue, files *fsutils.Files, log logutils
if r.path != nil && !r.path.MatchString(files.WithPathPrefix(issue.FilePath())) {
return false
}
+ if r.pathExcept != nil && r.pathExcept.MatchString(issue.FilePath()) {
+ return false
+ }
if len(r.linters) != 0 && !r.matchLinter(issue) {
return false
}
diff --git a/pkg/result/processors/exclude_rules.go b/pkg/result/processors/exclude_rules.go
index 42a24f64..2f7e30b4 100644
--- a/pkg/result/processors/exclude_rules.go
+++ b/pkg/result/processors/exclude_rules.go
@@ -47,6 +47,10 @@ func createRules(rules []ExcludeRule, prefix string) []excludeRule {
path := fsutils.NormalizePathInRegex(rule.Path)
parsedRule.path = regexp.MustCompile(path)
}
+ if rule.PathExcept != "" {
+ pathExcept := fsutils.NormalizePathInRegex(rule.PathExcept)
+ parsedRule.pathExcept = regexp.MustCompile(pathExcept)
+ }
parsedRules = append(parsedRules, parsedRule)
}
return parsedRules
diff --git a/pkg/result/processors/exclude_rules_test.go b/pkg/result/processors/exclude_rules_test.go
index d668f093..247cb320 100644
--- a/pkg/result/processors/exclude_rules_test.go
+++ b/pkg/result/processors/exclude_rules_test.go
@@ -34,6 +34,12 @@ func TestExcludeRulesMultiple(t *testing.T) {
Path: `_test\.go`,
},
},
+ {
+ BaseRule: BaseRule{
+ Text: "^nontestonly$",
+ PathExcept: `_test\.go`,
+ },
+ },
{
BaseRule: BaseRule{
Source: "^//go:generate ",
@@ -42,6 +48,7 @@ func TestExcludeRulesMultiple(t *testing.T) {
},
}, files, nil)
+ //nolint:dupl
cases := []issueTestCase{
{Path: "e.go", Text: "exclude", Linter: "linter"},
{Path: "e.go", Text: "some", Linter: "linter"},
@@ -49,6 +56,8 @@ func TestExcludeRulesMultiple(t *testing.T) {
{Path: "e_Test.go", Text: "normal", Linter: "testlinter"},
{Path: "e_test.go", Text: "another", Linter: "linter"},
{Path: "e_test.go", Text: "testonly", Linter: "linter"},
+ {Path: "e.go", Text: "nontestonly", Linter: "linter"},
+ {Path: "e_test.go", Text: "nontestonly", Linter: "linter"},
{Path: filepath.Join("testdata", "exclude_rules.go"), Line: 3, Linter: "lll"},
}
var issues []result.Issue
@@ -69,6 +78,7 @@ func TestExcludeRulesMultiple(t *testing.T) {
{Path: "e.go", Text: "some", Linter: "linter"},
{Path: "e_Test.go", Text: "normal", Linter: "testlinter"},
{Path: "e_test.go", Text: "another", Linter: "linter"},
+ {Path: "e_test.go", Text: "nontestonly", Linter: "linter"},
}
assert.Equal(t, expectedCases, resultingCases)
}
@@ -172,6 +182,7 @@ func TestExcludeRulesCaseSensitiveMultiple(t *testing.T) {
},
}, files, nil)
+ //nolint:dupl
cases := []issueTestCase{
{Path: "e.go", Text: "exclude", Linter: "linter"},
{Path: "e.go", Text: "excLude", Linter: "linter"},
diff --git a/pkg/result/processors/severity_rules.go b/pkg/result/processors/severity_rules.go
index 8174ae8c..0a4a643b 100644
--- a/pkg/result/processors/severity_rules.go
+++ b/pkg/result/processors/severity_rules.go
@@ -52,6 +52,10 @@ func createSeverityRules(rules []SeverityRule, prefix string) []severityRule {
path := fsutils.NormalizePathInRegex(rule.Path)
parsedRule.path = regexp.MustCompile(path)
}
+ if rule.PathExcept != "" {
+ pathExcept := fsutils.NormalizePathInRegex(rule.PathExcept)
+ parsedRule.pathExcept = regexp.MustCompile(pathExcept)
+ }
parsedRules = append(parsedRules, parsedRule)
}
return parsedRules
diff --git a/pkg/result/processors/severity_rules_test.go b/pkg/result/processors/severity_rules_test.go
index 637febe2..c142524f 100644
--- a/pkg/result/processors/severity_rules_test.go
+++ b/pkg/result/processors/severity_rules_test.go
@@ -39,6 +39,13 @@ func TestSeverityRulesMultiple(t *testing.T) {
Path: `_test\.go`,
},
},
+ {
+ Severity: "info",
+ BaseRule: BaseRule{
+ Text: "^nontestonly$",
+ PathExcept: `_test\.go`,
+ },
+ },
{
BaseRule: BaseRule{
Source: "^//go:generate ",
@@ -72,6 +79,8 @@ func TestSeverityRulesMultiple(t *testing.T) {
{Path: "ssl.go", Text: "ssl", Linter: "gosec"},
{Path: "e.go", Text: "some", Linter: "linter"},
{Path: "e_test.go", Text: "testonly", Linter: "testlinter"},
+ {Path: "e.go", Text: "nontestonly", Linter: "testlinter"},
+ {Path: "e_test.go", Text: "nontestonly", Linter: "testlinter"},
{Path: filepath.Join("testdata", "exclude_rules.go"), Line: 3, Linter: "lll"},
{Path: filepath.Join("testdata", "severity_rules.go"), Line: 3, Linter: "invalidgo"},
{Path: "someotherlinter.go", Text: "someotherlinter", Linter: "someotherlinter"},
@@ -97,6 +106,8 @@ func TestSeverityRulesMultiple(t *testing.T) {
{Path: "ssl.go", Text: "ssl", Linter: "gosec", Severity: "info"},
{Path: "e.go", Text: "some", Linter: "linter", Severity: "info"},
{Path: "e_test.go", Text: "testonly", Linter: "testlinter", Severity: "info"},
+ {Path: "e.go", Text: "nontestonly", Linter: "testlinter", Severity: "info"}, // matched
+ {Path: "e_test.go", Text: "nontestonly", Linter: "testlinter", Severity: "error"}, // not matched
{Path: filepath.Join("testdata", "exclude_rules.go"), Line: 3, Linter: "lll", Severity: "error"},
{Path: filepath.Join("testdata", "severity_rules.go"), Line: 3, Linter: "invalidgo", Severity: "info"},
{Path: "someotherlinter.go", Text: "someotherlinter", Linter: "someotherlinter", Severity: "info"},
diff --git a/test/testdata/configs/path-except.yml b/test/testdata/configs/path-except.yml
new file mode 100644
index 00000000..712bbd5a
--- /dev/null
+++ b/test/testdata/configs/path-except.yml
@@ -0,0 +1,13 @@
+linters-settings:
+ forbidigo:
+ forbid:
+ - fmt\.Print.*
+ - time.Sleep(# no sleeping!)?
+
+issues:
+ exclude-rules:
+ # Apply forbidigo only to test files, exclude
+ # it everywhere else.
+ - path-except: _test\.go
+ linters:
+ - forbidigo
diff --git a/test/testdata/path_except.go b/test/testdata/path_except.go
new file mode 100644
index 00000000..ab7b33a0
--- /dev/null
+++ b/test/testdata/path_except.go
@@ -0,0 +1,14 @@
+//golangcitest:args -Eforbidigo
+//golangcitest:config_path testdata/configs/path-except.yml
+//golangcitest:expected_exitcode 0
+package testdata
+
+import (
+ "fmt"
+ "time"
+)
+
+func Forbidigo() {
+ fmt.Printf("too noisy!!!")
+ time.Sleep(time.Nanosecond)
+}
diff --git a/test/testdata/path_except_test.go b/test/testdata/path_except_test.go
new file mode 100644
index 00000000..5f69f87a
--- /dev/null
+++ b/test/testdata/path_except_test.go
@@ -0,0 +1,14 @@
+//golangcitest:args -Eforbidigo
+//golangcitest:config_path testdata/configs/path-except.yml
+package testdata
+
+import (
+ "fmt"
+ "testing"
+ "time"
+)
+
+func TestForbidigo(t *testing.T) {
+ fmt.Printf("too noisy!!!") // want "use of `fmt\\.Printf` forbidden by pattern `fmt\\\\.Print\\.\\*`"
+ time.Sleep(time.Nanosecond) // want "no sleeping!"
+}