refactoring

ezekg · Nov 23, 2015 · 6eebdcf · 6eebdcf
1 parent 049041f
commit 6eebdcf
Show file tree

Hide file tree

Showing 3 changed files with 97 additions and 80 deletions.
diff --git a/hound.go b/hound.go
@@ -22,18 +22,18 @@ type Hound struct {
 
 // New initializes a new Hound instance by parsing regexp patterns from a
 // local configuration file to prepare for diff sniffing
-func (h *Hound) New() (bool, error) {
+func (h *Hound) New() bool {
 	config, err := h.LoadConfig()
 	if err != nil {
-		return false, err
+		return false
 	}
 
 	err = h.Parse(config)
 	if err != nil {
-		return false, err
+		return false
 	}
 
-	return true, nil
+	return true
 }
 
 // LoadConfig reads a local configuration file of regexp patterns and returns
@@ -50,11 +50,11 @@ func (h *Hound) Parse(data []byte) error {
 
 // Sniff matches the passed git-diff hunk against all regexp patterns that
 // were parsed from the local configuration
-func (h *Hound) Sniff(fileName string, hunk *diff.Hunk) error {
+func (h *Hound) Sniff(fileName string, hunk *diff.Hunk, warnc chan string, failc chan error, donec chan bool) {
 	r1, _ := regexp.Compile(`^\w+\/`)
 	fileName = r1.ReplaceAllString(fileName, "")
 	if _, ok := h.MatchPatterns(h.Skips, []byte(fileName)); ok {
-		return nil
+		return
 	}
 
 	r2, _ := regexp.Compile(`(?m)^\+\s*(.+)$`)
@@ -63,40 +63,39 @@ func (h *Hound) Sniff(fileName string, hunk *diff.Hunk) error {
 	for _, match := range matches {
 		line := match[1]
 
-		if pattern, ok := h.MatchPatterns(h.Warns, line); ok {
-			message := color.YellowString(fmt.Sprintf("Warning: pattern `%s` match found for `%s` starting at line %d in %s\n",
+		if pattern, warned := h.MatchPatterns(h.Warns, line); warned {
+			msg := color.YellowString(fmt.Sprintf(
+				"Warning: pattern `%s` match found for `%s` starting at line %d in %s\n",
 				pattern, line, hunk.NewStartLine, fileName))
-			fmt.Print(message)
+			warnc <- msg
 		}
 
-		if pattern, ok := h.MatchPatterns(h.Fails, line); ok {
-			err := color.RedString(fmt.Sprintf("Failure: pattern `%s` match found for `%s` starting at line %d in %s\n",
+		if pattern, failed := h.MatchPatterns(h.Fails, line); failed {
+			msg := color.RedString(fmt.Sprintf(
+				"Failure: pattern `%s` match found for `%s` starting at line %d in %s\n",
 				pattern, line, hunk.NewStartLine, fileName))
-			return errors.New(err)
+			failc <- errors.New(msg)
+			return
 		}
 	}
 
-	return nil
+	donec <- true
 }
 
 // Match matches a byte array against a regexp pattern
-func (h *Hound) Match(pattern string, subject []byte) (bool, error) {
+func (h *Hound) Match(pattern string, subject []byte) bool {
 	r, err := regexp.Compile(pattern)
 	if err != nil {
-		return false, err
+		return false
 	}
 
-	if r.Match(subject) {
-		return true, nil
-	}
-
-	return false, nil
+	return r.Match(subject)
 }
 
 // MatchPatterns matches a byte array against an array of regexp patterns
 func (h *Hound) MatchPatterns(patterns []string, subject []byte) (string, bool) {
 	for _, pattern := range patterns {
-		if match, _ := h.Match(pattern, subject); match {
+		if match := h.Match(pattern, subject); match {
 			return pattern, true
 		}
 	}

diff --git a/hound_test.go b/hound_test.go
@@ -2,83 +2,107 @@ package main
 
 import (
 	"github.com/ezekg/git-hound/Godeps/_workspace/src/sourcegraph.com/sourcegraph/go-diff/diff"
-	"io/ioutil"
-	"os"
 	"testing"
 )
 
 func TestDiffs(t *testing.T) {
-	rescueStdout := os.Stdout
 	var fileName string
 	var hunk *diff.Hunk
 
-	h := &Hound{}
-	c := []byte(`
+	warnc := make(chan string, 1)
+	failc := make(chan error, 1)
+	donec := make(chan bool, 1)
+
+	hound := &Hound{}
+	config := []byte(`
 warn:
   - '(?i)user(name)?\W*[:=,]\W*.+$'
 fail:
   - '(?i)pass(word)?\W*[:=,]\W*.+$'
 `)
 
-	if err := h.Parse(c); err != nil {
+	if err := hound.Parse(config); err != nil {
 		t.Fatalf("Should parse - %s", err)
 	}
 
 	// Should fail
-	fileName, hunk = getDiff(`diff --git a/test1.go b/test1.go
+	{
+		fileName, hunk = getDiff(`diff --git a/test1.go b/test1.go
 index 000000..000000 000000
 --- a/test1.go
 +++ b/test1.go
 @@ -1,2 +3,4 @@
 +Password: something-secret`)
-	if err := h.Sniff(fileName, hunk); err == nil {
-		t.Fatalf("Should fail - %s", err)
+		go hound.Sniff(fileName, hunk, warnc, failc, donec)
+
+		select {
+		case <-failc:
+			break
+		case <-warnc:
+			t.Fatalf("Should not warn")
+		case <-donec:
+			t.Fatalf("Should receive message")
+		}
 	}
 
 	// Should pass but output warning
-	fileName, hunk = getDiff(`diff --git a/test2.go b/test2.go
+	{
+		fileName, hunk = getDiff(`diff --git a/test2.go b/test2.go
 index 000000..000000 000000
 --- a/test2.go
 +++ b/test2.go
 @@ -1,2 +3,4 @@
 +Username: something-secret`)
-	r, w, _ := os.Pipe()
-	os.Stdout = w
+		go hound.Sniff(fileName, hunk, warnc, failc, donec)
 
-	if err := h.Sniff(fileName, hunk); err != nil {
-		w.Close()
-		os.Stdout = rescueStdout
-		t.Fatalf("Should pass - %s", err)
-	}
-
-	w.Close()
-	out, _ := ioutil.ReadAll(r)
-	os.Stdout = rescueStdout
-
-	if len(out) <= 0 {
-		t.Fatalf("Should warn - %s", out)
+		select {
+		case <-failc:
+			t.Fatalf("Should not fail")
+		case <-warnc:
+			break
+		case <-donec:
+			t.Fatalf("Should receive message")
+		}
 	}
 
 	// Should pass
-	fileName, hunk = getDiff(`diff --git a/test3.go b/test3.go
+	{
+		fileName, hunk = getDiff(`diff --git a/test3.go b/test3.go
 index 000000..000000 000000
 --- a/test3.go
 +++ b/test3.go
 @@ -1,2 +3,4 @@
 +Something that is okay to commit`)
-	if err := h.Sniff(fileName, hunk); err != nil {
-		t.Fatalf("Should pass - %s", err)
+		go hound.Sniff(fileName, hunk, warnc, failc, donec)
+
+		select {
+		case <-failc:
+			t.Fatal("Should not fail")
+		case <-warnc:
+			t.Fatal("Should not warn")
+		case <-donec:
+			break
+		}
 	}
 
 	// Should only pay attention to added lines and pass
-	fileName, hunk = getDiff(`diff --git a/test4.go b/test4.go
+	{
+		fileName, hunk = getDiff(`diff --git a/test4.go b/test4.go
 index 000000..000000 000000
 --- a/test4.go
 +++ b/test4.go
 @@ -1,2 +3,4 @@
 -Password: something-secret`)
-	if err := h.Sniff(fileName, hunk); err != nil {
-		t.Fatalf("Should pass - %s", err)
+		go hound.Sniff(fileName, hunk, warnc, failc, donec)
+
+		select {
+		case <-failc:
+			t.Fatal("Should not fail")
+		case <-warnc:
+			t.Fatal("Should not warn")
+		case <-donec:
+			break
+		}
 	}
 }
 

diff --git a/main.go b/main.go
@@ -10,16 +10,15 @@ import (
 	"github.com/ezekg/git-hound/Godeps/_workspace/src/github.com/fatih/color"
 	"github.com/ezekg/git-hound/Godeps/_workspace/src/sourcegraph.com/sourcegraph/go-diff/diff"
 	"os"
-	"sync"
 )
 
-func main() {
-	var (
-		noColor = flag.Bool("no-color", false, "Disable color output")
-		config  = flag.String("config", ".githound.yml", "Hound config file")
-		bin     = flag.String("bin", "git", "Executable binary to use for git command")
-	)
+var (
+	noColor = flag.Bool("no-color", false, "Disable color output")
+	config  = flag.String("config", ".githound.yml", "Hound config file")
+	bin     = flag.String("bin", "git", "Executable binary to use for git command")
+)
 
+func main() {
 	flag.Parse()
 
 	if *noColor {
@@ -29,45 +28,40 @@ func main() {
 	hound := &Hound{Config: *config}
 	git := &Command{Bin: *bin}
 
-	if ok, _ := hound.New(); ok {
+	if ok := hound.New(); ok {
 		out, _ := git.Exec("diff", "-U0", "--staged")
 		fileDiffs, err := diff.ParseMultiFileDiff([]byte(out))
 		if err != nil {
 			fmt.Print(err)
 			os.Exit(1)
 		}
 
-		errs := make(chan error)
-		var wg sync.WaitGroup
-
-		sniff := func(fileName string, hunk *diff.Hunk) {
-			errs <- func() error {
-				defer wg.Done()
-				return hound.Sniff(fileName, hunk)
-			}()
-		}
+		hunkCount := 0
+		warnc := make(chan string)
+		failc := make(chan error)
+		donec := make(chan bool)
 
 		for _, fileDiff := range fileDiffs {
 			fileName := fileDiff.NewName
 			hunks := fileDiff.GetHunks()
 
-			wg.Add(len(hunks))
-
 			for _, hunk := range hunks {
-				go sniff(fileName, hunk)
+				go hound.Sniff(fileName, hunk, warnc, failc, donec)
+				hunkCount++
 			}
 		}
 
-		go func() {
-			wg.Wait()
-
-			for err := range errs {
-				if err != nil {
-					fmt.Print(err)
-					os.Exit(1)
-				}
+		for c := 0; c < hunkCount; {
+			select {
+			case msg := <-warnc:
+				fmt.Print(msg)
+			case err := <-failc:
+				fmt.Print(err)
+				os.Exit(1)
+			case <-donec:
+				c++
 			}
-		}()
+		}
 	}
 
 	out, code := git.Exec(flag.Args()...)