interfaces: generalize constant folding for patterns

interfaces/prompting/patterns/parse_internal_test.go

@@ -68,3 +68,19 @@ func (s *parseSuite) TestParse(c *C) {
 	c.Assert(err, IsNil)
 	c.Check(tree, DeepEquals, handMadeTree)
 }
+
+func (s *parseSuite) TestParseWithLeadingLiteralOptimization(c *C) {
+	pattern := "/{foo,bar}"
+
+	handMadeTree := alt{
+		literal("/foo"),
+		literal("/bar"),
+	}
+
+	tokens, err := scan(pattern)
+	c.Assert(err, IsNil)
+	tree, err := parse(tokens)
+	c.Logf("Tree is: %v", tree)
+	c.Assert(err, IsNil)
+	c.Check(tree, DeepEquals, handMadeTree)
+}

interfaces/prompting/patterns/render.go

@@ -22,7 +22,6 @@ package patterns
 import (
 	"bytes"
 	"errors"
-	"strings"
 
 	"github.com/snapcore/snapd/logger"
 )
@@ -57,6 +56,7 @@ type renderNode interface {
 	NumVariants() int
 	// nodeEqual returns true if two nodes are recursively equal.
 	nodeEqual(renderNode) bool
+	prependLiteral(literal) renderNode
 }
 
 // renderAllVariants renders subsequent variants to a reusable buffer.
@@ -109,6 +109,10 @@ func (n literal) nodeEqual(other renderNode) bool {
 	return false
 }
 
