diff --git a/pkg/compiler/compiler.go b/pkg/compiler/compiler.go index dd5e0432..8e639360 100644 --- a/pkg/compiler/compiler.go +++ b/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) diff --git a/pkg/compiler/internal/diagnostics/error_listener.go b/pkg/compiler/internal/diagnostics/error_listener.go index 8024220f..200aadbd 100644 --- a/pkg/compiler/internal/diagnostics/error_listener.go +++ b/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()) -} diff --git a/pkg/compiler/internal/diagnostics/helpers.go b/pkg/compiler/internal/diagnostics/helpers.go index 289f8976..56d7c38f 100644 --- a/pkg/compiler/internal/diagnostics/helpers.go +++ b/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.+?)\s+expecting`) + match := re.FindStringSubmatch(msg) + return match[re.SubexpIndex("input")] +} + +func parseExtraneousInputAll(msg string) (string, []string) { + rx := regexp.MustCompile(`extraneous input\s+(?P.+?)\s+expecting\s+\{(?P.+?)\}`) + 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 +} diff --git a/pkg/compiler/internal/diagnostics/match_assignment_errors.go b/pkg/compiler/internal/diagnostics/match_assignment_errors.go index a66db4b1..3a284ae6 100644 --- a/pkg/compiler/internal/diagnostics/match_assignment_errors.go +++ b/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, "=") { diff --git a/pkg/compiler/internal/diagnostics/match_for_loop_errors.go b/pkg/compiler/internal/diagnostics/match_for_loop_errors.go index 75f7ebd3..6a4bc917 100644 --- a/pkg/compiler/internal/diagnostics/match_for_loop_errors.go +++ b/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 } diff --git a/pkg/compiler/internal/diagnostics/match_return_value_errors.go b/pkg/compiler/internal/diagnostics/match_return_value_errors.go index 91fce425..58af344d 100644 --- a/pkg/compiler/internal/diagnostics/match_return_value_errors.go +++ b/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?" diff --git a/pkg/parser/parser.go b/pkg/parser/parser.go index 2d00d6de..0d0274c9 100644 --- a/pkg/parser/parser.go +++ b/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)) } diff --git a/test/integration/base/assertions.go b/test/integration/base/assertions.go index 258b1c14..210d5e52 100644 --- a/test/integration/base/assertions.go +++ b/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) diff --git a/test/integration/compiler/compiler_errors_syntax_test.go b/test/integration/compiler/compiler_errors_syntax_test.go index 06adcbc3..848b220f 100644 --- a/test/integration/compiler/compiler_errors_syntax_test.go +++ b/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"), }) }