Check parent directories in Match and add git check-ignore conformance tests (#27)

* Check parent directories in Match and add git check-ignore conformance tests

Match now tests each parent directory of a path before the path itself,
so a file under an excluded directory is reported as ignored and a
negation cannot re-include it, matching git's per-directory traversal.

The implicit trailing ** and the /** rewrite are removed; matchSegments
takes a flag so a trailing ** must consume at least one segment. A
whole-segment run of two or more asterisks now compiles as **. Four
existing tests that asserted the opposite of git are corrected.

Fixes #25.
Fixes #26.

* Handle Git startup failures in conformance tests

* Add literal-suffix conformance case

Author: andrew

conformance_test.go

@@ -16,16 +16,16 @@ type segment struct {
 }
 
 type pattern struct {
-	segments      []segment
-	negate        bool
-	dirOnly       bool // trailing slash pattern or trailing /** pattern
-	hasConcrete   bool // has at least one non-** segment
-	anchored      bool
-	prefix        []string // directory scope segments for nested .gitignore
-	text          string   // original pattern text before compilation
-	source        string   // file path this pattern came from, empty for programmatic
-	line          int      // 1-based line number in source file
-	literalSuffix string   // fast-reject: last segment must end with this (e.g. ".log" from "*.log")
+	segments       []segment
+	negate         bool
+	dirOnly        bool // trailing slash pattern
+	tailDoubleStar bool // pattern ends in "/**"
+	anchored       bool
+	prefix         []string // directory scope segments for nested .gitignore
+	text           string   // original pattern text before compilation
+	source         string   // file path this pattern came from, empty for programmatic
+	line           int      // 1-based line number in source file
+	literalSuffix  string   // fast-reject: last segment must end with this (e.g. ".log" from "*.log")
 }
 
 // Matcher checks paths against gitignore rules collected from .gitignore files,
@@ -350,53 +350,72 @@ func (m *Matcher) MatchDetail(relPath string) MatchResult {
 	return m.matchDetail(relPath, isDir)
 }
 
+// match reports whether relPath is ignored. Git decides ignore status
+// while walking the tree and does not enter an excluded directory, so a
+// path is ignored if any of its parent directories is. That is checked
+// here by testing each proper prefix of the path as a directory before
+// testing the full path.
 func (m *Matcher) match(relPath string, isDir bool) bool {
 	pathSegs := strings.Split(relPath, "/")
-	lastSeg := pathSegs[len(pathSegs)-1]
-
-	for i := len(m.patterns) - 1; i >= 0; i-- {
-		p := &m.patterns[i]
-		if p.literalSuffix != "" && !p.dirOnly && !strings.HasSuffix(lastSeg, p.literalSuffix) {
-			continue
-		}
-		if !matchPattern(p, pathSegs, isDir) {
-			continue
+	for end := 1; end < len(pathSegs); end++ {
+		if idx := m.findMatch(pathSegs[:end], true); idx >= 0 && !m.patterns[idx].negate {
+			return true
 		}
-		return !p.negate
 	}
-	return false
+	idx := m.findMatch(pathSegs, isDir)
+	return idx >= 0 && !m.patterns[idx].negate
 }
 
 func (m *Matcher) matchDetail(relPath string, isDir bool) MatchResult {
 	pathSegs := strings.Split(relPath, "/")
-	lastSeg := pathSegs[len(pathSegs)-1]
+	for end := 1; end < len(pathSegs); end++ {
+		if idx := m.findMatch(pathSegs[:end], true); idx >= 0 && !m.patterns[idx].negate {
+			return m.resultFor(idx)
+		}
+	}
+	if idx := m.findMatch(pathSegs, isDir); idx >= 0 {
+		return m.resultFor(idx)
+	}
+	return MatchResult{}
+}
 
+// findMatch returns the index of the last pattern that matches pathSegs,
+// or -1 if none does. Patterns are scanned from last to first because
+// gitignore uses last-match-wins ordering.
+func (m *Matcher) findMatch(pathSegs []string, isDir bool) int {
+	lastSeg := pathSegs[len(pathSegs)-1]
 	for i := len(m.patterns) - 1; i >= 0; i-- {
 		p := &m.patterns[i]
-		if p.literalSuffix != "" && !p.dirOnly && !strings.HasSuffix(lastSeg, p.literalSuffix) {
-			continue
-		}
-		if !matchPattern(p, pathSegs, isDir) {
+		if p.literalSuffix != "" && !strings.HasSuffix(lastSeg, p.literalSuffix) {
 			continue
 		}
-		return MatchResult{
-			Ignored: !p.negate,
-			Matched: true,
-			Pattern: p.text,
-			Source:  p.source,
-			Line:    p.line,
-			Negate:  p.negate,
+		if matchPattern(p, pathSegs, isDir) {
+			return i
 		}
 	}
-	return MatchResult{}
+	return -1
+}
+
+func (m *Matcher) resultFor(idx int) MatchResult {
+	p := &m.patterns[idx]
+	return MatchResult{
+		Ignored: !p.negate,
+		Matched: true,
+		Pattern: p.text,
+		Source:  p.source,
+		Line:    p.line,
+		Negate:  p.negate,
+	}
 }
 
 // matchPattern checks whether pathSegs matches the compiled pattern,
 // including the directory prefix scope and dirOnly handling.
 func matchPattern(p *pattern, pathSegs []string, isDir bool) bool {
 	segs := pathSegs
 	if n := len(p.prefix); n > 0 {
-		if len(segs) < n {
+		// Rules from a nested .gitignore apply only to entries strictly
+		// inside that directory, never to the directory itself.
+		if len(segs) <= n {
 			return false
 		}
 		for i, ps := range p.prefix {
@@ -406,34 +425,10 @@ func matchPattern(p *pattern, pathSegs []string, isDir bool) bool {
 		}
 		segs = segs[n:]
 	}
-
-	if p.dirOnly {
-		// Dir-only patterns (trailing slash): match the directory itself,
-		// or match descendants (files/dirs under the matched directory).
-		if matchSegments(p.segments, segs) {
-			// A non-dir path may still be under a matched directory, so let
-			// exclusions fall through; negations don't inherit downwards.
-			if isDir || p.negate {
-				return isDir
-			}
-		}
-		// Only do descendant matching when the pattern identifies a specific
-		// directory (has at least one non-** segment). Pure ** patterns like
-		// "**/" only match directory paths directly.
-		if !p.hasConcrete {
-			return false
-		}
-		// Check if the path is a descendant of a matched directory by trying
-		// the pattern against every prefix of the path segments.
-		for end := len(segs) - 1; end >= 1; end-- {
-			if matchSegments(p.segments, segs[:end]) {
-				return true
-			}
-		}
+	if p.dirOnly && !isDir {
 		return false
 	}
-
-	return matchSegments(p.segments, segs)
+	return matchSegments(p.segments, segs, p.tailDoubleStar)
 }
 
 func (m *Matcher) addPatterns(data []byte, dir, source string) {
@@ -516,31 +511,23 @@ func compilePattern(line, dir string) (pattern, string) {
 		}
 	}
 
