Skip to content

Commit 6e5a8fe

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 6e5a8fe

12 files changed

+110
-5
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

+12-4
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 {
133133
Linters []string
134134
Path string
135+
PathExcept string `mapstructure:"path-except"`
135136
Text string
136137
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

+2
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,7 @@ func getExcludeRulesProcessor(cfg *config.Issues, log logutils.Log, files *fsuti
279279
Text: r.Text,
280280
Source: r.Source,
281281
Path: r.Path,
282+
PathExcept: r.PathExcept,
282283
Linters: r.Linters,
283284
},
284285
})
@@ -322,6 +323,7 @@ func getSeverityRulesProcessor(cfg *config.Severity, log logutils.Log, files *fs
322323
Text: r.Text,
323324
Source: r.Source,
324325
Path: r.Path,
326+
PathExcept: r.PathExcept,
325327
Linters: r.Linters,
326328
},
327329
})

pkg/result/processors/base_rule.go

+6-1
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,20 @@ type BaseRule struct {
1212
Text string
1313
Source string
1414
Path string
15+
PathExcept string
1516
Linters []string
1617
}
1718

1819
type baseRule struct {
1920
text *regexp.Regexp
2021
source *regexp.Regexp
2122
path *regexp.Regexp
23+
pathExcept *regexp.Regexp
2224
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)