Skip to content

Commit

Permalink
Fix _NegateMatcher bug in grepper
Browse files Browse the repository at this point in the history
Unlike positive matchers that is valid on first match, the exit criteria
is met when all matchers are matched.

Negate matchers must be tested with the whole file and none of negate
matchers can be matched. So it's different logic than testing with
finder. Because finder only test with one line of file path, grepper
needs the whole file.
  • Loading branch information
iwat committed May 11, 2017
1 parent 9538e80 commit 69d9b69
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 13 deletions.
6 changes: 3 additions & 3 deletions internal/finder.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,11 @@ func newFinder(queries ...string) *_Finder {
finder := &_Finder{
matchers: matchers,
walker: make(chan string),
done: make(chan bool),
done: make(chan bool, 1),
}

go func() {
defer close(finder.walker)
_ = filepath.Walk(".", func(path string, info os.FileInfo, err error) error {
if finder.shouldSkip(info.Name()) {
if info.IsDir() {
Expand Down Expand Up @@ -59,8 +60,6 @@ func newFinder(queries ...string) *_Finder {

return nil
})
close(finder.walker)
close(finder.done)
}()

return finder
Expand All @@ -72,6 +71,7 @@ func (f *_Finder) channel() <-chan string {

func (f *_Finder) reset() {
f.done <- true
close(f.done)
}

func (f *_Finder) shouldSkip(basematcher string) bool {
Expand Down
16 changes: 13 additions & 3 deletions internal/grepper.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,18 @@ func newGrepper() *_Grepper {
}

func (g *_Grepper) grep(file string, keywords ...string) (bool, error) {
matchers := make(map[string]_Matcher, len(keywords))
matchers := make(map[string]_Matcher)
negateMatchers := make(map[string]_Matcher)
for _, keyword := range keywords {
matcher, err := newMatcher(keyword)
if err != nil {
continue
}
matchers[keyword] = matcher
if _, ok := matcher.(*_NegateMatcher); ok {
negateMatchers[keyword] = matcher
} else {
matchers[keyword] = matcher
}
}

f, err := os.Open(file)
Expand All @@ -40,7 +45,12 @@ func (g *_Grepper) grep(file string, keywords ...string) (bool, error) {
delete(matchers, keyword)
}
}
if len(matchers) == 0 {
for _, negateMatcher := range negateMatchers {
if !negateMatcher.Matches(scanner.Text()) {
return false, nil
}
}
if len(matchers) == 0 && len(negateMatchers) == 0 {
break
}
}
Expand Down
15 changes: 8 additions & 7 deletions internal/grepper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,18 @@ package internal

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestGrep(t *testing.T) {
grepper := newGrepper()

match, err := grepper.grep("grepper_test.go", "package", "func", "NewGrepper")
if err != nil {
t.Fatal(err)
}
assert.Nil(t, err)
assert.True(t, match)

t.Log(match)
if !match {
t.Fatal("expected main.go to match")
}
match, err = grepper.grep("grepper_test.go", "package", "func", "-NewGrepper")
assert.Nil(t, err)
assert.False(t, match)
}
13 changes: 13 additions & 0 deletions internal/matcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

type _Matcher interface {
Matches(input string) bool
String() string
}

type _StringMatcher struct {
Expand Down Expand Up @@ -48,6 +49,10 @@ func (m _StringMatcher) Matches(input string) bool {
return strings.Contains(lower, m.pattern)
}

func (m _StringMatcher) String() string {
return m.pattern
}

type _RegexpMatcher struct {
pattern *regexp.Regexp
}
Expand All @@ -65,6 +70,10 @@ func (m _RegexpMatcher) Matches(input string) bool {
return m.pattern.FindStringSubmatch(input) != nil
}

func (m *_RegexpMatcher) String() string {
return m.pattern.String()
}

type _NegateMatcher struct {
original _Matcher
}
Expand All @@ -76,3 +85,7 @@ func newNegateMatcher(matcher _Matcher) *_NegateMatcher {
func (m _NegateMatcher) Matches(input string) bool {
return !m.original.Matches(input)
}

func (m _NegateMatcher) String() string {
return "-" + m.original.String()
}

0 comments on commit 69d9b69

Please sign in to comment.