Skip to content

Commit fc98692

Browse files
committed
rules: support inverted path match
It is not possible to use `exclude-rules: path` to exclude issues for non-test files because it is impossible to write a regexp that reliably matches those (#2440). The new `path-except` check solves that by inverting the meaning: it excludes if the pattern does not match. Besides test files, this is also useful for large code bases where some specific code paths have additional constraints. For example, in Kubernetes the usage of certain functions is forbidden in test/e2e/framework but okay elsewhere. Without `path-except`, a regular expression that carefully matches all other paths has to be used, which is very cumbersome: - path: ^(api|cluster|cmd|hack|pkg|plugin|staging|test/(cmd|conformance|e2e/(apimachinery|apps|architecture|auth|autoscaling|chaosmonkey|cloud|common|dra|instrumentation|kubectl|lifecycle|network|node|perftype|reporters|scheduling|storage|testing-manifests|upgrades|windows)|e2e_kubeadm|e2e_node|fixtures|fuzz|images|instrumentation|integration|kubemark|list|soak|typecheck|utils)|third_party|vendor)/ msg: "E2E framework:" linters: - forbidigo With path-except, this becomes much simpler: - path-except: ^test/e2e/framework/ msg: "E2E framework:" linters: - forbidigo
1 parent f648894 commit fc98692

12 files changed

+130
-25
lines changed

.golangci.reference.yml

+7
Original file line numberDiff line numberDiff line change
@@ -2331,6 +2331,13 @@ issues:
23312331
- dupl
23322332
- gosec
23332333

2334+
2335+
# Run some linter only for test files by excluding its issues
2336+
# for everything else.
2337+
- path-except: _test\.go
2338+
linters:
2339+
- forbidigo
2340+
23342341
# Exclude known linters from partially hard-vendored code,
23352342
# which is impossible to exclude via `nolint` comments.
23362343
# `/` will be replaced by current OS file path separator to properly work on Windows.

docs/src/docs/usage/false-positives.mdx

+12
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,18 @@ issues:
8181
- goconst
8282
```
8383
84+
The opposite, excluding reports *except* for specific paths, is also possible.
85+
In the following example, only test files get checked:
86+
87+
```yml
88+
issues:
89+
exclude-rules:
90+
- path-except: '(.+)_test\.go'
91+
linters:
92+
- funlen
93+
- goconst
94+
```
95+
8496
In the following example, all the reports related to the files (`skip-files`) are excluded:
8597

8698
```yml

pkg/config/issues.go

+16-8
Original file line numberDiff line numberDiff line change
@@ -125,21 +125,25 @@ type ExcludeRule struct {
125125
BaseRule `mapstructure:",squash"`
126126
}
127127

128-
func (e ExcludeRule) Validate() error {
128+
func (e *ExcludeRule) Validate() error {
129129
return e.BaseRule.Validate(excludeRuleMinConditionsCount)
130130
}
131131

132132
type BaseRule struct {
133-
Linters []string
134-
Path string
135-
Text string
136-
Source string
133+
Linters []string
134+
Path string
135+
PathExcept string `mapstructure:"path-except"`
136+
Text string
137+
Source string
137138
}
138139

139-
func (b BaseRule) Validate(minConditionsCount int) error {
140+
func (b *BaseRule) Validate(minConditionsCount int) error {
140141
if err := validateOptionalRegex(b.Path); err != nil {
141142
return fmt.Errorf("invalid path regex: %v", err)
142143
}
144+
if err := validateOptionalRegex(b.PathExcept); err != nil {
145+
return fmt.Errorf("invalid path-except regex: %v", err)
146+
}
143147
if err := validateOptionalRegex(b.Text); err != nil {
144148
return fmt.Errorf("invalid text regex: %v", err)
145149
}
@@ -150,7 +154,11 @@ func (b BaseRule) Validate(minConditionsCount int) error {
150154
if len(b.Linters) > 0 {
151155
nonBlank++
152156
}
153-
if b.Path != "" {
157+
// Filtering by path counts as one condition, regardless how it is done
158+
// (one or both). Otherwise a rule with Path and PathExcept set would
159+
// pass validation whereas before the introduction of path-except that
160+
// would't have been precise enough.
161+
if b.Path != "" || b.PathExcept != "" {
154162
nonBlank++
155163
}
156164
if b.Text != "" {
@@ -160,7 +168,7 @@ func (b BaseRule) Validate(minConditionsCount int) error {
160168
nonBlank++
161169
}
162170
if nonBlank < minConditionsCount {
163-
return fmt.Errorf("at least %d of (text, source, path, linters) should be set", minConditionsCount)
171+
return fmt.Errorf("at least %d of (text, source, [not-]path, linters) should be set", minConditionsCount)
164172
}
165173
return nil
166174
}

pkg/lint/runner.go

+10-8
Original file line numberDiff line numberDiff line change
@@ -276,10 +276,11 @@ func getExcludeRulesProcessor(cfg *config.Issues, log logutils.Log, files *fsuti
276276
for _, r := range cfg.ExcludeRules {
277277
excludeRules = append(excludeRules, processors.ExcludeRule{
278278
BaseRule: processors.BaseRule{
279-
Text: r.Text,
280-
Source: r.Source,
281-
Path: r.Path,
282-
Linters: r.Linters,
279+
Text: r.Text,
280+
Source: r.Source,
281+
Path: r.Path,
282+
PathExcept: r.PathExcept,
283+
Linters: r.Linters,
283284
},
284285
})
285286
}
@@ -319,10 +320,11 @@ func getSeverityRulesProcessor(cfg *config.Severity, log logutils.Log, files *fs
319320
severityRules = append(severityRules, processors.SeverityRule{
320321
Severity: r.Severity,
321322
BaseRule: processors.BaseRule{
322-
Text: r.Text,
323-
Source: r.Source,
324-
Path: r.Path,
325-
Linters: r.Linters,
323+
Text: r.Text,
324+
Source: r.Source,
325+
Path: r.Path,
326+
PathExcept: r.PathExcept,
327+
Linters: r.Linters,
326328
},
327329
})
328330
}

pkg/result/processors/base_rule.go

+14-9
Original file line numberDiff line numberDiff line change
@@ -9,21 +9,23 @@ import (
99
)
1010

1111
type BaseRule struct {
12-
Text string
13-
Source string
14-
Path string
15-
Linters []string
12+
Text string
13+
Source string
14+
Path string
15+
PathExcept string
16+
Linters []string
1617
}
1718

1819
type baseRule struct {
19-
text *regexp.Regexp
20-
source *regexp.Regexp
21-
path *regexp.Regexp
22-
linters []string
20+
text *regexp.Regexp
21+
source *regexp.Regexp
22+
path *regexp.Regexp
23+
pathExcept *regexp.Regexp
24+
linters []string
2325
}
2426

2527
func (r *baseRule) isEmpty() bool {
26-
return r.text == nil && r.source == nil && r.path == nil && len(r.linters) == 0
28+
return r.text == nil && r.source == nil && r.path == nil && r.pathExcept == nil && len(r.linters) == 0
2729
}
2830

2931
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
3638
if r.path != nil && !r.path.MatchString(files.WithPathPrefix(issue.FilePath())) {
3739
return false
3840
}
41+
if r.pathExcept != nil && r.pathExcept.MatchString(issue.FilePath()) {
42+
return false
43+
}
3944
if len(r.linters) != 0 && !r.matchLinter(issue) {
4045
return false
4146
}

pkg/result/processors/exclude_rules.go

+4
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ func createRules(rules []ExcludeRule, prefix string) []excludeRule {
4747
path := fsutils.NormalizePathInRegex(rule.Path)
4848
parsedRule.path = regexp.MustCompile(path)
4949
}
50+
if rule.PathExcept != "" {
51+
pathExcept := fsutils.NormalizePathInRegex(rule.PathExcept)
52+
parsedRule.pathExcept = regexp.MustCompile(pathExcept)
53+
}
5054
parsedRules = append(parsedRules, parsedRule)
5155
}
5256
return parsedRules

pkg/result/processors/exclude_rules_test.go

+11
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@ func TestExcludeRulesMultiple(t *testing.T) {
3434
Path: `_test\.go`,
3535
},
3636
},
37+
{
38+
BaseRule: BaseRule{
39+
Text: "^nontestonly$",
40+
PathExcept: `_test\.go`,
41+
},
42+
},
3743
{
3844
BaseRule: BaseRule{
3945
Source: "^//go:generate ",
@@ -42,13 +48,16 @@ func TestExcludeRulesMultiple(t *testing.T) {
4248
},
4349
}, files, nil)
4450

51+
//nolint:dupl
4552
cases := []issueTestCase{
4653
{Path: "e.go", Text: "exclude", Linter: "linter"},
4754
{Path: "e.go", Text: "some", Linter: "linter"},
4855
{Path: "e_test.go", Text: "normal", Linter: "testlinter"},
4956
{Path: "e_Test.go", Text: "normal", Linter: "testlinter"},
5057
{Path: "e_test.go", Text: "another", Linter: "linter"},
5158
{Path: "e_test.go", Text: "testonly", Linter: "linter"},
59+
{Path: "e.go", Text: "nontestonly", Linter: "linter"},
60+
{Path: "e_test.go", Text: "nontestonly", Linter: "linter"},
5261
{Path: filepath.Join("testdata", "exclude_rules.go"), Line: 3, Linter: "lll"},
5362
}
5463
var issues []result.Issue
@@ -69,6 +78,7 @@ func TestExcludeRulesMultiple(t *testing.T) {
6978
{Path: "e.go", Text: "some", Linter: "linter"},
7079
{Path: "e_Test.go", Text: "normal", Linter: "testlinter"},
7180
{Path: "e_test.go", Text: "another", Linter: "linter"},
81+
{Path: "e_test.go", Text: "nontestonly", Linter: "linter"},
7282
}
7383
assert.Equal(t, expectedCases, resultingCases)
7484
}
@@ -172,6 +182,7 @@ func TestExcludeRulesCaseSensitiveMultiple(t *testing.T) {
172182
},
173183
}, files, nil)
174184

185+
//nolint:dupl
175186
cases := []issueTestCase{
176187
{Path: "e.go", Text: "exclude", Linter: "linter"},
177188
{Path: "e.go", Text: "excLude", Linter: "linter"},

pkg/result/processors/severity_rules.go

+4
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,10 @@ func createSeverityRules(rules []SeverityRule, prefix string) []severityRule {
5252
path := fsutils.NormalizePathInRegex(rule.Path)
5353
parsedRule.path = regexp.MustCompile(path)
5454
}
55+
if rule.PathExcept != "" {
56+
pathExcept := fsutils.NormalizePathInRegex(rule.PathExcept)
57+
parsedRule.pathExcept = regexp.MustCompile(pathExcept)
58+
}
5559
parsedRules = append(parsedRules, parsedRule)
5660
}
5761
return parsedRules

pkg/result/processors/severity_rules_test.go

+11
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,13 @@ func TestSeverityRulesMultiple(t *testing.T) {
3939
Path: `_test\.go`,
4040
},
4141
},
42+
{
43+
Severity: "info",
44+
BaseRule: BaseRule{
45+
Text: "^nontestonly$",
46+
PathExcept: `_test\.go`,
47+
},
48+
},
4249
{
4350
BaseRule: BaseRule{
4451
Source: "^//go:generate ",
@@ -72,6 +79,8 @@ func TestSeverityRulesMultiple(t *testing.T) {
7279
{Path: "ssl.go", Text: "ssl", Linter: "gosec"},
7380
{Path: "e.go", Text: "some", Linter: "linter"},
7481
{Path: "e_test.go", Text: "testonly", Linter: "testlinter"},
82+
{Path: "e.go", Text: "nontestonly", Linter: "testlinter"},
83+
{Path: "e_test.go", Text: "nontestonly", Linter: "testlinter"},
7584
{Path: filepath.Join("testdata", "exclude_rules.go"), Line: 3, Linter: "lll"},
7685
{Path: filepath.Join("testdata", "severity_rules.go"), Line: 3, Linter: "invalidgo"},
7786
{Path: "someotherlinter.go", Text: "someotherlinter", Linter: "someotherlinter"},
@@ -97,6 +106,8 @@ func TestSeverityRulesMultiple(t *testing.T) {
97106
{Path: "ssl.go", Text: "ssl", Linter: "gosec", Severity: "info"},
98107
{Path: "e.go", Text: "some", Linter: "linter", Severity: "info"},
99108
{Path: "e_test.go", Text: "testonly", Linter: "testlinter", Severity: "info"},
109+
{Path: "e.go", Text: "nontestonly", Linter: "testlinter", Severity: "info"}, // matched
110+
{Path: "e_test.go", Text: "nontestonly", Linter: "testlinter", Severity: "error"}, // not matched
100111
{Path: filepath.Join("testdata", "exclude_rules.go"), Line: 3, Linter: "lll", Severity: "error"},
101112
{Path: filepath.Join("testdata", "severity_rules.go"), Line: 3, Linter: "invalidgo", Severity: "info"},
102113
{Path: "someotherlinter.go", Text: "someotherlinter", Linter: "someotherlinter", Severity: "info"},
+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
linters-settings:
2+
forbidigo:
3+
forbid:
4+
- fmt\.Print.*
5+
- time.Sleep(# no sleeping!)?
6+
7+
issues:
8+
exclude-rules:
9+
# Apply forbidigo only to test files, exclude
10+
# it everywhere else.
11+
- path-except: _test\.go
12+
linters:
13+
- forbidigo

test/testdata/forbidigo_exclude.go

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
//golangcitest:args -Eforbidigo
2+
//golangcitest:config_path testdata/configs/forbidigo_tests.yml
3+
//golangcitest:expected_exitcode 0
4+
package testdata
5+
6+
import (
7+
"fmt"
8+
"time"
9+
)
10+
11+
func Forbidigo() {
12+
fmt.Printf("too noisy!!!")
13+
time.Sleep(time.Nanosecond)
14+
}
+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
//golangcitest:args -Eforbidigo
2+
//golangcitest:config_path testdata/configs/forbidigo_tests.yml
3+
package testdata
4+
5+
import (
6+
"fmt"
7+
"testing"
8+
"time"
9+
)
10+
11+
func TestForbidigo(t *testing.T) {
12+
fmt.Printf("too noisy!!!") // want "use of `fmt\\.Printf` forbidden by pattern `fmt\\\\.Print\\.\\*`"
13+
time.Sleep(time.Nanosecond) // want "no sleeping!"
14+
}

0 commit comments

Comments
 (0)