Skip to content

Commit

Permalink
fix(cli): support .gitignore ignoring of directories (#6880)
Browse files Browse the repository at this point in the history
Minor enhancement to support the .gitignore trailing-/ so it only
matches directories.

---

### Changes are visible to end-users: no

### Test plan

- Covered by existing test cases
- New test cases added

GitOrigin-RevId: 2711ceec5d8d5ffe9ec2f045b7c0994d4b0c5d6c
  • Loading branch information
jbedard committed Oct 23, 2024
1 parent dd768d1 commit a331f1d
Show file tree
Hide file tree
Showing 9 changed files with 56 additions and 12 deletions.
8 changes: 5 additions & 3 deletions gazelle/common/git/gitignore.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import (
// Must align with patched bazel-gazelle
const ASPECT_GITIGNORE = "__aspect:gitignore"

type isGitIgnoredFunc = func(string, bool) bool

// Directive to enable/disable gitignore support
const Directive_GitIgnore = "gitignore"

Expand Down Expand Up @@ -97,14 +99,14 @@ func parseIgnore(rel string, ignoreReader io.Reader) []gitignore.Pattern {
return matcherPatterns
}

func createMatcherFunc(c *config.Config) func(string) bool {
func createMatcherFunc(c *config.Config) isGitIgnoredFunc {
patterns, patternsFound := c.Exts[ignorePatternsExt]
if !patternsFound {
return nil
}

matcher := gitignore.NewMatcher(patterns.([]gitignore.Pattern))
return func(s string) bool {
return matcher.Match(strings.Split(s, "/"), false)
return func(s string, isDir bool) bool {
return matcher.Match(strings.Split(s, "/"), isDir)
}
}
24 changes: 22 additions & 2 deletions gazelle/common/git/gitignore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,23 @@ func TestGitIgnore(t *testing.T) {
shouldMatch := func(what string, c *config.Config, matches ...string) {
matcher := createMatcherFunc(c)
for _, m := range matches {
if !(matcher != nil && matcher(m)) {
// Use trailing slash in test data to indicate directory
isDir := strings.HasSuffix(m, "/")
m = strings.TrimSuffix(m, "/")

if !(matcher != nil && matcher(m, isDir)) {
t.Error(fmt.Sprintf("%s should match '%s'", what, m))
}
}
}
shouldNotMatch := func(what string, c *config.Config, matches ...string) {
matcher := createMatcherFunc(c)
for _, m := range matches {
if matcher != nil && matcher(m) {
// Use trailing slash in test data to indicate directory
isDir := strings.HasSuffix(m, "/")
m = strings.TrimSuffix(m, "/")

if matcher != nil && matcher(m, isDir) {
t.Error(fmt.Sprintf("%s should NOT match '%s'", what, m))
}
}
Expand Down Expand Up @@ -183,6 +191,18 @@ func TestGitIgnore(t *testing.T) {
shouldMatch("double wildcard", c, "subdir/x.starstar.ts", "subdir/a/b/c/x.starstar.ts", "subdir/a/.starstar.ts")
shouldNotMatch("double wildcard different subdir", c, "startstar.ts", ".startstar.ts", "a/starstar.ts")
})

t.Run("dir specific matches", func(t *testing.T) {
c := config.New()
addIgnoreFileContent(c, "", `
**/node_modules/
**/foo.js/
`)

shouldMatch("dir pattern", c, "node_modules/", "a/b/node_modules/")
shouldMatch("dir pattern that looks like a file", c, "foo.js/", "a/b/foo.js/")
shouldNotMatch("ending file that looks like dir", c, "node_modules", "x/node_modules", "foo.js", "x/foo.js")
})
}

// Util method to invoke GitIgnore.AddIgnore() with the trimmed string
Expand Down
3 changes: 3 additions & 0 deletions gazelle/js/tests/ignore_config_files/.test-gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,6 @@ subtarget/generated.ts

# A global glob configured from the root with no slash
**-wild.ts

# An ignore pattern exclusively matching directories
**/modules.ts/
1 change: 1 addition & 0 deletions gazelle/js/tests/ignore_config_files/BUILD.out
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ ts_project(
"subdir/nested/index.ts",
"subdir/not.nested-ig.ts",
"subdir2/direct-ig.ts",
"subdir2/modules.ts",
"subdir2/not.nested-ig.ts",
],
deps = [
Expand Down
1 change: 1 addition & 0 deletions gazelle/js/tests/ignore_config_files/modules.ts/a.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import 'bad-import-but-this-dir-is-ignored';
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import 'bad-import-but-this-dir-is-ignored';
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import 'bad-import-but-this-dir-is-ignored';
Empty file.
29 changes: 22 additions & 7 deletions patches/bazelbuild_bazel-gazelle_aspect-gitignore.patch
Original file line number Diff line number Diff line change
Expand Up @@ -10,30 +10,45 @@ index 23b1083..1841650 100644

func New() *Config {
return &Config{
diff --git a/walk/config.go b/walk/config.go
index 1292146..07cdb3d 100644
--- a/walk/config.go
+++ b/walk/config.go
@@ -109,6 +109,7 @@ func (cr *Configurer) Configure(c *config.Config, rel string, f *rule.File) {
}

type isIgnoredFunc = func(string) bool
+type isGitIgnoredFunc = func(string, bool) bool

var nothingIgnored isIgnoredFunc = func(string) bool { return false }

diff --git a/walk/walk.go b/walk/walk.go
index 7814088..8ebb086 100644
index 7814088..1ba1b47 100644
--- a/walk/walk.go
+++ b/walk/walk.go
@@ -165,6 +165,13 @@ func visit(c *config.Config, cexts []config.Configurer, knownDirectives map[stri
@@ -165,6 +165,14 @@ func visit(c *config.Config, cexts []config.Configurer, knownDirectives map[stri
return nil, false
}

+ // PATCH(gitignore) ---
+ isGitIgnored, hasGitIgnore := c.Exts[config.ASPECT_GITIGNORE].(isIgnoredFunc)
+ if !hasGitIgnore || isGitIgnored == nil {
+ isGitIgnored = nothingIgnored
+ var isGitIgnored isGitIgnoredFunc
+ isGitIgnoredExt, hasGitIgnore := c.Exts[config.ASPECT_GITIGNORE]
+ if hasGitIgnore && isGitIgnoredExt != nil {
+ isGitIgnored = isGitIgnoredExt.(isGitIgnoredFunc)
+ }
+ // END PATCH(gitignore) ---
+
var dirs, subdirs, regularFiles []string
for _, ent := range ents {
base := ent.Name()
@@ -172,6 +179,11 @@ func visit(c *config.Config, cexts []config.Configurer, knownDirectives map[stri
@@ -172,6 +180,13 @@ func visit(c *config.Config, cexts []config.Configurer, knownDirectives map[stri
if wc.isExcluded(entRel) {
continue
}
+ // PATCH(gitignore) ---
+ if isGitIgnored(entRel) {
+ // NOTE: .gitignore must be applied *before* symlinks are followed
+ // See notes on symlink examples at https://git-scm.com/docs/gitignore#_examples
+ if isGitIgnored != nil && isGitIgnored(entRel, ent.IsDir()) {
+ continue
+ }
+ // END PATCH(gitignore) ---
Expand Down

0 comments on commit a331f1d

Please sign in to comment.