From b69f8af7162a540dcda4fea0d76564b2d7b2816d Mon Sep 17 00:00:00 2001 From: Tim Voronov Date: Sun, 10 Aug 2025 12:30:07 -0400 Subject: [PATCH] Add syntax error tests for FOR loops and COLLECT statements: enhance diagnostics for missing values, incomplete clauses, and unexpected syntax to improve error handling and clarity. --- pkg/compiler/internal/diagnostics/helpers.go | 16 ++ .../diagnostics/match_assignment_errors.go | 19 +- .../diagnostics/match_common_errors.go | 4 + .../diagnostics/match_for_loop_errors.go | 44 +++- .../diagnostics/match_return_value_errors.go | 2 + pkg/file/helpers.go | 12 ++ pkg/file/snippet.go | 1 - pkg/file/source.go | 26 ++- .../compiler_errors_syntax_loop_test.go | 197 ++++++++++++++++++ .../compiler/compiler_errors_syntax_test.go | 171 ++++----------- 10 files changed, 355 insertions(+), 137 deletions(-) create mode 100644 test/integration/compiler/compiler_errors_syntax_loop_test.go diff --git a/pkg/compiler/internal/diagnostics/helpers.go b/pkg/compiler/internal/diagnostics/helpers.go index 21e15c6e..625d7e89 100644 --- a/pkg/compiler/internal/diagnostics/helpers.go +++ b/pkg/compiler/internal/diagnostics/helpers.go @@ -110,10 +110,26 @@ func is(node *TokenNode, expected string) bool { return strings.ToUpper(node.GetText()) == expected } +func anyIs(first, second *TokenNode, expected string) *TokenNode { + if is(first, expected) { + return first + } + + if is(second, expected) { + return second + } + + return nil +} + func has(msg string, substr string) bool { return strings.Contains(strings.ToLower(msg), strings.ToLower(substr)) } +func isMismatched(msg string) bool { + return has(msg, "mismatched input") +} + func isNoAlternative(msg string) bool { return has(msg, "no viable alternative at input") } diff --git a/pkg/compiler/internal/diagnostics/match_assignment_errors.go b/pkg/compiler/internal/diagnostics/match_assignment_errors.go index 3a284ae6..fcff3054 100644 --- a/pkg/compiler/internal/diagnostics/match_assignment_errors.go +++ b/pkg/compiler/internal/diagnostics/match_assignment_errors.go @@ -13,11 +13,11 @@ func matchMissingAssignmentValue(src *file.Source, err *CompilationError, offend prev := offending.Prev() - if is(offending, "LET") || is(prev, "=") { - span := spanFromTokenSafe(prev.Token(), src) + if node := anyIs(offending, prev, "="); node != nil { + span := spanFromTokenSafe(node.Token(), src) span.Start++ span.End++ - err.Message = fmt.Sprintf("Expected expression after '=' for variable '%s'", prev.Prev()) + err.Message = fmt.Sprintf("Expected expression after '=' for variable '%s'", node.Prev()) err.Hint = "Did you forget to provide a value?" err.Spans = []ErrorSpan{ NewMainErrorSpan(span, "missing value"), @@ -26,5 +26,18 @@ func matchMissingAssignmentValue(src *file.Source, err *CompilationError, offend return true } + if is(offending, "LET") { + span := spanFromTokenSafe(offending.Token(), src) + span.Start = span.End + span.End = span.Start + 1 + err.Message = "Expected variable name" + err.Hint = "Did you forget to provide a variable name?" + err.Spans = []ErrorSpan{ + NewMainErrorSpan(span, "missing name"), + } + + return true + } + return false } diff --git a/pkg/compiler/internal/diagnostics/match_common_errors.go b/pkg/compiler/internal/diagnostics/match_common_errors.go index 316592c5..87bae372 100644 --- a/pkg/compiler/internal/diagnostics/match_common_errors.go +++ b/pkg/compiler/internal/diagnostics/match_common_errors.go @@ -19,5 +19,9 @@ func matchCommonErrors(src *file.Source, err *CompilationError, offending *Token } } + if isMismatched(err.Message) { + + } + return false } diff --git a/pkg/compiler/internal/diagnostics/match_for_loop_errors.go b/pkg/compiler/internal/diagnostics/match_for_loop_errors.go index ff8fef06..93acee27 100644 --- a/pkg/compiler/internal/diagnostics/match_for_loop_errors.go +++ b/pkg/compiler/internal/diagnostics/match_for_loop_errors.go @@ -62,6 +62,48 @@ func matchForLoopErrors(src *file.Source, err *CompilationError, offending *Toke NewMainErrorSpan(span, "missing variable"), } + return true + } else if isNoAlternative(msg) { + span := spanFromTokenSafe(offending.Token(), src) + span.Start = span.End + span.End = span.Start + 1 + + err.Message = "Incomplete COLLECT clause" + err.Hint = "COLLECT must specify a grouping key, an AGGREGATE clause, or WITH COUNT." + err.Spans = []ErrorSpan{ + NewMainErrorSpan(span, "missing grouping or aggregation"), + } + + return true + } + } + + if is(offending, "INTO") { + span := spanFromTokenSafe(offending.Token(), src) + span.Start = span.End + 1 + span.End = span.Start + 1 + + err.Message = "Expected variable name after INTO" + err.Hint = "Provide a variable name to store grouped values, e.g. INTO groups." + err.Spans = []ErrorSpan{ + NewMainErrorSpan(span, "missing variable name"), + } + + return true + } + + if is(offending, "AGGREGATE") { + if isNoAlternative(err.Message) { + span := spanFromTokenSafe(offending.Token(), src) + span.Start = span.End + 1 + span.End = span.Start + 1 + + err.Message = "Expected variable assignment after AGGREGATE" + err.Hint = "Provide at least one variable assignment, e.g. AGGREGATE total = COUNT(x)." + err.Spans = []ErrorSpan{ + NewMainErrorSpan(span, "missing variable assignment"), + } + return true } } @@ -71,7 +113,7 @@ func matchForLoopErrors(src *file.Source, err *CompilationError, offending *Toke span.Start = span.End span.End = span.Start + 1 - err.Message = "Expected condition after 'FILTER'" + err.Message = "Incomplete FILTER clause" err.Hint = "FILTER requires a boolean expression." err.Spans = []ErrorSpan{ NewMainErrorSpan(span, "missing expression"), diff --git a/pkg/compiler/internal/diagnostics/match_return_value_errors.go b/pkg/compiler/internal/diagnostics/match_return_value_errors.go index 58af344d..bf3b7919 100644 --- a/pkg/compiler/internal/diagnostics/match_return_value_errors.go +++ b/pkg/compiler/internal/diagnostics/match_return_value_errors.go @@ -26,6 +26,8 @@ func matchMissingReturnValue(src *file.Source, err *CompilationError, offending } span := spanFromTokenSafe(offending.Token(), src) + span.Start = span.End + span.End = span.Start + 1 err.Message = fmt.Sprintf("Expected expression after '%s'", offending) err.Hint = "Did you forget to provide a value to return?" err.Spans = []ErrorSpan{ diff --git a/pkg/file/helpers.go b/pkg/file/helpers.go index d64c4687..4a32c901 100644 --- a/pkg/file/helpers.go +++ b/pkg/file/helpers.go @@ -13,3 +13,15 @@ func SkipWhitespaceForward(content string, offset int) int { return offset } + +func SkipHorizontalWhitespaceForward(content string, offset int) int { + for offset < len(content) { + ch := content[offset] + // Skip spaces and tabs only; do NOT cross line breaks + if ch != ' ' && ch != '\t' { + break + } + offset++ + } + return offset +} diff --git a/pkg/file/snippet.go b/pkg/file/snippet.go index 4c534dea..5cee9568 100644 --- a/pkg/file/snippet.go +++ b/pkg/file/snippet.go @@ -34,7 +34,6 @@ func NewSnippetWithCaret(lines []string, span Span, line int) Snippet { endCol := computeVisualOffset(srcLine, span.End-lineStartOffset+1) caret := "" - if endCol <= startCol+1 { caret = strings.Repeat(" ", startCol) + "^" } else { diff --git a/pkg/file/source.go b/pkg/file/source.go index 580b7b0a..49e7769e 100644 --- a/pkg/file/source.go +++ b/pkg/file/source.go @@ -43,20 +43,36 @@ func (s *Source) LocationAt(span Span) (line, column int) { return 0, 0 } - total := 0 offset := span.Start + total := 0 for i, l := range s.lines { - lineLen := len(l) + 1 // +1 for newline + lineLen := len(l) + 1 // +1 for '\n' + lineStart := total + lineEndWithNL := total + lineLen - if total+lineLen > offset { + // If offset is exactly at the start of this line (not the very first line), + // treat it as the end of the previous line. + if offset == lineStart && i > 0 { + prev := s.lines[i-1] + return i, len(prev) + 1 + } + + if lineEndWithNL > offset { + // Normal case: offset lives on this line return i + 1, offset - total + 1 } - total += lineLen + total = lineEndWithNL } - return total, 1 + // If we somehow fell through, clamp to last line end + if len(s.lines) > 0 { + last := s.lines[len(s.lines)-1] + return len(s.lines), len(last) + 1 + } + + return 0, 0 } func (s *Source) Snippet(span Span) []Snippet { diff --git a/test/integration/compiler/compiler_errors_syntax_loop_test.go b/test/integration/compiler/compiler_errors_syntax_loop_test.go new file mode 100644 index 00000000..d32ae1c1 --- /dev/null +++ b/test/integration/compiler/compiler_errors_syntax_loop_test.go @@ -0,0 +1,197 @@ +package compiler_test + +import ( + "testing" + + "github.com/MontFerret/ferret/pkg/compiler" +) + +func TestForLoopSyntaxErrors(t *testing.T) { + RunUseCases(t, []UseCase{ + ErrorCase( + ` + FOR i IN [1, 2, 3] + RETURN + `, E{ + Kind: compiler.SyntaxError, + Message: "Expected expression after 'RETURN'", + Hint: "Did you forget to provide a value to return?", + }, "Missing return value in for loop"), + + ErrorCase( + ` + FOR i IN + RETURN i + `, E{ + Kind: compiler.SyntaxError, + 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: "Expected loop variable before 'IN'", + Hint: "FOR must declare a variable.", + }, "FOR without variable"), + + ErrorCase( + ` + LET users = [] + FOR x IN users + FILTER + RETURN x + `, E{ + Kind: compiler.SyntaxError, + Message: "Incomplete FILTER clause", + 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: "Dangling comma in LIMIT clause", + Hint: "LIMIT accepts one or two arguments. Did you forget to add a value?", + }, "LIMIT unexpected comma 2"), + + ErrorCase( + ` + LET users = [] + FOR x IN users + LIMIT , + RETURN x + `, E{ + Kind: compiler.SyntaxError, + Message: "Dangling comma in LIMIT clause", + Hint: "LIMIT accepts one or two arguments. Did you forget to add a value?", + }, "LIMIT unexpected comma 3"), + + ErrorCase( + ` + LET users = [] + FOR x IN users + LIMIT, + RETURN x + `, E{ + Kind: compiler.SyntaxError, + Message: "Dangling comma in LIMIT clause", + Hint: "LIMIT accepts one or two arguments. Did you forget to add a value?", + }, "LIMIT unexpected comma 4"), + + ErrorCase( + ` + LET users = [] + FOR x IN users + COLLECT = + RETURN x + `, E{ + Kind: compiler.SyntaxError, + 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 + RETURN x + `, E{ + Kind: compiler.SyntaxError, + Message: "Incomplete COLLECT clause", + Hint: "COLLECT must specify a grouping key, an AGGREGATE clause, or WITH COUNT.", + }, "COLLECT with no variables"), + + ErrorCase( + ` + LET users = [] + FOR i IN users + COLLECT gender = i.gender INTO + RETURN { + gender, + values + }`, E{ + Kind: compiler.SyntaxError, + Message: "Expected variable name after INTO", + Hint: "Provide a variable name to store grouped values, e.g. INTO groups.", + }, "COLLECT INTO with no variable"), + + ErrorCase( + ` + LET users = [] + FOR x IN users + 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 + COLLECT AGGREGATE + RETURN total + `, E{ + Kind: compiler.SyntaxError, + Message: "Expected variable assignment after AGGREGATE", + Hint: "Provide at least one variable assignment, e.g. AGGREGATE total = COUNT(x).", + }, "COLLECT AGGREGATE without expression 2"), + }) +} diff --git a/test/integration/compiler/compiler_errors_syntax_test.go b/test/integration/compiler/compiler_errors_syntax_test.go index 60ecdca2..e1632d61 100644 --- a/test/integration/compiler/compiler_errors_syntax_test.go +++ b/test/integration/compiler/compiler_errors_syntax_test.go @@ -8,6 +8,25 @@ import ( func TestSyntaxErrors(t *testing.T) { RunUseCases(t, []UseCase{ + ErrorCase( + ` + LET + `, E{ + Kind: compiler.SyntaxError, + Message: "Expected variable name", + Hint: "Did you forget to provide a variable name?", + }, "Missing variable name"), + + ErrorCase( + ` + LET + RETURN 5 + `, E{ + Kind: compiler.SyntaxError, + Message: "Expected variable name", + Hint: "Did you forget to provide a variable name?", + }, "Missing variable name 2"), + ErrorCase( ` LET i = NONE @@ -27,16 +46,6 @@ func TestSyntaxErrors(t *testing.T) { Hint: "Did you forget to provide a value to return?", }, "Missing return value"), - ErrorCase( - ` - FOR i IN [1, 2, 3] - RETURN - `, E{ - Kind: compiler.SyntaxError, - Message: "Expected expression after 'RETURN'", - Hint: "Did you forget to provide a value to return?", - }, "Missing return value in for loop"), - ErrorCase( ` LET i = @@ -49,138 +58,46 @@ func TestSyntaxErrors(t *testing.T) { ErrorCase( ` - FOR i IN - RETURN i + LET i = + LET j = 5 + RETURN i `, E{ Kind: compiler.SyntaxError, - 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: "Expected loop variable before 'IN'", - Hint: "FOR must declare a variable.", - }, "FOR without variable"), - - ErrorCase( - ` - LET users = [] - FOR x IN users - COLLECT = - RETURN x - `, E{ - Kind: compiler.SyntaxError, - 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 AGGREGATE total = - RETURN total - `, E{ - Kind: compiler.SyntaxError, - Message: "Expected expression after '=' for variable 'total'", + Message: "Expected expression after '=' for variable 'i'", Hint: "Did you forget to provide a value?", - }, "COLLECT AGGREGATE without expression"), + }, "Missing variable assignment value 2"), ErrorCase( ` - LET users = [] - FOR x IN users - FILTER - RETURN x + LET i = + FOR j IN [1, 2, 3] RETURN j `, E{ Kind: compiler.SyntaxError, - Message: "Expected condition after 'FILTER'", - Hint: "FILTER requires a boolean expression.", - }, "FILTER with no expression"), + Message: "Expected expression after '=' for variable 'i'", + Hint: "Did you forget to provide a value?", + }, "Missing variable assignment value 3"), - ErrorCase( + SkipErrorCase( ` - LET users = [] - FOR x IN users - LIMIT - RETURN x + LET o = { foo: "bar" } + LET i = o. + RETURN i `, E{ Kind: compiler.SyntaxError, - Message: "Expected number after 'LIMIT'", - Hint: "LIMIT requires a numeric value.", - }, "LIMIT with no value"), + Message: "Expected expression after '=' for variable 'i'", + Hint: "Did you forget to provide a value?", + }, "Incomplete member access"), - ErrorCase( + SkipErrorCase( ` - LET users = [] - FOR x IN users - LIMIT 1, 2, 3 - RETURN x + LET o = { foo: "bar" } + LET i = o. + FUNC(i) + RETURN i `, 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: "Dangling comma in LIMIT clause", - Hint: "LIMIT accepts one or two arguments. Did you forget to add a value?", - }, "LIMIT unexpected comma 2"), - ErrorCase( - ` - LET users = [] - FOR x IN users - LIMIT , - RETURN x - `, E{ - Kind: compiler.SyntaxError, - Message: "Dangling comma in LIMIT clause", - Hint: "LIMIT accepts one or two arguments. Did you forget to add a value?", - }, "LIMIT unexpected comma 3"), - ErrorCase( - ` - LET users = [] - FOR x IN users - LIMIT, - RETURN x - `, E{ - Kind: compiler.SyntaxError, - Message: "Dangling comma in LIMIT clause", - Hint: "LIMIT accepts one or two arguments. Did you forget to add a value?", - }, "LIMIT unexpected comma 4"), + Message: "Expected expression after '=' for variable 'i'", + Hint: "Did you forget to provide a value?", + }, "Incomplete member access 2"), }) }