Refactor syntax error handling: enhance error analysis for FOR loops and return values, update error messages, and streamline matcher functions for improved clarity and maintainability.

pkg/compiler/compiler.go

@@ -1,9 +1,10 @@
 package compiler
 
 import (
-	"github.com/MontFerret/ferret/pkg/compiler/internal/diagnostics"
 	goruntime "runtime"
 
+	"github.com/MontFerret/ferret/pkg/compiler/internal/diagnostics"
+
 	"github.com/antlr4-go/antlr/v4"
 
 	"github.com/MontFerret/ferret/pkg/file"

pkg/compiler/internal/diagnostics/error_analyzer.go

@@ -8,8 +8,9 @@ type SyntaxErrorMatcher func(src *file.Source, err *CompilationError, offending
 
 func AnalyzeSyntaxError(src *file.Source, err *CompilationError, offending *TokenNode) bool {
 	matchers := []SyntaxErrorMatcher{
-		missingAssignmentValueMatcher,
-		missingReturnValueMatcher,
+		matchMissingAssignmentValue,
+		matchForLoopErrors,
+		matchMissingReturnValue,
 	}
 
 	for _, matcher := range matchers {

pkg/compiler/internal/diagnostics/error_listener.go

@@ -93,9 +93,5 @@ func (d *ErrorListener) noViableAltError(err *CompilationError) bool {
 		return false
 	}
 
-	if d.history.Size() < 2 {
-		return false
-	}
-
 	return AnalyzeSyntaxError(d.src, err, d.history.Last())
 }

pkg/compiler/internal/diagnostics/handler.go

@@ -71,15 +71,6 @@ func (h *ErrorHandler) Add(err *CompilationError) {
 	}
 }
 
-func (h *ErrorHandler) UnexpectedToken(ctx antlr.ParserRuleContext) {
-	h.Add(&CompilationError{
-		Message: fmt.Sprintf("Unexpected token '%s'", ctx.GetText()),
-		Source:  h.src,
-		Spans:   []ErrorSpan{NewMainErrorSpan(SpanFromRuleContext(ctx), "Unexpected token")},
-		Kind:    SyntaxError,
-	})
-}
-
 func (h *ErrorHandler) VariableNotUnique(ctx antlr.ParserRuleContext, name string) {
 	// TODO: Add information where the variable was defined
 	h.Add(&CompilationError{
@@ -98,3 +89,21 @@ func (h *ErrorHandler) VariableNotFound(token antlr.Token, name string) {
 		Kind:    NameError,
 	})
 }
+
+func (h *ErrorHandler) MissingReturnValue(ctx antlr.ParserRuleContext) {
+	//span := spanFromTokenSafe(offending.Token(), src)
+	//err.Message = fmt.Sprintf("Expected expression after '%s'", offending)
+	//err.Hint = "Did you forget to provide a value to return?"
+	//err.Spans = []ErrorSpan{
+	//	NewMainErrorSpan(span, "missing return value"),
+	//}
+
+	h.Add(&CompilationError{
+		Message: fmt.Sprintf("Expected expression after '%s'", ctx.GetText()),
+		Hint:    "Did you forget to provide a value to return?",
+		Source:  h.src,
+		Spans: []ErrorSpan{
+			NewMainErrorSpan(SpanFromRuleContext(ctx), "missing return value")},
+		Kind: SyntaxError,
+	})
+}

pkg/compiler/internal/diagnostics/helpers.go

@@ -1,9 +1,10 @@
 package diagnostics
 
 import (
+	"strings"
+
 	"github.com/MontFerret/ferret/pkg/parser/fql"
 	"github.com/antlr4-go/antlr/v4"
-	"strings"
 
 	"github.com/MontFerret/ferret/pkg/file"
 )
@@ -56,14 +57,6 @@ func spanFromTokenSafe(tok antlr.Token, src *file.Source) file.Span {
 	return file.Span{Start: start, End: end}
 }
 
-func stringify(token *TokenNode) string {
-	if token == nil {
-		return ""
-	}
-
-	return strings.ToUpper(strings.TrimSpace(token.GetText()))
-}
-
 func isIdentifier(node *TokenNode) bool {
 	if node == nil {
 		return false

pkg/compiler/internal/diagnostics/match_assignment_errors.go

@@ -2,13 +2,13 @@ package diagnostics
 
 import (
 	"fmt"
+
 	"github.com/MontFerret/ferret/pkg/file"
 )
 
-func missingAssignmentValueMatcher(src *file.Source, err *CompilationError, offending *TokenNode) bool {
+func matchMissingAssignmentValue(src *file.Source, err *CompilationError, offending *TokenNode) bool {
 	prev := offending.Prev()
 
-	// CASE: LET x = [missing value]
 	if is(offending, "LET") || is(prev, "=") {
 		span := spanFromTokenSafe(prev.Token(), src)
 		span.Start++

pkg/compiler/internal/diagnostics/match_for_loop_errors.go

@@ -0,0 +1,50 @@
+package diagnostics
+
+import (
+	"github.com/MontFerret/ferret/pkg/file"
+)
+
+func matchForLoopErrors(src *file.Source, err *CompilationError, offending *TokenNode) bool {
+	prev := offending.Prev()
+
+	if is(prev, "IN") {
+		span := spanFromTokenSafe(prev.Token(), src)
+		span.Start = span.End + 1
+		span.End = span.Start + 1
+		err.Message = "Expected expression after 'IN'"
+		err.Hint = "Each FOR loop must iterate over a collection or range."
+		err.Spans = []ErrorSpan{
+			NewMainErrorSpan(span, "missing value"),
+		}
+
+		return true
+	}
+
+	if is(prev, "FOR") {
+		span := spanFromTokenSafe(offending.Token(), src)
+		span.Start = span.End
+		span.End = span.Start + 1
+		err.Message = "Expected 'IN' after loop variable"
+		err.Hint = "Use 'FOR x IN [iterable]' syntax."
+		err.Spans = []ErrorSpan{
+			NewMainErrorSpan(span, "missing keyword"),
+		}
+
+		return true
+	}
+
+	if is(offending, "FOR") {
+		span := spanFromTokenSafe(offending.Token(), src)
+		span.Start = span.End
+		span.End = span.Start + 1
+		err.Message = "Expected loop variable before 'IN'"
+		err.Hint = "FOR must declare a variable."
+		err.Spans = []ErrorSpan{
+			NewMainErrorSpan(span, "missing variable"),
+		}
+
+		return true
+	}
+
+	return false
+}

pkg/compiler/internal/diagnostics/match_return_value_errors.go

@@ -2,20 +2,15 @@ package diagnostics
 
 import (
 	"fmt"
+
 	"github.com/MontFerret/ferret/pkg/file"
 )
 
-func missingReturnValueMatcher(src *file.Source, err *CompilationError, offending *TokenNode) bool {
+func matchMissingReturnValue(src *file.Source, err *CompilationError, offending *TokenNode) bool {
 	if !is(offending, "RETURN") {
 		return false
 	}
 
-	prev := offending.Prev()
-
-	if prev != nil && isKeyword(prev) && !is(prev, "NONE") && !is(prev, "NULL") {
-		return false
-	}
-
 	span := spanFromTokenSafe(offending.Token(), src)
 	err.Message = fmt.Sprintf("Expected expression after '%s'", offending)
 	err.Hint = "Did you forget to provide a value to return?"

pkg/compiler/internal/expr.go

@@ -57,8 +57,6 @@ func (c *ExprCompiler) Compile(ctx fql.IExpressionContext) vm.Operand {
 		return c.compilePredicate(p)
 	}
 
-	c.ctx.Errors.UnexpectedToken(ctx)
-
 	return vm.NoopOperand
 }
 
@@ -87,8 +85,6 @@ func (c *ExprCompiler) compileUnary(ctx fql.IUnaryOperatorContext, parent fql.IE
 	} else if ctx.Plus() != nil {
 		op = vm.OpFlipPositive
 	} else {
-		c.ctx.Errors.UnexpectedToken(ctx)
-
 		return vm.NoopOperand
 	}
 
@@ -271,8 +267,6 @@ func (c *ExprCompiler) compilePredicate(ctx fql.IPredicateContext) vm.Operand {
 		case "<=":
 			opcode = vm.OpLte
 		default:
-			c.ctx.Errors.UnexpectedToken(ctx)
-
 			return vm.NoopOperand
 		}
 	} else if op := ctx.ArrayOperator(); op != nil {
@@ -325,8 +319,6 @@ func (c *ExprCompiler) compileAtom(ctx fql.IExpressionAtomContext) vm.Operand {
 		case "%":
 			opcode = vm.OpMod
 		default:
-			c.ctx.Errors.UnexpectedToken(ctx)
-
 			return vm.NoopOperand
 		}
 	} else if op := ctx.AdditiveOperator(); op != nil {
@@ -338,8 +330,6 @@ func (c *ExprCompiler) compileAtom(ctx fql.IExpressionAtomContext) vm.Operand {
 		case "-":
 			opcode = vm.OpSub
 		default:
-			c.ctx.Errors.UnexpectedToken(ctx)
-
 			return vm.NoopOperand
 		}
 
@@ -352,8 +342,6 @@ func (c *ExprCompiler) compileAtom(ctx fql.IExpressionAtomContext) vm.Operand {
 		case "!~":
 			opcode = vm.OpRegexpNegative
 		default:
-			c.ctx.Errors.UnexpectedToken(ctx)
-
 			return vm.NoopOperand
 		}
 	}
@@ -726,7 +714,5 @@ func (c *ExprCompiler) compileRangeOperand(ctx fql.IRangeOperandContext) vm.Oper
 		return c.ctx.LiteralCompiler.CompileIntegerLiteral(il)
 	}
 
-	c.ctx.Errors.UnexpectedToken(ctx)
-
 	return vm.NoopOperand
 }

pkg/compiler/internal/literal.go

@@ -51,8 +51,6 @@ func (c *LiteralCompiler) Compile(ctx fql.ILiteralContext) vm.Operand {
 		return c.CompileNoneLiteral(nl)
 	}
 
-	c.ctx.Errors.UnexpectedToken(ctx)
-
 	return vm.NoopOperand
 }
 
@@ -178,7 +176,8 @@ func (c *LiteralCompiler) CompileBooleanLiteral(ctx fql.IBooleanLiteralContext)
 	case "false":
 		c.ctx.Emitter.EmitBoolean(reg, false)
 	default:
-		c.ctx.Errors.UnexpectedToken(ctx)
+		c.ctx.Registers.Free(reg)
+		reg = vm.NoopOperand
 	}
 
 	return reg
@@ -308,8 +307,6 @@ func (c *LiteralCompiler) CompilePropertyName(ctx fql.IPropertyNameContext) vm.O
 		// Unsafe reserved word (e.g., { for: value })
 		name = word.GetText()
 	} else {
-		c.ctx.Errors.UnexpectedToken(ctx)
-
 		return vm.NoopOperand
 	}
 

pkg/compiler/internal/loop.go

@@ -233,9 +233,6 @@ func (c *LoopCompiler) compileForExpressionSource(ctx fql.IForExpressionSourceCo
 		return c.ctx.LiteralCompiler.CompileObjectLiteral(ol)
 	}
 
-	// If none of the above, the source expression is invalid
-	c.ctx.Errors.UnexpectedToken(ctx)
-
 	return vm.NoopOperand
 }
 
@@ -319,9 +316,6 @@ func (c *LoopCompiler) compileLimitClauseValue(ctx fql.ILimitClauseValueContext)
 		return c.ctx.ExprCompiler.CompileFunctionCallExpression(fce)
 	}
 
-	// If none of the above, the limit value expression is invalid
-	c.ctx.Errors.UnexpectedToken(ctx)
-
 	return vm.NoopOperand
 }
 

