Bug fixes (#11)

* `Do`: Return tuple `([]UnsatisfiedRule, error)`

* Add `UnsatisfiedRules.String()`

* `[]UnsatisfiedRule` to `UnsatisfiedRules`

Author: EthanThatOneKid

cli/main.go

@@ -1,5 +1,5 @@
 // Run:
-// git diff | go run cli/main.go
+// git diff | go run cli/main.go --verbose
 
 package main
 

difflint.go

@@ -3,6 +3,7 @@ package difflint
 import (
 	"fmt"
 	"io"
+	"os"
 	"path/filepath"
 	"strings"
 
@@ -49,10 +50,6 @@ type LintOptions struct {
 // TemplatesFromFile returns the directive templates for the given file type.
 func (o *LintOptions) TemplatesFromFile(file string) ([]string, error) {
 	fileType := strings.TrimPrefix(filepath.Ext(file), ".")
-	if fileType == "" {
-		return nil, errors.Errorf("file %q has no extension", file)
-	}
-
 	templateIndices, ok := o.FileExtMap[fileType]
 	if !ok {
 		templateIndices = []int{o.DefaultTemplate}
@@ -87,6 +84,7 @@ type UnsatisfiedRule struct {
 	UnsatisfiedTargets map[int]struct{}
 }
 
+// UnsatisfiedRules is a list of unsatisfied rules.
 type UnsatisfiedRules []UnsatisfiedRule
 
 // String returns a string representation of the unsatisfied rules.
@@ -122,6 +120,45 @@ type LintResult struct {
 	UnsatisfiedRules UnsatisfiedRules
 }
 
+// Walk walks the file tree rooted at root, calling callback for each file or
+// directory in the tree, including root.
+func Walk(root string, include []string, exclude []string, callback filepath.WalkFunc) error {
+	isHidden := func(path string) bool {
+		components := strings.Split(path, string(os.PathSeparator))
+		for _, component := range components {
+			if strings.HasPrefix(component, ".") && component != "." && component != ".." {
+				return true
+			}
+		}
+		return false
+	}
+
+	return filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+
+		if info.IsDir() {
+			return nil
+		}
+
+		if isHidden(path) {
+			return nil
+		}
+
+		included, err := Include(path, include, exclude)
+		if err != nil {
+			return err
+		}
+
+		if !included {
+			return nil
+		}
+
+		return callback(path, info, nil)
+	})
+}
+
 // Lint lints the given hunks against the given rules and returns the result.
 func Lint(o LintOptions) (*LintResult, error) {
 	// Parse the diff hunks.
@@ -131,18 +168,33 @@ func Lint(o LintOptions) (*LintResult, error) {
 	}
 
 	// Parse rules from hunks.
-	rulesMap, _, err := RulesMapFromHunks(hunks, o)
+	rulesMap, presentTargetsMap, err := RulesMapFromHunks(hunks, o)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to parse rules from hunks")
 	}
 
 	// Collect the rules that are not satisfied.
-	unsatisfiedRules, err := Check(rulesMap)
+	unsatisfiedRules, err := Check(rulesMap, presentTargetsMap)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to check rules")
 	}
 
-	return &LintResult{UnsatisfiedRules: unsatisfiedRules}, nil
+	// Filter out rules that are not intended to be included in the output.
+	var filteredUnsatisfiedRules UnsatisfiedRules
+	for _, rule := range unsatisfiedRules {
+		included, err := Include(rule.Rule.Hunk.File, o.Include, o.Exclude)
+		if err != nil {
+			return nil, errors.Wrap(err, "failed to check if file is included")
+		}
+
+		if !included {
+			continue
+		}
+
+		filteredUnsatisfiedRules = append(filteredUnsatisfiedRules, rule)
+	}
+
+	return &LintResult{UnsatisfiedRules: filteredUnsatisfiedRules}, nil
 }
 
 // TargetKey returns the key for the given target.
@@ -176,52 +228,35 @@ func isRelativeToCurrentDirectory(path string) bool {
 }
 
 // Check returns the list of unsatisfied rules for the given map of rules.
-func Check(rulesMap map[string][]Rule) (UnsatisfiedRules, error) {
+func Check(rulesMap map[string][]Rule, targetsMap map[string]struct{}) (UnsatisfiedRules, error) {
 	var unsatisfiedRules UnsatisfiedRules
-	for pathnameA, rulesA := range rulesMap {
-	outer:
-		for i, ruleA := range rulesA {
-			// Skip if ruleA is not present or if it has no targets.
-			if len(ruleA.Targets) == 0 || !ruleA.Present {
-				continue
-			}
 
-			for pathnameB, rulesB := range rulesMap {
-			inner:
-				for j, ruleB := range rulesB {
-					// Skip if both rules are present or if ruleA is the same as ruleB.
-					if ruleB.Present || (pathnameA == pathnameB && i == j) {
-						continue
-					}
-
-					// Given that ruleA is present and ruleB is not present, check if ruleA
-					// is satisfied by ruleB.
-					unsatisfiedTargetIndices := make(map[int]struct{})
-					for k, target := range ruleA.Targets {
-						// ruleA is satisfied by ruleB if ruleB matches a target of ruleA.
-						satisfied := target.ID == ruleB.ID && ((target.File == nil && pathnameA == pathnameB) || (*target.File == pathnameB))
-						if satisfied {
-							continue inner
-						}
-
-						// Otherwise, add the target index to the list of unsatisfied targets.
-						unsatisfiedTargetIndices[k] = struct{}{}
-					}
-
-					unsatisfiedRules = append(unsatisfiedRules, UnsatisfiedRule{
-						Rule:               ruleA,
-						UnsatisfiedTargets: unsatisfiedTargetIndices,
-					})
-					continue outer
+	// Check each rule.
+	for _, rules := range rulesMap {
+		for _, rule := range rules {
+			unsatisfiedTargets := make(map[int]struct{}, len(rule.Targets))
+			for i, target := range rule.Targets {
+				key := TargetKey(rule.Hunk.File, target)
+				if _, ok := targetsMap[key]; rule.Present != ok {
+					unsatisfiedTargets[i] = struct{}{}
 				}
 			}
+
+			// If there are unsatisfied targets, then the rule is unsatisfied.
+			if len(unsatisfiedTargets) > 0 {
+				unsatisfiedRules = append(unsatisfiedRules, UnsatisfiedRule{
+					Rule:               rule,
+					UnsatisfiedTargets: unsatisfiedTargets,
+				})
+			}
 		}
 	}
 
+	// Return the unordered list of unsatisfied rules.
 	return unsatisfiedRules, nil
 }
 
-// Entrypoint for the difflint command.
+// Do is the difflint command's entrypoint.
 func Do(r io.Reader, include, exclude []string, extMapPath string) (UnsatisfiedRules, error) {
 	// Parse options.
 	extMap := NewExtMap(extMapPath)
@@ -239,25 +274,6 @@ func Do(r io.Reader, include, exclude []string, extMapPath string) (UnsatisfiedR
 		return nil, errors.Wrap(err, "failed to lint hunks")
 	}
 
-	// If there are no unsatisfied rules, return nil.
-	if len(result.UnsatisfiedRules) == 0 {
-		return nil, nil
-	}
-
-	// Print the unsatisfied rules.
-	var included bool
-	for _, rule := range result.UnsatisfiedRules {
-		// Skip if the rule is not intended to be included in the output.
-		included, err = Include(rule.Hunk.File, include, exclude)
-		if err != nil {
-			return nil, errors.Wrap(err, "failed to check if file is included")
-		}
-
-		if !included {
-			continue
-		}
-	}
-
 	return result.UnsatisfiedRules, nil
 }
 

rules.go

@@ -3,7 +3,6 @@ package difflint
 import (
 	"log"
 	"os"
-	"sync"
 
 	"github.com/pkg/errors"
 )
@@ -34,9 +33,8 @@ type Rule struct {
 }
 
 // RulesMapFromHunks parses rules from the given hunks by file name and
-// returns the map of rules.
+// returns the map of rules and the set of all the target keys that are present.
 func RulesMapFromHunks(hunks []Hunk, options LintOptions) (map[string][]Rule, map[string]struct{}, error) {
-	// Separate hunks by file name and construct a set of all the target keys that exist.
 	targetsMap := make(map[string]struct{}, len(hunks))
 	rangesMap := make(map[string][]Range, len(hunks))
 	for _, hunk := range hunks {
@@ -49,87 +47,95 @@ func RulesMapFromHunks(hunks []Hunk, options LintOptions) (map[string][]Rule, ma
 		rangesMap[hunk.File] = []Range{hunk.Range}
 	}
 
-	// Populate rules map.
 	rulesMap := make(map[string][]Rule, len(hunks))
-	visited := make(map[string]struct{})
-	var wg sync.WaitGroup
-	for pathname, ranges := range rangesMap {
-		rules, err := RulesFromFile(pathname, ranges, visited, &wg, options)
+	err := Walk(".", nil, nil, func(file string, info os.FileInfo, err error) error {
 		if err != nil {
-			return nil, nil, err
+			return err
+		}
+
+		f, err := os.Open(file)
+		if err != nil {
+			return errors.Wrapf(err, "failed to open file %s", file)
+		}
+		defer f.Close()
+
+		templates, err := options.TemplatesFromFile(file)
+		if err != nil {
+			return errors.Wrapf(err, "failed to parse templates for file %s", file)
+		}
+
+		tokens, err := lex(f, lexOptions{file, templates})
+		if err != nil {
+			return errors.Wrapf(err, "failed to lex file %s", file)
+		}
+
+		rules, err := parseRules(file, tokens, rangesMap[file])
+		if err != nil {
+			return errors.Wrapf(err, "failed to parse rules for file %s", file)
 		}
 
 		for _, rule := range rules {
-			targetsMap[TargetKey(pathname, Target{ID: rule.ID})] = struct{}{}
+			if rule.Hunk.File != file {
+				continue
+			}
+
+			ranges, ok := rangesMap[file]
+			if !ok {
+				continue
+			}
+
+			for _, rng := range ranges {
+				if !Intersects(rule.Hunk.Range, rng) {
+					continue
+				}
+
+				key := TargetKey(file, Target{
+					File: &rule.Hunk.File,
+					ID:   rule.ID,
+				})
+				targetsMap[key] = struct{}{}
+			}
 		}
 
-		rulesMap[pathname] = rules
-	}
+		if len(rules) > 0 {
+			rulesMap[file] = rules
+		}
 
-	wg.Wait()
+		return nil
+	})
+	if err != nil {
+		return nil, nil, errors.Wrap(err, "failed to walk files")
+	}
 
 	return rulesMap, targetsMap, nil
 }
 
 // RulesFromFile parses rules from the given file and returns the list of rules.
-func RulesFromFile(pathname string, ranges []Range, visited map[string]struct{}, wg *sync.WaitGroup, options LintOptions) ([]Rule, error) {
-	visited[pathname] = struct{}{}
-
+func RulesFromFile(file string, ranges []Range, options LintOptions) ([]Rule, error) {
 	// Parse rules for the file.
-	log.Println("Parsing rules for file", pathname)
-	file, err := os.Open(pathname)
+	log.Println("parsing rules for file", file)
+	f, err := os.Open(file)
 	if err != nil {
-		return nil, errors.Wrapf(err, "failed to open file %s", pathname)
+		return nil, errors.Wrapf(err, "failed to open file %s", file)
 	}
 
-	defer file.Close()
+	defer f.Close()
 
-	templates, err := options.TemplatesFromFile(pathname)
+	templates, err := options.TemplatesFromFile(file)
 	if err != nil {
-		return nil, errors.Wrapf(err, "failed to parse templates for file %s", pathname)
+		return nil, errors.Wrapf(err, "failed to parse templates for file %s", file)
 	}
 
-	tokens, err := lex(file, lexOptions{
-		file:      pathname,
-		templates: templates,
-	})
+	tokens, err := lex(f, lexOptions{file, templates})
 	if err != nil {
-		return nil, errors.Wrapf(err, "failed to lex file %s", pathname)
+		return nil, errors.Wrapf(err, "failed to lex file %s", file)
 	}
 
-	rules, err := parseRules(pathname, tokens, ranges)
+	rules, err := parseRules(file, tokens, ranges)
 	if err != nil {
-		return nil, errors.Wrapf(err, "failed to parse rules for file %s", pathname)
-	}
-
-	var innerWg sync.WaitGroup // WaitGroup for inner goroutines
-	for _, rule := range rules {
-		for _, target := range rule.Targets {
-			if target.File == nil {
-				continue
-			}
-
-			innerWg.Add(1)
-			go func(pathname string) {
-				defer innerWg.Done()
-
-				if _, ok := visited[pathname]; ok {
-					return
-				}
-
-				moreRules, err := RulesFromFile(pathname, nil, visited, &innerWg, options)
-				if err != nil {
-					return
-				}
-
-				rules = append(rules, moreRules...)
-			}(*target.File)
-		}
+		return nil, errors.Wrapf(err, "failed to parse rules for file %s", file)
 	}
 
-	// Wait for all inner goroutines to complete before returning.
-	innerWg.Wait()
-
 	// Add rules to the map.
 	return rules, nil
 }