diff --git a/hound.go b/hound.go index 7786f13..26e27a5 100644 --- a/hound.go +++ b/hound.go @@ -1,9 +1,6 @@ package main import ( - "errors" - "fmt" - "github.com/ezekg/git-hound/Godeps/_workspace/src/github.com/fatih/color" "github.com/ezekg/git-hound/Godeps/_workspace/src/gopkg.in/yaml.v2" "github.com/ezekg/git-hound/Godeps/_workspace/src/sourcegraph.com/sourcegraph/go-diff/diff" "io/ioutil" @@ -14,21 +11,21 @@ import ( // A Hound contains the local configuration filename and all regexp patterns // used for sniffing git-diffs. type Hound struct { - Config string Fails []string `yaml:"fail"` Warns []string `yaml:"warn"` Skips []string `yaml:"skip"` + config string } // New initializes a new Hound instance by parsing regexp patterns from a // local configuration file and returns a status bool. func (h *Hound) New() bool { - config, err := h.LoadConfig() + config, err := h.loadConfig() if err != nil { return false } - err = h.Parse(config) + err = h.parse(config) if err != nil { return false } @@ -36,53 +33,59 @@ func (h *Hound) New() bool { return true } -// LoadConfig reads a local configuration file of regexp patterns and returns -// the contents of the file. -func (h *Hound) LoadConfig() ([]byte, error) { - filename, _ := filepath.Abs(h.Config) - return ioutil.ReadFile(filename) -} - -// Parse parses a configuration byte array and returns an error. -func (h *Hound) Parse(data []byte) error { - return yaml.Unmarshal(data, h) -} - // 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, warnc chan string, failc chan error, donec chan bool) { - defer func() { donec <- true }() +func (h *Hound) Sniff(fileName string, hunk *diff.Hunk, smells chan<- smell, done chan<- bool) { + defer func() { done <- true }() - r1, _ := regexp.Compile(`^\w+\/`) - fileName = r1.ReplaceAllString(fileName, "") - if _, ok := h.MatchPatterns(h.Skips, []byte(fileName)); ok { + rxFileName, _ := regexp.Compile(`^\w+\/`) + fileName = rxFileName.ReplaceAllString(fileName, "") + if _, ok := h.matchPatterns(h.Skips, []byte(fileName)); ok { return } - r2, _ := regexp.Compile(`(?m)^\+\s*(.+)$`) - matches := r2.FindAllSubmatch(hunk.Body, -1) + rxModLines, _ := regexp.Compile(`(?m)^\+\s*(.+)$`) + matches := rxModLines.FindAllSubmatch(hunk.Body, -1) for _, match := range matches { line := match[1] - 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)) - warnc <- msg + if pattern, warned := h.matchPatterns(h.Warns, line); warned { + smells <- smell{ + pattern: pattern, + fileName: fileName, + line: line, + lineNum: hunk.NewStartLine, + severity: 1, + } } - 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)) - failc <- errors.New(msg) + if pattern, failed := h.matchPatterns(h.Fails, line); failed { + smells <- smell{ + pattern: pattern, + fileName: fileName, + line: line, + lineNum: hunk.NewStartLine, + severity: 2, + } } } } -// Match matches a byte array against a regexp pattern and returns a bool. -func (h *Hound) Match(pattern string, subject []byte) bool { +// loadConfig reads a local configuration file of regexp patterns and returns +// the contents of the file. +func (h *Hound) loadConfig() ([]byte, error) { + filename, _ := filepath.Abs(h.config) + return ioutil.ReadFile(filename) +} + +// parse parses a configuration byte array and returns an error. +func (h *Hound) parse(config []byte) error { + return yaml.Unmarshal(config, h) +} + +// match matches a byte array against a regexp pattern and returns a bool. +func (h *Hound) match(pattern string, subject []byte) bool { r, err := regexp.Compile(pattern) if err != nil { panic(err) @@ -91,11 +94,11 @@ func (h *Hound) Match(pattern string, subject []byte) bool { return r.Match(subject) } -// MatchPatterns matches a byte array against an array of regexp patterns and +// matchPatterns matches a byte array against an array of regexp patterns and // returns the matched pattern and a bool. -func (h *Hound) MatchPatterns(patterns []string, subject []byte) (string, bool) { +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 index 4c88de5..8def103 100644 --- a/hound_test.go +++ b/hound_test.go @@ -16,133 +16,199 @@ skip: - '\.test$' `) - if err := hound.Parse(config); err != nil { + if err := hound.parse(config); err != nil { t.Fatalf("Should parse - %s", err) } - // Should fail + // Should fail with a warning { - fileName, hunk := getDiff(`diff --git a/test1.go b/test1.go + fileName, hunk := getDiff(t, `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 ++Username: something-secret`) + warnCount := 0 + failCount := 0 + + smells := make(chan smell) + done := make(chan bool) + + go hound.Sniff(fileName, hunk, smells, done) + + for c := 0; c < 1; { + select { + case s := <-smells: + switch s.severity { + case 1: + warnCount++ + case 2: + failCount++ + default: + t.Fatalf("Should receive smell") + } + case <-done: + c++ + } + } + + if warnCount == 0 { + t.Fatalf("Should warn") + } + + if failCount == 0 { + t.Fatalf("Should fail") + } + } + + // Should fail + { + fileName, hunk := getDiff(t, `diff --git a/test2.go b/test2.go +index 000000..000000 000000 +--- a/test2.go ++++ b/test2.go +@@ -1,2 +3,4 @@ +Password: something-secret`) - warnc := make(chan string) - failc := make(chan error) - donec := make(chan bool) + smells := make(chan smell) + done := make(chan bool) - go hound.Sniff(fileName, hunk, warnc, failc, donec) + go hound.Sniff(fileName, hunk, smells, done) select { - case <-failc: - break - case <-warnc: - t.Fatalf("Should not warn") - case <-donec: - t.Fatalf("Should receive message") + case s := <-smells: + switch s.severity { + case 1: + t.Fatalf("Should not warn") + case 2: + t.Logf("Did fail") + default: + t.Fatalf("Should fail") + } + case <-done: + t.Fatalf("Should receive smell") } } // Should pass but output warning { - fileName, hunk := getDiff(`diff --git a/test2.go b/test2.go + fileName, hunk := getDiff(t, `diff --git a/test3.go b/test3.go index 000000..000000 000000 ---- a/test2.go -+++ b/test2.go +--- a/test3.go ++++ b/test3.go @@ -1,2 +3,4 @@ +Username: something-secret`) - warnc := make(chan string) - failc := make(chan error) - donec := make(chan bool) + smells := make(chan smell) + done := make(chan bool) - go hound.Sniff(fileName, hunk, warnc, failc, donec) + go hound.Sniff(fileName, hunk, smells, done) select { - case <-failc: - t.Fatalf("Should not fail") - case <-warnc: - break - case <-donec: - t.Fatalf("Should receive message") + case s := <-smells: + switch s.severity { + case 1: + t.Logf("Did warn") + case 2: + t.Fatalf("Should not fail") + default: + t.Fatalf("Should warn") + } + case <-done: + t.Fatalf("Should receive smell") } } // Should pass { - fileName, hunk := getDiff(`diff --git a/test3.go b/test3.go + fileName, hunk := getDiff(t, `diff --git a/test4.go b/test4.go index 000000..000000 000000 ---- a/test3.go -+++ b/test3.go +--- a/test4.go ++++ b/test4.go @@ -1,2 +3,4 @@ +Something that is okay to commit`) - warnc := make(chan string) - failc := make(chan error) - donec := make(chan bool) + smells := make(chan smell) + done := make(chan bool) - go hound.Sniff(fileName, hunk, warnc, failc, donec) + go hound.Sniff(fileName, hunk, smells, done) select { - case <-failc: - t.Fatal("Should not fail") - case <-warnc: - t.Fatal("Should not warn") - case <-donec: - break + case s := <-smells: + switch s.severity { + case 1: + t.Fatalf("Should not warn") + case 2: + t.Fatalf("Should not fail") + default: + t.Fatalf("Should not receive smell") + } + case <-done: + t.Logf("Did pass") } } // Should only pay attention to added lines and pass { - fileName, hunk := getDiff(`diff --git a/test4.go b/test4.go + fileName, hunk := getDiff(t, `diff --git a/test5.go b/test5.go index 000000..000000 000000 ---- a/test4.go -+++ b/test4.go +--- a/test5.go ++++ b/test5.go @@ -1,2 +3,4 @@ -Password: something-secret`) - warnc := make(chan string) - failc := make(chan error) - donec := make(chan bool) + smells := make(chan smell) + done := make(chan bool) - go hound.Sniff(fileName, hunk, warnc, failc, donec) + go hound.Sniff(fileName, hunk, smells, done) select { - case <-failc: - t.Fatal("Should not fail") - case <-warnc: - t.Fatal("Should not warn") - case <-donec: - break + case s := <-smells: + switch s.severity { + case 1: + t.Fatalf("Should not warn") + case 2: + t.Fatalf("Should not fail") + default: + t.Fatalf("Should not receive smell") + } + case <-done: + t.Logf("Did pass") } } // Should skip even with failures { - fileName, hunk := getDiff(`diff --git a/test4.test b/test4.test + fileName, hunk := getDiff(t, `diff --git a/test6.test b/test6.test index 000000..000000 000000 ---- a/test4.test -+++ b/test4.test +--- a/test6.test ++++ b/test6.test @@ -1,2 +3,4 @@ +Password: something-secret`) - warnc := make(chan string) - failc := make(chan error) - donec := make(chan bool) + smells := make(chan smell) + done := make(chan bool) - go hound.Sniff(fileName, hunk, warnc, failc, donec) + go hound.Sniff(fileName, hunk, smells, done) select { - case <-failc: - t.Fatal("Should not fail") - case <-warnc: - t.Fatal("Should not warn") - case <-donec: - break + case s := <-smells: + switch s.severity { + case 1: + t.Fatalf("Should not warn") + case 2: + t.Fatalf("Should not fail") + default: + t.Fatalf("Should not receive message") + } + case <-done: + t.Logf("Did pass") } } } -func getDiff(diffContents string) (string, *diff.Hunk) { - fileDiff, _ := diff.ParseFileDiff([]byte(diffContents)) +func getDiff(t *testing.T, diffContents string) (string, *diff.Hunk) { + fileDiff, err := diff.ParseFileDiff([]byte(diffContents)) + if err != nil { + t.Fatalf("Should parse fileDiff") + } + fileName := fileDiff.NewName for _, hunk := range fileDiff.GetHunks() { diff --git a/main.go b/main.go index 7cf5caf..1baac3c 100644 --- a/main.go +++ b/main.go @@ -25,23 +25,22 @@ func main() { color.NoColor = true } - hound := &Hound{Config: *config} - git := &Command{Bin: *bin} + hound := &Hound{config: *config} + git := &Command{bin: *bin} if ok := hound.New(); ok { out, _ := git.Exec("diff", "-U0", "--staged") fileDiffs, err := diff.ParseMultiFileDiff([]byte(out)) if err != nil { - fmt.Print(err) + color.Red(fmt.Sprintf("%s\n", err)) os.Exit(1) } + severeSmellCount := 0 hunkCount := 0 - failCount := 0 - warnc := make(chan string) - failc := make(chan error) - donec := make(chan bool) + smells := make(chan smell) + done := make(chan bool) for _, fileDiff := range fileDiffs { fileName := fileDiff.NewName @@ -51,11 +50,11 @@ func main() { go func(hunk *diff.Hunk) { defer func() { if r := recover(); r != nil { - fmt.Print(color.RedString(fmt.Sprintf("%s\n", r))) + color.Red(fmt.Sprintf("%s\n", r)) os.Exit(1) } }() - hound.Sniff(fileName, hunk, warnc, failc, donec) + hound.Sniff(fileName, hunk, smells, done) }(hunk) hunkCount++ } @@ -63,18 +62,26 @@ func main() { for c := 0; c < hunkCount; { select { - case msg := <-warnc: - fmt.Print(msg) - case err := <-failc: - fmt.Print(err) - failCount++ - case <-donec: + case s := <-smells: + if s.severity > 1 { + severeSmellCount++ + } + + switch s.severity { + case 1: + color.Yellow(fmt.Sprintf("warning: %s\n", s.String())) + case 2: + color.Red(fmt.Sprintf("failure: %s\n", s.String())) + default: + color.Red(fmt.Sprintf("error: unknown severity given - %d\n", s.severity)) + } + case <-done: c++ } } - if failCount > 0 { - fmt.Printf("%d failures detected - please fix them before you can commit.\n", failCount) + if severeSmellCount > 0 { + fmt.Printf("%d severe smell(s) detected - please fix them before you can commit\n", severeSmellCount) os.Exit(1) } } diff --git a/smell.go b/smell.go new file mode 100644 index 0000000..113b654 --- /dev/null +++ b/smell.go @@ -0,0 +1,21 @@ +package main + +import ( + "fmt" +) + +// A smell contains information about a code smell, such as line, filename, +// line number and severity. +type smell struct { + pattern string + fileName string + line []byte + lineNum int32 + severity int +} + +// String returns a string representation of the smell instance. +func (s *smell) String() string { + return fmt.Sprintf("pattern `%s` match found for `%s` starting at line %d in %s", + s.pattern, s.line, s.lineNum, s.fileName) +}