Skip to content

Commit c05858e

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 `not-path` 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 `not-path`, 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 not-path, this becomes much simpler: - not-path: ^test/e2e/framework/ msg: "E2E framework:" linters: - forbidigo
1 parent defde13 commit c05858e

12 files changed

+108
-4
lines changed

.golangci.reference.yml

+7
Original file line numberDiff line numberDiff line change
@@ -2250,6 +2250,13 @@ issues:
22502250
- dupl
22512251
- gosec
22522252

2253+
2254+
# Run some linter only for test files by excluding its issues
2255+
# for everything else.
2256+
- not-path: _test\.go
2257+
linters:
2258+
- forbidigo
2259+
22532260
# Exclude known linters from partially hard-vendored code,
22542261
# which is impossible to exclude via `nolint` comments.
22552262
# `/` will be replaced by current OS file path separator to properly work on Windows.

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

+14-1
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,8 @@ issues:
6666

6767
Exclude issues in path by `run.skip-dirs`, `run.skip-files` or `issues.exclude-rules` config options.
6868

69-
In the following example, all the reports from the linters (`linters`) that concerns the path (`path`) are excluded:
69+
In the following example, all the reports from the linters (`linters`) that concerns the path (`path`) are excluded
70+
and therefore test files do not get checked:
7071

7172
```yml
7273
issues:
@@ -77,6 +78,18 @@ issues:
7778
- goconst
7879
```
7980
81+
The opposite, excluding reports *except* for specific paths, is also possible.
82+
In the following example, only test files get checked:
83+
84+
```yml
85+
issues:
86+
exclude-rules:
87+
- not-path: '(.+)_test\.go'
88+
linters:
89+
- funlen
90+
- goconst
91+
```
92+
8093
In the following example, all the reports related to the files (`skip-files`) are excluded:
8194

