Skip to content

Commit 50a74a5

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 bfa2006 commit 50a74a5

12 files changed

+112
-6
lines changed

.golangci.reference.yml

+7
Original file line numberDiff line numberDiff line change
@@ -2329,6 +2329,13 @@ issues:
23292329
- dupl
23302330
- gosec
23312331

2332+
2333+
# Run some linter only for test files by excluding its issues
2334+
# for everything else.
2335+
- not-path: _test\.go
2336+
linters:
2337+
- forbidigo
2338+
23322339
# Exclude known linters from partially hard-vendored code,
23332340
# which is impossible to exclude via `nolint` comments.
23342341
# `/` 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
@@ -70,7 +70,8 @@ Beware that the paths that get matched here are relative to the current working
7070
When the configuration contains path patterns that check for specific directories,
7171
the `--path-prefix` parameter can be used to extend the paths before matching.
7272

73-
In the following example, all the reports from the linters (`linters`) that concerns the path (`path`) are excluded:
73+
In the following example, all the reports from the linters (`linters`) that concerns the path (`path`) are excluded
74+
and therefore test files do not get checked by these linters:
7475

7576
```yml
7677
issues:
@@ -81,6 +82,18 @@ issues:
8182
- goconst
8283
```
8384
85+
The opposite, excluding reports *except* for specific paths, is also possible.
86+
In the following example, only test files get checked with `funlen` and `goconst`:
87+
88+
```yml
89+
issues:
90+
exclude-rules:
91+
- not-path: '(.+)_test\.go'
92+
linters:
93+
- funlen
94+
- goconst
95+
```
96+
8497
In the following example, all the reports related to the files (`skip-files`) are excluded:
8598

8699
```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+
NotPath string `mapstructure:"not-path"`
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.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
@@ -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+
NotPath: r.NotPath,
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+
NotPath: r.NotPath,
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+
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, 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.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

+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+
NotPath: `_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.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
@@ -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+
NotPath: `_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+
- 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)