Skip to content

Commit

Permalink
Include inline ignored violations when --no-ignore flag is used (#55)
Browse files Browse the repository at this point in the history
  • Loading branch information
caitlinelfring authored Mar 6, 2021
1 parent 69f1664 commit e33a005
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 34 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ Flags:
--debug Enable debug logging
--exit-1-on-failure Exit with exit code 1 on failures
-h, --help help for woke
--no-ignore Files matching entries in .gitignore/.wokeignore are parsed
--no-ignore Ignored files in .gitignore/.wokeignore and inline ignores are processed
-o, --output string Output type [text,simple,github-actions,json] (default "text")
--stdin Read from stdin
-v, --version version for woke
Expand Down
2 changes: 1 addition & 1 deletion cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ func init() {
rootCmd.PersistentFlags().BoolVar(&exitOneOnFailure, "exit-1-on-failure", false, "Exit with exit code 1 on failures")
rootCmd.PersistentFlags().BoolVar(&stdin, "stdin", false, "Read from stdin")
rootCmd.PersistentFlags().BoolVar(&debug, "debug", false, "Enable debug logging")
rootCmd.PersistentFlags().BoolVar(&noIgnore, "no-ignore", false, "Files matching entries in .gitignore/.wokeignore are parsed")
rootCmd.PersistentFlags().BoolVar(&noIgnore, "no-ignore", false, "Ignored files in .gitignore/.wokeignore and inline ignores are processed")
rootCmd.PersistentFlags().StringVarP(&outputName, "output", "o", printer.OutFormatText, fmt.Sprintf("Output type [%s]", printer.OutFormatsString))
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/parser/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func NewParser(rules []*rule.Rule, ignorer *ignore.Ignore) *Parser {
func (p *Parser) ParsePaths(print printer.Printer, paths ...string) int {
// data provided through stdin
if util.InSlice(os.Stdin.Name(), paths) {
r, _ := generateFileViolations(os.Stdin, p.Rules)
r, _ := p.generateFileViolations(os.Stdin)
if r.Len() > 0 {
print.Print(output.Stdout, r)
}
Expand Down Expand Up @@ -88,7 +88,7 @@ func (p *Parser) processFiles(files <-chan string, done chan bool, wg *sync.Wait
go func(f string) {
defer wg.Done()

v, _ := generateFileViolationsFromFilename(f, p.Rules)
v, _ := p.generateFileViolationsFromFilename(f)
if v == nil || len(v.Results) == 0 {
return
}
Expand Down
32 changes: 25 additions & 7 deletions pkg/parser/parser_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package parser

import (
"fmt"
"go/token"
"io"
"io/ioutil"
Expand Down Expand Up @@ -35,10 +34,10 @@ func parsePathTests(t *testing.T) {
t.Run("violation", func(t *testing.T) {
f, err := newFile(t, "i have a whitelist")
assert.NoError(t, err)

pr := new(testPrinter)
p := testParser()
violations := p.ParsePaths(pr, f.Name())

assert.Len(t, pr.results, 1)
assert.Equal(t, len(pr.results), violations)

Expand Down Expand Up @@ -75,8 +74,6 @@ func parsePathTests(t *testing.T) {
p := testParser()
pr := new(testPrinter)
violations := p.ParsePaths(pr, f.Name())

assert.NoError(t, err)
assert.Len(t, pr.results, 0)
assert.Equal(t, len(pr.results), violations)
})
Expand All @@ -87,7 +84,6 @@ func parsePathTests(t *testing.T) {
p := testParser()
pr := new(testPrinter)
violations := p.ParsePaths(pr, f.Name())
assert.NoError(t, err)
assert.Len(t, pr.results, 0)
assert.Equal(t, len(pr.results), violations)
})
Expand All @@ -102,8 +98,6 @@ func parsePathTests(t *testing.T) {
p := testParser()
pr := new(testPrinter)
violations := p.ParsePaths(pr, f1.Name(), f2.Name())
assert.NoError(t, err)
fmt.Println(pr.results)
assert.Len(t, pr.results, 1)
assert.Equal(t, len(pr.results), violations)
})
Expand All @@ -117,11 +111,35 @@ func parsePathTests(t *testing.T) {
pr := new(testPrinter)

violations := p.ParsePaths(pr, f.Name())
assert.Len(t, pr.results, 0)
assert.Equal(t, len(pr.results), violations)
})

t.Run("ignored inline", func(t *testing.T) {
f, err := newFile(t, "i have a whitelist violation, but am ignored # wokeignore:rule=whitelist\n")
assert.NoError(t, err)

p := testParser()
pr := new(testPrinter)

violations := p.ParsePaths(pr, f.Name())
assert.Len(t, pr.results, 0)
assert.Equal(t, len(pr.results), violations)
})

t.Run("ignored inline with no ignorer", func(t *testing.T) {
f, err := newFile(t, "i have a whitelist violation, but am ignored # wokeignore:rule=whitelist\n")
assert.NoError(t, err)

p := testParser()
p.Ignorer = nil
pr := new(testPrinter)

violations := p.ParsePaths(pr, f.Name())
assert.Len(t, pr.results, 1)
assert.Equal(t, len(pr.results), violations)
})

t.Run("default path", func(t *testing.T) {
// Test default path (which would run tests against the parser package)
p := testParser()
Expand Down
20 changes: 14 additions & 6 deletions pkg/parser/violations.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,22 @@ import (
"time"

"github.com/get-woke/woke/pkg/result"
"github.com/get-woke/woke/pkg/rule"

"github.com/rs/zerolog/log"
)

func generateFileViolationsFromFilename(filename string, rules []*rule.Rule) (*result.FileResults, error) {
func (p *Parser) generateFileViolationsFromFilename(filename string) (*result.FileResults, error) {
file, err := os.Open(filename)
if err != nil {
return nil, err
}
defer file.Close()
return generateFileViolations(file, rules)
return p.generateFileViolations(file)
}

// generateFileViolations reads the file and returns results of places where rules are broken
// this function will not close the file, that should be handled by the caller
func generateFileViolations(file *os.File, rules []*rule.Rule) (*result.FileResults, error) {
func (p *Parser) generateFileViolations(file *os.File) (*result.FileResults, error) {
filename := filepath.ToSlash(file.Name())
start := time.Now()
defer func() {
Expand All @@ -40,7 +39,7 @@ func generateFileViolations(file *os.File, rules []*rule.Rule) (*result.FileResu
}

// Check for violations in the filename itself
for _, pathResult := range result.MatchPathRules(rules, file.Name()) {
for _, pathResult := range result.MatchPathRules(p.Rules, file.Name()) {
results.Results = append(results.Results, pathResult)
}

Expand All @@ -54,7 +53,16 @@ Loop:
case err == nil || (err == io.EOF && text != ""):
text = strings.TrimSuffix(text, "\n")

for _, r := range rules {
for _, r := range p.Rules {
if p.Ignorer != nil && r.CanIgnoreLine(text) {
log.Debug().
Str("rule", r.Name).
Str("file", filename).
Int("line", line).
Msg("ignoring via in-line")
continue
}

lineResults := result.FindResults(r, results.Filename, text, line)
results.Results = append(results.Results, lineResults...)
}
Expand Down
13 changes: 8 additions & 5 deletions pkg/parser/violations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ func TestGenerateFileViolations(t *testing.T) {
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)
p := testParser()
res, err := p.generateFileViolationsFromFilename(f.Name())
assert.NoError(t, err)

filename := filepath.ToSlash(f.Name())
Expand Down Expand Up @@ -60,15 +60,17 @@ func TestGenerateFileViolations(t *testing.T) {
})
}
t.Run("missing file", func(t *testing.T) {
_, err := generateFileViolationsFromFilename("missing.file", rule.DefaultRules)
p := testParser()
_, err := p.generateFileViolationsFromFilename("missing.file")
assert.Error(t, err)
})

t.Run("filename violation", func(t *testing.T) {
f, err := newFileWithPrefix(t, "whitelist-", "content")
assert.NoError(t, err)

res, err := generateFileViolationsFromFilename(f.Name(), rule.DefaultRules)
p := testParser()
res, err := p.generateFileViolationsFromFilename(f.Name())
assert.NoError(t, err)
assert.Len(t, res.Results, 1)
assert.Regexp(t, "^Filename violation: ", res.Results[0].Reason())
Expand Down Expand Up @@ -114,7 +116,8 @@ func TestGenerateFileViolationsOverlappingRules(t *testing.T) {
f, err := newFile(t, tc.content)
assert.NoError(t, err)

res, err := generateFileViolationsFromFilename(f.Name(), rule.DefaultRules)
p := testParser()
res, err := p.generateFileViolationsFromFilename(f.Name())
assert.NoError(t, err)
assert.Len(t, res.Results, tc.matches)
})
Expand Down
11 changes: 0 additions & 11 deletions pkg/result/lineresult.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import (
"go/token"

"github.com/get-woke/woke/pkg/rule"

"github.com/rs/zerolog/log"
)

// MaxLineLength is the max line length that this printer
Expand Down Expand Up @@ -47,15 +45,6 @@ func NewLineResult(r *rule.Rule, violation, filename string, line, startColumn,
// FindResults returns the results that match the rule for the given text.
// filename and line are only used for the Position
func FindResults(r *rule.Rule, filename, text string, line int) (rs []Result) {
if r.CanIgnoreLine(text) {
log.Debug().
Str("rule", r.Name).
Str("file", filename).
Int("line", line).
Msg("ignoring via in-line")
return
}

idxs := r.FindMatchIndexes(text)

for _, idx := range idxs {
Expand Down
3 changes: 2 additions & 1 deletion pkg/result/lineresult_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ func TestFindResults(t *testing.T) {
rs = FindResults(&rule.WhitelistRule, "my/file", "this has no rule violations", 1)
assert.Len(t, rs, 0)

// inline-ignoring is handled in Parser.generateFileViolations, not FindResults
rs = FindResults(&rule.WhitelistRule, "my/file", "this has the term whitelist #wokeignore:rule=whitelist", 1)
assert.Len(t, rs, 0)
assert.Len(t, rs, 1)
}

func TestLineResult_MarshalJSON(t *testing.T) {
Expand Down

0 comments on commit e33a005

Please sign in to comment.