Skip to content

Commit 6d0498c

Browse files
alexandearalexandear
authored
refactor: simplify File.disabledIntervals, add tests (#1216)
1 parent dc3de10 commit 6d0498c

File tree

3 files changed

+191
-13
lines changed

3 files changed

+191
-13
lines changed

‎lint/file.go

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -135,15 +135,16 @@ type enableDisableConfig struct {
135135
position int
136136
}
137137

138+
type disabledIntervalsMap = map[string][]DisabledInterval
139+
138140
const (
139-
directiveRE = `^//[\s]*revive:(enable|disable)(?:-(line|next-line))?(?::([^\s]+))?[\s]*(?: (.+))?$`
140141
directivePos = 1
141142
modifierPos = 2
142143
rulesPos = 3
143144
reasonPos = 4
144145
)
145146

146-
var re = regexp.MustCompile(directiveRE)
147+
var directiveRegexp = regexp.MustCompile(`^//[\s]*revive:(enable|disable)(?:-(line|next-line))?(?::([^\s]+))?[\s]*(?: (.+))?$`)
147148

148149
func (f *File) disabledIntervals(rules []Rule, mustSpecifyDisableReason bool, failures chan Failure) disabledIntervalsMap {
149150
enabledDisabledRulesMap := map[string][]enableDisableConfig{}
@@ -194,8 +195,7 @@ func (f *File) disabledIntervals(rules []Rule, mustSpecifyDisableReason bool, fa
194195
enabledDisabledRulesMap[name] = existing
195196
}
196197

197-
handleRules := func(_, modifier string, isEnabled bool, line int, ruleNames []string) []DisabledInterval {
198-
var result []DisabledInterval
198+
handleRules := func(modifier string, isEnabled bool, line int, ruleNames []string) {
199199
for _, name := range ruleNames {
200200
switch modifier {
201201
case "line":
@@ -208,13 +208,12 @@ func (f *File) disabledIntervals(rules []Rule, mustSpecifyDisableReason bool, fa
208208
handleConfig(isEnabled, line, name)
209209
}
210210
}
211-
return result
212211
}
213212

214-
handleComment := func(filename string, c *ast.CommentGroup, line int) {
213+
handleComment := func(c *ast.CommentGroup, line int) {
215214
comments := c.List
216215
for _, c := range comments {
217-
match := re.FindStringSubmatch(c.Text)
216+
match := directiveRegexp.FindStringSubmatch(c.Text)
218217
if len(match) == 0 {
219218
continue
220219
}
@@ -247,13 +246,12 @@ func (f *File) disabledIntervals(rules []Rule, mustSpecifyDisableReason bool, fa
247246
}
248247
}
249248

250-
handleRules(filename, match[modifierPos], match[directivePos] == "enable", line, ruleNames)
249+
handleRules(match[modifierPos], match[directivePos] == "enable", line, ruleNames)
251250
}
252251
}
253252

254-
comments := f.AST.Comments
255-
for _, c := range comments {
256-
handleComment(f.Name, c, f.ToPosition(c.End()).Line)
253+
for _, c := range f.AST.Comments {
254+
handleComment(c, f.ToPosition(c.End()).Line)
257255
}
258256

259257
return getEnabledDisabledIntervals()

‎lint/file_test.go

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
package lint
2+
3+
import (
4+
"go/ast"
5+
"go/token"
6+
"testing"
7+
)
8+
9+
func TestFile_disabledIntervals(t *testing.T) {
10+
buildCommentGroups := func(comments ...string) []*ast.CommentGroup {
11+
commentGroups := make([]*ast.CommentGroup, 0, len(comments))
12+
for _, c := range comments {
13+
commentGroups = append(commentGroups, &ast.CommentGroup{
14+
List: []*ast.Comment{
15+
{Text: c},
16+
},
17+
})
18+
}
19+
return commentGroups
20+
}
21+
22+
tests := []struct {
23+
name string
24+
comments []*ast.CommentGroup
25+
expected disabledIntervalsMap
26+
}{
27+
{
28+
name: "no directives",
29+
comments: buildCommentGroups("// some comment"),
30+
expected: disabledIntervalsMap{},
31+
},
32+
{
33+
name: "disable rule",
34+
comments: buildCommentGroups("//revive:disable:rule1"),
35+
expected: disabledIntervalsMap{
36+
"rule1": {
37+
{
38+
RuleName: "rule1",
39+
From: token.Position{
40+
Filename: "test.go",
41+
},
42+
To: token.Position{
43+
Filename: "test.go",
44+
Line: 2147483647,
45+
},
46+
},
47+
},
48+
},
49+
},
50+
{
51+
name: "enable rule",
52+
comments: buildCommentGroups("//revive:enable:rule1"),
53+
expected: disabledIntervalsMap{
54+
"rule1": {},
55+
},
56+
},
57+
{
58+
name: "disable and enable rule",
59+
comments: buildCommentGroups("//revive:disable:rule1", "//revive:enable:rule1"),
60+
expected: disabledIntervalsMap{
61+
"rule1": {
62+
{
63+
RuleName: "rule1",
64+
From: token.Position{
65+
Filename: "test.go",
66+
},
67+
To: token.Position{
68+
Filename: "test.go",
69+
},
70+
},
71+
},
72+
},
73+
},
74+
{
75+
name: "disable-line rule",
76+
comments: buildCommentGroups("//revive:disable-line:rule1"),
77+
expected: disabledIntervalsMap{
78+
"rule1": {
79+
{
80+
RuleName: "rule1",
81+
From: token.Position{
82+
Filename: "test.go",
83+
},
84+
To: token.Position{
85+
Filename: "test.go",
86+
},
87+
},
88+
},
89+
},
90+
},
91+
{
92+
name: "enable-line rule",
93+
comments: buildCommentGroups("//revive:enable-line:rule1"),
94+
expected: disabledIntervalsMap{
95+
"rule1": {
96+
{
97+
RuleName: "rule1",
98+
From: token.Position{
99+
Filename: "test.go",
100+
},
101+
To: token.Position{
102+
Filename: "test.go",
103+
Line: 2147483647,
104+
},
105+
},
106+
},
107+
},
108+
},
109+
{
110+
name: "disable-next-line rule",
111+
comments: buildCommentGroups("//revive:disable-next-line:rule1"),
112+
expected: disabledIntervalsMap{
113+
"rule1": {
114+
{
115+
RuleName: "rule1",
116+
From: token.Position{
117+
Filename: "test.go",
118+
Line: 1,
119+
},
120+
To: token.Position{
121+
Filename: "test.go",
122+
Line: 1,
123+
},
124+
},
125+
},
126+
},
127+
},
128+
{
129+
name: "enable-next-line rule",
130+
comments: buildCommentGroups("//revive:enable-next-line:rule1"),
131+
expected: disabledIntervalsMap{
132+
"rule1": {
133+
{
134+
RuleName: "rule1",
135+
From: token.Position{
136+
Filename: "test.go",
137+
Line: 1,
138+
},
139+
To: token.Position{
140+
Filename: "test.go",
141+
Line: 2147483647,
142+
},
143+
},
144+
},
145+
},
146+
},
147+
}
148+
149+
for _, tt := range tests {
150+
t.Run(tt.name, func(t *testing.T) {
151+
f := &File{
152+
Name: "test.go",
153+
Pkg: &Package{
154+
fset: token.NewFileSet(),
155+
},
156+
AST: &ast.File{
157+
Comments: tt.comments,
158+
},
159+
}
160+
got := f.disabledIntervals(nil, false, make(chan Failure, 10))
161+
if len(got) != len(tt.expected) {
162+
t.Errorf("disabledIntervals() = got %v, want %v", got, tt.expected)
163+
}
164+
for rule, intervals := range got {
165+
expectedIntervals, ok := tt.expected[rule]
166+
if !ok {
167+
t.Errorf("unexpected rule %q", rule)
168+
continue
169+
}
170+
if len(intervals) != len(expectedIntervals) {
171+
t.Errorf("intervals for rule %q = got %+v, want %+v", rule, intervals, expectedIntervals)
172+
continue
173+
}
174+
for i, interval := range intervals {
175+
if interval != expectedIntervals[i] {
176+
t.Errorf("interval %d for rule %q = got %+v, want %+v", i, rule, interval, expectedIntervals[i])
177+
}
178+
}
179+
}
180+
})
181+
}
182+
}

‎lint/linter.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@ import (
1919
// ReadFile defines an abstraction for reading files.
2020
type ReadFile func(path string) (result []byte, err error)
2121

22-
type disabledIntervalsMap = map[string][]DisabledInterval
23-
2422
// Linter is used for linting set of files.
2523
type Linter struct {
2624
reader ReadFile

0 commit comments

Comments
 (0)