Fixes the pattern parser validation. (#4308) (#4322)

We should never have 2 consecutive capture, since when we do we can't figure which data goes where.
The previous validation was validating two nodes at the time and moving to the next two nodes so in the end it wasn't validating properly.

:facepalm:

Signed-off-by: Cyril Tovena <[email protected]>

Co-authored-by: Cyril Tovena <[email protected]>

pkg/logql/log/pattern/ast.go

@@ -19,19 +19,18 @@ func (e expr) validate() error {
 	if !e.hasCapture() {
 		return ErrNoCapture
 	}
-	// if there is at least 2 node, verify that none are consecutive.
-	if len(e) >= 2 {
-		for i := 0; i < len(e); i = i + 2 {
-			if i+1 >= len(e) {
-				break
-			}
-			if _, ok := e[i].(capture); ok {
-				if _, ok := e[i+1].(capture); ok {
-					return fmt.Errorf("found consecutive capture: %w", ErrInvalidExpr)
-				}
+	// Consecutive captures are not allowed.
+	for i, n := range e {
+		if i+1 >= len(e) {
+			break
+		}
+		if _, ok := n.(capture); ok {
+			if _, ok := e[i+1].(capture); ok {
+				return fmt.Errorf("found consecutive capture '%s': %w", n.String()+e[i+1].String(), ErrInvalidExpr)
 			}
 		}
 	}
+
 	caps := e.captures()
 	uniq := map[string]struct{}{}
 	for _, c := range caps {
@@ -46,7 +45,7 @@ func (e expr) validate() error {
 func (e expr) captures() (captures []string) {
 	for _, n := range e {
 		if c, ok := n.(capture); ok && !c.isUnamed() {
-			captures = append(captures, c.String())
+			captures = append(captures, c.Name())
 		}
 	}
 	return
@@ -59,6 +58,10 @@ func (e expr) captureCount() (count int) {
 type capture string
 
 func (c capture) String() string {
+	return "<" + string(c) + ">"
+}
+
+func (c capture) Name() string {
 	return string(c)
 }
 

pkg/logql/log/pattern/pattern_test.go

@@ -150,9 +150,11 @@ func Test_Error(t *testing.T) {
 		{"<_>", ErrNoCapture},
 		{"foo <_> bar <_>", ErrNoCapture},
 		{"foo bar buzz", ErrNoCapture},
-		{"<f><f>", fmt.Errorf("found consecutive capture: %w", ErrInvalidExpr)},
-		{"<f> f<d><b>", fmt.Errorf("found consecutive capture: %w", ErrInvalidExpr)},
+		{"<f><f>", fmt.Errorf("found consecutive capture '<f><f>': %w", ErrInvalidExpr)},
+		{"<f> f<d><b>", fmt.Errorf("found consecutive capture '<d><b>': %w", ErrInvalidExpr)},
 		{"<f> f<f>", fmt.Errorf("duplicate capture name (f): %w", ErrInvalidExpr)},
+		{`f<f><_>`, fmt.Errorf("found consecutive capture '<f><_>': %w", ErrInvalidExpr)},
+		{`<f>f<f><_>`, fmt.Errorf("found consecutive capture '<f><_>': %w", ErrInvalidExpr)},
 	} {
 		t.Run(tt.name, func(t *testing.T) {
 			_, err := New(tt.name)