Skip to content

Commit

Permalink
refactor(pattern-matcher): add CanSkipDir method in patternMatcher
Browse files Browse the repository at this point in the history
1. Remove deprecated method: `Matches` in `PatternMatcher`
2. Add `CanSkipDir` to `MatchesResult`
3. Improve `MatchesResult`
   1. Remove the `matches` and `excludes` fields (no one is using these two fields)

Signed-off-by: Black-Hole1 <bh@bugs.cc>
  • Loading branch information
BlackHole1 committed Jun 30, 2023
1 parent fab5175 commit eb17cc5
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 25 deletions.
44 changes: 35 additions & 9 deletions pkg/fileutils/fileutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func NewPatternMatcher(patterns []string) (*PatternMatcher, error) {
pm.exclusions = true
}
// Do some syntax checking on the pattern.
// filepath's Match() has some really weird rules that are inconsistent
// filepath.Match() has some really weird rules that are inconsistent
// so instead of trying to dup their logic, just call Match() for its
// error state and if there is an error in the pattern return it.
// If this becomes an issue we can remove this since its really only
Expand Down Expand Up @@ -89,11 +89,11 @@ func (pm *PatternMatcher) Matches(file string) (bool, error) {
}

type MatchResult struct {
isMatched bool
matches, excludes uint
isMatched, canSkipDir bool
matches, excludes uint
}

// Excludes returns true if the overall result is matched
// IsMatched returns true if the overall result is matched
func (m *MatchResult) IsMatched() bool {
return m.isMatched
}
Expand All @@ -108,12 +108,20 @@ func (m *MatchResult) Excludes() uint {
return m.excludes
}

// CanSkipDir returns true if it is possible to use filepath.SkipDir
// Only when the pattern is very simple (does not start with ! and does not contain ? and *)
// and is not influenced by other patterns, will it return true.
func (m *MatchResult) CanSkipDir() bool {
return m.canSkipDir
}

// MatchesResult verifies the provided filepath against all patterns.
// It returns the `*MatchResult` result for the patterns on success, otherwise
// an error. This method is not safe to be called concurrently.
func (pm *PatternMatcher) MatchesResult(file string) (res *MatchResult, err error) {
file = filepath.FromSlash(file)
res = &MatchResult{false, 0, 0}
res = &MatchResult{false, false, 0, 0}
matchPatterns := make([]*Pattern, 0, len(pm.patterns))

for _, pattern := range pm.patterns {
negative := false
Expand All @@ -129,16 +137,34 @@ func (pm *PatternMatcher) MatchesResult(file string) (res *MatchResult, err erro

if match {
res.isMatched = !negative
if negative {
res.excludes++
} else {
if res.isMatched {
res.matches++
matchPatterns = append(matchPatterns, pattern)
} else {
res.excludes++
}
}
}

if res.matches > 0 {
if res.isMatched {
logrus.Debugf("Skipping excluded path: %s", file)

isPatternsEqual := true
first := matchPatterns[0].String()
if len(matchPatterns) != 1 {
for _, pattern := range matchPatterns[1:] {
if first != pattern.String() {
isPatternsEqual = false
break
}
}
}

if isPatternsEqual {
if !strings.Contains(first, "*") && !strings.Contains(first, "?") {
res.canSkipDir = true
}
}
}

return res, nil
Expand Down
35 changes: 19 additions & 16 deletions pkg/fileutils/fileutils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ func TestMatches(t *testing.T) {
assert.Equal(t, err, filepath.ErrBadPattern)
} else {
assert.Nil(t, err)
assert.Equal(t, test.match, res.isMatched, desc)
assert.Equal(t, test.match, res.IsMatched(), desc)
}
}
})
Expand Down Expand Up @@ -591,22 +591,24 @@ func TestMatch(t *testing.T) {

func TestMatchesAmount(t *testing.T) {
testData := []struct {
patterns []string
input string
matches uint
excludes uint
isMatch bool
patterns []string
input string
matches uint
excludes uint
isMatch bool
canSkipDir bool
}{
{[]string{"1", "2", "3"}, "2", 1, 0, true},
{[]string{"2", "!2", "!2"}, "2", 1, 2, false},
{[]string{"1", "2", "2"}, "2", 2, 0, true},
{[]string{"1", "2", "2", "2"}, "2", 3, 0, true},
{[]string{"/prefix/path", "/prefix/other"}, "/prefix/path", 1, 0, true},
{[]string{"/prefix*", "!/prefix/path"}, "/prefix/match", 1, 0, true},
{[]string{"/prefix*", "!/prefix/path"}, "/prefix/path", 1, 0, true},
{[]string{"/prefix*", "!/prefix/path"}, "prefix/path", 0, 1, false},
{[]string{"/prefix*", "!./prefix/path"}, "prefix/path", 0, 1, false},
{[]string{"/prefix*", "!prefix/path"}, "prefix/path", 0, 1, false},
{[]string{"1", "2", "3"}, "2", 1, 0, true, true},
{[]string{"!1", "1"}, "1", 1, 1, true, true},
{[]string{"2", "!2", "!2"}, "2", 1, 2, false, false},
{[]string{"1", "2", "2"}, "2", 2, 0, true, true},
{[]string{"1", "2", "2", "2"}, "2", 3, 0, true, true},
{[]string{"/prefix/path", "/prefix/other"}, "/prefix/path", 1, 0, true, true},
{[]string{"/prefix*", "!/prefix/path"}, "/prefix/match", 1, 0, true, false},
{[]string{"/prefix*", "!/prefix/path"}, "/prefix/path", 1, 0, true, false},
{[]string{"/prefix*", "!/prefix/path"}, "prefix/path", 0, 1, false, false},
{[]string{"/prefix*", "!./prefix/path"}, "prefix/path", 0, 1, false, false},
{[]string{"/prefix*", "!prefix/path"}, "prefix/path", 0, 1, false, false},
}

for _, testCase := range testData {
Expand All @@ -618,6 +620,7 @@ func TestMatchesAmount(t *testing.T) {
assert.Equal(t, testCase.excludes, res.Excludes(), desc)
assert.Equal(t, testCase.matches, res.Matches(), desc)
assert.Equal(t, testCase.isMatch, res.IsMatched(), desc)
assert.Equal(t, testCase.canSkipDir, res.CanSkipDir(), desc)

isMatch, err := pm.IsMatch(testCase.input)
require.NoError(t, err)
Expand Down

0 comments on commit eb17cc5

Please sign in to comment.