pkg/compiler/internal/wait.go

@@ -132,9 +132,6 @@ func (c *WaitCompiler) CompileWaitForEventName(ctx fql.IWaitForEventNameContext)
 		return c.ctx.ExprCompiler.CompileFunctionCallExpression(fce)
 	}
 
-	// If none of the above, the event name expression is invalid
-	c.ctx.Errors.UnexpectedToken(ctx)
-
 	return vm.NoopOperand
 }
 
@@ -164,9 +161,6 @@ func (c *WaitCompiler) CompileWaitForEventSource(ctx fql.IWaitForEventSourceCont
 		return c.ctx.ExprCompiler.CompileFunctionCallExpression(fce)
 	}
 
-	// If none of the above, the event source expression is invalid
-	c.ctx.Errors.UnexpectedToken(ctx)
-
 	return vm.NoopOperand
 }
 
@@ -185,9 +179,6 @@ func (c *WaitCompiler) CompileOptionsClause(ctx fql.IOptionsClauseContext) vm.Op
 		return c.ctx.LiteralCompiler.CompileObjectLiteral(ol)
 	}
 
-	// If not an object literal, the options expression is invalid
-	c.ctx.Errors.UnexpectedToken(ctx)
-
 	return vm.NoopOperand
 }
 
@@ -227,8 +218,5 @@ func (c *WaitCompiler) CompileTimeoutClauseContext(ctx fql.ITimeoutClauseContext
 		return c.ctx.ExprCompiler.CompileFunctionCall(fc, false)
 	}
 
