refactor: simplify File.disabledIntervals, add tests

lint/file.go

@@ -135,15 +135,16 @@ type enableDisableConfig struct {
 	position int
 }
 
+type disabledIntervalsMap = map[string][]DisabledInterval
+
 const (
-	directiveRE  = `^//[\s]*revive:(enable|disable)(?:-(line|next-line))?(?::([^\s]+))?[\s]*(?: (.+))?$`
 	directivePos = 1
 	modifierPos  = 2
 	rulesPos     = 3
 	reasonPos    = 4
 )
 
-var re = regexp.MustCompile(directiveRE)
+var directiveRegexp = regexp.MustCompile(`^//[\s]*revive:(enable|disable)(?:-(line|next-line))?(?::([^\s]+))?[\s]*(?: (.+))?$`)
 
 func (f *File) disabledIntervals(rules []Rule, mustSpecifyDisableReason bool, failures chan Failure) disabledIntervalsMap {
 	enabledDisabledRulesMap := map[string][]enableDisableConfig{}
@@ -194,8 +195,7 @@ func (f *File) disabledIntervals(rules []Rule, mustSpecifyDisableReason bool, fa
 		enabledDisabledRulesMap[name] = existing
 	}
 
-	handleRules := func(_, modifier string, isEnabled bool, line int, ruleNames []string) []DisabledInterval {
-		var result []DisabledInterval
+	handleRules := func(modifier string, isEnabled bool, line int, ruleNames []string) {
 		for _, name := range ruleNames {
 			switch modifier {
 			case "line":
@@ -208,13 +208,12 @@ func (f *File) disabledIntervals(rules []Rule, mustSpecifyDisableReason bool, fa
 				handleConfig(isEnabled, line, name)
 			}
 		}
-		return result
 	}
 
-	handleComment := func(filename string, c *ast.CommentGroup, line int) {
+	handleComment := func(c *ast.CommentGroup, line int) {
 		comments := c.List
 		for _, c := range comments {
-			match := re.FindStringSubmatch(c.Text)
+			match := directiveRegexp.FindStringSubmatch(c.Text)
 			if len(match) == 0 {
 				continue
 			}
@@ -247,13 +246,12 @@ func (f *File) disabledIntervals(rules []Rule, mustSpecifyDisableReason bool, fa
 				}
 			}
 
-			handleRules(filename, match[modifierPos], match[directivePos] == "enable", line, ruleNames)
+			handleRules(match[modifierPos], match[directivePos] == "enable", line, ruleNames)
 		}
 	}
 
-	comments := f.AST.Comments
-	for _, c := range comments {
-		handleComment(f.Name, c, f.ToPosition(c.End()).Line)
+	for _, c := range f.AST.Comments {
+		handleComment(c, f.ToPosition(c.End()).Line)
 	}
 
 	return getEnabledDisabledIntervals()

lint/file_test.go

@@ -0,0 +1,182 @@
+package lint
+
+import (
+	"go/ast"
+	"go/token"
+	"testing"
+)
+
+func TestFile_disabledIntervals(t *testing.T) {
+	buildCommentGroups := func(comments ...string) []*ast.CommentGroup {
+		commentGroups := make([]*ast.CommentGroup, 0, len(comments))
+		for _, c := range comments {
+			commentGroups = append(commentGroups, &ast.CommentGroup{
+				List: []*ast.Comment{
+					{Text: c},
+				},
+			})
+		}
+		return commentGroups
+	}
+
+	tests := []struct {
+		name     string
+		comments []*ast.CommentGroup
+		expected disabledIntervalsMap
+	}{
+		{
+			name:     "no directives",
+			comments: buildCommentGroups("// some comment"),
+			expected: disabledIntervalsMap{},
+		},
+		{
+			name:     "disable rule",
+			comments: buildCommentGroups("//revive:disable:rule1"),
+			expected: disabledIntervalsMap{
+				"rule1": {
+					{
+						RuleName: "rule1",
+						From: token.Position{
+							Filename: "test.go",
+						},
+						To: token.Position{
+							Filename: "test.go",
+							Line:     2147483647,
+						},
+					},
+				},
+			},
+		},
+		{
+			name:     "enable rule",
+			comments: buildCommentGroups("//revive:enable:rule1"),
+			expected: disabledIntervalsMap{
+				"rule1": {},
+			},
+		},
+		{
+			name:     "disable and enable rule",
+			comments: buildCommentGroups("//revive:disable:rule1", "//revive:enable:rule1"),
+			expected: disabledIntervalsMap{
+				"rule1": {
+					{
+						RuleName: "rule1",
+						From: token.Position{
+							Filename: "test.go",
+						},
+						To: token.Position{
+							Filename: "test.go",
+						},
+					},
+				},
+			},
+		},
+		{
+			name:     "disable-line rule",
+			comments: buildCommentGroups("//revive:disable-line:rule1"),
+			expected: disabledIntervalsMap{
+				"rule1": {
+					{
+						RuleName: "rule1",
+						From: token.Position{
+							Filename: "test.go",
+						},
+						To: token.Position{
+							Filename: "test.go",
+						},
+					},
+				},
+			},
+		},
+		{
+			name:     "enable-line rule",
+			comments: buildCommentGroups("//revive:enable-line:rule1"),
+			expected: disabledIntervalsMap{
+				"rule1": {
+					{
+						RuleName: "rule1",
+						From: token.Position{
+							Filename: "test.go",
+						},
+						To: token.Position{
+							Filename: "test.go",
+							Line:     2147483647,
+						},
+					},
+				},
+			},
+		},
+		{
+			name:     "disable-next-line rule",
+			comments: buildCommentGroups("//revive:disable-next-line:rule1"),
+			expected: disabledIntervalsMap{
+				"rule1": {
+					{
+						RuleName: "rule1",
+						From: token.Position{
+							Filename: "test.go",
+							Line:     1,
+						},
+						To: token.Position{
+							Filename: "test.go",
+							Line:     1,
+						},
+					},
+				},
+			},
+		},
+		{
+			name:     "enable-next-line rule",
+			comments: buildCommentGroups("//revive:enable-next-line:rule1"),
+			expected: disabledIntervalsMap{
+				"rule1": {
+					{
+						RuleName: "rule1",
+						From: token.Position{
+							Filename: "test.go",
+							Line:     1,
+						},
+						To: token.Position{
+							Filename: "test.go",
+							Line:     2147483647,
+						},
+					},
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			f := &File{
+				Name: "test.go",
+				Pkg: &Package{
+					fset: token.NewFileSet(),
+				},
+				AST: &ast.File{
+					Comments: tt.comments,
+				},
+			}
+			got := f.disabledIntervals(nil, false, make(chan Failure, 10))
+			if len(got) != len(tt.expected) {
+				t.Errorf("disabledIntervals() = got %v, want %v", got, tt.expected)
+			}
+			for rule, intervals := range got {
+				expectedIntervals, ok := tt.expected[rule]
+				if !ok {
+					t.Errorf("unexpected rule %q", rule)
+					continue
+				}
+				if len(intervals) != len(expectedIntervals) {
+					t.Errorf("intervals for rule %q = got %+v, want %+v", rule, intervals, expectedIntervals)
+					continue
+				}
+				for i, interval := range intervals {
+					if interval != expectedIntervals[i] {
+						t.Errorf("interval %d for rule %q = got %+v, want %+v", i, rule, interval, expectedIntervals[i])
+					}
+				}
+			}
+		})
+	}
+}

lint/linter.go

@@ -19,8 +19,6 @@ import (
 // ReadFile defines an abstraction for reading files.
 type ReadFile func(path string) (result []byte, err error)
 
-type disabledIntervalsMap = map[string][]DisabledInterval
-
 // Linter is used for linting set of files.
 type Linter struct {
 	reader         ReadFile