+	// A pattern ending "/**" matches everything inside the named directory
+	// but not the directory itself. Record that here so matchSegments can
+	// require the trailing ** to consume at least one path segment. Git
+	// treats any run of two or more asterisks as ** when it forms a whole
+	// segment, so "/***" and beyond count too.
+	if i := strings.LastIndexByte(line, '/'); i >= 0 {
+		p.tailDoubleStar = allStars(line[i+1:])
+	}
+
 	segs, anchored := buildSegments(line, hasLeadingSlash)
 	p.anchored = anchored
 
 	if msg := validateSegmentBrackets(segs); msg != "" {
 		return pattern{}, msg
 	}
 
-	// Trailing /** means "match directory and its contents, not files with the
-	// same name". In git, "data/**" matches data/ and data/file but not data
-	// (as a file). This is equivalent to dirOnly semantics, so strip the
-	// trailing ** and set dirOnly.
-	if !p.dirOnly && len(segs) >= 2 && segs[len(segs)-1].doubleStar {
-		segs = segs[:len(segs)-1]
-		p.dirOnly = true
-	}
-
-	segs = appendTrailingDoubleStar(segs, p.dirOnly)
-
 	p.segments = segs
-	for _, s := range segs {
-		if !s.doubleStar {
-			p.hasConcrete = true
-			break
-		}
-	}
 	p.literalSuffix = extractLiteralSuffix(segs)
 	return p, ""
 }
