From 69d9b692c97b57064674759ed7f5e0f7fe985a6d Mon Sep 17 00:00:00 2001
From: iwat <iwatth@gmail.com>
Date: Thu, 11 May 2017 11:13:35 +0700
Subject: [PATCH] Fix _NegateMatcher bug in grepper

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.
---
 internal/finder.go       |  6 +++---
 internal/grepper.go      | 16 +++++++++++++---
 internal/grepper_test.go | 15 ++++++++-------
 internal/matcher.go      | 13 +++++++++++++
 4 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/internal/finder.go b/internal/finder.go
index ccec417..c5baca2 100644
--- a/internal/finder.go
+++ b/internal/finder.go
@@ -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() {
@@ -59,8 +60,6 @@ func newFinder(queries ...string) *_Finder {
 
 			return nil
 		})
-		close(finder.walker)
-		close(finder.done)
 	}()
 
 	return finder
@@ -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 {
diff --git a/internal/grepper.go b/internal/grepper.go
index 9a7451c..6527b55 100644
--- a/internal/grepper.go
+++ b/internal/grepper.go
@@ -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)
@@ -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
 		}
 	}
diff --git a/internal/grepper_test.go b/internal/grepper_test.go
index 6347028..0d704c0 100644
--- a/internal/grepper_test.go
+++ b/internal/grepper_test.go
@@ -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)
 }
diff --git a/internal/matcher.go b/internal/matcher.go
index 9c2b0a8..2af35f4 100644
--- a/internal/matcher.go
+++ b/internal/matcher.go
@@ -8,6 +8,7 @@ import (
 
 type _Matcher interface {
 	Matches(input string) bool
+	String() string
 }
 
 type _StringMatcher struct {
@@ -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
 }
@@ -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
 }
@@ -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()
+}