Skip to content

Commit 9c8988e

Browse files
Ignore scoped files anywhere in a repo
When checking if an Addition belongs to any scopes, we should compare just the basename of the Addition instead of using the standard filename/path matching logic. Authored-by: Owen Nelson <[email protected]>
1 parent a2301ba commit 9c8988e

File tree

5 files changed

+70
-69
lines changed

5 files changed

+70
-69
lines changed

gitrepo/gitrepo.go

+20-6
Original file line numberDiff line numberDiff line change
@@ -187,19 +187,27 @@ func (repo GitRepo) CheckIfFileExists(fileName string) bool {
187187
return true
188188
}
189189

190-
// Matches states whether the addition matches the given pattern.
191-
// If the pattern ends in a path separator, then all files inside a directory with that name are matched. However, files with that name itself will not be matched.
192-
// If a pattern contains the path separator in any other location, the match works according to the pattern logic of the default golang glob mechanism
193-
// If there are other special characters in the pattern, the pattern is matched against the base name of the file. Thus, the pattern will match files with that pattern anywhere in the repository.
194-
// If there are no special characters in the pattern, then it means exact filename is provided as pattern like file.txt. Thus, the pattern is matched against the file path so that not all files with the same name in the repo are not returned.
190+
// Matches reports whether the addition matches the given pattern.
191+
//
192+
// If the pattern ends in a path separator, then all files inside a directory with that name are matched.
193+
// However, files with that name itself will not be matched.
194+
//
195+
// If a pattern contains the path separator in any other location,
196+
// the match works according to the pattern logic of the default golang glob mechanism.
197+
//
198+
// If there are other special characters in the pattern, the pattern is matched against the base name of the file.
199+
// Thus, the pattern will match files with that pattern anywhere in the repository.
200+
//
201+
// If there are no special characters in the pattern, then it means exact filename is provided as pattern like file.txt.
202+
// Thus, the pattern is matched against the file path so that not all files with the same name in the repo are not returned.
195203
func (a Addition) Matches(pattern string) bool {
196204
var result bool
197205
if pattern[len(pattern)-1] == '/' { // If the pattern ends in a path separator, then all files inside a directory with that name are matched. However, files with that name itself will not be matched.
198206
result = strings.HasPrefix(string(a.Path), pattern)
199207
} else if strings.ContainsRune(pattern, '/') { // If a pattern contains the path separator in any other location, the match works according to the pattern logic of the default golang glob mechanism
200208
result, _ = path.Match(pattern, string(a.Path))
201209
} else if strings.ContainsAny(pattern, "*?[]\\") { // If there are other special characters in the pattern, the pattern is matched against the base name of the file. Thus, the pattern will match files with that pattern anywhere in the repository.
202-
result, _ = path.Match(pattern, string(a.Name))
210+
result = a.NameMatches(pattern)
203211
} else { // If there are no special characters in the pattern, then it means exact filename is provided as pattern like file.txt. Thus, the pattern is matched against the file path so that not all files with the same name in the repo are not returned.
204212
result = strings.Compare(string(a.Path), pattern) == 0
205213
}
@@ -211,6 +219,12 @@ func (a Addition) Matches(pattern string) bool {
211219
return result
212220
}
213221

222+
// NameMatches reports whether the basename of the Addition matches the given pattern
223+
func (a Addition) NameMatches(pattern string) bool {
224+
result, _ := path.Match(pattern, string(a.Name))
225+
return result
226+
}
227+
214228
// TrackedFilesAsAdditions returns all of the tracked files in a GitRepo as Additions
215229
func (repo GitRepo) TrackedFilesAsAdditions() []Addition {
216230
trackedFilePaths := repo.trackedFilePaths()

gitrepo/gitrepo_test.go

+34-56
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package gitrepo
22

33
import (
44
"fmt"
5-
"io/ioutil"
65
"os"
76
"os/exec"
87
"path"
@@ -44,9 +43,9 @@ func TestGetDiffForStagedFiles(t *testing.T) {
4443
modifiedAddition := additions[0]
4544
createdAddition := additions[1]
4645

47-
aTxtFileContents, err := ioutil.ReadFile(path.Join(cloneLocation, "a.txt"))
46+
aTxtFileContents, err := os.ReadFile(path.Join(cloneLocation, "a.txt"))
4847
assert.NoError(t, err)
49-
newTxtFileContents, err := ioutil.ReadFile(path.Join(cloneLocation, "new.txt"))
48+
newTxtFileContents, err := os.ReadFile(path.Join(cloneLocation, "new.txt"))
5049
assert.NoError(t, err)
5150

5251
expectedModifiedAddition := Addition{
@@ -81,7 +80,7 @@ func TestGetDiffForStagedFilesWithSpacesInPath(t *testing.T) {
8180
if assert.Len(t, additions, 1) {
8281
modifiedAddition := additions[0]
8382

84-
aTxtFileContents, err := ioutil.ReadFile(path.Join(cloneLocation, "folder b/c.txt"))
83+
aTxtFileContents, err := os.ReadFile(path.Join(cloneLocation, "folder b/c.txt"))
8584
assert.NoError(t, err)
8685

8786
expectedModifiedAddition := Addition{
@@ -204,17 +203,6 @@ func TestStagedAdditionsShouldNotIncludeDeletedFiles(t *testing.T) {
204203
assert.Len(t, stagedAdditions, 0)
205204
}
206205

207-
func TestMatchShouldAllowWildcardPatternMatches(t *testing.T) {
208-
file1 := Addition{Path: "bigfile", Name: "bigfile"}
209-
file2 := Addition{Path: "anotherbigfile", Name: "anotherbigfile"}
210-
file3 := Addition{Path: "somefile", Name: "somefile"}
211-
pattern := "*bigfile"
212-
213-
assert.True(t, file1.Matches(pattern))
214-
assert.True(t, file2.Matches(pattern))
215-
assert.False(t, file3.Matches(pattern))
216-
}
217-
218206
func TestMatchShouldMatchExactFileIfNoPatternIsProvided(t *testing.T) {
219207
file1 := Addition{Path: "bigfile", Name: "bigfile"}
220208
file2 := Addition{Path: "subfolder/bigfile", Name: "bigfile"}
@@ -226,49 +214,39 @@ func TestMatchShouldMatchExactFileIfNoPatternIsProvided(t *testing.T) {
226214
assert.False(t, file3.Matches(pattern))
227215
}
228216

229-
func TestMatchShouldAllowStarPattern(t *testing.T) {
230-
file1 := Addition{Path: "GitRepoPath1/File1.txt", Name: "File1.txt"}
231-
file2 := Addition{Path: "GitRepoPath1/File2.txt", Name: "File2.txt"}
232-
file3 := Addition{Path: "GitRepoPath1/somefile", Name: "somefile"}
233-
file4 := Addition{Path: "somefile.jpg", Name: "somefile.jpg"}
234-
file5 := Addition{Path: "somefile.txt", Name: "somefile.txt"}
235-
file6 := Addition{Path: "File1.txt", Name: "File1.txt"}
236-
file7 := Addition{Path: "File3.txt", Name: "File3.txt"}
217+
func TestMatchingWithPatterns(t *testing.T) {
218+
files := []Addition{
219+
NewAddition("GitRepoPath1/File1.txt", nil),
220+
NewAddition("GitRepoPath1/File2.txt", nil),
221+
NewAddition("GitRepoPath1/somefile", nil),
222+
NewAddition("somefile.jpg", nil),
223+
NewAddition("somefile.txt", nil),
224+
NewAddition("File1.txt", nil),
225+
NewAddition("File3.txt", nil),
226+
}
237227

238-
pattern := "GitRepoPath1/*.txt"
228+
expectedToMatch := map[string][]bool{
229+
"GitRepoPath1/*.txt": {true, true, false, false, false, false, false},
230+
"*.txt": {true, true, false, false, true, true, true},
231+
"File?.txt": {true, true, false, false, false, true, true},
232+
"File[1].txt": {true, false, false, false, false, true, false},
233+
"File[1-2].txt": {true, true, false, false, false, true, false},
234+
"File\\1.txt": {true, false, false, false, false, true, false},
235+
}
239236

240-
assert.True(t, file1.Matches(pattern))
241-
assert.True(t, file2.Matches(pattern))
242-
assert.False(t, file3.Matches(pattern))
243-
assert.False(t, file4.Matches(pattern))
244-
assert.False(t, file5.Matches(pattern))
245-
246-
pattern1 := "*.txt"
247-
assert.True(t, file1.Matches(pattern1))
248-
assert.True(t, file2.Matches(pattern1))
249-
assert.False(t, file3.Matches(pattern1))
250-
assert.False(t, file4.Matches(pattern1))
251-
assert.True(t, file5.Matches(pattern1))
252-
253-
pattern2 := "File?.txt"
254-
assert.True(t, file1.Matches(pattern2))
255-
assert.True(t, file2.Matches(pattern2))
256-
assert.True(t, file6.Matches(pattern2))
257-
258-
pattern3 := "File[1].txt"
259-
assert.True(t, file1.Matches(pattern3))
260-
assert.False(t, file2.Matches(pattern3))
261-
assert.True(t, file6.Matches(pattern3))
262-
263-
pattern4 := "File[1-2].txt"
264-
assert.True(t, file1.Matches(pattern4))
265-
assert.True(t, file2.Matches(pattern4))
266-
assert.True(t, file6.Matches(pattern4))
267-
assert.False(t, file7.Matches(pattern4))
268-
269-
pattern5 := "File\\1.txt"
270-
assert.True(t, file1.Matches(pattern5))
271-
assert.True(t, file6.Matches(pattern5))
237+
for pattern, expectedResults := range expectedToMatch {
238+
t.Run(fmt.Sprintf("Testing matches for pattern %s", pattern), func(t *testing.T) {
239+
for i := range files {
240+
assert.Equal(t, expectedResults[i], files[i].Matches(pattern))
241+
}
242+
})
243+
}
244+
}
245+
246+
func TestMatchingAdditionBasename(t *testing.T) {
247+
addition := NewAddition("subdirectory/nested-file", nil)
248+
assert.False(t, addition.Matches("nested-file"))
249+
assert.True(t, addition.NameMatches("nested-file"))
272250
}
273251

274252
func setupOriginAndClones(originLocation, cloneLocation string) (*git_testing.GitTesting, GitRepo) {

talismanrc/scopes.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
package talismanrc
22

3+
// Mapping of language scopes to names of files that should be ignored anywhere in a repository
34
var knownScopes = map[string][]string{
45
"node": {"pnpm-lock.yaml", "yarn.lock", "package-lock.json"},
56
"go": {"makefile", "go.mod", "go.sum", "Gopkg.toml", "Gopkg.lock", "glide.yaml", "glide.lock"},
67
"images": {"*.jpeg", "*.jpg", "*.png", "*.tiff", "*.bmp"},
78
"bazel": {"*.bzl"},
8-
"terraform": {".terraform.lock.hcl", "*.terraform.lock.hcl"},
9+
"terraform": {".terraform.lock.hcl"},
910
"php": {"composer.lock"},
1011
"python": {"poetry.lock", "Pipfile.lock", "requirements.txt"},
1112
}

talismanrc/talismanrc.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func (tRC *TalismanRC) RemoveScopedFiles(additions []gitrepo.Addition) []gitrepo
4444
for _, addition := range additions {
4545
isFilePresentInScope := false
4646
for _, fileName := range applicableScopeFileNames {
47-
if addition.Matches(fileName) {
47+
if addition.NameMatches(fileName) {
4848
isFilePresentInScope = true
4949
break
5050
}

talismanrc/talismanrc_test.go

+13-5
Original file line numberDiff line numberDiff line change
@@ -90,13 +90,19 @@ func TestIgnoreAdditionsByScope(t *testing.T) {
9090
"node": {
9191
testAddition("yarn.lock"),
9292
testAddition("pnpm-lock.yaml"),
93-
testAddition("package-lock.json")},
93+
testAddition("package-lock.json"),
94+
},
9495
"go": {
9596
testAddition("Gopkg.lock"),
9697
testAddition("makefile"),
97-
testAddition("go.mod"), testAddition("go.sum"),
98-
testAddition("Gopkg.toml"), testAddition("Gopkg.lock"),
99-
testAddition("glide.yaml"), testAddition("glide.lock"),
98+
testAddition("go.mod"),
99+
testAddition("go.sum"),
100+
testAddition("submodule/go.mod"),
101+
testAddition("submodule/go.sum"),
102+
testAddition("Gopkg.toml"),
103+
testAddition("Gopkg.lock"),
104+
testAddition("glide.yaml"),
105+
testAddition("glide.lock"),
100106
},
101107
"images": {
102108
testAddition("img.jpeg"),
@@ -105,7 +111,9 @@ func TestIgnoreAdditionsByScope(t *testing.T) {
105111
testAddition("img.tiff"),
106112
testAddition("img.bmp"),
107113
},
108-
"bazel": {testAddition("bazelfile.bzl")},
114+
"bazel": {
115+
testAddition("bazelfile.bzl"),
116+
},
109117
"terraform": {
110118
testAddition(".terraform.lock.hcl"),
111119
testAddition("foo/.terraform.lock.hcl"),

0 commit comments

Comments
 (0)