8295
```yml

pkg/config/issues.go

+10-2
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ func (e ExcludeRule) Validate() error {
132132
type BaseRule struct {
133133
Linters []string
134134
Path string
135+
NotPath string `mapstructure:"not-path"`
135136
Text string
136137
Source string
137138
}
@@ -140,6 +141,9 @@ 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.NotPath); err != nil {
145+
return fmt.Errorf("invalid not-path 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 NotPath set would
159+
// pass validation whereas before the introduction of not-path that
160+
// would't have been precise enough.
161+
if b.Path != "" || b.NotPath != "" {
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
@@ -268,6 +268,7 @@ func getExcludeRulesProcessor(cfg *config.Issues, log logutils.Log, lineCache *f
268268
Text: r.Text,
269269
Source: r.Source,
270270
Path: r.Path,
271+
NotPath: r.NotPath,
271272
Linters: r.Linters,
272273
},
273274
})
@@ -311,6 +312,7 @@ func getSeverityRulesProcessor(cfg *config.Severity, log logutils.Log, lineCache
311312
Text: r.Text,
312313
Source: r.Source,
313314
Path: r.Path,
315+
NotPath: r.NotPath,
314316
Linters: r.Linters,
315317
},
316318
})

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+
NotPath string
1516
Linters []string
1617
}
1718

1819
type baseRule struct {
1920
text *regexp.Regexp
2021
source *regexp.Regexp
2122
path *regexp.Regexp
23+
notPath *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.notPath == nil && len(r.linters) == 0
2729
}
2830

2931
func (r *baseRule) match(issue *result.Issue, lineCache *fsutils.LineCache, log logutils.Log) bool {
@@ -36,6 +38,9 @@ func (r *baseRule) match(issue *result.Issue, lineCache *fsutils.LineCache, log
3638
if r.path != nil && !r.path.MatchString(issue.FilePath()) {
3739
return false
3840
}
41+
if r.notPath != nil && r.notPath.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.NotPath != "" {
51+
notPath := fsutils.NormalizePathInRegex(rule.NotPath)
52+
parsedRule.notPath = regexp.MustCompile(notPath)
53+
}
5054
parsedRules = append(parsedRules, parsedRule)
5155
}
5256
return parsedRules

pkg/result/processors/exclude_rules_test.go

+9
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,12 @@ func TestExcludeRulesMultiple(t *testing.T) {
3131
Path: `_test\.go`,
3232
},
3333
},
34+
{
35+
BaseRule: BaseRule{
36+
Text: "^nontestonly$",
37+
NotPath: `_test\.go`,
38+
},
39+
},
3440
{
3541
BaseRule: BaseRule{
3642
Source: "^//go:generate ",
@@ -46,6 +52,8 @@ func TestExcludeRulesMultiple(t *testing.T) {
4652
{Path: "e_Test.go", Text: "normal", Linter: "testlinter"},
4753
{Path: "e_test.go", Text: "another", Linter: "linter"},
4854
{Path: "e_test.go", Text: "testonly", Linter: "linter"},
55+
{Path: "e.go", Text: "nontestonly", Linter: "linter"},
56+
{Path: "e_test.go", Text: "nontestonly", Linter: "linter"},
4957
{Path: filepath.Join("testdata", "exclude_rules.go"), Line: 3, Linter: "lll"},
5058
}
5159
var issues []result.Issue
@@ -66,6 +74,7 @@ func TestExcludeRulesMultiple(t *testing.T) {
6674
{Path: "e.go", Text: "some", Linter: "linter"},
6775
{Path: "e_Test.go", Text: "normal", Linter: "testlinter"},
6876
{Path: "e_test.go", Text: "another", Linter: "linter"},
77+
{Path: "e_test.go", Text: "nontestonly", Linter: "linter"},
6978
}
7079
assert.Equal(t, expectedCases, resultingCases)
7180
}

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.NotPath != "" {
56+
notPath := fsutils.NormalizePathInRegex(rule.NotPath)
57+
parsedRule.notPath = regexp.MustCompile(notPath)
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
@@ -37,6 +37,13 @@ func TestSeverityRulesMultiple(t *testing.T) {
3737
Path: `_test\.go`,
3838
},
3939
},
40+
{
41+
Severity: "info",
42+
BaseRule: BaseRule{
43+
Text: "^nontestonly$",
44+
NotPath: `_test\.go`,
45+
},
46+
},
4047
{
4148
BaseRule: BaseRule{
4249
Source: "^//go:generate ",
@@ -70,6 +77,8 @@ func TestSeverityRulesMultiple(t *testing.T) {
7077
{Path: "ssl.go", Text: "ssl", Linter: "gosec"},
7178
{Path: "e.go", Text: "some", Linter: "linter"},
7279
{Path: "e_test.go", Text: "testonly", Linter: "testlinter"},
80+
{Path: "e.go", Text: "nontestonly", Linter: "testlinter"},
81+
{Path: "e_test.go", Text: "nontestonly", Linter: "testlinter"},
7382
{Path: filepath.Join("testdata", "exclude_rules.go"), Line: 3, Linter: "lll"},
7483
{Path: filepath.Join("testdata", "severity_rules.go"), Line: 3, Linter: "invalidgo"},
7584
{Path: "someotherlinter.go", Text: "someotherlinter", Linter: "someotherlinter"},
@@ -95,6 +104,8 @@ func TestSeverityRulesMultiple(t *testing.T) {
95104
{Path: "ssl.go", Text: "ssl", Linter: "gosec", Severity: "info"},
96105
{Path: "e.go", Text: "some", Linter: "linter", Severity: "info"},
97106
{Path: "e_test.go", Text: "testonly", Linter: "testlinter", Severity: "info"},
107+
{Path: "e.go", Text: "nontestonly", Linter: "testlinter", Severity: "info"}, // matched
108+
{Path: "e_test.go", Text: "nontestonly", Linter: "testlinter", Severity: "error"}, // not matched
98109
{Path: filepath.Join("testdata", "exclude_rules.go"), Line: 3, Linter: "lll", Severity: "error"},
99110
{Path: filepath.Join("testdata", "severity_rules.go"), Line: 3, Linter: "invalidgo", Severity: "info"},
100111
{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+
- not-path: _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)