Refactor error handling and diagnostics: enhance error messages for syntax errors, improve handling of extraneous input, and streamline error listener logic for better clarity and maintainability.

pkg/compiler/compiler.go

@@ -64,6 +64,9 @@ func (c *Compiler) Compile(src *file.Source) (program *vm.Program, err error) {
 	p := parser.New(src.Content(), func(stream antlr.TokenStream) antlr.TokenStream {
 		return diagnostics.NewTrackingTokenStream(stream, tokenHistory)
 	})
+	// Remove all default error listeners
+	p.RemoveErrorListeners()
+	// Add custom error listener
 	p.AddErrorListener(diagnostics.NewErrorListener(src, l.Ctx.Errors, tokenHistory))
 	p.Visit(l)
 

pkg/compiler/internal/diagnostics/error_listener.go

@@ -1,8 +1,6 @@
 package diagnostics
 
 import (
-	"strings"
-
 	"github.com/antlr4-go/antlr/v4"
 
 	"github.com/MontFerret/ferret/pkg/file"
@@ -38,8 +36,8 @@ func (d *ErrorListener) SyntaxError(_ antlr.Recognizer, offendingSymbol interfac
 		offending = tok
 	}
 
-	if err := d.parseError(msg, offending); err != nil {
-		if !d.handler.HasErrorOnLine(line) {
+	if !d.handler.HasErrorOnLine(line) {
+		if err := d.parseError(msg, offending); err != nil {
 			d.handler.Add(err)
 		}
 	}
@@ -58,53 +56,7 @@ func (d *ErrorListener) parseError(msg string, offending antlr.Token) *Compilati
 		},
 	}
 
-	for _, handler := range []func(*CompilationError) bool{
-		d.extraneousInput,
-		d.noViableAlternative,
-		d.mismatchedInput,
-	} {
-		if handler(err) {
-			break
-		}
-	}
+	AnalyzeSyntaxError(d.src, err, d.history.Last())
 
 	return err
 }
-
-func (d *ErrorListener) extraneousInput(err *CompilationError) (matched bool) {
-	if !strings.Contains(err.Message, "extraneous input") {
-		return false
-	}
-
-	last := d.history.Last()
-
-	if last == nil {
-		return false
-	}
-
-	span := spanFromTokenSafe(last.Token(), d.src)
-	err.Spans = []ErrorSpan{
-		NewMainErrorSpan(span, "query must end with a value"),
-	}
-
-	err.Message = "Expected a RETURN or FOR clause at end of query"
-	err.Hint = "All queries must return a value. Add a RETURN statement to complete the query."
-
-	return true
-}
-
-func (d *ErrorListener) noViableAlternative(err *CompilationError) bool {
-	if !strings.Contains(err.Message, "viable alternative at input") {
-		return false
-	}
-
-	return AnalyzeSyntaxError(d.src, err, d.history.Last())
-}
-
-func (d *ErrorListener) mismatchedInput(err *CompilationError) bool {
-	if !strings.Contains(err.Message, "mismatched input") {
-		return false
-	}
-
-	return AnalyzeSyntaxError(d.src, err, d.history.Last())
-}

pkg/compiler/internal/diagnostics/helpers.go

@@ -1,11 +1,13 @@
 package diagnostics
 
 import (
+	"regexp"
 	"strings"
 
-	"github.com/MontFerret/ferret/pkg/parser/fql"
 	"github.com/antlr4-go/antlr/v4"
 
+	"github.com/MontFerret/ferret/pkg/parser/fql"
+
 	"github.com/MontFerret/ferret/pkg/file"
 )
 
@@ -111,3 +113,34 @@ func is(node *TokenNode, expected string) bool {
 func has(msg string, substr string) bool {
 	return strings.Contains(strings.ToLower(msg), strings.ToLower(substr))
 }
+
+func isExtraneous(msg string) bool {
+	return has(msg, "extraneous input")
+}
+
+func parseExtraneousInput(msg string) string {
+	re := regexp.MustCompile(`extraneous input\s+(?P<input>.+?)\s+expecting`)
+	match := re.FindStringSubmatch(msg)
+	return match[re.SubexpIndex("input")]
+}
+
+func parseExtraneousInputAll(msg string) (string, []string) {
+	rx := regexp.MustCompile(`extraneous input\s+(?P<input>.+?)\s+expecting\s+\{(?P<expected>.+?)\}`)
+	matches := rx.FindStringSubmatch(msg)
+
+	if len(matches) != 3 {
+		return "", nil
+	}
+
+	input := strings.TrimSpace(matches[1])
+	expectedRaw := strings.TrimSpace(matches[2])
+	var expected []string
+
+	for _, part := range strings.Split(expectedRaw, ",") {
+		part = strings.TrimSpace(part)
+		part = strings.Trim(part, "'")
+		expected = append(expected, part)
+	}
+
+	return input, expected
+}

pkg/compiler/internal/diagnostics/match_assignment_errors.go

@@ -7,6 +7,10 @@ import (
 )
 
 func matchMissingAssignmentValue(src *file.Source, err *CompilationError, offending *TokenNode) bool {
+	if isExtraneous(err.Message) {
+		return false
+	}
+
 	prev := offending.Prev()
 
 	if is(offending, "LET") || is(prev, "=") {

pkg/compiler/internal/diagnostics/match_for_loop_errors.go

@@ -64,5 +64,63 @@ func matchForLoopErrors(src *file.Source, err *CompilationError, offending *Toke
 		}
 	}
 
+	if is(prev, "FILTER") {
+		span := spanFromTokenSafe(prev.Token(), src)
+		span.Start = span.End
+		span.End = span.Start + 1
+
+		err.Message = "Expected condition after 'FILTER'"
+		err.Hint = "FILTER requires a boolean expression."
+		err.Spans = []ErrorSpan{
+			NewMainErrorSpan(span, "missing expression"),
+		}
+
+		return true
+	}
+
+	if is(prev, "LIMIT") {
+		span := spanFromTokenSafe(prev.Token(), src)
+		span.Start = span.End
+		span.End = span.Start + 1
+
+		err.Message = "Expected number after 'LIMIT'"
+		err.Hint = "LIMIT requires a numeric value."
+		err.Spans = []ErrorSpan{
+			NewMainErrorSpan(span, "missing expression"),
+		}
+
+		return true
+	}
+
+	if isExtraneous(err.Message) {
+		input := parseExtraneousInput(err.Message)
+
+		if input != "','" {
+			return false
+		}
+
+		var steps int
+
+		// We walk back two tokens to find if the keyword is LIMIT.
+		for ; steps < 2 && prev != nil; steps++ {
+			prev = prev.Prev()
+		}
+
+		if is(prev, "LIMIT") {
+			limitSpan := spanFromTokenSafe(prev.Token(), src)
+			span := spanFromTokenSafe(offending.Token(), src)
+			span.Start = limitSpan.End + 1
+			span.End += 4
+
+			err.Message = "Too many arguments provided to LIMIT clause"
+			err.Hint = "LIMIT accepts at most two arguments: offset and count."
+			err.Spans = []ErrorSpan{
+				NewMainErrorSpan(span, "unexpected third argument"),
+			}
+
+			return true
+		}
+	}
+
 	return false
 }

pkg/compiler/internal/diagnostics/match_return_value_errors.go

@@ -7,10 +7,24 @@ import (
 )
 
 func matchMissingReturnValue(src *file.Source, err *CompilationError, offending *TokenNode) bool {
-	if !is(offending, "RETURN") {
+	extraneous := isExtraneous(err.Message)
+
+	if !is(offending, "RETURN") && !extraneous {
 		return false
 	}
 
+	if extraneous {
+		span := spanFromTokenSafe(offending.Token(), src)
+		err.Spans = []ErrorSpan{
+			NewMainErrorSpan(span, "query must end with a value"),
+		}
+
+		err.Message = "Expected a RETURN or FOR clause at end of query"
+		err.Hint = "All queries must return a value. Add a RETURN statement to complete the query."
+
+		return true
+	}
+
 	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/parser/parser.go

@@ -40,6 +40,10 @@ func (p *Parser) AddErrorListener(listener antlr.ErrorListener) {
 	p.tree.AddErrorListener(listener)
 }
 
+func (p *Parser) RemoveErrorListeners() {
+	p.tree.RemoveErrorListeners()
+}
+
 func (p *Parser) Visit(visitor fql.FqlParserVisitor) interface{} {
 	return visitor.VisitProgram(p.tree.Program().(*fql.ProgramContext))
 }

test/integration/base/assertions.go

@@ -2,6 +2,7 @@ package base
 
 import (
 	"fmt"
+
 	"github.com/smarty/assertions"
 
 	"github.com/MontFerret/ferret/pkg/compiler"
@@ -110,7 +111,13 @@ func ShouldBeCompilationError(actual any, expected ...any) string {
 		err, ok := actual.(*compiler.CompilationError)
 
 		if !ok {
-			return "expected a compilation error"
+			err2, ok := actual.(*compiler.MultiCompilationError)
+
+			if !ok {
+				return "expected a compilation error"
+			}
+
+			err = err2.Errors[0]
 		}
 
 		msg = assertExpectedError(err, ex)

test/integration/compiler/compiler_errors_syntax_test.go

@@ -16,6 +16,7 @@ func TestSyntaxErrors(t *testing.T) {
 				Message: "Expected a RETURN or FOR clause at end of query",
 				Hint:    "All queries must return a value. Add a RETURN statement to complete the query.",
 			}, "Missing return statement"),
+
 		ErrorCase(
 			`
 			LET i = NONE
@@ -25,6 +26,7 @@ func TestSyntaxErrors(t *testing.T) {
 				Message: "Expected expression after 'RETURN'",
 				Hint:    "Did you forget to provide a value to return?",
 			}, "Missing return value"),
+
 		ErrorCase(
 			`
 			FOR i IN [1, 2, 3]
@@ -34,6 +36,7 @@ func TestSyntaxErrors(t *testing.T) {
 				Message: "Expected expression after 'RETURN'",
 				Hint:    "Did you forget to provide a value to return?",
 			}, "Missing return value in for loop"),
+
 		ErrorCase(
 			`
 			LET i =
@@ -43,6 +46,7 @@ func TestSyntaxErrors(t *testing.T) {
 				Message: "Expected expression after '=' for variable 'i'",
 				Hint:    "Did you forget to provide a value?",
 			}, "Missing variable assignment value"),
+
 		ErrorCase(
 			`
 			FOR i IN 
@@ -52,6 +56,7 @@ func TestSyntaxErrors(t *testing.T) {
 				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]
@@ -61,6 +66,7 @@ func TestSyntaxErrors(t *testing.T) {
 				Message: "Expected 'IN' after loop variable",
 				Hint:    "Use 'FOR x IN [iterable]' syntax.",
 			}, "Missing IN in FOR"),
+
 		ErrorCase(
 			`
 			FOR IN [1, 2, 3]
@@ -70,6 +76,7 @@ func TestSyntaxErrors(t *testing.T) {
 				Message: "Expected loop variable before 'IN'",
 				Hint:    "FOR must declare a variable.",
 			}, "FOR without variable"),
+
 		ErrorCase(
 			`
 			LET users = []
@@ -81,16 +88,77 @@ func TestSyntaxErrors(t *testing.T) {
 				Message: "Expected variable before '=' in COLLECT",
 				Hint:    "COLLECT must group by a variable.",
 			}, "COLLECT with no variable"),
+
 		ErrorCase(
 			`
 			LET users = []
 			FOR x IN users
-				COLLECT i =
+				COLLECT AGGREGATE total = 
+				RETURN total
+		`, E{
+				Kind:    compiler.SyntaxError,
+				Message: "Expected expression after '=' for variable 'total'",
+				Hint:    "Did you forget to provide a value?",
+			}, "COLLECT AGGREGATE without expression"),
+
+		ErrorCase(
+			`
+			LET users = []
+			FOR x IN users
+				FILTER
 				RETURN x
 		`, E{
 				Kind:    compiler.SyntaxError,
-				Message: "Expected expression after '=' for variable 'i'",
-				Hint:    "Did you forget to provide a value?",
-			}, "COLLECT with no variable assignment"),
+				Message: "Expected condition after 'FILTER'",
+				Hint:    "FILTER requires a boolean expression.",
+			}, "FILTER with no expression"),
+
+		ErrorCase(
+			`
+			LET users = []
+			FOR x IN users
+				LIMIT
+				RETURN x
+		`, E{
+				Kind:    compiler.SyntaxError,
+				Message: "Expected number after 'LIMIT'",
+				Hint:    "LIMIT requires a numeric value.",
+			}, "LIMIT with no value"),
+
+		ErrorCase(
+			`
+			LET users = []
+			FOR x IN users
+				LIMIT 1, 2, 3
+				RETURN x
+		`, E{
+				Kind:    compiler.SyntaxError,
+				Message: "Too many arguments provided to LIMIT clause",
+				Hint:    "LIMIT accepts at most two arguments: offset and count.",
+			}, "LIMIT with too many values"),
+
+		ErrorCase(
+			`
+			LET users = []
+			FOR x IN users
+				LIMIT 1, 2,
+				RETURN x
+		`, E{
+				Kind:    compiler.SyntaxError,
+				Message: "Too many arguments provided to LIMIT clause",
+				Hint:    "LIMIT accepts at most two arguments: offset and count.",
+			}, "LIMIT unexpected comma"),
+
+		ErrorCase(
+			`
+			LET users = []
+			FOR x IN users
+				LIMIT 1,
+				RETURN x
+		`, E{
+				Kind:    compiler.SyntaxError,
+				Message: "---",
+				Hint:    "FILTER requires a boolean expression.",
+			}, "LIMIT unexpected comma 2"),
 	})
 }