@@ -559,7 +546,7 @@ func buildSegments(line string, hasLeadingSlash bool) ([]segment, bool) {
 	}
 
 	for _, raw := range rawSegs {
-		if raw == "**" {
+		if allStars(raw) {
 			segs = append(segs, segment{doubleStar: true})
 		} else {
 			segs = append(segs, segment{raw: raw})
@@ -576,6 +563,20 @@ func buildSegments(line string, hasLeadingSlash bool) ([]segment, bool) {
 	return collapsed, anchored
 }
 
+// allStars reports whether s consists of two or more '*' bytes and nothing
+// else. Git's wildmatch treats such a segment the same as **.
+func allStars(s string) bool {
+	if len(s) < 2 {
+		return false
+	}
+	for i := 0; i < len(s); i++ {
+		if s[i] != '*' {
+			return false
+		}
+	}
+	return true
+}
+
 // validateSegmentBrackets checks bracket expressions in all concrete segments.
 func validateSegmentBrackets(segs []segment) string {
 	for _, seg := range segs {
@@ -589,15 +590,6 @@ func validateSegmentBrackets(segs []segment) string {
 	return ""
 }
 
-// appendTrailingDoubleStar adds an implicit ** at the end for non-dir-only
-// patterns so that matching "foo" also matches "foo/anything".
-func appendTrailingDoubleStar(segs []segment, dirOnly bool) []segment {
-	if !dirOnly && (len(segs) == 0 || !segs[len(segs)-1].doubleStar) {
-		segs = append(segs, segment{doubleStar: true})
-	}
-	return segs
-}
-
 // extractLiteralSuffix finds the literal trailing portion of the last concrete
 // segment, for fast rejection. For example, "*.log" yields ".log", "test_*.go"
 // yields ".go". Only extracts a suffix when the segment is a simple star-prefix
@@ -628,10 +620,12 @@ func extractLiteralSuffix(segs []segment) string {
 		return ""
 	}
 
-	// Bail if the suffix contains wildcards, brackets, or escapes.
+	// Bail if the suffix contains wildcards, brackets, or escapes. A ']'
+	// here means the '*' found above was inside a bracket expression and
+	// is literal, so the suffix boundary is wrong.
 	for i := 0; i < len(suffix); i++ {
 		switch suffix[i] {
-		case '*', '?', '[', '\\':
+		case '*', '?', '[', ']', '\\':
 			return ""
 		}
 	}

gitignore.go

@@ -745,16 +745,18 @@ func TestMatchNegateAnchored(t *testing.T) {
 func TestMatchDoubleStarSlash(t *testing.T) {
 	m := setupMatcher(t, "**/\n")
 
-	// **/ matches any directory
+	// **/ matches any directory, and therefore any path under a
+	// directory, since git does not enter an excluded directory.
 	shouldMatch := []string{
 		"a/",
 		"a/b/",
 		"deep/nested/dir/",
+		"a/b",             // file inside excluded directory a/
+		"deep/nested/dir", // file at any depth
 	}
 	shouldNotMatch := []string{
-		"a",   // file, not directory
-		"b",   // file
-		"a/b", // file inside directory
+		"a", // top-level file
+		"b", // top-level file
 	}
 
 	for _, path := range shouldMatch {
@@ -780,11 +782,9 @@ func TestMatchEscapedCharacters(t *testing.T) {
 	}
 }
 
-// TestMatchAgainstGitCheckIgnore verifies our implementation matches
-// git check-ignore for a variety of patterns. Each subtest creates a
-// real git repo, writes a .gitignore, and compares our result against
-// git's actual output.
-func TestMatchAgainstGitCheckIgnore(t *testing.T) {
+// TestMatchTable exercises a variety of pattern shapes against fixed
+// expectations. See TestConformance for direct comparison with git.
+func TestMatchTable(t *testing.T) {
 	tests := []struct {
 		name     string
 		patterns string
@@ -994,21 +994,19 @@ func TestMatchDirOnlyFrotz(t *testing.T) {
 }
 
 func TestMatchCannotReincludeUnderExcludedParent(t *testing.T) {
-	// From docs: "It is not possible to re-include a file if a parent directory
-	// of that file is excluded."
-	// Since our callers SkipDir on excluded directories, we test that the
-	// directory itself is excluded (the caller won't descend into it).
+	// gitignore(5): "It is not possible to re-include a file if a parent
+	// directory of that file is excluded." dir/ excludes the directory,
+	// so the negation on line 2 has no effect.
 	m := setupMatcher(t, "dir/\n!dir/important.txt\n")
 
-	// The directory is still excluded
 	if !m.Match("dir/") {
 		t.Error("expected dir/ to be ignored")
 	}
-	// The file would be re-included by the pattern, but since callers
-	// SkipDir on dir/, they never check this file. We verify the pattern
-	// semantics still work for completeness.
-	if m.Match("dir/important.txt") {
-		t.Error("negation should re-include dir/important.txt in pattern matching")
+	if !m.Match("dir/important.txt") {
+		t.Error("expected dir/important.txt to be ignored (parent dir/ is excluded)")
+	}
+	if !m.Match("dir/other.txt") {
+		t.Error("expected dir/other.txt to be ignored (parent dir/ is excluded)")
 	}
 }
 
@@ -1072,8 +1070,9 @@ func TestMatchStarExtension(t *testing.T) {
 func TestMatchDoubleStarTrailingDir(t *testing.T) {
 	m := setupMatcher(t, "foo/**/\n")
 
-	shouldMatch := []string{"foo/", "foo/abc/", "foo/x/y/z/"}
-	shouldNotMatch := []string{"foo"}
+	// foo/**/ matches directories inside foo, not foo itself.
+	shouldMatch := []string{"foo/abc/", "foo/x/y/z/"}
+	shouldNotMatch := []string{"foo", "foo/"}
 
 	for _, path := range shouldMatch {
 		if !m.Match(path) {
@@ -1106,13 +1105,15 @@ func TestMatchDoubleStarWithExtension(t *testing.T) {
 }
 
 func TestMatchNegationSubdirectoryFilter(t *testing.T) {
+	// abc excludes the directory abc, so nothing under it can be
+	// re-included. abc/* would be needed for the negation to take effect.
 	m := setupMatcher(t, "abc\n!abc/b\n")
 
 	if !m.Match("abc/a.js") {
 		t.Error("expected abc/a.js to match")
 	}
-	if m.Match("abc/b/b.js") {
-		t.Error("expected abc/b/b.js to not match")
+	if !m.Match("abc/b/b.js") {
+		t.Error("expected abc/b/b.js to match (parent abc is excluded)")
 	}
 }
 

gitignore_test.go

@@ -0,0 +1,3 @@
+/root-only
+unanchored
+foo/*

testdata/conformance/anchored-and-wildcard/gitignore

@@ -0,0 +1,8 @@
+root-only
+sub/root-only
+unanchored
+sub/unanchored
+foo/
+foo/test.json
+foo/bar/
+foo/bar/hello.c

testdata/conformance/anchored-and-wildcard/paths

@@ -0,0 +1,2 @@
+[a*]b
+x[c*d]y

testdata/conformance/bracket-with-star/gitignore

@@ -0,0 +1,6 @@
+ab
+zb
+xcy
+xdy
+xzy
+xccdy

testdata/conformance/bracket-with-star/paths

@@ -0,0 +1,2 @@
+build/
+*.egg-info/

testdata/conformance/dir-only-basics/gitignore

@@ -0,0 +1,8 @@
+buildfile
+build/
+build/output.js
+sub/build/
+sub/build/x
+mypkg.egg-info/
+mypkg.egg-info/PKG-INFO
+mypkg.egg-info-backup

testdata/conformance/dir-only-basics/paths

@@ -0,0 +1 @@
+data/**/