From fbfd72574d7ff4385d6e0a95200d414b10adb0ec Mon Sep 17 00:00:00 2001 From: Oliver Calder Date: Mon, 5 Aug 2024 14:46:28 -0500 Subject: [PATCH] i/prompting: implement path pattern precedence (#13868) * i/prompting: implement path pattern precedence When a path matches multiple patterns, the most specific of those patterns must be selected so that the associated rule can take precedence. When comparing the precedence of path patterns, those patterns cannot contain groups, since it is necessary to know which variants of the pattern were matched against the path in question. Thus, path pattern precedence must only be computed between patterns which have already been rendered. In general, at the first character where two patterns differ, the one which is more restrictive has precedence (i.e. '/' over '?', '?' over '*'). Less restrictive patterns are repeatedly pruned until a single most restrictive pattern remains. That pattern has precedence. One property which emerges from this precedence system is that the pattern which most specifically restricts the location of matching paths takes precedence. For example, the following patterns all match `/foo/bar/baz/myfile.txt` (though this is not an exhaustive list) and are sorted from highest to lowest precedence: 1. /foo/bar/baz/myfile.txt 2. /foo/bar/baz/m?file.* 3. /foo/bar/baz/m*file.txt 4. /foo/bar/baz/m*file* 5. /foo/bar/baz/* 6. /foo/bar/*/myfile.txt 7. /foo/bar/*/myfile* 8. /foo/bar/*/* 9. /foo/bar/**/baz/myfile.txt 10. /foo/bar/**/myfile.txt 11. /foo/bar/**/baz/*.txt 12. /foo/bar/**/baz/* 13. /foo/bar/** 14. /foo/b?r/baz/myfile.txt 15. /foo/?*/baz/myfile.txt 16. /foo/?*/** 17. /foo/*?/baz/myfile.txt 18. /foo/*/baz/myfile.txt 19. /foo/*/baz/* 20. /foo/*/*/* 21. /foo/*/**/baz/myfile.txt 22. /foo/**/baz/myfile.txt 23. /foo/**/baz/** 24. /**/myfile.txt 25. /** Signed-off-by: Oliver Calder i/prompting: add tests for precedence between example patterns Signed-off-by: Oliver Calder * i/prompting/patterns: add pattern variant type for precedence comparison After a path pattern is rendered into variants, parse each variant into its constituent components so that precedence between any variants can be decided based on the order and occurrence of those components. For example, `/foo/*/bar/b?z/**` has precedence over `/foo/*/b?r/baz/**`. There are several types of scenarios which make it difficult to compute precedence in all cases using a simple mechanism: - `/foo/` > `/foo` > `/foo/**` - `/foo/**/bar` > `/foo/**/` > `/foo/**` - `/foo/bar/` > `/foo/b*r/` - `/foo/**/x/b*r` > `/foo/**/bar/` - `/foo/bar/**` > `/foo/b?r/**/baz` > `/foo/b*r/baz` The current implementation fails to address all these scenarios, namely because it relies on the length of literals (to catch `/foo/bar/` vs `/foo/b*r/`, for example), but this falls apart for literals which occur after a previous `/**/`, since a shorter literal may match an earlier component of a hypothetical path. Additionally, we cannot always compute precedence between two variants: Both `/foo/**/bar/**` and `/foo/**/baz/**` match `/foo/bar/baz`, but nothing about the order or length of components can tell us which should have precedence. In this case, we would likely say the former has precedence, since `bar` matches earlier in the path, but if the path were `/foo/baz/bar`, we would likely say the latter has precedence. Thus, we must use the path against which the matches were made when computing precedence between pattern variants. Signed-off-by: Oliver Calder * i/prompting/patterns: compute precedence based on matching path With knowledge of the path which matched every pattern variant we're comparing, we can compute which of those variants has the highest precedence. For example, the precedence of the following pattern variants cannot be computed a priori: - /foo/**/fizz/** - /foo/**/buzz/** However, with the matching path "/foo/bar/fizz/buzz", we can say that "/foo/**/fizz/**" has precedence, since "fizz" matches earlier in the path than "buzz". If instead the path were "/foo/bar/buzz/fizz", then "/foo/**/buzz/**" would have precedence, for the same reason. To compute this, we add a regular expression snippet for every component, concatenate and compile them into a single regex for the pattern variant, and match it against the path when computing precedence, keeping track of submatches. As we iterate through the components of the pattern variant, we also iterate through the submatches, and we are thus able to tell more about how many bytes each component matched. For variable-length component types (e.g. globstars and doublestars), matching fewer bytes yields higher precedence. Conversely, for literals, matching more bytes yields higher precedence, as this implies that the literal itself is longer and thus more specific, as literals may be followed by variable-length components. This also helps reliable compute less-pathological cases, such as the following: - /foo/**/myfile*/** - /foo/**/bar/myfile.txt We may be able to tell that the latter has precedence, but a deterministic algorithm to compute this is difficult to write and reason about. By including a matching path, such as "/foo/bar/myfile.txt", we are able to see that the "/**/" in the former pattern matches more characters than that of the latter, and thus is a less precise match, and has lower precedence. Signed-off-by: Oliver Calder * i/prompting/patterns: use component submatch instead of storing length Signed-off-by: Oliver Calder * i/prompting/patterns: fix bugs around handling doublestars and escaped runes Signed-off-by: Oliver Calder * i/prompting/patterns: stack allocate pattern variants and improve documentation Signed-off-by: Oliver Calder * i/prompting/patterns: add test case and improve robustness Signed-off-by: Oliver Calder * i/prompting/patterns: fix path pattern json marshalling Signed-off-by: Oliver Calder * i/prompting/patterns: improve documentation and unexport parsePatternVariant Signed-off-by: Oliver Calder * i/prompting/patterns: add documentation and test cases Signed-off-by: Oliver Calder --------- Signed-off-by: Oliver Calder --- interfaces/prompting/patterns/export_test.go | 22 + interfaces/prompting/patterns/patterns.go | 74 +- .../prompting/patterns/patterns_test.go | 856 +++++++++++++++++- interfaces/prompting/patterns/render.go | 12 +- .../patterns/render_internal_test.go | 4 +- interfaces/prompting/patterns/variant.go | 505 +++++++++++ .../patterns/variant_internal_test.go | 371 ++++++++ 7 files changed, 1794 insertions(+), 50 deletions(-) create mode 100644 interfaces/prompting/patterns/export_test.go create mode 100644 interfaces/prompting/patterns/variant.go create mode 100644 interfaces/prompting/patterns/variant_internal_test.go diff --git a/interfaces/prompting/patterns/export_test.go b/interfaces/prompting/patterns/export_test.go new file mode 100644 index 00000000000..9b9ef4b61a4 --- /dev/null +++ b/interfaces/prompting/patterns/export_test.go @@ -0,0 +1,22 @@ +// -*- Mode: Go; indent-tabs-mode: t -*- + +/* + * Copyright (C) 2024 Canonical Ltd + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package patterns + +var ParsePatternVariant = parsePatternVariant diff --git a/interfaces/prompting/patterns/patterns.go b/interfaces/prompting/patterns/patterns.go index e76a9971e5f..eca7f4345b3 100644 --- a/interfaces/prompting/patterns/patterns.go +++ b/interfaces/prompting/patterns/patterns.go @@ -21,13 +21,15 @@ package patterns import ( "encoding/json" + "errors" "fmt" - "regexp" "strings" doublestar "github.com/bmatcuk/doublestar/v4" ) +var ErrNoPatterns = errors.New("cannot establish precedence: no patterns given") + // Limit the number of expanded path patterns for a particular pattern. // When fully expanded, the number of patterns for a given unexpanded pattern // may not exceed this limit. @@ -76,7 +78,7 @@ func (p *PathPattern) Match(path string) (bool, error) { // MarshalJSON implements json.Marshaller for PathPattern. func (p *PathPattern) MarshalJSON() ([]byte, error) { - return []byte(p.original), nil + return json.Marshal(p.original) } // UnmarshalJSON implements json.Unmarshaller for PathPattern. @@ -95,46 +97,13 @@ func (p *PathPattern) NumVariants() int { } // RenderAllVariants enumerates every alternative for each group in the path -// pattern and renders each one into a string which is then passed into the -// given observe closure, along with the index of the variant. +// pattern and renders each one into a PatternVariant which is then passed into +// the given observe closure, along with the index of the variant. // // The given observe closure should perform some action with the rendered // variant, such as adding it to a data structure. -func (p *PathPattern) RenderAllVariants(observe func(index int, variant string)) { - cleanThenObserve := func(i int, variant string) { - cleaned := cleanPattern(variant) - observe(i, cleaned) - } - renderAllVariants(p.renderTree, cleanThenObserve) -} - -var ( - duplicateSlashes = regexp.MustCompile(`(^|[^\\])/+`) - charsDoublestar = regexp.MustCompile(`([^/\\])\*\*+`) - doublestarChars = regexp.MustCompile(`([^\\])\*\*+([^/])`) - duplicateDoublestar = regexp.MustCompile(`/\*\*(/\*\*)+`) // relies on charsDoublestar running first - starsAnyMaybeStars = regexp.MustCompile(`([^\\])\*+(\?\**)+`) -) - -func cleanPattern(pattern string) string { - pattern = duplicateSlashes.ReplaceAllString(pattern, `${1}/`) - pattern = charsDoublestar.ReplaceAllString(pattern, `${1}*`) - pattern = doublestarChars.ReplaceAllString(pattern, `${1}*${2}`) - pattern = duplicateDoublestar.ReplaceAllString(pattern, `/**`) - pattern = starsAnyMaybeStars.ReplaceAllStringFunc(pattern, func(s string) string { - deleteStars := func(r rune) rune { - if r == '*' { - return -1 - } - return r - } - return strings.Map(deleteStars, s) + "*" - }) - if strings.HasSuffix(pattern, "/**/*") { - // Strip trailing "/*" from suffix - return pattern[:len(pattern)-len("/*")] - } - return pattern +func (p *PathPattern) RenderAllVariants(observe func(index int, variant PatternVariant)) { + renderAllVariants(p.renderTree, observe) } // PathPatternMatches returns true if the given pattern matches the given path. @@ -173,3 +142,30 @@ func PathPatternMatches(pattern string, path string) (bool, error) { // match paths like `/foo/`. return doublestar.Match(pattern+"/", path) } + +// HighestPrecedencePattern determines which of the given path patterns is the +// most specific, and thus has the highest precedence. +// +// Precedence is only defined between patterns which match the same path. +// Precedence is determined according to which pattern places the earliest and +// greatest restriction on the path. +func HighestPrecedencePattern(patterns []PatternVariant, matchingPath string) (PatternVariant, error) { + switch len(patterns) { + case 0: + return PatternVariant{}, ErrNoPatterns + case 1: + return patterns[0], nil + } + + currHighest := patterns[0] + for _, contender := range patterns[1:] { + result, err := currHighest.Compare(contender, matchingPath) + if err != nil { + return PatternVariant{}, err + } + if result < 0 { + currHighest = contender + } + } + return currHighest, nil +} diff --git a/interfaces/prompting/patterns/patterns_test.go b/interfaces/prompting/patterns/patterns_test.go index af6e9ce9c46..0bede7a9ccb 100644 --- a/interfaces/prompting/patterns/patterns_test.go +++ b/interfaces/prompting/patterns/patterns_test.go @@ -275,17 +275,20 @@ func (s *patternsSuite) TestPathPatternMatch(c *C) { } } -func (s *patternsSuite) TestPathPatternMarshalJSON(c *C) { +func (s *patternsSuite) TestPathPatternMarshalUnmarshalJSON(c *C) { for _, pattern := range []string{ "/foo", - "/foo/ba{r,s}/**", + "/f?o/ba{r,s}/**", "/{a,b}{c,d}{e,f}{g,h}", } { pathPattern, err := patterns.ParsePathPattern(pattern) c.Check(err, IsNil) marshalled, err := pathPattern.MarshalJSON() c.Check(err, IsNil) - c.Check(marshalled, DeepEquals, []byte(pattern)) + c.Check(marshalled, DeepEquals, []byte(`"`+pattern+`"`)) + unmarshalled := patterns.PathPattern{} + err = unmarshalled.UnmarshalJSON(marshalled) + c.Check(err, IsNil) } } @@ -300,8 +303,7 @@ func (s *patternsSuite) TestPathPatternUnmarshalJSONHappy(c *C) { c.Check(err, IsNil) marshalled, err := pathPattern.MarshalJSON() c.Check(err, IsNil) - // Marshalled pattern excludes surrounding '"' for some reason - c.Check(marshalled, DeepEquals, pattern[1:len(pattern)-1]) + c.Check(marshalled, DeepEquals, pattern) } } @@ -473,8 +475,8 @@ func (s *patternsSuite) TestPathPatternRenderAllVariants(c *C) { pathPattern, err := patterns.ParsePathPattern(testCase.pattern) c.Check(err, IsNil, Commentf("testCase: %+v", testCase)) expanded := make([]string, 0, pathPattern.NumVariants()) - pathPattern.RenderAllVariants(func(i int, str string) { - expanded = append(expanded, str) + pathPattern.RenderAllVariants(func(i int, variant patterns.PatternVariant) { + expanded = append(expanded, variant.String()) }) c.Check(expanded, DeepEquals, testCase.expanded, Commentf("test case: %+v", testCase)) } @@ -864,3 +866,843 @@ func (s *patternsSuite) TestPathPatternMatchesErrors(c *C) { c.Check(err, Equals, doublestar.ErrBadPattern) c.Check(matches, Equals, false) } + +func (s *patternsSuite) TestHighestPrecedencePattern(c *C) { + for i, testCase := range []struct { + matchingPath string + patterns []string + highestPrecedence string + }{ + // A single pattern + { + "/foo", + []string{ + "/foo", + }, + "/foo", + }, + // Test cases from componentType documentation + // Literal + { + "/foo/bar", + []string{ + "/foo/bar", + "/foo/?ar", + "/foo/ba?", + "/foo/*", + }, + "/foo/bar", + }, + { + "/foo/bar/", + []string{ + "/foo/b*r", + "/foo/b*", + "/foo/b*/", + }, + "/foo/b*r", + }, + { + "/foo/bar", + []string{ + "/f*o/bar", + "/f*/bar", + }, + "/f*o/bar", + }, + // Wildcard '?' + { + "/foo/bar/", + []string{ + "/foo/ba*?", + "/foo/ba*/", + }, + "/foo/ba?*", + }, + { + "/foo/bar", + []string{ + "/foo/?ar", + "/foo/*bar", + }, + "/foo/?ar", + }, + { + "/foo/bar", + []string{ + "/foo/*a?", + "/foo/*r/**", + }, + "/foo/*a?", + }, + // Separator '/' + { + "/foo/bar/", + []string{ + "/foo/bar/", + "/foo/bar", + "/foo/bar*", + "/foo/bar/**", + "/foo/bar/**/", + }, + "/foo/bar/", + }, + { + "/foo/bar", + []string{ + "/foo/bar", + "/foo/**/bar", + }, + "/foo/bar", + }, + // Terminal + { + "/foo/bar/", + []string{ + "/foo/bar", + "/foo/bar/**", + "/foo/bar/**/", + "/foo/bar*", + }, + "/foo/bar", + }, + // Non-terminal "/**" + { + "/foo/bar/", + []string{ + "/foo/**/bar", + "/foo/**/", + "/foo/**", + "/foo*/bar", + }, + "/foo/**/bar", + }, + // Terminal "/**/" + { + "/foo/bar/", + []string{ + "/foo/**/", + "/foo/**", + }, + "/foo/**/", + }, + { + "/foo/bar/", + []string{ + "/foo/bar/**/", + "/foo/bar*", + }, + "/foo/bar/**/", + }, + // Terminal "/**" + { + "/foo/bar", + []string{ + "/foo/bar/**", + "/foo/bar*", + }, + "/foo/bar/**", + }, + // Test cases from Compare documentation + { + "/foo/bar", + []string{ + "/foo/*b*", + "/foo/*a*", + "/foo/*r*", + }, + "/foo/*b*", + }, + { + "/foo/bar", + []string{ + "/foo/bar*", + "/foo/ba*", + "/foo/b*", + }, + "/foo/bar*", + }, + { + "/foo/bar", + []string{ + "/foo/*bar", + "/foo/*ar", + "/foo/*r", + }, + "/foo/*bar", + }, + { + "/foo/bar/bazz/quxxx", + []string{ + "/foo/**/bar/bazz/quxxx", + "/foo/**/bazz/quxxx", + "/foo/**/quxxx", + }, + "/foo/**/bar/bazz/quxxx", + }, + // Related test cases + { + "/foo", + []string{ + "/f?o", + "/fo?", + }, + "/fo?", + }, + { + "/foo/aaa", + []string{ + "/foo/*a?", + "/foo/*a??", + }, + "/foo/*a??", + }, + { + "/foo/aaa", + []string{ + "/foo/*?a", + "/foo/*??a", + }, + "/foo/??*a", + }, + { + "/foo/bar", + []string{ + "/foo/bar", + "/foo/ba?", + "/foo/b??", + }, + "/foo/bar", + }, + // Other test cases + { + "/foo/bar/baz", + []string{ + "/foo/bar/baz", + "/foo/bar/ba?", + }, + "/foo/bar/baz", + }, + { + "/foo/bar/baz", + []string{ + "/foo/bar/b?z", + "/foo/bar/baz", + }, + "/foo/bar/baz", + }, + { + "/foo/bar/baz", + []string{ + "/foo/bar/baz*", + "/foo/bar/baz", + }, + "/foo/bar/baz", + }, + { + "/foo/bar/baz", + []string{ + "/foo/b?r/baz", + "/foo/bar/**", + }, + "/foo/bar/**", + }, + { + "/foo/bar/", + []string{ + "/foo/bar/", + "/foo/bar", + }, + "/foo/bar/", + }, + { + "/foo/bar/", + []string{ + "/foo/bar/", + "/foo/bar/*", + }, + "/foo/bar/", + }, + { + "/foo/bar/", + []string{ + "/foo/bar/", + "/foo/bar/**", + }, + "/foo/bar/", + }, + { + "/foo/bar/", + []string{ + "/foo/bar/", + "/foo/bar/**/", + }, + "/foo/bar/", + }, + { + "/foo/bar", + []string{ + "/foo/bar", + "/foo/bar/**", + }, + "/foo/bar", + }, + { + "/foo/barxbaz", + []string{ + "/foo/bar?baz", + "/foo/bar*baz", + }, + "/foo/bar?baz", + }, + { + "/foo/barxbaz", + []string{ + "/foo/bar?baz", + "/foo/bar**baz", + }, + "/foo/bar?baz", + }, + { + "/foo/bar/baz/", + []string{ + "/foo/bar/baz", + "/foo/b*r/baz/", + }, + "/foo/bar/baz", + }, + { + "/foo/bar/x/baz", + []string{ + "/foo/bar/*/baz", + "/foo/bar/*/*baz", + }, + "/foo/bar/*/baz", + }, + { + "/foo/bar/x/baz", + []string{ + "/foo/bar/*/baz", + "/foo/bar/*/*", + }, + "/foo/bar/*/baz", + }, + { + "/foo/bar/baz/", + []string{ + "/foo/bar/*/", + "/foo/bar/*", + }, + "/foo/bar/*/", + }, + { + "/foo/bar/baz/", + []string{ + "/foo/bar/*/", + "/foo/bar/*/**/", + }, + "/foo/bar/*/", + }, + { + "/foo/bar/baz/", + []string{ + "/foo/bar/*/", + "/foo/bar/*/**", + }, + "/foo/bar/*/", + }, + { + "/foo/bar/x/baz", + []string{ + "/foo/bar/*/*baz", + "/foo/bar/*/*", + }, + "/foo/bar/*/*baz", + }, + { + "/foo/bar/x/baz", + []string{ + "/foo/bar/*/*baz", + "/foo/bar/*/**", + }, + "/foo/bar/*/*baz", + }, + { + "/foo/bar/x/baz", + []string{ + "/foo/bar/*/*", + "/foo/bar/*/**", + }, + "/foo/bar/*/*", + }, + { + "/foo/bar/baz", + []string{ + "/foo/bar/*", + "/foo/bar/*/**", + }, + "/foo/bar/*", + }, + { + "/foo/bar/baz", + []string{ + "/foo/bar/*", + "/foo/bar/**/baz", + }, + "/foo/bar/*", + }, + { + "/foo/bar/baz", + []string{ + "/foo/bar/*/**", + "/foo/bar/**/baz", + }, + "/foo/bar/*/**", + }, + { + "/foo/barxbaz", + []string{ + "/foo/bar*baz", + "/foo/bar*", + }, + "/foo/bar*baz", + }, + { + "/foo/bar/baz", + []string{ + "/foo/bar*/baz", + "/foo/bar*/*", + }, + "/foo/bar*/baz", + }, + { + "/foo/bar/baz", + []string{ + "/foo/bar*/baz", + "/foo/bar*/baz/**", + }, + "/foo/bar*/baz", + }, + { + "/foo/bar/baz", + []string{ + "/foo/bar*/baz", + "/foo/bar/**", + }, + "/foo/bar/**", + }, + { + "/foo/barxxx/xxxbaz", + []string{ + "/foo/bar*/*", + "/foo/bar*/*baz", + }, + "/foo/bar*/*baz", + }, + { + "/foo/barxxx", + []string{ + "/foo/bar*/**", + "/foo/bar*", + }, + "/foo/bar*", + }, + { + "/foo/bar/", + []string{ + "/foo/bar*/", + "/foo/bar*/**", + }, + "/foo/bar*/", + }, + { + "/foo/bar/", + []string{ + "/foo/bar*/", + "/foo/bar*/**/", + }, + "/foo/bar*/", + }, + { + "/foo/bar/", + []string{ + "/foo/bar*/", + "/foo/bar/**/", + }, + "/foo/bar/**/", + }, + { + "/foo/bar/baz/", + []string{ + "/foo/bar*/*baz", + "/foo/bar/**/baz", + }, + "/foo/bar/**/baz", + }, + { + "/foo/bar/baz/", + []string{ + "/foo/bar*/*baz", + "/foo/bar*/**/baz", + }, + "/foo/bar*/*baz", + }, + { + "/foo/bar/x/baz/", + []string{ + "/foo/bar*/*/baz", + "/foo/bar*/*/*", + }, + "/foo/bar*/*/baz", + }, + { + "/foo/bar/x/baz/", + []string{ + "/foo/bar*/*/baz", + "/foo/bar/**/baz", + }, + "/foo/bar/**/baz", + }, + { + "/foo/bar/baz/", + []string{ + "/foo/bar*/*/", + "/foo/bar*/*", + }, + "/foo/bar*/*/", + }, + { + "/foo/bar/baz/", + []string{ + "/foo/bar/**/baz", + "/foo/bar/**/*baz", + }, + "/foo/bar/**/baz", + }, + { + "/foo/bar/baz/", + []string{ + "/foo/bar/**/baz", + "/foo/bar/**", + }, + "/foo/bar/**/baz", + }, + // Prioritize earlier matches after /**/ + { + "/foo/bar/fizz/buzz/file.txt", + []string{ + "/foo/bar/**/fizz/**", + "/foo/bar/**/buzz/**", + }, + "/foo/bar/**/fizz/**", + }, + { + "/foo/bar/buzz/fizz/file.txt", + []string{ + "/foo/bar/**/fizz/**", + "/foo/bar/**/buzz/**", + }, + "/foo/bar/**/buzz/**", + }, + { + "/foo/bar/baz/", + []string{ + "/foo/bar/**/*baz/", + "/foo/bar/**/*baz", + }, + "/foo/bar/**/*baz/", + }, + { + "/foo/bar/baz/", + []string{ + "/foo/bar/**/*baz/", + "/foo/bar/**/", + }, + "/foo/bar/**/*baz/", + }, + { + "/foo/bar/baz/", + []string{ + "/foo/bar/**/*baz", + "/foo/bar/**/", + }, + "/foo/bar/**/*baz", + }, + { + "/foo/bar/x/baz", + []string{ + "/foo/bar/**/*baz", + "/foo/bar/**", + }, + "/foo/bar/**/*baz", + }, + { + "/foo/bar/fizz/buzz/baz", + []string{ + "/foo/bar/**/*baz", + "/foo/bar*/**/baz", + }, + "/foo/bar/**/*baz", + }, + { + "/foo/bar/fizz/buzz/baz/", + []string{ + "/foo/bar/**/", + "/foo/bar/**", + }, + "/foo/bar/**/", + }, + { + "/foo/bar/fizz/buzz/baz/", + []string{ + "/foo/bar/**/", + "/foo/bar*/**/baz/", + }, + "/foo/bar/**/", + }, + { + "/foo/bar/fizz/buzz/baz/", + []string{ + "/foo/bar/**", + "/foo/bar*/**/baz/", + }, + "/foo/bar/**", + }, + { + "/foo/bar/fizz/buzz/baz/", + []string{ + "/foo/bar*/**/baz", + "/foo/bar*/**/", + }, + "/foo/bar*/**/baz", + }, + { + "/foo/barfizz/buzz/baz/", + []string{ + "/foo/bar*/**/", + "/foo/bar*/**", + }, + "/foo/bar*/**/", + }, + { + "/foo/bar/file.tar.gz", + []string{ + "/foo/bar/*.gz", + "/foo/bar/*.tar.gz", + }, + "/foo/bar/*.tar.gz", + }, + { + "/foo/bar/file.tar.gz", + []string{ + "/foo/bar/**/*.gz", + "/foo/**/*.tar.gz", + }, + "/foo/bar/**/*.gz", + }, + { + "/foo/bar/x/y/z/file.tar.gz", + []string{ + "/foo/bar/x/**/*.gz", + "/foo/bar/**/*.tar.gz", + }, + "/foo/bar/x/**/*.gz", + }, + { + "/foo/bar/file.tar.gz", + []string{ + "/foo/bar/**/*.tar.gz", + "/foo/bar/*", + }, + "/foo/bar/*", + }, + { + "/foo/bar/baz/x/y/z/file.txt", + []string{ + "/foo/bar/**", + "/foo/bar/baz/**", + "/foo/bar/baz/**/*.txt", + }, + "/foo/bar/baz/**/*.txt", + }, + { + "/foo/bar", + []string{ + "/foo/bar*", + "/foo/bar/**", + }, + "/foo/bar/**", + }, + { + `/foo/\`, + []string{ + `/foo/\\`, + `/foo/*/**`, + }, + `/foo/\\`, + }, + { + `/foo/*fizz/bar/x*`, + []string{ + `/foo/\**/b\ar/*\*`, + `/foo/*/bar/x\*`, + }, + `/foo/\**/bar/*\*`, + }, + { + "/foo/barxxxbaz", + []string{ + "/foo/bar**", + "/foo/bar**baz", + }, + "/foo/bar*baz", + }, + { + "/foo/xxxbar", + []string{ + "/foo/**", + "/foo/**bar", + }, + "/foo/*bar", + }, + { + "/foo/x/y/z/bar/baz/", + []string{ + "/foo/**/bar/*/", + "/foo/**/b?r/baz/", + }, + "/foo/**/bar/*/", + }, + { + "/foo/x/y/z/bar/fizz", + []string{ + "/foo/**/fizz", + "/foo/**/bar/fizz", + }, + "/foo/**/bar/fizz", + }, + { + "/foo/x/y/z/bar", + []string{ + "/foo/**/*/bar", + "/foo/**/*", + }, + "/foo/*/**/bar", + }, + { + "/foo/bar", + []string{ + "/foo/**/*", + "/**", + }, + "/foo/**", + }, + // Duplicate patterns should never be passed into HighestPrecedencePattern, + // but if they are, handle them correctly. + { + "/foo/bar/", + []string{ + "/foo/bar/", + "/foo/bar/", + }, + "/foo/bar/", + }, + { + "/foo/bar/", + []string{ + "/foo/bar/", + "/foo/bar/", + "/foo/bar", + }, + "/foo/bar/", + }, + { + "/foo/bar/baz/", + []string{ + "/foo/bar/**", + "/foo/bar/**", + "/foo/bar/*", + }, + "/foo/bar/*", + }, + } { + variants := make([]patterns.PatternVariant, len(testCase.patterns)) + variantsReversed := make([]patterns.PatternVariant, len(testCase.patterns)) + for i, pattern := range testCase.patterns { + variant, err := patterns.ParsePatternVariant(pattern) + c.Assert(err, IsNil, Commentf("pattern: %s", pattern)) + variants[i] = variant + variantsReversed[len(variantsReversed)-1-i] = variant + // Check that the rendered variant actually matches the path + matches, err := patterns.PathPatternMatches(variant.String(), testCase.matchingPath) + c.Check(err, IsNil, Commentf("testCase: %+v\npath: %s\nvariant: %s", testCase, testCase.matchingPath, variant.String())) + c.Check(matches, Equals, true, Commentf("testCase: %+v\npath: %s\nvariant: %s", testCase, testCase.matchingPath, variant.String())) + } + highestPrecedence, err := patterns.HighestPrecedencePattern(variants, testCase.matchingPath) + c.Check(err, IsNil, Commentf("Error occurred during test case %d:\n%+v\nerror: %v", i, testCase, err)) + if err != nil { + continue + } + c.Check(highestPrecedence.String(), Equals, testCase.highestPrecedence, Commentf("Highest precedence pattern incorrect for test case %d:\n%+v", i, testCase)) + highestPrecedence, err = patterns.HighestPrecedencePattern(variantsReversed, testCase.matchingPath) + c.Check(err, IsNil, Commentf("Error occurred during test case %d:\n%+v\nerror: %v", i, testCase, err)) + if err != nil { + continue + } + c.Check(highestPrecedence.String(), Equals, testCase.highestPrecedence, Commentf("Highest precedence pattern incorrect for reversed test case %d:\n%+v", i, testCase)) + } +} + +func (s *patternsSuite) TestHighestPrecedencePatternOrdered(c *C) { + matchingPath := "/foo/bar/baz/myfile.txt" + orderedPatterns := []string{ + "/foo/bar/baz/myfile.txt", + "/foo/bar/baz/m?file.*", + "/foo/bar/baz/m*file.txt", + "/foo/bar/baz/m*file*", + "/foo/bar/baz/*", + "/foo/bar/*/myfile.txt", + "/foo/bar/*/myfile*", + "/foo/bar/*/*.txt", + "/foo/bar/*/*", + "/foo/bar/**/baz/myfile.txt", + "/foo/bar/**/baz/*.txt", + "/foo/bar/**/baz/*", + "/foo/bar/**/myfile.txt", + "/foo/bar/**", + "/foo/ba*r/baz/myfile.txt", + "/foo/b?r/baz/myfile.txt", + "/foo/b*r/baz/myfile.txt", + "/foo/?*/baz/myfile.txt", + "/foo/?*/**", + "/foo/*/baz/myfile.txt", + "/foo/*/baz/*", + "/foo/*/*/*", + "/foo/*/**/baz/myfile.txt", + "/foo/**/bar/baz/myfile.txt", + "/foo/**/baz/myfile.txt", + "/foo/**/baz/**", + "/foo/**/myfile.txt", + "/**/foo/bar/baz/myfile.txt", + "/**/myfile.txt", + "/**", + } + for i := 0; i < len(orderedPatterns); i++ { + window := orderedPatterns[i:] + variants := make([]patterns.PatternVariant, 0, len(window)) + for _, pattern := range window { + variant, err := patterns.ParsePatternVariant(pattern) + c.Assert(err, IsNil, Commentf("pattern: %s", pattern)) + variants = append(variants, variant) + } + result, err := patterns.HighestPrecedencePattern(variants, matchingPath) + c.Assert(err, IsNil, Commentf("Error occurred while computing precedence between %v: %v", window, err)) + c.Check(result.String(), Equals, window[0], Commentf("patterns: %+v", window)) + } +} + +func (s *patternsSuite) TestHighestPrecedencePatternUnhappy(c *C) { + result, err := patterns.HighestPrecedencePattern([]patterns.PatternVariant{}, "") + c.Check(err, Equals, patterns.ErrNoPatterns) + c.Check(result, DeepEquals, patterns.PatternVariant{}) +} diff --git a/interfaces/prompting/patterns/render.go b/interfaces/prompting/patterns/render.go index 838b76e34ae..90fba610597 100644 --- a/interfaces/prompting/patterns/render.go +++ b/interfaces/prompting/patterns/render.go @@ -23,6 +23,8 @@ import ( "bytes" "errors" "strings" + + "github.com/snapcore/snapd/logger" ) // variantState is the current variant of a render node. @@ -63,7 +65,7 @@ type renderNode interface { // for each group in the path pattern. The given observe closure is then called // on each variant, along with its index, and it can perform some action with // the variant, such as adding it to a data structure. -func renderAllVariants(n renderNode, observe func(index int, variant string)) { +func renderAllVariants(n renderNode, observe func(index int, variant PatternVariant)) { var buf bytes.Buffer v, length := n.InitialVariant() @@ -74,7 +76,13 @@ func renderAllVariants(n renderNode, observe func(index int, variant string)) { buf.Truncate(lengthUnchanged) buf.Grow(length - lengthUnchanged) v.Render(&buf, lengthUnchanged) - observe(i, buf.String()) + variant, err := parsePatternVariant(buf.String()) + if err != nil { + // This should never occur + logger.Noticef("patterns: cannot parse pattern variant '%s': %v", variant, err) + continue + } + observe(i, variant) length, lengthUnchanged, moreRemain = v.NextVariant() } } diff --git a/interfaces/prompting/patterns/render_internal_test.go b/interfaces/prompting/patterns/render_internal_test.go index 6ca3dac4c92..2252515f335 100644 --- a/interfaces/prompting/patterns/render_internal_test.go +++ b/interfaces/prompting/patterns/render_internal_test.go @@ -86,8 +86,8 @@ func (s *renderSuite) TestRenderAllVariants(c *C) { } variants := make([]string, 0, len(expectedVariants)) - renderAllVariants(parsed, func(index int, variant string) { - variants = append(variants, variant) + renderAllVariants(parsed, func(index int, variant PatternVariant) { + variants = append(variants, variant.String()) }) c.Check(variants, DeepEquals, expectedVariants) diff --git a/interfaces/prompting/patterns/variant.go b/interfaces/prompting/patterns/variant.go new file mode 100644 index 00000000000..75bbf4f2b39 --- /dev/null +++ b/interfaces/prompting/patterns/variant.go @@ -0,0 +1,505 @@ +// -*- Mode: Go; indent-tabs-mode: t -*- + +/* + * Copyright (C) 2024 Canonical Ltd + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package patterns + +import ( + "errors" + "fmt" + "io" + "regexp" + "strings" +) + +type componentType int + +// Component types in order from lowest to highest precedence. +// +// A literal exactly matches the next characters in the path, so it has the +// highest precedence. +// - `/foo/bar` has precedence over `/foo/ba?`, `/foo/?ar`, and `/foo/*` +// - `/foo/b*r` has precedence over `/foo/b*` and `/foo/b*/` (though this is caught by match length of '*') +// - `/f*o/bar` has precedence over `/f*/bar` (though this is caught by match length of '*') +// +// A wildcard '?' character matches exactly one non-separator character, so it +// has precedence over variable-width components such as globstars and double- +// stars. The parser ensures that all '?'s are moved before any adjacent '*', +// so a '?' can never match at the same position as a separator or terminal, +// without precedence having decided by a previous component. Thus, it doesn't +// actually matter whether '?' has precedence over '/' or terminal. +// - `/foo/ba*?` (parsed to `/foo/ba?*`) has precedence over `/foo/ba*/` +// - `/foo/?ar` has precedence over `/foo/*bar` +// - `/foo/*a?` has precedence over `/foo/*r/**` (though this is caught by match length of '*') +// +// A separator '/' character is like a literal, except that when following a +// '*', it precludes any more information being given about the length or +// content of the path prior to the separator. Thus, '/' has lower precedence +// than literals, though in practice, any situation where '/' would be compared +// against a literal must follow a variable-length component (e.g. '*' or "/**") +// where the component prior to the literal would match fewer characters in the +// path than the component prior to the '/', and thus precedence would already +// be given to the component prior to the literal. Patterns with trailing '/' +// match only directories, while patterns without match both files and +// directories, so '/' has precedence over terminals. +// - `/foo/bar/` has precedence over `/foo/bar` and `/foo/bar*` +// - `/foo/bar/` has precedence over `/foo/bar/**` and `/foo/bar/**/` +// - `/foo/bar` has precedence over `/foo/**/bar` +// +// Terminals match when there is no path left, so they have precedence over all +// the variable-length component types, which may match zero or more characters. +// - `/foo/bar` has precedence over `/foo/bar/**/`, `/foo/bar/**` +// - `/foo/bar` has precedence over `/foo/bar*` +// +// The remaining component types are variable-width, as they may match zero or +// more characters in the path. When two variable-width components of the same +// type are compared, the one which matches fewer characters in the path has +// precedence, since that means that the next component matches earlier in the +// path, and thus provides greater specificity earlier. +// +// The next three component types relate to doublestars, which always follow a +// '/' character, and may match zero or more characters; in order to know +// whether a '/' is followed by a "**" or not without looking ahead in the list +// of components, we group these components together, along with their suffix +// when relevant. Since "/**" components match zero or more characters, +// including the terminal in the component allows us to know whether there are +// more non-terminal components to come without needing to look ahead. +// +// The non-terminal "/**" must be followed by a component which gives more +// information about the matching path, so it has the highest precedence of the +// three doublestar component types. +// - `/foo/**/bar` has precedence over `/foo/**/` and `/foo/**` +// - `/foo/**/bar` has precedence over `/foo*/bar +// +// The terminal "/**/" component means that the variant only matches +// directories, while the terminal "/**" component can match files or +// directories, so the former has precedence over the latter. +// - `/foo/**/` has precedence over `/foo/**` +// - `/foo/bar/**/` has precedence over `/foo/bar*` +// +// The terminal "/**" has lower precedence than "/**/", as discussed above. +// However, it still has higher precedence than '*', since the leading separator +// which is built into the "/**" puts a constraint on the length/content of the +// path segment preceding it, while '*' does not. +// - `/foo/bar/**` has precedence over `/foo/bar*` +// +// The globstar '*' has the lowest precedence, since all other component +// types begin with more information about the length or content of the next +// characters in the path: as discussed above, "/foo/**" has precedence over +// "/foo*" since the former matches "/foo" exactly or a path in the "/foo" +// directory, while "/foo*" matches any path which happens to begin with "/foo". +const ( + compUnset componentType = iota + compGlobstar + compSeparatorDoublestarTerminal // need to bundle separator and terminal marker with /** + compSeparatorDoublestarSeparatorTerminal // need to bundle separators and terminal marker with /**/ + compSeparatorDoublestar + compTerminal // marker of end of pattern. + compSeparator + compAnySingle // ? has precedence over / so that /foo*?/ has precedence over /foo*/ + compLiteral +) + +type component struct { + compType componentType + compText string +} + +// String returns the globstar-style pattern string associated with the given +// component. +func (c component) String() string { + switch c.compType { + case compGlobstar: + return "*" + case compSeparatorDoublestarTerminal: + return "/**" + case compSeparatorDoublestarSeparatorTerminal: + return "/**/" + case compSeparatorDoublestar: + return "/**" + case compTerminal: // end of pattern + return "" + case compSeparator: + return "/" + case compAnySingle: + return "?" + case compLiteral: + return c.compText + } + panic(fmt.Sprintf("unknown component type: %d", int(c.compType))) +} + +// componentRegex returns a regular expression corresponding to the bash-style +// globstar matching behavior of the receiving component. +// +// For example, "*" matches any non-separator characters, so we return the regex +// `((?:[^/]|\\/)*)` for the globstar component type. +// +// The returned regexps should each be enclosed in a capturing group with no +// capturing groups within. This allows a single regex to be constructed for a +// given pattern variant by concatenating all the component regular expressions +// together, and the resulting regex has exactly one capturing group for each +// component, in order. +func (c component) componentRegex() string { + switch c.compType { + case compGlobstar: + return `((?:[^/]|\\/)*)` + case compSeparatorDoublestarTerminal: + return `((?:/.+)?/?)` + case compSeparatorDoublestarSeparatorTerminal: + return `((?:/.+)?/)` + case compSeparatorDoublestar: + return `((?:/.+)?)` + case compTerminal: + return `(/?)` + case compSeparator: + return `(/)` + case compAnySingle: + return `([^/])` // does escaped '/' (e.g. `\\/`) count as one character? + case compLiteral: + return `(` + regexp.QuoteMeta(unescapeLiteral(c.compText)) + `)` + } + return `()` +} + +var escapeFinder = regexp.MustCompile(`\\(.)`) + +// unescapeLiteral removes any `\` characters which are used to escape another +// character. Note that escaped `\` characters are not removed, since they are +// not acting as an escape character in those instances. That is, `\\` is +// reduced to `\`. +func unescapeLiteral(literal string) string { + return escapeFinder.ReplaceAllString(literal, "${1}") +} + +type PatternVariant struct { + variant string + components []component + regex *regexp.Regexp +} + +// String returns the rendered string associated with the pattern variant. +func (v PatternVariant) String() string { + return v.variant +} + +// parsePatternVariant parses a rendered variant string into a PatternVariant +// whose precedence can be compared against others. +func parsePatternVariant(variant string) (PatternVariant, error) { + var components []component + var runes []rune + + // prevComponentsAre returns true if the most recent components have types + // matching the given target types from least recent to most recent. + prevComponentsAre := func(target []componentType) bool { + if len(components) < len(target) { + return false + } + for i, t := range target { + if components[len(components)-len(target)+i].compType != t { + return false + } + } + return true + } + + // addGlobstar adds a globstar to the components if the previous component + // was not a globstar or a doublestar. + addGlobstar := func() { + if !prevComponentsAre([]componentType{compGlobstar}) && !prevComponentsAre([]componentType{compSeparatorDoublestar}) { + components = append(components, component{compType: compGlobstar}) + } + } + + // reducePrevDoublestar checks if the most recent component was a + // doublestar, and if it was, replaces it with a globstar '*'. + // This is necessary because a doublestar followed by anything except a + // separator is treated as a globstar '*' instead. + reducePrevDoublestar := func() { + if prevComponentsAre([]componentType{compSeparatorDoublestar}) { + // SeparatorDoublestar followed by anything except separator should + // replaced by a separator '/' and globstar '*'. + components[len(components)-1] = component{compType: compSeparator} + components = append(components, component{compType: compGlobstar}) + } + } + + // consumeText writes any accumulated runes as a literal component. + consumeText := func() { + if len(runes) > 0 { + reducePrevDoublestar() + components = append(components, component{compType: compLiteral, compText: string(runes)}) + runes = nil + } + } + + preparedVariant := prepareVariantForParsing(variant) + + rr := strings.NewReader(preparedVariant) + for { + r, _, err := rr.ReadRune() + if err != nil { + if errors.Is(err, io.EOF) { + break + } + // Should not occur, err is only set if no rune available to read + return PatternVariant{}, fmt.Errorf("internal error: failed to read rune while scanning variant: %w", err) + } + + switch r { + case '/': + consumeText() + if prevComponentsAre([]componentType{compSeparatorDoublestar, compSeparator, compGlobstar}) { + // Replace previous /**/* with /*/** before adding separator + components[len(components)-3] = component{compType: compSeparator} + components[len(components)-2] = component{compType: compGlobstar} + components[len(components)-1] = component{compType: compSeparatorDoublestar} + } + if !prevComponentsAre([]componentType{compSeparator}) { + // Collapse repeated separators + components = append(components, component{compType: compSeparator}) + } + case '?': + reducePrevDoublestar() + consumeText() + if prevComponentsAre([]componentType{compGlobstar}) { + // Insert '?' before previous '*' + components[len(components)-1] = component{compType: compAnySingle} + components = append(components, component{compType: compGlobstar}) + } else { + components = append(components, component{compType: compAnySingle}) + } + case '⁑': + consumeText() + if prevComponentsAre([]componentType{compSeparatorDoublestar, compSeparator}) { + // Reduce /**/** to simply /** by removing most recent separator + components = components[:len(components)-1] + } else if prevComponentsAre([]componentType{compSeparator}) { + // Replace previous separator with separatorDoublestar + components[len(components)-1] = component{compType: compSeparatorDoublestar} + } else { + // Reduce to * since previous component is not a separator + addGlobstar() + } + case '*': + reducePrevDoublestar() + consumeText() + addGlobstar() + case '\\': + r2, _, err := rr.ReadRune() + if err != nil { + // Should be impossible, we just rendered this variant + return PatternVariant{}, errors.New(`internal error: trailing unescaped '\' character`) + } + switch r2 { + case '*', '?', '[', ']', '{', '}', '\\': + runes = append(runes, r, r2) + default: + // do not add r to runes if it's unnecessary + runes = append(runes, r2) + } + case '[', ']', '{', '}': + // Should be impossible, we just rendered this variant + return PatternVariant{}, fmt.Errorf(`internal error: unexpected unescaped '%v' character`, r) + default: + runes = append(runes, r) + } + } + + consumeText() + + if prevComponentsAre([]componentType{compSeparatorDoublestar, compSeparator, compGlobstar}) { + // If components end with /**/*, strip trailing /* + components = components[:len(components)-2] + } + + // Add terminal marker or convert existing doublestar to terminal doublestar + if prevComponentsAre([]componentType{compSeparatorDoublestar, compSeparator}) { + components = components[:len(components)-2] + components = append(components, component{compType: compSeparatorDoublestarSeparatorTerminal}) + } else if prevComponentsAre([]componentType{compSeparatorDoublestar}) { + components[len(components)-1] = component{compType: compSeparatorDoublestarTerminal} + } else { + components = append(components, component{compType: compTerminal}) + } + + var variantBuf strings.Builder + var regexBuf strings.Builder + regexBuf.WriteRune('^') + for _, c := range components { + variantBuf.WriteString(c.String()) + regexBuf.WriteString(c.componentRegex()) + } + regexBuf.WriteRune('$') + regex := regexpMustCompileLongest(regexBuf.String()) + + v := PatternVariant{ + variant: variantBuf.String(), + components: components, + regex: regex, + } + + return v, nil +} + +// regexpMustCompileLongest compiles the given string into a Regexp and then +// calls Longest() on it before returning it. +func regexpMustCompileLongest(str string) *regexp.Regexp { + re := regexp.MustCompile(str) + re.Longest() + return re +} + +// Need to escape any unescaped literal "⁑" runes before we use that symbol to +// indicate the presence of a "**" doublestar. +var doublestarEscaper = regexpMustCompileLongest(`((\\)*)⁑`) + +// Need to replace unescaped "**", but must be careful about an escaped '\\' +// before the first '*', since that doesn't escape the first '*'. +var doublestarReplacer = regexpMustCompileLongest(`((\\)*)\*\*`) + +// prepareVariantForParsing escapes any unescaped '⁑' characters and then +// replaces any unescaped "**" with a single '⁑' so that doublestars can be +// identified without needing to look ahead whenever a '*' is seen. +func prepareVariantForParsing(variant string) string { + escaped := doublestarEscaper.ReplaceAllStringFunc(variant, func(s string) string { + if (len(s)-len("⁑"))%2 == 1 { + // Odd number of leading '\\'s, so already escaped + return s + } + // Escape any unescaped literal "⁑" + return s[:len(s)-len("⁑")] + `\` + "⁑" + }) + prepared := doublestarReplacer.ReplaceAllStringFunc(escaped, func(s string) string { + if (len(s)-len("**"))%2 == 1 { + // Odd number of leading '\\'s, so escaped + return s + } + // Discard trailing "**", add "⁑" instead + return s[:len(s)-2] + "⁑" + }) + return prepared +} + +type componentReader struct { + components []component + submatches []string + index int +} + +func (r *componentReader) next() (*component, string) { + if r.index >= len(r.components) { + return &component{compType: compTerminal}, "" + } + comp := &r.components[r.index] + submatch := r.submatches[r.index] + r.index++ + return comp, submatch +} + +// Compare returns the relative precence of the receiver and the given pattern +// variant when considering the given matching path. +// +// Returns one of the following, if no error occurs: +// -1 if v has lower precedence than other +// 0 if v and other have equal precedence (only possible if v == other) +// 1 if v has higher precedence than other. +func (v PatternVariant) Compare(other PatternVariant, matchingPath string) (int, error) { + selfSubmatches := v.regex.FindStringSubmatch(matchingPath) + switch { + case selfSubmatches == nil: + return 0, fmt.Errorf("internal error: no matches for pattern variant against given path:\ncomponents: %+v\nregex: %s\npath: %s", v.components, v.regex.String(), matchingPath) + case len(selfSubmatches)-1 != len(v.components): + return 0, fmt.Errorf("internal error: submatch count not equal to component count:\ncomponents: %+v\nregex: %s\npath: %s", v.components, v.regex.String(), matchingPath) + } + + otherSubmatches := other.regex.FindStringSubmatch(matchingPath) + if otherSubmatches == nil { + return 0, fmt.Errorf("internal error: no matches for pattern variant against given path\ncomponents: %+v\nregex: %s\npath: %s", other.components, other.regex.String(), matchingPath) + } else if len(otherSubmatches)-1 != len(other.components) { + return 0, fmt.Errorf("internal error: submatch count not equal to component count:\ncomponents: %+v\nregex: %s\npath: %s", other.components, other.regex.String(), matchingPath) + } + + selfReader := componentReader{components: v.components, submatches: selfSubmatches[1:]} + otherReader := componentReader{components: other.components, submatches: otherSubmatches[1:]} + +loop: + for { + selfComp, selfSubmatch := selfReader.next() + otherComp, otherSubmatch := otherReader.next() + if selfComp.compType < otherComp.compType { + return -1, nil + } else if selfComp.compType > otherComp.compType { + return 1, nil + } + switch selfComp.compType { + case compGlobstar, compSeparatorDoublestar: + // Prioritize shorter matches for variable-width non-terminal + // components. We do this because the fewer characters are matched + // by a variable-width component (i.e. "*" or "/**", as terminal + // doublestar characters always match to the end of the path), the + // earlier in the path the next component matches, and thus provides + // provides greater specificity. Given equality up to the current + // position in the patterns, whichever pattern matches with greater + // specificity earlier in the path has precedence. + // + // For example, when computing precedence after matching `/foo/bar`: + // - `/foo/*b*` has precedence over + // - `/foo/*a*`, which has precedence over + // - `/foo/*r*`. + // All these patterns are {separator, literal, separator, globstar, + // literal, globstar, terminal}, but the distinction is that the + // globstar which matches fewer characters in the pattern implies + // that the next literal in the pattern matches earlier in the path, + // and so that pattern has precedence. + // + // Note: similar logic applies to: + // - `/foo/bar*` vs `/foo/ba*` vs `/foo/b*` and + // - `/foo/*bar` vs `/foo/*ar` vs `/foo/*r`. + // In these cases, however, the relative lengths of the literals + // would be sufficient to determine precedence. + // + // Likewise, all of the following match `/foo/bar/bazz/quxxx`: + // - `/foo/**/bar/bazz/quxxx` has precedence over + // - `/foo/**/bazz/quxxx`, which has precedence over + // - `/foo/**/quxxx`. + // If not for the precedence for fewest characters matched by the + // `/**`, the longest literal after the next separator, `quxxx`, + // would take precedence incorrectly. By prioritizing the doublestar + // which matches the fewest characters, the next component matches + // the earliest position in the path, and thus indicates precedence. + if len(selfSubmatch) > len(otherSubmatch) { + return -1, nil + } else if len(selfSubmatch) < len(otherSubmatch) { + return 1, nil + } + case compSeparatorDoublestarTerminal, compSeparatorDoublestarSeparatorTerminal, compTerminal: + break loop + case compLiteral: + // Prioritize longer literals (which must match exactly) + if selfSubmatch < otherSubmatch { + return -1, nil + } else if selfSubmatch > otherSubmatch { + return 1, nil + } + default: + continue + } + } + return 0, nil +} diff --git a/interfaces/prompting/patterns/variant_internal_test.go b/interfaces/prompting/patterns/variant_internal_test.go new file mode 100644 index 00000000000..79684d3b1c8 --- /dev/null +++ b/interfaces/prompting/patterns/variant_internal_test.go @@ -0,0 +1,371 @@ +// -*- Mode: Go; indent-tabs-mode: t -*- + +/*", + * Copyright (C) 2024 Canonical Ltd + *", + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * published by the Free Software Foundation. + *", + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + *", + * You should have received a copy of the GNU General Public License + * along with this program. If not, Text: see . + *", + */ + +package patterns + +import ( + . "gopkg.in/check.v1" +) + +type variantSuite struct{} + +var _ = Suite(&variantSuite{}) + +func (s *variantSuite) TestParsePatternVariant(c *C) { + for _, testCase := range []struct { + pattern string + preparedPattern string + components []component + variantStr string + }{ + { + "/foo/bar/baz", + "/foo/bar/baz", + []component{ + {compType: compSeparator}, + {compType: compLiteral, compText: "foo"}, + {compType: compSeparator}, + {compType: compLiteral, compText: "bar"}, + {compType: compSeparator}, + {compType: compLiteral, compText: "baz"}, + {compType: compTerminal}, + }, + "/foo/bar/baz", + }, + { + "/foo/bar/baz/", + "/foo/bar/baz/", + []component{ + {compType: compSeparator}, + {compType: compLiteral, compText: "foo"}, + {compType: compSeparator}, + {compType: compLiteral, compText: "bar"}, + {compType: compSeparator}, + {compType: compLiteral, compText: "baz"}, + {compType: compSeparator}, + {compType: compTerminal}, + }, + "/foo/bar/baz/", + }, + { + "/?o*/b?r/*a?/", + "/?o*/b?r/*a?/", + []component{ + {compType: compSeparator}, + {compType: compAnySingle}, + {compType: compLiteral, compText: "o"}, + {compType: compGlobstar}, + {compType: compSeparator}, + {compType: compLiteral, compText: "b"}, + {compType: compAnySingle}, + {compType: compLiteral, compText: "r"}, + {compType: compSeparator}, + {compType: compGlobstar}, + {compType: compLiteral, compText: "a"}, + {compType: compAnySingle}, + {compType: compSeparator}, + {compType: compTerminal}, + }, + "/?o*/b?r/*a?/", + }, + { + "/foo////bar", + "/foo////bar", + []component{ + {compType: compSeparator}, + {compType: compLiteral, compText: "foo"}, + {compType: compSeparator}, + {compType: compLiteral, compText: "bar"}, + {compType: compTerminal}, + }, + "/foo/bar", + }, + { + "/foo**/bar", + "/foo⁑/bar", + []component{ + {compType: compSeparator}, + {compType: compLiteral, compText: "foo"}, + {compType: compGlobstar}, + {compType: compSeparator}, + {compType: compLiteral, compText: "bar"}, + {compType: compTerminal}, + }, + "/foo*/bar", + }, + { + "/foo/**bar", + "/foo/⁑bar", + []component{ + {compType: compSeparator}, + {compType: compLiteral, compText: "foo"}, + {compType: compSeparator}, + {compType: compGlobstar}, + {compType: compLiteral, compText: "bar"}, + {compType: compTerminal}, + }, + "/foo/*bar", + }, + { + "/foo/**/**/bar", + "/foo/⁑/⁑/bar", + []component{ + {compType: compSeparator}, + {compType: compLiteral, compText: "foo"}, + {compType: compSeparatorDoublestar}, + {compType: compSeparator}, + {compType: compLiteral, compText: "bar"}, + {compType: compTerminal}, + }, + "/foo/**/bar", + }, + { + "/foo/**/*/bar", + "/foo/⁑/*/bar", + []component{ + {compType: compSeparator}, + {compType: compLiteral, compText: "foo"}, + {compType: compSeparator}, + {compType: compGlobstar}, + {compType: compSeparatorDoublestar}, + {compType: compSeparator}, + {compType: compLiteral, compText: "bar"}, + {compType: compTerminal}, + }, + "/foo/*/**/bar", + }, + { + "/foo/**/**/", + "/foo/⁑/⁑/", + []component{ + {compType: compSeparator}, + {compType: compLiteral, compText: "foo"}, + {compType: compSeparatorDoublestarSeparatorTerminal}, + }, + "/foo/**/", + }, + { + "/foo/**/**", + "/foo/⁑/⁑", + []component{ + {compType: compSeparator}, + {compType: compLiteral, compText: "foo"}, + {compType: compSeparatorDoublestarTerminal}, + }, + "/foo/**", + }, + { + "/foo/**/*", + "/foo/⁑/*", + []component{ + {compType: compSeparator}, + {compType: compLiteral, compText: "foo"}, + {compType: compSeparatorDoublestarTerminal}, + }, + "/foo/**", + }, + { + "/foo/**?/*?*?*", + "/foo/⁑?/*?*?*", + []component{ + {compType: compSeparator}, + {compType: compLiteral, compText: "foo"}, + {compType: compSeparator}, + {compType: compAnySingle}, + {compType: compGlobstar}, + {compType: compSeparator}, + {compType: compAnySingle}, + {compType: compAnySingle}, + {compType: compGlobstar}, + {compType: compTerminal}, + }, + "/foo/?*/??*", + }, + { + "/foo/**?/***?***?***", + "/foo/⁑?/⁑*?⁑*?⁑*", + []component{ + {compType: compSeparator}, + {compType: compLiteral, compText: "foo"}, + {compType: compSeparator}, + {compType: compAnySingle}, + {compType: compGlobstar}, + {compType: compSeparator}, + {compType: compAnySingle}, + {compType: compAnySingle}, + {compType: compGlobstar}, + {compType: compTerminal}, + }, + "/foo/?*/??*", + }, + // Check that unicode in patterns treated as a single rune, and that escape + // characters are not counted, even when escaping unicode runes. + { + "/foo/🚵🚵", + "/foo/🚵🚵", + []component{ + {compType: compSeparator}, + {compType: compLiteral, compText: "foo"}, + {compType: compSeparator}, + {compType: compLiteral, compText: "🚵🚵"}, + {compType: compTerminal}, + }, + "/foo/🚵🚵", + }, + { + `/foo/\🚵\🚵\🚵\🚵\🚵`, + `/foo/\🚵\🚵\🚵\🚵\🚵`, + []component{ + {compType: compSeparator}, + {compType: compLiteral, compText: "foo"}, + {compType: compSeparator}, + {compType: compLiteral, compText: `🚵🚵🚵🚵🚵`}, + {compType: compTerminal}, + }, + `/foo/🚵🚵🚵🚵🚵`, + }, + { + `/foo/\\`, + `/foo/\\`, + []component{ + {compType: compSeparator}, + {compType: compLiteral, compText: "foo"}, + {compType: compSeparator}, + {compType: compLiteral, compText: `\\`}, + {compType: compTerminal}, + }, + `/foo/\\`, + }, + { + `/foo/⁑⁑⁑⁑⁑`, + `/foo/\⁑\⁑\⁑\⁑\⁑`, + []component{ + {compType: compSeparator}, + {compType: compLiteral, compText: "foo"}, + {compType: compSeparator}, + {compType: compLiteral, compText: `⁑⁑⁑⁑⁑`}, + {compType: compTerminal}, + }, + `/foo/⁑⁑⁑⁑⁑`, + }, + { + `/foo/⁑\\⁑\\⁑\\⁑\\⁑`, + `/foo/\⁑\\\⁑\\\⁑\\\⁑\\\⁑`, + []component{ + {compType: compSeparator}, + {compType: compLiteral, compText: "foo"}, + {compType: compSeparator}, + {compType: compLiteral, compText: `⁑\\⁑\\⁑\\⁑\\⁑`}, + {compType: compTerminal}, + }, + `/foo/⁑\\⁑\\⁑\\⁑\\⁑`, + }, + { + `/foo/⁑\⁑\\⁑\\\⁑\\\\⁑`, + `/foo/\⁑\⁑\\\⁑\\\⁑\\\\\⁑`, + []component{ + {compType: compSeparator}, + {compType: compLiteral, compText: "foo"}, + {compType: compSeparator}, + {compType: compLiteral, compText: `⁑⁑\\⁑\\⁑\\\\⁑`}, + {compType: compTerminal}, + }, + `/foo/⁑⁑\\⁑\\⁑\\\\⁑`, + }, + { + `/foo/**********`, + `/foo/⁑⁑⁑⁑⁑`, + []component{ + {compType: compSeparator}, + {compType: compLiteral, compText: "foo"}, + {compType: compSeparatorDoublestarTerminal}, + }, + `/foo/**`, + }, + { + `/foo/\**\**\**\**\**`, + `/foo/\**\**\**\**\**`, + []component{ + {compType: compSeparator}, + {compType: compLiteral, compText: "foo"}, + {compType: compSeparator}, + {compType: compLiteral, compText: `\*`}, + {compType: compGlobstar}, + {compType: compLiteral, compText: `\*`}, + {compType: compGlobstar}, + {compType: compLiteral, compText: `\*`}, + {compType: compGlobstar}, + {compType: compLiteral, compText: `\*`}, + {compType: compGlobstar}, + {compType: compLiteral, compText: `\*`}, + {compType: compGlobstar}, + {compType: compTerminal}, + }, + `/foo/\**\**\**\**\**`, + }, + { + `/foo/**\\**\\**\\**\\**`, + `/foo/⁑\\⁑\\⁑\\⁑\\⁑`, + []component{ + {compType: compSeparator}, + {compType: compLiteral, compText: "foo"}, + {compType: compSeparator}, + {compType: compGlobstar}, + {compType: compLiteral, compText: `\\`}, + {compType: compGlobstar}, + {compType: compLiteral, compText: `\\`}, + {compType: compGlobstar}, + {compType: compLiteral, compText: `\\`}, + {compType: compGlobstar}, + {compType: compLiteral, compText: `\\`}, + {compType: compGlobstar}, + {compType: compTerminal}, + }, + `/foo/*\\*\\*\\*\\*`, + }, + { + `/foo/⁑**⁑\**\⁑*\\*⁑\\\**\\\⁑`, + `/foo/\⁑⁑\⁑\**\⁑*\\*\⁑\\\**\\\⁑`, + []component{ + {compType: compSeparator}, + {compType: compLiteral, compText: "foo"}, + {compType: compSeparator}, + {compType: compLiteral, compText: `⁑`}, + {compType: compGlobstar}, + {compType: compLiteral, compText: `⁑\*`}, + {compType: compGlobstar}, + {compType: compLiteral, compText: `⁑`}, + {compType: compGlobstar}, + {compType: compLiteral, compText: `\\`}, + {compType: compGlobstar}, + {compType: compLiteral, compText: `⁑\\\*`}, + {compType: compGlobstar}, + {compType: compLiteral, compText: `\\⁑`}, + {compType: compTerminal}, + }, + `/foo/⁑*⁑\**⁑*\\*⁑\\\**\\⁑`, + }, + } { + c.Check(prepareVariantForParsing(testCase.pattern), Equals, testCase.preparedPattern, Commentf("testCase: %+v", testCase)) + variant, err := parsePatternVariant(testCase.pattern) + c.Assert(err, IsNil, Commentf("testCase: %+v", testCase)) + c.Check(variant.components, DeepEquals, testCase.components, Commentf("testCase: %+v", testCase)) + c.Check(variant.String(), DeepEquals, testCase.variantStr, Commentf("testCase: %+v", testCase)) + } +}