[pkg/ottl] Refactor grammar validation to use the AST visitor and combining errors

.chloggen/ottl-grammar-custom-errors-visitor.yaml

@@ -0,0 +1,27 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: enhancement
+
+# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
+component: pkg/ottl
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: "Refactor grammar's validation to use the AST visitor and to combine all errors."
+
+# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
+issues: [35728]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext: Parsing invalid statements and conditions now combines all errors instead of returning the first one found.
+
+# If your change doesn't affect end users or the exported elements of any package,
+# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: [user]

pkg/ottl/functions.go

@@ -33,16 +33,7 @@ func buildOriginalText(path *path) string {
 	for i, f := range path.Fields {
 		builder.WriteString(f.Name)
 		if len(f.Keys) > 0 {
-			for _, k := range f.Keys {
-				builder.WriteString("[")
-				if k.Int != nil {
-					builder.WriteString(strconv.FormatInt(*k.Int, 10))
-				}
-				if k.String != nil {
-					builder.WriteString(*k.String)
-				}
-				builder.WriteString("]")
-			}
+			builder.WriteString(buildOriginalKeysText(f.Keys))
 		}
 		if i != len(path.Fields)-1 {
 			builder.WriteString(".")
@@ -51,6 +42,23 @@ func buildOriginalText(path *path) string {
 	return builder.String()
 }
 
+func buildOriginalKeysText(keys []key) string {
+	var builder strings.Builder
+	if len(keys) > 0 {
+		for _, k := range keys {
+			builder.WriteString("[")
+			if k.Int != nil {
+				builder.WriteString(strconv.FormatInt(*k.Int, 10))
+			}
+			if k.String != nil {
+				builder.WriteString(*k.String)
+			}
+			builder.WriteString("]")
+		}
+	}
+	return builder.String()
+}
+
 func (p *Parser[K]) newPath(path *path) (*basePath[K], error) {
 	if len(path.Fields) == 0 {
 		return nil, fmt.Errorf("cannot make a path from zero fields")

pkg/ottl/grammar.go

@@ -5,6 +5,7 @@ package ottl // import "github.com/open-telemetry/opentelemetry-collector-contri
 
 import (
 	"encoding/hex"
+	"errors"
 	"fmt"
 
 	"github.com/alecthomas/participle/v2/lexer"
@@ -19,17 +20,17 @@ type parsedStatement struct {
 }
 
 func (p *parsedStatement) checkForCustomError() error {
+	validator := &grammarCustomErrorsVisitor{}
 	if p.Converter != nil {
-		return fmt.Errorf("editor names must start with a lowercase letter but got '%v'", p.Converter.Function)
-	}
-	err := p.Editor.checkForCustomError()
-	if err != nil {
-		return err
+		validator.add(fmt.Errorf("editor names must start with a lowercase letter but got '%v'", p.Converter.Function))
 	}
+
+	p.Editor.accept(validator)
 	if p.WhereClause != nil {
-		return p.WhereClause.checkForCustomError()
+		p.WhereClause.accept(validator)
 	}
-	return nil
+
+	return validator.join()
 }
 
 type constExpr struct {
@@ -47,16 +48,6 @@ type booleanValue struct {
 	SubExpr    *booleanExpression `parser:"| '(' @@ ')' )"`
 }
 
-func (b *booleanValue) checkForCustomError() error {
-	if b.Comparison != nil {
-		return b.Comparison.checkForCustomError()
-	}
-	if b.SubExpr != nil {
-		return b.SubExpr.checkForCustomError()
-	}
-	return nil
-}
-
 func (b *booleanValue) accept(v grammarVisitor) {
 	if b.Comparison != nil {
 		b.Comparison.accept(v)
@@ -75,10 +66,6 @@ type opAndBooleanValue struct {
 	Value    *booleanValue `parser:"@@"`
 }
 
-func (b *opAndBooleanValue) checkForCustomError() error {
-	return b.Value.checkForCustomError()
-}
-
 func (b *opAndBooleanValue) accept(v grammarVisitor) {
 	if b.Value != nil {
 		b.Value.accept(v)
@@ -91,20 +78,6 @@ type term struct {
 	Right []*opAndBooleanValue `parser:"@@*"`
 }
 
-func (b *term) checkForCustomError() error {
-	err := b.Left.checkForCustomError()
-	if err != nil {
-		return err
-	}
-	for _, r := range b.Right {
-		err = r.checkForCustomError()
-		if err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
 func (b *term) accept(v grammarVisitor) {
 	if b.Left != nil {
 		b.Left.accept(v)
@@ -122,10 +95,6 @@ type opOrTerm struct {
 	Term     *term  `parser:"@@"`
 }
 
-func (b *opOrTerm) checkForCustomError() error {
-	return b.Term.checkForCustomError()
-}
-
 func (b *opOrTerm) accept(v grammarVisitor) {
 	if b.Term != nil {
 		b.Term.accept(v)
@@ -140,17 +109,9 @@ type booleanExpression struct {
 }
 
 func (b *booleanExpression) checkForCustomError() error {
-	err := b.Left.checkForCustomError()
-	if err != nil {
-		return err
-	}
-	for _, r := range b.Right {
-		err = r.checkForCustomError()
-		if err != nil {
-			return err
-		}
-	}
-	return nil
+	validator := &grammarCustomErrorsVisitor{}
+	b.accept(validator)
+	return validator.join()
 }
 
 func (b *booleanExpression) accept(v grammarVisitor) {
@@ -224,15 +185,6 @@ type comparison struct {
 	Right value     `parser:"@@"`
 }
 
-func (c *comparison) checkForCustomError() error {
-	err := c.Left.checkForCustomError()
-	if err != nil {
-		return err
-	}
-	err = c.Right.checkForCustomError()
-	return err
-}
-
 func (c *comparison) accept(v grammarVisitor) {
 	c.Left.accept(v)
 	c.Right.accept(v)
@@ -246,21 +198,6 @@ type editor struct {
 	Keys []key `parser:"( @@ )*"`
 }
 
-func (i *editor) checkForCustomError() error {
-	var err error
-
-	for _, arg := range i.Arguments {
-		err = arg.checkForCustomError()
-		if err != nil {
-			return err
-		}
-	}
-	if i.Keys != nil {
-		return fmt.Errorf("only paths and converters may be indexed, not editors, but got %v %v", i.Function, i.Keys)
-	}
-	return nil
-}
-
 func (i *editor) accept(v grammarVisitor) {
 	v.visitEditor(i)
 	for _, arg := range i.Arguments {
@@ -289,10 +226,6 @@ type argument struct {
 	FunctionName *string `parser:"| @(Uppercase(Uppercase | Lowercase)*) )"`
 }
 
-func (a *argument) checkForCustomError() error {
-	return a.Value.checkForCustomError()
-}
-
 func (a *argument) accept(v grammarVisitor) {
 	a.Value.accept(v)
 }
@@ -311,16 +244,6 @@ type value struct {
 	List           *list            `parser:"| @@)"`
 }
 
-func (v *value) checkForCustomError() error {
-	if v.Literal != nil {
-		return v.Literal.checkForCustomError()
-	}
-	if v.MathExpression != nil {
-		return v.MathExpression.checkForCustomError()
-	}
-	return nil
-}
-
 func (v *value) accept(vis grammarVisitor) {
 	vis.visitValue(v)
 	if v.Literal != nil {
@@ -416,13 +339,6 @@ type mathExprLiteral struct {
 	Path      *path      `parser:"| @@ )"`
 }
 
-func (m *mathExprLiteral) checkForCustomError() error {
-	if m.Editor != nil {
-		return fmt.Errorf("converter names must start with an uppercase letter but got '%v'", m.Editor.Function)
-	}
-	return nil
-}
-
 func (m *mathExprLiteral) accept(v grammarVisitor) {
 	v.visitMathExprLiteral(m)
 	if m.Path != nil {
@@ -441,13 +357,6 @@ type mathValue struct {
 	SubExpression *mathExpression  `parser:"| '(' @@ ')' )"`
 }
 
-func (m *mathValue) checkForCustomError() error {
-	if m.Literal != nil {
-		return m.Literal.checkForCustomError()
-	}
-	return m.SubExpression.checkForCustomError()
-}
-
 func (m *mathValue) accept(v grammarVisitor) {
 	if m.Literal != nil {
 		m.Literal.accept(v)
@@ -462,10 +371,6 @@ type opMultDivValue struct {
 	Value    *mathValue `parser:"@@"`
 }
 
-func (m *opMultDivValue) checkForCustomError() error {
-	return m.Value.checkForCustomError()
-}
-
 func (m *opMultDivValue) accept(v grammarVisitor) {
 	if m.Value != nil {
 		m.Value.accept(v)
@@ -477,20 +382,6 @@ type addSubTerm struct {
 	Right []*opMultDivValue `parser:"@@*"`
 }
 
-func (m *addSubTerm) checkForCustomError() error {
-	err := m.Left.checkForCustomError()
-	if err != nil {
-		return err
-	}
-	for _, r := range m.Right {
-		err = r.checkForCustomError()
-		if err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
 func (m *addSubTerm) accept(v grammarVisitor) {
 	if m.Left != nil {
 		m.Left.accept(v)
@@ -507,13 +398,9 @@ type opAddSubTerm struct {
 	Term     *addSubTerm `parser:"@@"`
 }
 
-func (m *opAddSubTerm) checkForCustomError() error {
-	return m.Term.checkForCustomError()
-}
-
-func (m *opAddSubTerm) accept(v grammarVisitor) {
-	if m.Term != nil {
-		m.Term.accept(v)
+func (r *opAddSubTerm) accept(v grammarVisitor) {
+	if r.Term != nil {
+		r.Term.accept(v)
 	}
 }
 
@@ -522,20 +409,6 @@ type mathExpression struct {
 	Right []*opAddSubTerm `parser:"@@*"`
 }
 
-func (m *mathExpression) checkForCustomError() error {
-	err := m.Left.checkForCustomError()
-	if err != nil {
-		return err
-	}
-	for _, r := range m.Right {
-		err = r.checkForCustomError()
-		if err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
 func (m *mathExpression) accept(v grammarVisitor) {
 	if m.Left != nil {
 		m.Left.accept(v)
@@ -627,3 +500,35 @@ type grammarVisitor interface {
 	visitValue(v *value)
 	visitMathExprLiteral(v *mathExprLiteral)
 }
+
+// grammarCustomErrorsVisitor is used to execute custom validations on the grammar AST.
+type grammarCustomErrorsVisitor struct {
+	errors []error
+}
+
+func (g *grammarCustomErrorsVisitor) add(err error) {
+	g.errors = append(g.errors, err)
+}
+
+func (g *grammarCustomErrorsVisitor) join() error {
+	if len(g.errors) == 0 {
+		return nil
+	}
+	return errors.Join(g.errors...)
+}
+
+func (g *grammarCustomErrorsVisitor) visitPath(_ *path) {}
+
+func (g *grammarCustomErrorsVisitor) visitValue(_ *value) {}
+
+func (g *grammarCustomErrorsVisitor) visitEditor(v *editor) {
+	if v.Keys != nil {
+		g.add(fmt.Errorf("only paths and converters may be indexed, not editors, but got %s%s", v.Function, buildOriginalKeysText(v.Keys)))
+	}
+}
+
+func (g *grammarCustomErrorsVisitor) visitMathExprLiteral(v *mathExprLiteral) {
+	if v.Editor != nil {
+		g.add(fmt.Errorf("converter names must start with an uppercase letter but got '%v'", v.Editor.Function))
+	}
+}