-	// If none of the above, the timeout expression is invalid
-	c.ctx.Errors.UnexpectedToken(ctx)
-
 	return vm.NoopOperand
 }

test/integration/compiler/compiler_errors_syntax_test.go

@@ -46,11 +46,29 @@ func TestSyntaxErrors(t *testing.T) {
 		ErrorCase(
 			`
 			FOR i IN 
-			RETURN i
+				RETURN i
 		`, E{
 				Kind:    compiler.SyntaxError,
-				Message: "__FAIL__",
-				Hint:    "Did you forget to provide a value?",
+				Message: "Expected expression after 'IN'",
+				Hint:    "Each FOR loop must iterate over a collection or range.",
 			}, "Missing iterable in FOR"),
+		ErrorCase(
+			`
+			FOR i [1, 2, 3]
+				RETURN i
+		`, E{
+				Kind:    compiler.SyntaxError,
+				Message: "Expected 'IN' after loop variable",
+				Hint:    "Use 'FOR x IN [iterable]' syntax.",
+			}, "Missing IN in FOR"),
+		ErrorCase(
+			`
+			FOR IN [1, 2, 3]
+				RETURN i
+		`, E{
+				Kind:    compiler.SyntaxError,
+				Message: "--",
+				Hint:    "Use 'FOR x IN [iterable]' syntax.",
+			}, "FOR without variable"),
 	})
 }