Skip to content

Commit

Permalink
Fix triggering violations within a wokeignore inline ignore comment (
Browse files Browse the repository at this point in the history
…#43)

* Add tests for inline ignore rule name violation

* Remove inline ignores from text to avoid matching against other rules
  • Loading branch information
caitlinelfring authored Nov 11, 2020
1 parent f603505 commit fc0fcf6
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 1 deletion.
23 changes: 23 additions & 0 deletions pkg/parser/violations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,26 @@ func newFileWithPrefix(t *testing.T, prefix, text string) (*os.File, error) {

return tmpFile, err
}

// Tests for when a rule name is used with inline wokeignore, when that rule name matches another rule
func TestGenerateFileViolationsOverlappingRules(t *testing.T) {
tests := []struct {
desc string
content string
matches int
}{
{"overlapping rule", "this has master #wokeignore:rule=master-slave", 0},
{"overlapping rule two ignores", "this has master #wokeignore:rule=master-slave,slave", 0},
{"wrong rule", "this has whitelist # wokeignore:rule=blacklist", 1},
}
for _, tc := range tests {
t.Run(tc.desc, func(t *testing.T) {
f, err := newFile(t, tc.content)
assert.NoError(t, err)

res, err := generateFileViolationsFromFilename(f.Name(), rule.DefaultRules)
assert.NoError(t, err)
assert.Len(t, res.Results, tc.matches)
})
}
}
26 changes: 25 additions & 1 deletion pkg/rule/rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"regexp"
"strings"
"unicode"

"github.com/get-woke/woke/pkg/util"
)
Expand Down Expand Up @@ -37,7 +38,9 @@ func (r *Rule) FindMatchIndexes(text string) [][]int {
}

r.SetRegexp()
matches := r.findAllStringSubmatchIndex(text)

// Remove inline ignores from text to avoid matching against other rules
matches := r.findAllStringSubmatchIndex(removeInlineIgnore(text))
if matches == nil {
return [][]int(nil)
}
Expand Down Expand Up @@ -160,6 +163,27 @@ func escape(ss []string) []string {
return ss
}

// removeInlineIgnore removes the entire match of the ignoreRuleRegex from the line
// and replaces it with the unicode replacement character so the rule matcher won't
// attempt to find violations within
func removeInlineIgnore(line string) string {
inlineIgnoreMatch := ignoreRuleRegex.FindStringIndex(line)
if inlineIgnoreMatch == nil || len(inlineIgnoreMatch) < 2 {
return line
}

lineWithoutIgnoreRule := []rune(line)

start := inlineIgnoreMatch[0]
end := inlineIgnoreMatch[1]

for i := start; i < end; i++ {
lineWithoutIgnoreRule[i] = unicode.ReplacementChar
}

return string(lineWithoutIgnoreRule)
}

// Disabled denotes if the rule is disabled
// If no terms are provided, this essentially disables the rule
// which is helpful for disabling default rules. Eventually, there should be a better
Expand Down
24 changes: 24 additions & 0 deletions pkg/rule/rule_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,3 +130,27 @@ func TestRule_EmptyTerms(t *testing.T) {
})
}
}

func Test_removeInlineIgnore(t *testing.T) {
tests := []struct {
desc string
line string
expected string
}{
{
desc: "replace wokeignore:rule",
line: "wokeignore:rule=master-slave",
expected: "����������������������������",
},
{
desc: "not replace wokeignore:rule",
line: "no inline ignore",
expected: "no inline ignore",
},
}
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
assert.Equal(t, tt.expected, removeInlineIgnore(tt.line))
})
}
}

0 comments on commit fc0fcf6

Please sign in to comment.