Skip to content

Commit

Permalink
rules: support inverted path match (#3617)
Browse files Browse the repository at this point in the history
  • Loading branch information
pohly committed May 31, 2023
1 parent 0b8ebea commit 8fde463
Show file tree
Hide file tree
Showing 12 changed files with 127 additions and 25 deletions.
5 changes: 5 additions & 0 deletions .golangci.reference.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
12 changes: 12 additions & 0 deletions docs/src/docs/usage/false-positives.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 15 additions & 8 deletions pkg/config/issues.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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 != "" {
Expand All @@ -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
}
Expand Down
18 changes: 10 additions & 8 deletions pkg/lint/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
})
}
Expand Down Expand Up @@ -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,
},
})
}
Expand Down
23 changes: 14 additions & 9 deletions pkg/result/processors/base_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
Expand Down
4 changes: 4 additions & 0 deletions pkg/result/processors/exclude_rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions pkg/result/processors/exclude_rules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ func TestExcludeRulesMultiple(t *testing.T) {
Path: `_test\.go`,
},
},
{
BaseRule: BaseRule{
Text: "^nontestonly$",
PathExcept: `_test\.go`,
},
},
{
BaseRule: BaseRule{
Source: "^//go:generate ",
Expand All @@ -42,13 +48,16 @@ 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"},
{Path: "e_test.go", Text: "normal", Linter: "testlinter"},
{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
Expand All @@ -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)
}
Expand Down Expand Up @@ -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"},
Expand Down
4 changes: 4 additions & 0 deletions pkg/result/processors/severity_rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions pkg/result/processors/severity_rules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 ",
Expand Down Expand Up @@ -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"},
Expand All @@ -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"},
Expand Down
13 changes: 13 additions & 0 deletions test/testdata/configs/path-except.yml
Original file line number Diff line number Diff line change
@@ -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
14 changes: 14 additions & 0 deletions test/testdata/path_except.go
Original file line number Diff line number Diff line change
@@ -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)
}
14 changes: 14 additions & 0 deletions test/testdata/path_except_test.go
Original file line number Diff line number Diff line change
@@ -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!"
}

0 comments on commit 8fde463

Please sign in to comment.