+func (n literal) prependLiteral(l literal) renderNode {
+	return l + n
+}
+
 func (n literal) Render(buf *bytes.Buffer, alreadyRendered int) int {
 	if alreadyRendered > 0 {
 		return alreadyRendered - len(n)
@@ -176,44 +180,56 @@ func (n seq) nodeEqual(other renderNode) bool {
 // empty seq to a literal "", and reduces a seq with one element to that
 // element.
 func (n seq) optimize() (renderNode, error) {
-	var b strings.Builder
-
+	// Constant-fold leading literal into whatever that follows it.
+	var prefix literal
 	var newSeq seq
-
 	for _, item := range n {
-		if l, ok := item.(literal); ok {
-			if l == "" {
-				continue
-			}
-
-			b.WriteString(string(l))
-		} else {
-			if b.Len() > 0 {
-				newSeq = append(newSeq, literal(b.String()))
-				b.Reset()
-			}
+		if prefix != "" {
+			// XXX: prependItem may return modified item.
+			item = item.prependLiteral(prefix)
+		}
 
-			newSeq = append(newSeq, item)
+		if item, ok := item.(literal); ok {
+			prefix = item
+			continue
 		}
+
+		newSeq = append(newSeq, item)
+		prefix = ""
 	}
 
-	if b.Len() > 0 {
-		newSeq = append(newSeq, literal(b.String()))
-		b.Reset()
+	if prefix != "" {
+		newSeq = append(newSeq, prefix)
 	}
 
+	n = newSeq
+
 	// Reduce strength
-	switch len(newSeq) {
+	switch len(n) {
 	case 0:
 		return literal(""), nil
 	case 1:
-		return newSeq[0], nil
+		return n[0], nil
 	}
 
-	return newSeq, nil
+	return n, nil
+}
+
+func (n seq) prependLiteral(l literal) renderNode {
+	if l == "" {
+		return n
+	}
+
+	if len(n) == 0 {
+		return l
+	}
+
+	n[0] = n[0].prependLiteral(l)
+
+	return n
 }
 
-// seqVariant is the variant state for a seqeunce of render nodes.
+// seqVariant is the variant state for a sequence of render nodes.
 //
 // Each render node in the seq has a corresponding variant at the same index.
 // of the elements list.
@@ -314,9 +330,24 @@ func (n alt) nodeEqual(other renderNode) bool {
 // optimize for alt eliminates equivalent items and reduces alts with one item
 // to that item.
 func (n alt) optimize() (renderNode, error) {
-	var seen []renderNode
 	var newAlt alt
 
+	// This pass adds items that are alts back to us to unify the set of
+	// alts, effectively.
+	for _, item := range n {
+		if innerAlt, ok := item.(alt); ok {
+			newAlt = append(newAlt, innerAlt...)
+		} else {
+			newAlt = append(newAlt, item)
+		}
+	}
+
+	n = newAlt
+	newAlt = nil
+
+	var seen []renderNode
+
+	// This pass eliminates duplicate entries.
 outer:
 	for _, item := range n {
 		for _, seenItem := range seen {
@@ -342,6 +373,22 @@ outer:
 	return newAlt, nil
 }
 
+func (n alt) prependLiteral(l literal) renderNode {
+	if l == "" {
+		return n
+	}
+
+	if len(n) == 0 {
+		return l
+	}
+
+	for i := range n {
+		n[i] = n[i].prependLiteral(l)
+	}
+
+	return n
+}
+
 // altVariant is the variant state for an set of alternative render nodes.
 type altVariant struct {
 	alt     alt          // Alt associated with this variant state.

interfaces/prompting/patterns/render_internal_test.go

@@ -190,31 +190,25 @@ func (s *renderSuite) TestOptimizeNodeEqualTrue(c *C) {
 		},
 		{
 			"/foo/{a,b}",
-			seq{
-				literal("/foo/"),
-				alt{
-					literal("a"),
-					literal("b"),
-				},
+			alt{
+				literal("/foo/a"),
+				literal("/foo/b"),
 			},
 		},
 		{
 			"/foo/{{a,b},{a,b}}",
-			seq{
-				literal("/foo/"),
-				alt{
-					literal("a"),
-					literal("b"),
-				},
+			alt{
+				literal("/foo/a"),
+				literal("/foo/b"),
 			},
 		},
 	} {
 		scanned, err := scan(testCase.pattern)
 		c.Assert(err, IsNil, Commentf("testCase: %+v", testCase))
 		parsed, err := parse(scanned)
 		c.Assert(err, IsNil, Commentf("testCase: %+v", testCase))
-		c.Check(parsed.nodeEqual(testCase.byHand), Equals, true, Commentf("testCase: %+v\nparsed: %+v", testCase, parsed))
-		c.Check(testCase.byHand.nodeEqual(parsed), Equals, true, Commentf("testCase: %+v\nparsed: %+v", testCase, parsed))
+		c.Check(parsed.nodeEqual(testCase.byHand), Equals, true, Commentf("testCase: %+v\nparsed: %#v", testCase, parsed))
+		c.Check(testCase.byHand.nodeEqual(parsed), Equals, true, Commentf("testCase: %+v\nparsed: %#v", testCase, parsed))
 	}
 }
 
@@ -281,3 +275,56 @@ func (s *renderSuite) TestOptimizeNodeEqualFalse(c *C) {
 		c.Check(testCase.byHand.nodeEqual(parsed), Equals, false, Commentf("testCase: %+v\nparsed: %+v", testCase, parsed))
 	}
 }
+
+func (s *renderSuite) TestRenderAllVariantsConflicts(c *C) {
+	// all variants are the exact same path
+	for _, testCase := range []struct {
+		pattern          string
+		expectedVariants []string
+	}{
+		{
+			"/{fo{obar},foo{b{ar,ar},bar,{ba}r}}",
+			[]string{"/foobar"},
+		},
+		{
+			"/{{foo/{bar,baz},123},{123,foo{/bar,/baz}}}",
+			[]string{
+				"/foo/bar",
+				"/foo/baz",
+				"/123",
+			},
+		},
+		{
+			"/{{a,DUP},{b,DUP}}",
+			[]string{
+				"/a",
+				"/DUP",
+				"/b",
+			},
+		},
+		{
+			"/{{foo,bar}/baz/{fizz,buzz},{foo/,bar/}baz{/fizz,/buzz}}",
+			[]string{
+				"/foo/baz/fizz",
+				"/foo/baz/buzz",
+				"/bar/baz/fizz",
+				"/bar/baz/buzz",
+				"/foo/baz/fizz",
+				"/foo/baz/buzz",
+				"/bar/baz/fizz",
+				"/bar/baz/buzz",
+			},
+		},
+	} {
+		scanned, err := scan(testCase.pattern)
+		c.Assert(err, IsNil)
+		parsed, err := parse(scanned)
+		c.Assert(err, IsNil)
+		variants := make([]string, 0, len(testCase.expectedVariants))
+		renderAllVariants(parsed, func(index int, variant PatternVariant) {
+			variants = append(variants, variant.String())
+		})
+		c.Check(variants, DeepEquals, testCase.expectedVariants)
+		c.Check(variants, HasLen, parsed.NumVariants())
+	}
+}