From 4fc66941de4aa271be930340c720d11054d3da1d Mon Sep 17 00:00:00 2001 From: Black-Hole1 Date: Fri, 30 Jun 2023 09:26:47 +0800 Subject: [PATCH] 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 --- pkg/fileutils/fileutils.go | 80 +++++++++++++-------------------- pkg/fileutils/fileutils_test.go | 35 +++++++-------- 2 files changed, 48 insertions(+), 67 deletions(-) diff --git a/pkg/fileutils/fileutils.go b/pkg/fileutils/fileutils.go index 9d0714b1b9..e78afe49b3 100644 --- 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,55 +57,21 @@ 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. @@ -113,7 +79,8 @@ func (m *MatchResult) Excludes() uint { // 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 index 3131720b1f..fccea0c209 100644 --- 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)