refactor(pattern-matcher): add CanSkipDir method in patternMatcher

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 <[email protected]>
containers · Jun 30, 2023 · 4fc6694 · 4fc6694
1 parent fab5175
commit 4fc6694
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 67 deletions.
diff --git a/pkg/fileutils/fileutils.go b/pkg/fileutils/fileutils.go
@@ -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
@@ -57,63 +57,30 @@ func NewPatternMatcher(patterns []string) (*PatternMatcher, error) {
 	return pm, nil
 }
 
-// Deprecated: Please use the `MatchesResult` method instead.
-// Matches matches path against all the patterns. Matches is not safe to be
-// called concurrently
-func (pm *PatternMatcher) Matches(file string) (bool, error) {
-	matched := false
-	file = filepath.FromSlash(file)
-
-	for _, pattern := range pm.patterns {
-		negative := false
-
-		if pattern.exclusion {
-			negative = true
-		}
-
-		match, err := pattern.match(file)
-		if err != nil {
-			return false, err
-		}
-
-		if match {
-			matched = !negative
-		}
-	}
-
-	if matched {
-		logrus.Debugf("Skipping excluded path: %s", file)
-	}
-
-	return matched, nil
-}
-
 type MatchResult struct {
-	isMatched         bool
-	matches, excludes uint
+	isMatched  bool
+	canSkipDir bool
 }
 
-// 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
 }
 
-// Excludes returns the amount of matches of an MatchResult
-func (m *MatchResult) Matches() uint {
-	return m.matches
-}
-
-// Excludes returns the amount of excludes of an MatchResult
-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}
+	matchPatterns := make([]*Pattern, 0, len(pm.patterns))
 
 	for _, pattern := range pm.patterns {
 		negative := false
@@ -129,16 +96,31 @@ func (pm *PatternMatcher) MatchesResult(file string) (res *MatchResult, err erro
 
 		if match {
 			res.isMatched = !negative
-			if negative {
-				res.excludes++
-			} else {
-				res.matches++
+			if res.isMatched {
+				matchPatterns = append(matchPatterns, pattern)
 			}
 		}
 	}
 
-	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

diff --git a/pkg/fileutils/fileutils_test.go b/pkg/fileutils/fileutils_test.go
@@ -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)
 				}
 			}
 		})
@@ -591,22 +591,22 @@ 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
+		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", true, true},
+		{[]string{"!1", "1"}, "1", true, true},
+		{[]string{"2", "!2", "!2"}, "2", false, false},
+		{[]string{"1", "2", "2"}, "2", true, true},
+		{[]string{"1", "2", "2", "2"}, "2", true, true},
+		{[]string{"/prefix/path", "/prefix/other"}, "/prefix/path", true, true},
+		{[]string{"/prefix*", "!/prefix/path"}, "/prefix/match", true, false},
+		{[]string{"/prefix*", "!/prefix/path"}, "/prefix/path", true, false},
+		{[]string{"/prefix*", "!/prefix/path"}, "prefix/path", false, false},
+		{[]string{"/prefix*", "!./prefix/path"}, "prefix/path", false, false},
+		{[]string{"/prefix*", "!prefix/path"}, "prefix/path", false, false},
 	}
 
 	for _, testCase := range testData {
@@ -615,9 +615,8 @@ func TestMatchesAmount(t *testing.T) {
 		res, err := pm.MatchesResult(testCase.input)
 		require.NoError(t, err)
 		desc := fmt.Sprintf("pattern=%q input=%q", testCase.patterns, testCase.input)
-		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)