From 9d3313995ed698a8290175dc490b7179bb11e7ef Mon Sep 17 00:00:00 2001 From: dkanus Date: Wed, 29 Apr 2026 13:50:45 +0700 Subject: [PATCH 1/2] Add delimeter matching to lexer --- dev_tests/src/pretty.rs | 13 - dev_tests/src/uc_lexer_verify.rs | 92 ++-- dev_tests/src/verify_expr.rs | 136 ++--- rottlib/src/lexer/mod.rs | 467 +++++++++++++++++- rottlib/src/parser/cursor.rs | 27 + .../expression/control_flow/for_loop.rs | 4 +- .../grammar/expression/control_flow/mod.rs | 28 +- .../parser/grammar/expression/primary/mod.rs | 4 +- .../parser/grammar/expression/primary/new.rs | 4 +- rottlib/src/parser/recovery.rs | 204 ++++++++ rottlib/tests/lexer_diagnostics.rs | 350 +++++++++++++ 11 files changed, 1196 insertions(+), 133 deletions(-) delete mode 100644 dev_tests/src/pretty.rs create mode 100644 rottlib/tests/lexer_diagnostics.rs diff --git a/dev_tests/src/pretty.rs b/dev_tests/src/pretty.rs deleted file mode 100644 index 1ff1ffe..0000000 --- a/dev_tests/src/pretty.rs +++ /dev/null @@ -1,13 +0,0 @@ -// diagnostics_render.rs - -use rottlib::diagnostics::{Diagnostic}; -use rottlib::lexer::TokenizedFile; - -pub fn render_diagnostic( - diag: &Diagnostic, - _file: &TokenizedFile, - file_name: Option<&str>, - colors: bool, -) { - diag.render(_file, file_name.unwrap_or("")); -} diff --git a/dev_tests/src/uc_lexer_verify.rs b/dev_tests/src/uc_lexer_verify.rs index 3080fac..e451402 100644 --- a/dev_tests/src/uc_lexer_verify.rs +++ b/dev_tests/src/uc_lexer_verify.rs @@ -19,8 +19,6 @@ use rottlib::diagnostics::Diagnostic as Diag; use rottlib::lexer::TokenizedFile; use rottlib::parser::Parser; -mod pretty; - // ---------- CONFIG ---------- const FILE_LIMIT: usize = 10000; // cap on files scanned const DIAG_SHOW_FIRST: usize = 12; // show first N diagnostics @@ -35,6 +33,43 @@ const ALSO_PRINT_DEBUG_AFTER_PRETTY: bool = true; // chardet = "0.2" // encoding_rs = "0.8" +fn render_diagnostic(diag: &Diag, file: &TokenizedFile<'_>, file_name: &str) { + diag.render(file, file_name); +} + +fn render_diagnostics_window(diags: &[Diag], tf: &TokenizedFile<'_>, file_name: &str) { + let total = diags.len(); + let first_n = DIAG_SHOW_FIRST.min(total); + let last_n = DIAG_SHOW_LAST.min(total.saturating_sub(first_n)); + + if total > first_n + last_n { + for (k, d) in diags.iter().take(first_n).enumerate() { + render_diagnostic(d, tf, file_name); + if ALSO_PRINT_DEBUG_AFTER_PRETTY { + eprintln!("#{}: {:#?}", k + 1, d); + } + } + + eprintln!("... {} diagnostics omitted ...", total - (first_n + last_n)); + + let start = total - last_n; + for (offset, d) in diags.iter().skip(start).enumerate() { + let idx_global = start + offset + 1; + render_diagnostic(d, tf, file_name); + if ALSO_PRINT_DEBUG_AFTER_PRETTY { + eprintln!("#{idx_global}: {d:#?}"); + } + } + } else { + for (k, d) in diags.iter().enumerate() { + render_diagnostic(d, tf, file_name); + if ALSO_PRINT_DEBUG_AFTER_PRETTY { + eprintln!("#{}: {:#?}", k + 1, d); + } + } + } +} + // Linux-only accurate RSS in MB. Fallback uses sysinfo. fn rss_mb() -> u64 { #[cfg(target_os = "linux")] @@ -230,11 +265,24 @@ fn main() { mark("after_tokenize", t0); // If tokenization error: wait, dump tokens for the first failing file, then exit. + // If tokenization error: wait, print lexer diagnostics for the first failing file, then exit. if let Some(idx) = tk_error_idx { - let (bad_path, _) = &tokenized[idx]; - wait_before_errors("Tokenization error found. Press Enter to dump tokens..."); - eprintln!("--- Tokenization error in: {}", bad_path.display()); - //bad_tf.dump_debug_layout(); // from DebugTools + let (bad_path, bad_tf) = &tokenized[idx]; + wait_before_errors("Tokenization issues detected. Press Enter to print diagnostics..."); + + let fname = bad_path.display().to_string(); + + eprintln!("--- Tokenization issues in first failing file ---"); + eprintln!("File: {fname}"); + + let diags = bad_tf.diagnostics(); + + if diags.is_empty() { + eprintln!("(no diagnostics captured)"); + } else { + render_diagnostics_window(diags, bad_tf, &fname); + } + std::process::exit(1); } @@ -298,38 +346,8 @@ fn main() { if diags.is_empty() && fatal.is_none() { eprintln!("(no diagnostics captured)"); } else { - let use_colors = is_terminal::is_terminal(io::stderr()); let fname = path.display().to_string(); - let total = diags.len(); - let first_n = DIAG_SHOW_FIRST.min(total); - let last_n = DIAG_SHOW_LAST.min(total.saturating_sub(first_n)); - - if total > first_n + last_n { - // first window - for (k, d) in diags.iter().take(first_n).enumerate() { - let s = pretty::render_diagnostic(d, tf, Some(&fname), use_colors); - if ALSO_PRINT_DEBUG_AFTER_PRETTY { - eprintln!("#{}: {:#?}", k + 1, d); - } - } - eprintln!("... {} diagnostics omitted ...", total - (first_n + last_n)); - // last window - let start = total - last_n; - for (offset, d) in diags.iter().skip(start).enumerate() { - let idx_global = start + offset + 1; - let s = pretty::render_diagnostic(d, tf, Some(&fname), use_colors); - if ALSO_PRINT_DEBUG_AFTER_PRETTY { - eprintln!("#{idx_global}: {d:#?}"); - } - } - } else { - for (k, d) in diags.iter().enumerate() { - let s = pretty::render_diagnostic(d, tf, Some(&fname), use_colors); - if ALSO_PRINT_DEBUG_AFTER_PRETTY { - eprintln!("#{}: {:#?}", k + 1, d); - } - } - } + render_diagnostics_window(&diags, tf, &fname); } std::process::exit(1); } diff --git a/dev_tests/src/verify_expr.rs b/dev_tests/src/verify_expr.rs index 8736688..e37fc12 100644 --- a/dev_tests/src/verify_expr.rs +++ b/dev_tests/src/verify_expr.rs @@ -7,69 +7,63 @@ )] use rottlib::arena::Arena; +use rottlib::diagnostics::Diagnostic; use rottlib::lexer::TokenizedFile; use rottlib::parser::Parser; -mod pretty; - -// a * * * - -/// Expressions to test. +/// Lexer-focused fixtures. /// -/// Add, remove, or edit entries here. -/// Using `(&str, &str)` gives each case a human-readable label. -/// Expressions to test. -/// -/// Add, remove, or edit entries here. -/// Using `(&str, &str)` gives each case a human-readable label. +/// Keep these small: the goal is to inspect lexer diagnostics and delimiter +/// recovery behavior, not full parser behavior. const TEST_CASES: &[(&str, &str)] = &[ - // P0022: invalid return value start + // L0001: invalid or unknown token ( - "files/P0022_01.uc", - "return ] ;", - ), - ( - "files/P0022_02.uc", - "return\n ]\n;\n", - ), - ( - "files/P0022_03.uc", - "return\n}\n", + "files/L0001_01.uc", + "`", ), - // P0023: invalid break value start + // L0002: unexpected closing delimiter ( - "files/P0023_01.uc", - "break ] ;", - ), - ( - "files/P0023_02.uc", - "break\n \n\n\n\n ]\n;\n", - ), - ( - "files/P0023_03.uc", - "break\n}\n", + "files/L0002_01.uc", + "]", ), - // P0024: goto target is missing or not a label token + // L0003: unclosed delimiter before later closing delimiter + // + // The `}` can still recover by matching the earlier `{`. ( - "files/P0024_01.uc", - "goto;", + "files/L0003_01.uc", + "{\n foo(\n}\n", ), + + // L0004: mismatched closing delimiter ( - "files/P0024_02.uc", - "goto\n ;\n", + "files/L0004_01.uc", + "(]", ), + + // L0005: unclosed delimiter at end of file ( - "files/P0024_03.uc", - "goto\n ]\n;\n", + "files/L0005_01.uc", + "foo(", ), + + // Mixed recovery case: + // + // `)` recovers by matching `(` after treating `[` as unclosed; + // the following `]` is then unexpected. ( - "files/P0024_04.uc", - "goto", + "files/L_mixed_01.uc", + "([)]", ), ]; +/// If true, also run the parser after tokenization. +/// +/// For lexer-focused fixtures this is usually noisy, so keep it off unless you +/// want to inspect how parser recovery behaves after lexer diagnostics. +const RUN_PARSER: bool = false; + /// If true, print the parsed expression using Debug formatting. const PRINT_PARSED_EXPR: bool = false; @@ -82,24 +76,46 @@ fn main() { let mut had_any_problem = false; for (idx, (label, source)) in TEST_CASES.iter().enumerate() { + println!("============================================================"); + println!("Case {}: {}", idx + 1, label); + println!("------------------------------------------------------------"); + let tf = TokenizedFile::tokenize(source); - let mut parser = Parser::new(&tf, &arena); - let expr = parser.parse_expression(); + let lexer_diagnostics = tf.diagnostics(); - if PRINT_PARSED_EXPR { - println!("Parsed expression:"); - println!("{expr:#?}"); - } - - if parser.diagnostics.is_empty() { - println!("Diagnostics: none"); + if lexer_diagnostics.is_empty() { + println!("Lexer diagnostics: none"); } else { had_any_problem = true; + if ALWAYS_PRINT_DIAGNOSTICS { - let use_colors = false; - for (k, diag) in parser.diagnostics.iter().enumerate() { - pretty::render_diagnostic(diag, &tf, Some(label), use_colors); + println!("Lexer diagnostics:"); + for diag in lexer_diagnostics { + render_diagnostic(diag, &tf, Some(label), false); + } + } + } + + if RUN_PARSER { + let mut parser = Parser::new(&tf, &arena); + let expr = parser.parse_expression(); + + if PRINT_PARSED_EXPR { + println!("Parsed expression:"); + println!("{expr:#?}"); + } + + if parser.diagnostics.is_empty() { + println!("Parser diagnostics: none"); + } else { + had_any_problem = true; + + if ALWAYS_PRINT_DIAGNOSTICS { + println!("Parser diagnostics:"); + for diag in &parser.diagnostics { + render_diagnostic(diag, &tf, Some(label), false); + } } } } @@ -108,10 +124,20 @@ fn main() { } println!("============================================================"); + if had_any_problem { - println!("Done. At least one case had tokenization or parse diagnostics."); + println!("Done. At least one case had lexer or parser diagnostics."); std::process::exit(1); } else { println!("Done. All cases completed without diagnostics."); } } + +fn render_diagnostic( + diag: &Diagnostic, + file: &TokenizedFile<'_>, + file_name: Option<&str>, + _colors: bool, +) { + diag.render(file, file_name.unwrap_or("")); +} \ No newline at end of file diff --git a/rottlib/src/lexer/mod.rs b/rottlib/src/lexer/mod.rs index 41b8ac1..4fec620 100644 --- a/rottlib/src/lexer/mod.rs +++ b/rottlib/src/lexer/mod.rs @@ -30,6 +30,7 @@ use std::ops::Range; use logos::Logos; +use crate::diagnostics::{Diagnostic, DiagnosticBuilder}; use raw_lexer::RawToken; pub use raw_lexer::BraceKind; @@ -143,6 +144,10 @@ pub struct TokenizedFile<'src> { /// Records only exists for multiline tokens and ranges can be empty for /// lines that only contain line break boundary. multi_line_map: HashMap>, + /// Sparse map between matched opening and closing delimiters. + delimiter_matches: DelimiterMatches, + /// Diagnostics produced during tokenization and delimiter matching. + diagnostics: Vec, /// Simple flag for marking erroneous state. had_errors: bool, } @@ -205,6 +210,12 @@ struct Tokenizer<'src> { /// that started on `line_number`; it is consumed exactly once by /// [`Self::commit_current_line`]. multi_line_start_line: Option, + /// Tracks delimiter pairs and delimiter diagnostics. + delimiter_matcher: DelimiterMatcher, + /// Tracks crude line indentation for delimiter recovery diagnostics. + indent_tracker: IndentTracker, + /// Diagnostics produced while tokenizing. + diagnostics: Vec, /// Set to `true` if the lexer reported any error tokens. had_errors: bool, } @@ -226,7 +237,8 @@ impl<'src> TokenizedFile<'src> { RawToken::Error }); let token_piece = make_token_data(Token::from(token), lexer.slice()); - tokenizer.process_token_piece(token_piece); + let position = tokenizer.process_token_piece(token_piece); + tokenizer.after_token(position); } tokenizer.into_tokenized_file() } @@ -244,6 +256,18 @@ impl<'src> TokenizedFile<'src> { pub const fn iter(&self) -> Tokens<'_, 'src> { Tokens::new(self) } + + /// Returns diagnostics produced during tokenization and delimiter matching. + #[must_use] + pub fn diagnostics(&self) -> &[Diagnostic] { + &self.diagnostics + } + + /// Returns the matching delimiter token, if this token is a matched delimiter. + #[must_use] + pub fn matching_delimiter(&self, position: TokenPosition) -> Option { + self.delimiter_matches.mate_of(position) + } } impl Line { @@ -298,17 +322,22 @@ impl<'src> Tokenizer<'src> { line_number: 0, uncommitted_start_index: 0, multi_line_start_line: None, + delimiter_matcher: DelimiterMatcher::default(), + indent_tracker: IndentTracker::new(), + diagnostics: Vec::new(), had_errors: false, } } /// Handles a token span and dispatches to the appropriate handler. - fn process_token_piece(&mut self, token_piece: TokenData<'src>) { + fn process_token_piece(&mut self, token_piece: TokenData<'src>) -> TokenPosition { + let position = TokenPosition(self.buffer.len()); if token_piece.token.can_span_lines() { self.process_multi_line_token(token_piece); } else { self.process_single_line_token(token_piece); } + position } /// Handles simple tokens that *never* span multiple lines, allowing us to @@ -387,6 +416,26 @@ impl<'src> Tokenizer<'src> { } } + /// Runs post-processing that needs the token to already have a stable + /// [`TokenPosition`]. + fn after_token(&mut self, position: TokenPosition) { + let token_piece = self.buffer[position.0]; + + if token_piece.token == Token::Error { + self.had_errors = true; + self.diagnostics + .push(diagnostic_invalid_token(position, token_piece.lexeme)); + } + + self.delimiter_matcher.observe( + position, + token_piece.token, + self.indent_tracker.current_indent(), + ); + + self.indent_tracker.observe_token(token_piece); + } + /// Finishes tokenization, converting accumulated data into /// [`TokenizedFile`]. fn into_tokenized_file(mut self) -> TokenizedFile<'src> { @@ -399,6 +448,12 @@ impl<'src> Tokenizer<'src> { self.lines.push(Line::continued(from)); } + let (delimiter_matches, delimiter_diagnostics) = self.delimiter_matcher.finish(); + if !delimiter_diagnostics.is_empty() { + self.had_errors = true; + } + self.diagnostics.extend(delimiter_diagnostics); + self.buffer.shrink_to_fit(); self.lines.shrink_to_fit(); @@ -406,6 +461,8 @@ impl<'src> Tokenizer<'src> { buffer: self.buffer, lines: self.lines, multi_line_map: self.multi_line_map, + delimiter_matches, + diagnostics: self.diagnostics, had_errors: self.had_errors, } } @@ -507,3 +564,409 @@ impl<'file, 'src> IntoIterator for &'file TokenizedFile<'src> { self.iter() } } + +/// Sparse map between matched delimiter tokens. +/// +/// Stores both directions: +/// +/// - opening delimiter -> closing delimiter +/// - closing delimiter -> opening delimiter +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct DelimiterMatches { + mates: HashMap, +} + +impl DelimiterMatches { + /// Returns the matching delimiter token for `position`, if known. + #[must_use] + pub fn mate_of(&self, position: TokenPosition) -> Option { + self.mates.get(&position).copied() + } + + fn insert_pair(&mut self, left: TokenPosition, right: TokenPosition) { + self.mates.insert(left, right); + self.mates.insert(right, left); + } +} + +#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq)] +enum DelimiterKind { + Parenthesis, + Bracket, + Brace, +} + +#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq)] +enum DelimiterSide { + Open, + Close, +} + +#[derive(Clone, Copy, Debug)] +struct OpenDelimiter { + kind: DelimiterKind, + position: TokenPosition, + indent: usize, +} + +#[derive(Debug, Default)] +struct DelimiterMatcher { + stack: Vec, + matches: DelimiterMatches, + diagnostics: Vec, + stopped_at_defaultproperties: bool, +} + +impl DelimiterMatcher { + fn handle_defaultproperties_boundary(&mut self, defaultproperties_position: TokenPosition) { + while let Some(open) = self.stack.pop() { + self.diagnostics + .push(diagnostic_unclosed_delimiter_before_defaultproperties( + open, + defaultproperties_position, + )); + } + } + + fn observe(&mut self, position: TokenPosition, token: Token, indent: usize) { + if self.stopped_at_defaultproperties { + return; + } + + if token == Token::Keyword(Keyword::DefaultProperties) { + self.handle_defaultproperties_boundary(position); + self.stopped_at_defaultproperties = true; + return; + } + + let Some((side, kind)) = delimiter_of(token) else { + return; + }; + + match side { + DelimiterSide::Open => { + self.stack.push(OpenDelimiter { + kind, + position, + indent, + }); + } + DelimiterSide::Close => { + self.handle_close(kind, position, indent); + } + } + } + + fn handle_close( + &mut self, + close_kind: DelimiterKind, + close_position: TokenPosition, + close_indent: usize, + ) { + let Some(top) = self.stack.last().copied() else { + self.diagnostics + .push(diagnostic_unexpected_closing_delimiter( + close_kind, + close_position, + )); + return; + }; + + if top.kind == close_kind { + let open = self.stack.pop().expect("stack top was just checked"); + self.matches.insert_pair(open.position, close_position); + return; + } + + let earlier_same_kind = self.stack.iter().rposition(|open| open.kind == close_kind); + + match earlier_same_kind { + Some(index) => { + let bad_open = top; + let recovered_open = self.stack[index]; + + self.diagnostics + .push(diagnostic_unclosed_delimiter_before_later_close( + bad_open, + close_kind, + close_position, + recovered_open, + recovered_open.indent == close_indent, + )); + + while self.stack.len() - 1 > index { + self.stack.pop(); + } + + let open = self + .stack + .pop() + .expect("same-kind delimiter was found on the stack"); + + self.matches.insert_pair(open.position, close_position); + } + None => { + let bad_open = self.stack.pop().expect("stack top was known to exist"); + + self.diagnostics + .push(diagnostic_mismatched_closing_delimiter( + bad_open, + close_kind, + close_position, + )); + } + } + } + + fn finish(mut self) -> (DelimiterMatches, Vec) { + if !self.stopped_at_defaultproperties { + while let Some(open) = self.stack.pop() { + self.diagnostics + .push(diagnostic_unclosed_delimiter_at_eof(open)); + } + } + + (self.matches, self.diagnostics) + } +} + +fn delimiter_of(token: Token) -> Option<(DelimiterSide, DelimiterKind)> { + match token { + Token::LeftParenthesis => Some((DelimiterSide::Open, DelimiterKind::Parenthesis)), + Token::RightParenthesis => Some((DelimiterSide::Close, DelimiterKind::Parenthesis)), + + Token::LeftBracket => Some((DelimiterSide::Open, DelimiterKind::Bracket)), + Token::RightBracket => Some((DelimiterSide::Close, DelimiterKind::Bracket)), + + Token::LeftBrace => Some((DelimiterSide::Open, DelimiterKind::Brace)), + Token::RightBrace => Some((DelimiterSide::Close, DelimiterKind::Brace)), + + _ => None, + } +} + +fn open_delimiter_text(kind: DelimiterKind) -> &'static str { + match kind { + DelimiterKind::Parenthesis => "(", + DelimiterKind::Bracket => "[", + DelimiterKind::Brace => "{", + } +} + +fn close_delimiter_text(kind: DelimiterKind) -> &'static str { + match kind { + DelimiterKind::Parenthesis => ")", + DelimiterKind::Bracket => "]", + DelimiterKind::Brace => "}", + } +} + +/// Crude indentation tracker used only for delimiter recovery diagnostics. +/// +/// This is intentionally simple. It assumes indentation is represented by a +/// run of whitespace tokens before the first non-trivia token on a physical +/// line. It does not try to normalize tabs or handle every multiline trivia +/// edge case precisely, because indentation is only a diagnostic heuristic. +#[derive(Clone, Copy, Debug)] +struct IndentTracker { + current_indent: usize, + before_first_nontrivia_on_line: bool, +} + +impl IndentTracker { + fn new() -> Self { + Self { + current_indent: 0, + before_first_nontrivia_on_line: true, + } + } + + fn current_indent(&self) -> usize { + self.current_indent + } + + fn observe_token(&mut self, token_piece: TokenData<'_>) { + if token_is_newline(token_piece.token) { + self.current_indent = 0; + self.before_first_nontrivia_on_line = true; + return; + } + + if self.before_first_nontrivia_on_line && token_is_whitespace(token_piece.token) { + self.current_indent += token_piece.lexeme.chars().count(); + return; + } + + if token_is_trivia(token_piece.token) { + return; + } + + self.before_first_nontrivia_on_line = false; + } +} + +fn token_is_newline(token: Token) -> bool { + matches!(token, Token::Newline) +} + +fn token_is_whitespace(token: Token) -> bool { + matches!(token, Token::Whitespace) +} + +fn token_is_trivia(token: Token) -> bool { + matches!( + token, + Token::Whitespace | Token::Newline | Token::LineComment | Token::BlockComment + ) +} + +fn diagnostic_invalid_token(position: TokenPosition, lexeme: &str) -> Diagnostic { + let shown = display_lexeme(lexeme); + + DiagnosticBuilder::error(format!("invalid token: {}", shown)) + .code("L0001") + .primary_label( + TokenSpan::new(position), + format!("invalid token: {}", shown), + ) + .build() +} + +fn diagnostic_unexpected_closing_delimiter( + close_kind: DelimiterKind, + close_position: TokenPosition, +) -> Diagnostic { + let close = close_delimiter_text(close_kind); + + DiagnosticBuilder::error(format!("unexpected closing delimiter: `{}`", close)) + .code("L0002") + .primary_label( + TokenSpan::new(close_position), + "unexpected closing delimiter", + ) + .build() +} + +fn diagnostic_unclosed_delimiter_before_later_close( + bad_open: OpenDelimiter, + close_kind: DelimiterKind, + close_position: TokenPosition, + recovered_open: OpenDelimiter, + same_indent: bool, +) -> Diagnostic { + let bad_open_text = open_delimiter_text(bad_open.kind); + let close_text = close_delimiter_text(close_kind); + let recovered_open_text = open_delimiter_text(recovered_open.kind); + + let mut builder = + DiagnosticBuilder::error(format!("unclosed delimiter before `{}`", close_text)) + .code("L0003") + .primary_label( + TokenSpan::new(bad_open.position), + format!( + "this `{}` is not closed before `{}`", + bad_open_text, close_text + ), + ) + .secondary_label( + TokenSpan::new(close_position), + format!( + "this `{}` is matched with the earlier `{}`", + close_text, recovered_open_text + ), + ); + + if same_indent { + builder = builder.secondary_label( + TokenSpan::new(recovered_open.position), + format!( + "this `{}` is likely the intended match", + recovered_open_text, + ), + ); + } + + builder.build() +} + +fn diagnostic_mismatched_closing_delimiter( + bad_open: OpenDelimiter, + close_kind: DelimiterKind, + close_position: TokenPosition, +) -> Diagnostic { + let open = open_delimiter_text(bad_open.kind); + let close = close_delimiter_text(close_kind); + + DiagnosticBuilder::error(format!("mismatched closing delimiter: `{}`", close)) + .code("L0004") + .primary_label( + TokenSpan::new(close_position), + format!("closing delimiter does not match `{}`", open), + ) + .secondary_label( + TokenSpan::new(bad_open.position), + format!("`{}` opened here", open), + ) + .build() +} + +fn diagnostic_unclosed_delimiter_at_eof(open: OpenDelimiter) -> Diagnostic { + let open_text = open_delimiter_text(open.kind); + + DiagnosticBuilder::error(format!("unclosed delimiter: `{}`", open_text)) + .code("L0005") + .primary_label( + TokenSpan::new(open.position), + format!("this `{}` was never closed", open_text), + ) + .build() +} + +fn diagnostic_unclosed_delimiter_before_defaultproperties( + open: OpenDelimiter, + defaultproperties_position: TokenPosition, +) -> Diagnostic { + let open_text = open_delimiter_text(open.kind); + + DiagnosticBuilder::error("unclosed delimiter before `defaultproperties`") + .code("L0006") + .primary_label( + TokenSpan::new(open.position), + format!("this `{}` is not closed before `defaultproperties`", open_text), + ) + .secondary_label( + TokenSpan::new(defaultproperties_position), + "delimiter matching stops at `defaultproperties`", + ) + .build() +} + +fn display_lexeme(lexeme: &str) -> String { + if lexeme.is_empty() { + return "".to_string(); + } + + if lexeme == "`" { + return "backtick".to_string(); + } + + let escaped: String = lexeme.chars().flat_map(char::escape_default).collect(); + + const MAX_DISPLAY_CHARS: usize = 32; + + let mut display = String::new(); + let mut chars = escaped.chars(); + + for _ in 0..MAX_DISPLAY_CHARS { + let Some(character) = chars.next() else { + return format!("`{}`", display); + }; + display.push(character); + } + + if chars.next().is_some() { + display.push_str("..."); + } + + format!("`{}`", display) +} \ No newline at end of file diff --git a/rottlib/src/parser/cursor.rs b/rottlib/src/parser/cursor.rs index d0cca2b..812a9ab 100644 --- a/rottlib/src/parser/cursor.rs +++ b/rottlib/src/parser/cursor.rs @@ -384,4 +384,31 @@ impl<'src, 'arena> Parser<'src, 'arena> { } } } + + /// Returns the first significant token after `position`, without consuming it. + /// + /// This buffers through the cursor, so trivia is still recorded normally and + /// insignificant tokens are skipped consistently with the rest of the parser. + /// + /// Returns `None` if the stream ends before a later significant token is found. + #[must_use] + pub(crate) fn peek_token_after_position(&mut self, position: TokenPosition) -> Option { + let mut lookahead = 0usize; + + loop { + self.cursor + .ensure_lookahead_available(lookahead, &mut self.trivia); + + let Some((token_position, token_data)) = self.cursor.lookahead_buffer.get(lookahead) + else { + return None; + }; + + if *token_position > position { + return Some(token_data.token); + } + + lookahead += 1; + } + } } diff --git a/rottlib/src/parser/grammar/expression/control_flow/for_loop.rs b/rottlib/src/parser/grammar/expression/control_flow/for_loop.rs index 7e3cfd9..120c4ec 100644 --- a/rottlib/src/parser/grammar/expression/control_flow/for_loop.rs +++ b/rottlib/src/parser/grammar/expression/control_flow/for_loop.rs @@ -122,6 +122,8 @@ impl<'src, 'arena> Parser<'src, 'arena> { ) // Header recovery must not consume the next `;`; // it belongs to the surrounding `for` header. + // That's why `SyncLevel` match is preferable to syncing onto + // matching delimiter - latter wouldn't stop at preceding `;`. .sync_error_until(self, SyncLevel::CloseParenthesis) .unwrap_or_fallback(self), ) @@ -216,7 +218,7 @@ impl<'src, 'arena> Parser<'src, 'arena> { ) .widen_error_span_from(for_keyword_position) .related_token("for_header_start", left_parenthesis_position) - .sync_error_at(self, SyncLevel::CloseParenthesis) + .sync_error_at_matching_delimiter(self, left_parenthesis_position) .ok_or_report(self); } diff --git a/rottlib/src/parser/grammar/expression/control_flow/mod.rs b/rottlib/src/parser/grammar/expression/control_flow/mod.rs index 3712e40..d31dcd9 100644 --- a/rottlib/src/parser/grammar/expression/control_flow/mod.rs +++ b/rottlib/src/parser/grammar/expression/control_flow/mod.rs @@ -97,27 +97,13 @@ impl<'src, 'arena> Parser<'src, 'arena> { else { return None; }; - let mut nesting_depth: usize = 1; - let mut lookahead_token_offset: usize = 1; - while let Some(lookahead_token) = self.peek_token_at(lookahead_token_offset) { - match lookahead_token { - Token::LeftParenthesis => nesting_depth += 1, - Token::RightParenthesis => { - if nesting_depth == 1 { - return self - .peek_token_at(lookahead_token_offset + 1) - .is_some_and(|token| token.is_valid_identifier_name()) - .then_some(left_parenthesis_position); - } - nesting_depth -= 1; - } - _ => (), - } - lookahead_token_offset += 1; - } - // Recovery is left to normal expression parsing when the closing `)` - // is missing. - None + + let right_parenthesis_position = + self.file().matching_delimiter(left_parenthesis_position)?; + + self.peek_token_after_position(right_parenthesis_position) + .is_some_and(|token| token.is_valid_identifier_name()) + .then_some(left_parenthesis_position) } /// Parses a control-flow condition. diff --git a/rottlib/src/parser/grammar/expression/primary/mod.rs b/rottlib/src/parser/grammar/expression/primary/mod.rs index ca8eff9..38b8ced 100644 --- a/rottlib/src/parser/grammar/expression/primary/mod.rs +++ b/rottlib/src/parser/grammar/expression/primary/mod.rs @@ -196,7 +196,7 @@ impl<'src, 'arena> Parser<'src, 'arena> { .widen_error_span_from(left_parenthesis_position) .extend_blame_to_next_token(self) .related_token("left_parenthesis", left_parenthesis_position) - .sync_error_until(self, SyncLevel::CloseParenthesis) + .sync_error_at_matching_delimiter(self, left_parenthesis_position) .fallback(self); }; let inner_expression = self.parse_expression(); @@ -206,7 +206,7 @@ impl<'src, 'arena> Parser<'src, 'arena> { ParseErrorKind::ParenthesizedExpressionMissingClosingParenthesis, ) .widen_error_span_from(left_parenthesis_position) - .sync_error_at(self, SyncLevel::CloseParenthesis) + .sync_error_at_matching_delimiter(self, left_parenthesis_position) .extend_blame_start_to_covered_start() .related_token("left_parenthesis", left_parenthesis_position) .unwrap_or_fallback(self); diff --git a/rottlib/src/parser/grammar/expression/primary/new.rs b/rottlib/src/parser/grammar/expression/primary/new.rs index 17cd00d..0cc6b2d 100644 --- a/rottlib/src/parser/grammar/expression/primary/new.rs +++ b/rottlib/src/parser/grammar/expression/primary/new.rs @@ -277,7 +277,7 @@ impl<'src, 'arena> Parser<'src, 'arena> { error = error.related("first_extra_argument", span); } error - .sync_error_until(self, SyncLevel::CloseParenthesis) + .sync_error_until_matching_delimiter(self, state.left_parenthesis_position) .report_error(self); } } @@ -296,7 +296,7 @@ impl<'src, 'arena> Parser<'src, 'arena> { .related_token("new_keyword", state.new_keyword_position) .related_token("left_parenthesis", state.left_parenthesis_position); let class_specifier_parse_action = if self.next_token_definitely_cannot_start_expression() { - error = error.sync_error_at(self, SyncLevel::CloseParenthesis); + error = error.sync_error_at_matching_delimiter(self, state.left_parenthesis_position); // Skipping the class specifier avoids cascading errors when // the next token cannot start an expression anyway. NewClassSpecifierParseAction::Skip diff --git a/rottlib/src/parser/recovery.rs b/rottlib/src/parser/recovery.rs index 57f8e50..18df7e7 100644 --- a/rottlib/src/parser/recovery.rs +++ b/rottlib/src/parser/recovery.rs @@ -138,6 +138,22 @@ impl SyncLevel { } } } + + fn fallback_sync_level_for_delimiter_start(token: Option) -> SyncLevel { + match token { + Some(Token::LeftParenthesis) => SyncLevel::CloseParenthesis, + Some(Token::LeftBracket) => SyncLevel::CloseBracket, + Some(Token::LeftBrace) => SyncLevel::BlockBoundary, + _ => SyncLevel::CloseParenthesis, + } + } + + fn is_opening_delimiter(token: Token) -> bool { + matches!( + token, + Token::LeftParenthesis | Token::LeftBracket | Token::LeftBrace + ) + } } impl<'src, 'arena> Parser<'src, 'arena> { @@ -171,6 +187,105 @@ impl<'src, 'arena> Parser<'src, 'arena> { } } + /// Skips tokens until a token with exactly `level` is found, then consumes + /// that token. + /// + /// This mirrors the behavior used by [`ResultRecoveryExt::sync_error_at`]: + /// stronger sync tokens can stop [`Self::recover_until`], but they are not + /// consumed unless they are exactly the requested level. + fn recover_at_sync_level(&mut self, level: SyncLevel) { + self.recover_until(level); + + if self + .peek_token() + .and_then(SyncLevel::for_token) + .is_some_and(|next_level| next_level == level) + { + self.advance(); + } + } + + /// Recovers up to the lexer-produced matching delimiter for + /// `delimiter_start`, if possible, but does not consume it. + /// + /// If `delimiter_start` is not an opening delimiter, if no match is known, + /// or if the parser has already moved past the matching delimiter, this + /// falls back to ordinary sync-level recovery inferred from + /// `delimiter_start`. + pub(crate) fn recover_until_matching_delimiter_or_sync( + &mut self, + delimiter_start: TokenPosition, + ) { + let start_token = self.file().token_at(delimiter_start).map(|data| data.token); + let fallback_level = SyncLevel::fallback_sync_level_for_delimiter_start(start_token); + + let Some(start_token) = start_token else { + self.recover_until(fallback_level); + return; + }; + + if !SyncLevel::is_opening_delimiter(start_token) { + self.recover_until(fallback_level); + return; + } + + let Some(target) = self.file().matching_delimiter(delimiter_start) else { + self.recover_until(fallback_level); + return; + }; + + if self.peek_position_or_eof() > target { + self.recover_until(fallback_level); + return; + } + + while self.peek_position_or_eof() < target { + self.advance(); + } + } + + /// Recovers by using the lexer-produced matching delimiter for + /// `delimiter_start`, if possible. + /// + /// If `delimiter_start` is not an opening delimiter, if no match is known, + /// or if the parser has already moved past the matching delimiter, this + /// falls back to ordinary sync-level recovery inferred from + /// `delimiter_start`. + pub(crate) fn recover_at_matching_delimiter_or_sync(&mut self, delimiter_start: TokenPosition) { + let start_token = self.file().token_at(delimiter_start).map(|data| data.token); + let fallback_level = SyncLevel::fallback_sync_level_for_delimiter_start(start_token); + + let Some(start_token) = start_token else { + self.recover_at_sync_level(fallback_level); + return; + }; + + if !SyncLevel::is_opening_delimiter(start_token) { + self.recover_at_sync_level(fallback_level); + return; + } + + let Some(target) = self.file().matching_delimiter(delimiter_start) else { + self.recover_at_sync_level(fallback_level); + return; + }; + + if self.peek_position_or_eof() > target { + self.recover_at_sync_level(fallback_level); + return; + } + + while self.peek_position_or_eof() < target { + self.advance(); + } + + if self.peek_position_or_eof() == target { + self.advance(); + } else { + self.recover_at_sync_level(fallback_level); + } + } + /// Reports `error` and returns the recovery fallback for `T`. /// /// This is the primitive used when parsing must keep going with a @@ -234,6 +349,39 @@ pub trait ResultRecoveryExt<'src, 'arena, T>: Sized { #[must_use] fn sync_error_at(self, parser: &mut Parser<'src, 'arena>, level: SyncLevel) -> Self; + /// Extends the right end of the error span up to, but not including, the + /// known matching closing delimiter for `delimiter_start`, if that delimiter + /// is known and has not already been passed. + /// + /// If no usable delimiter match exists, falls back to ordinary + /// [`SyncLevel`]-based recovery. The fallback level is inferred from the + /// token at `delimiter_start`. + #[must_use] + fn sync_error_until_matching_delimiter( + self, + parser: &mut Parser<'src, 'arena>, + delimiter_start: TokenPosition, + ) -> Self; + + /// Extends the right end of the error span to include the known matching + /// closing delimiter for `delimiter_start`, if that delimiter is known and + /// has not already been passed. + /// + /// If no usable delimiter match exists, falls back to ordinary + /// [`SyncLevel`]-based recovery. The fallback level is inferred from the + /// token at `delimiter_start`: + /// + /// - `(` -> [`SyncLevel::CloseParenthesis`] + /// - `[` -> [`SyncLevel::CloseBracket`] + /// - `{` -> [`SyncLevel::BlockBoundary`] + /// - anything else -> [`SyncLevel::CloseParenthesis`] + #[must_use] + fn sync_error_at_matching_delimiter( + self, + parser: &mut Parser<'src, 'arena>, + delimiter_start: TokenPosition, + ) -> Self; + /// Either returns expected value or its best effort fallback. #[must_use] fn unwrap_or_fallback(self, parser: &mut Parser<'src, 'arena>) -> T @@ -305,6 +453,36 @@ impl<'src, 'arena, T> ResultRecoveryExt<'src, 'arena, T> for ParseResult<'src, ' self } + fn sync_error_until_matching_delimiter( + mut self, + parser: &mut Parser<'src, 'arena>, + delimiter_start: TokenPosition, + ) -> Self { + if let Err(ref mut error) = self { + parser.recover_until_matching_delimiter_or_sync(delimiter_start); + error.covered_span.end = std::cmp::max( + error.covered_span.end, + parser.last_consumed_position_or_start(), + ); + } + self + } + + fn sync_error_at_matching_delimiter( + mut self, + parser: &mut Parser<'src, 'arena>, + delimiter_start: TokenPosition, + ) -> Self { + if let Err(ref mut error) = self { + parser.recover_at_matching_delimiter_or_sync(delimiter_start); + error.covered_span.end = std::cmp::max( + error.covered_span.end, + parser.last_consumed_position_or_start(), + ); + } + self + } + fn unwrap_or_fallback(self, parser: &mut Parser<'src, 'arena>) -> T where T: RecoveryFallback<'src, 'arena>, @@ -390,6 +568,32 @@ impl<'src, 'arena> ResultRecoveryExt<'src, 'arena, ()> for ParseError { self } + fn sync_error_until_matching_delimiter( + mut self, + parser: &mut Parser<'src, 'arena>, + delimiter_start: TokenPosition, + ) -> Self { + parser.recover_until_matching_delimiter_or_sync(delimiter_start); + self.covered_span.end = std::cmp::max( + self.covered_span.end, + parser.last_consumed_position_or_start(), + ); + self + } + + fn sync_error_at_matching_delimiter( + mut self, + parser: &mut Parser<'src, 'arena>, + delimiter_start: TokenPosition, + ) -> Self { + parser.recover_at_matching_delimiter_or_sync(delimiter_start); + self.covered_span.end = std::cmp::max( + self.covered_span.end, + parser.last_consumed_position_or_start(), + ); + self + } + fn unwrap_or_fallback(self, parser: &mut Parser<'src, 'arena>) { parser.report_error(self); } diff --git a/rottlib/tests/lexer_diagnostics.rs b/rottlib/tests/lexer_diagnostics.rs new file mode 100644 index 0000000..42b39f7 --- /dev/null +++ b/rottlib/tests/lexer_diagnostics.rs @@ -0,0 +1,350 @@ +use std::collections::HashMap; + +use rottlib::diagnostics::{Diagnostic, Severity}; +use rottlib::lexer::{TokenPosition, TokenSpan, TokenizedFile}; + +#[derive(Debug)] +struct ExpectedLabel { + span: TokenSpan, + message: &'static str, +} + +#[derive(Debug)] +struct ExpectedDiagnostic<'a> { + headline: &'static str, + severity: Severity, + code: Option<&'static str>, + primary_label: Option, + secondary_labels: &'a [ExpectedLabel], + help: Option<&'static str>, + notes: &'a [&'static str], +} + +#[track_caller] +fn assert_diagnostic(actual: &Diagnostic, expected: &ExpectedDiagnostic<'_>) { + assert_eq!(actual.headline(), expected.headline); + assert_eq!(actual.severity(), expected.severity); + assert_eq!(actual.code(), expected.code); + assert_eq!(actual.help(), expected.help); + + match (actual.primary_label(), expected.primary_label.as_ref()) { + (None, None) => {} + (Some(actual), Some(expected)) => { + assert_eq!(actual.span, expected.span); + assert_eq!(actual.message, expected.message); + } + _ => panic!("primary label mismatch"), + } + + let actual_secondary = actual.secondary_labels(); + assert_eq!(actual_secondary.len(), expected.secondary_labels.len()); + + for (actual, expected) in actual_secondary + .iter() + .zip(expected.secondary_labels.iter()) + { + assert_eq!(actual.span, expected.span); + assert_eq!(actual.message, expected.message); + } + + let actual_notes = actual.notes(); + assert_eq!(actual_notes.len(), expected.notes.len()); + + for (actual, expected) in actual_notes.iter().zip(expected.notes.iter()) { + assert_eq!(actual, expected); + } +} + +#[derive(Debug, Clone, Copy)] +struct Fixture { + label: &'static str, + source: &'static str, +} + +type FixtureRun = Vec; + +struct FixtureRuns { + runs: HashMap<&'static str, FixtureRun>, +} + +impl FixtureRuns { + #[track_caller] + fn get(&self, label: &str) -> Option> { + self.runs.get(label).cloned() + } + + #[track_caller] + fn get_any(&self, label: &str) -> Diagnostic { + self.runs + .get(label) + .map(|fixture_run| fixture_run[0].clone()) + .unwrap_or_else(|| panic!("no fixture run for `{label}`")) + } + + #[track_caller] + fn get_by_code(&self, label: &str, code: &str) -> Diagnostic { + self.runs + .get(label) + .unwrap_or_else(|| panic!("no fixture run for `{label}`")) + .iter() + .find(|diagnostic| diagnostic.code() == Some(code)) + .unwrap_or_else(|| panic!("no `{code}` diagnostic in fixture `{label}`")) + .clone() + } +} + +const fn span(position: usize) -> TokenSpan { + TokenSpan { + start: TokenPosition(position), + end: TokenPosition(position), + } +} + +const LEXER_FIXTURES: &[Fixture] = &[ + Fixture { + label: "files/L0001_01.uc", + source: "`", + }, + Fixture { + label: "files/L0002_01.uc", + source: "]", + }, + Fixture { + label: "files/L0003_01.uc", + source: "{\n foo(\n}\n", + }, + Fixture { + label: "files/L0004_01.uc", + source: "(]", + }, + Fixture { + label: "files/L0005_01.uc", + source: "foo(", + }, + Fixture { + label: "files/L_mixed_01.uc", + source: "([)]", + }, +]; + +fn run_fixture(fixture: &'static Fixture) -> FixtureRun { + let file = TokenizedFile::tokenize(fixture.source); + file.diagnostics().to_vec() +} + +fn run_fixtures(fixtures: &'static [Fixture]) -> FixtureRuns { + let mut runs = HashMap::new(); + + for fixture in fixtures { + runs.insert(fixture.label, run_fixture(fixture)); + } + + FixtureRuns { runs } +} + +#[test] +fn check_lexer_diagnostic_counts() { + let runs = run_fixtures(LEXER_FIXTURES); + + assert_eq!(runs.get("files/L0001_01.uc").unwrap().len(), 1); + assert_eq!(runs.get("files/L0002_01.uc").unwrap().len(), 1); + assert_eq!(runs.get("files/L0003_01.uc").unwrap().len(), 1); + assert_eq!(runs.get("files/L0004_01.uc").unwrap().len(), 1); + assert_eq!(runs.get("files/L0005_01.uc").unwrap().len(), 1); + assert_eq!(runs.get("files/L_mixed_01.uc").unwrap().len(), 2); +} + +#[test] +fn check_l0001_invalid_token() { + let runs = run_fixtures(LEXER_FIXTURES); + + assert_diagnostic( + &runs.get_any("files/L0001_01.uc"), + &ExpectedDiagnostic { + headline: "invalid token: backtick", + severity: Severity::Error, + code: Some("L0001"), + primary_label: Some(ExpectedLabel { + span: span(0), + message: "invalid token: backtick", + }), + secondary_labels: &[], + help: None, + notes: &[], + }, + ); +} + +#[test] +fn check_l0002_unexpected_closing_delimiter() { + let runs = run_fixtures(LEXER_FIXTURES); + + assert_diagnostic( + &runs.get_any("files/L0002_01.uc"), + &ExpectedDiagnostic { + headline: "unexpected closing delimiter: `]`", + severity: Severity::Error, + code: Some("L0002"), + primary_label: Some(ExpectedLabel { + span: span(0), + message: "unexpected closing delimiter", + }), + secondary_labels: &[], + help: None, + notes: &[], + }, + ); +} + +#[test] +fn check_l0003_unclosed_delimiter_before_later_close() { + let runs = run_fixtures(LEXER_FIXTURES); + + assert_diagnostic( + &runs.get_any("files/L0003_01.uc"), + &ExpectedDiagnostic { + headline: "unclosed delimiter before `}`", + severity: Severity::Error, + code: Some("L0003"), + primary_label: Some(ExpectedLabel { + span: span(4), + message: "this `(` is not closed before `}`", + }), + secondary_labels: &[ + ExpectedLabel { + span: span(6), + message: "this `}` is matched with the earlier `{`", + }, + ExpectedLabel { + span: span(0), + message: "this `{` is likely the intended match", + }, + ], + help: None, + notes: &[], + }, + ); +} + +#[test] +fn check_l0004_mismatched_closing_delimiter() { + let runs = run_fixtures(LEXER_FIXTURES); + + assert_diagnostic( + &runs.get_any("files/L0004_01.uc"), + &ExpectedDiagnostic { + headline: "mismatched closing delimiter: `]`", + severity: Severity::Error, + code: Some("L0004"), + primary_label: Some(ExpectedLabel { + span: span(1), + message: "closing delimiter does not match `(`", + }), + secondary_labels: &[ExpectedLabel { + span: span(0), + message: "`(` opened here", + }], + help: None, + notes: &[], + }, + ); +} + +#[test] +fn check_l0005_unclosed_delimiter_at_eof() { + let runs = run_fixtures(LEXER_FIXTURES); + + assert_diagnostic( + &runs.get_any("files/L0005_01.uc"), + &ExpectedDiagnostic { + headline: "unclosed delimiter: `(`", + severity: Severity::Error, + code: Some("L0005"), + primary_label: Some(ExpectedLabel { + span: span(1), + message: "this `(` was never closed", + }), + secondary_labels: &[], + help: None, + notes: &[], + }, + ); +} + +#[test] +fn check_mixed_recovery_diagnostics() { + let runs = run_fixtures(LEXER_FIXTURES); + + assert_diagnostic( + &runs.get_by_code("files/L_mixed_01.uc", "L0003"), + &ExpectedDiagnostic { + headline: "unclosed delimiter before `)`", + severity: Severity::Error, + code: Some("L0003"), + primary_label: Some(ExpectedLabel { + span: span(1), + message: "this `[` is not closed before `)`", + }), + secondary_labels: &[ + ExpectedLabel { + span: span(2), + message: "this `)` is matched with the earlier `(`", + }, + ExpectedLabel { + span: span(0), + message: "this `(` is likely the intended match", + }, + ], + help: None, + notes: &[], + }, + ); + + assert_diagnostic( + &runs.get_by_code("files/L_mixed_01.uc", "L0002"), + &ExpectedDiagnostic { + headline: "unexpected closing delimiter: `]`", + severity: Severity::Error, + code: Some("L0002"), + primary_label: Some(ExpectedLabel { + span: span(3), + message: "unexpected closing delimiter", + }), + secondary_labels: &[], + help: None, + notes: &[], + }, + ); +} + +#[test] +fn check_recovered_delimiter_matches_are_stored() { + let file = TokenizedFile::tokenize("{\n foo(\n}\n"); + + assert_eq!( + file.matching_delimiter(TokenPosition(0)), + Some(TokenPosition(6)) + ); + assert_eq!( + file.matching_delimiter(TokenPosition(6)), + Some(TokenPosition(0)) + ); + assert_eq!(file.matching_delimiter(TokenPosition(4)), None); +} + +#[test] +fn check_mixed_recovery_delimiter_matches_are_stored() { + let file = TokenizedFile::tokenize("([)]"); + + assert_eq!( + file.matching_delimiter(TokenPosition(0)), + Some(TokenPosition(2)) + ); + assert_eq!( + file.matching_delimiter(TokenPosition(2)), + Some(TokenPosition(0)) + ); + + assert_eq!(file.matching_delimiter(TokenPosition(1)), None); + assert_eq!(file.matching_delimiter(TokenPosition(3)), None); +} \ No newline at end of file From b1f071448329299388e445d45d44a877e8b54d6c Mon Sep 17 00:00:00 2001 From: dkanus Date: Wed, 29 Apr 2026 20:13:58 +0700 Subject: [PATCH 2/2] Improve block body's diagnostics --- dev_tests/src/verify_expr.rs | 67 ++- .../parse_error_diagnostics/block_items.rs | 111 +++++ .../control_flow_expressions.rs | 2 +- .../parse_error_diagnostics/mod.rs | 13 + rottlib/src/lexer/raw_lexer.rs | 2 +- rottlib/src/parser/cursor.rs | 3 +- rottlib/src/parser/errors.rs | 10 +- rottlib/src/parser/grammar/class.rs | 4 +- .../grammar/declarations/enum_definition.rs | 4 +- .../grammar/declarations/struct_definition.rs | 2 +- .../declarations/variable_declarators.rs | 4 +- .../src/parser/grammar/expression/block.rs | 213 +++++--- .../parser/grammar/expression/primary/mod.rs | 2 +- .../src/parser/grammar/expression/switch.rs | 6 +- rottlib/src/parser/grammar/statement.rs | 3 +- rottlib/src/parser/recovery.rs | 14 +- .../tests/parser_diagnostics/block_items.rs | 455 ++++++++++++++++++ rottlib/tests/parser_diagnostics/mod.rs | 1 + 18 files changed, 790 insertions(+), 126 deletions(-) create mode 100644 rottlib/src/diagnostics/parse_error_diagnostics/block_items.rs create mode 100644 rottlib/tests/parser_diagnostics/block_items.rs diff --git a/dev_tests/src/verify_expr.rs b/dev_tests/src/verify_expr.rs index e37fc12..978b618 100644 --- a/dev_tests/src/verify_expr.rs +++ b/dev_tests/src/verify_expr.rs @@ -16,45 +16,40 @@ use rottlib::parser::Parser; /// Keep these small: the goal is to inspect lexer diagnostics and delimiter /// recovery behavior, not full parser behavior. const TEST_CASES: &[(&str, &str)] = &[ - // L0001: invalid or unknown token - ( - "files/L0001_01.uc", - "`", - ), - - // L0002: unexpected closing delimiter - ( - "files/L0002_01.uc", - "]", - ), - - // L0003: unclosed delimiter before later closing delimiter + // P0027: `else` without a matching `if` // - // The `}` can still recover by matching the earlier `{`. + // `else` cannot start a standalone block item. The parser should report + // P0027 at `else`, then recover by bailing out of the current block. ( - "files/L0003_01.uc", - "{\n foo(\n}\n", + "files/P0027_01.uc", + "{\n local bool bReady;\n bReady = CheckReady();\n else { StartMatch(); }\n NotifyReady();\n}\n", ), - // L0004: mismatched closing delimiter - ( - "files/L0004_01.uc", - "(]", - ), - - // L0005: unclosed delimiter at end of file - ( - "files/L0005_01.uc", - "foo(", - ), - - // Mixed recovery case: + // P0027: `case` outside of a `switch` // - // `)` recovers by matching `(` after treating `[` as unclosed; - // the following `]` is then unexpected. + // `case` is a switch-arm boundary, not a valid statement or expression + // starter in an ordinary block. ( - "files/L_mixed_01.uc", - "([)]", + "files/P0027_02.uc", + "{ local int Count; Count = 3; case 3: Count++; UpdateHud();}", + ), + + // P0027: standalone `until` without a preceding `do` + // + // `until` is only meaningful as the tail of `do ... until`, so it should + // not be accepted as a normal block item. + ( + "files/P0027_03.uc", + "{\n local bool bDone;\n bDone = false;\n until (bDone)\n TickWork();\n}\n", + ), + + // P0027: preprocessor/exec directive inside a statement block + // + // `#exec` is declaration/top-level-like syntax, not a valid statement or + // expression inside a braced statement block. + ( + "files/P0027_04.uc", + "{\n local int Count;\n Count = 0;\n #exec TEXTURE IMPORT NAME=Bad FILE=Bad.bmp\n Count++;\n}\n", ), ]; @@ -62,12 +57,12 @@ const TEST_CASES: &[(&str, &str)] = &[ /// /// For lexer-focused fixtures this is usually noisy, so keep it off unless you /// want to inspect how parser recovery behaves after lexer diagnostics. -const RUN_PARSER: bool = false; +const RUN_PARSER: bool = true; /// If true, print the parsed expression using Debug formatting. const PRINT_PARSED_EXPR: bool = false; -/// If true, print diagnostics even when parsing returned a value. +const PRINT_LEXER_DIAGNOSTICS: bool = false; const ALWAYS_PRINT_DIAGNOSTICS: bool = true; fn main() { @@ -89,7 +84,7 @@ fn main() { } else { had_any_problem = true; - if ALWAYS_PRINT_DIAGNOSTICS { + if PRINT_LEXER_DIAGNOSTICS { println!("Lexer diagnostics:"); for diag in lexer_diagnostics { render_diagnostic(diag, &tf, Some(label), false); diff --git a/rottlib/src/diagnostics/parse_error_diagnostics/block_items.rs b/rottlib/src/diagnostics/parse_error_diagnostics/block_items.rs new file mode 100644 index 0000000..d3fcc35 --- /dev/null +++ b/rottlib/src/diagnostics/parse_error_diagnostics/block_items.rs @@ -0,0 +1,111 @@ +use super::{Diagnostic, DiagnosticBuilder, FoundAt, found_at}; +use crate::lexer::{Token, TokenSpan, TokenizedFile}; +use crate::parser::ParseError; + +/// P0025 +pub(super) fn diagnostic_block_missing_semicolon_after_expression<'src>( + error: ParseError, + file: &TokenizedFile<'src>, +) -> Diagnostic { + let expression_span = error.related_spans.get("expression_span").copied(); + + let primary_span = TokenSpan::new(error.blame_span.end); + + let primary_text = match found_at(file, primary_span.end) { + FoundAt::Token(token_text) => format!("expected `;` before `{}`", token_text), + FoundAt::EndOfFile => "expected `;` before end of file".to_string(), + FoundAt::Unknown => "expected `;` here".to_string(), + }; + + let mut builder = DiagnosticBuilder::error("missing `;` after expression statement"); + + if let Some(expression_span) = expression_span { + if file.same_line(expression_span.start, primary_span.end) { + builder = builder.secondary_label(expression_span, "expression statement"); + } else { + builder = builder.secondary_label( + TokenSpan::new(expression_span.end), + "expression statement ends here", + ); + } + } + + builder + .primary_label(primary_span, primary_text) + .code("P0025") + .build() +} + +/// P0026 +pub(super) fn diagnostic_block_missing_closing_brace<'src>( + error: ParseError, + file: &TokenizedFile<'src>, +) -> Diagnostic { + let left_brace_span = error.related_spans.get("left_brace").copied(); + let unexpected_token_span = error.related_spans.get("unexpected_token").copied(); + + let (mut primary_span, primary_text) = + if let Some(unexpected_token_span) = unexpected_token_span { + let primary_span = TokenSpan::new(unexpected_token_span.end); + + let primary_text = match found_at(file, primary_span.end) { + FoundAt::Token(token_text) => format!("expected `}}` before `{}`", token_text), + FoundAt::EndOfFile => "expected `}` before end of file".to_string(), + FoundAt::Unknown => "expected `}` here".to_string(), + }; + + (primary_span, primary_text) + } else { + ( + TokenSpan::new(error.blame_span.end), + "expected `}` before end of file".to_string(), + ) + }; + + let builder = DiagnosticBuilder::error("missing `}` to close block"); + + if let Some(left_brace_span) = left_brace_span + && !file.same_line(left_brace_span.start, primary_span.end) + { + primary_span.start = left_brace_span.start; + } + + builder + .primary_label(primary_span, primary_text) + .code("P0026") + .build() +} + +/// P0027 +pub(super) fn diagnostic_block_expected_item<'src>( + error: ParseError, + file: &TokenizedFile<'src>, +) -> Diagnostic { + let primary_span = error.blame_span; + + let (title, primary_text) = match file.token_at(primary_span.start).map(|data| data.token) { + Some(Token::ExecDirective) => ( + "expected statement or expression, found `#exec` directive".to_string(), + "`#exec` directives are not allowed in a statement block".to_string(), + ), + _ => match found_at(file, primary_span.start) { + FoundAt::Token(token_text) => ( + format!("expected statement or expression, found `{}`", token_text), + format!("unexpected `{}`", token_text), + ), + FoundAt::EndOfFile => ( + "expected statement or expression, found end of file".to_string(), + "reached end of file here".to_string(), + ), + FoundAt::Unknown => ( + "expected statement or expression".to_string(), + "expected statement or expression here".to_string(), + ), + }, + }; + + DiagnosticBuilder::error(title) + .primary_label(primary_span, primary_text) + .code("P0027") + .build() +} diff --git a/rottlib/src/diagnostics/parse_error_diagnostics/control_flow_expressions.rs b/rottlib/src/diagnostics/parse_error_diagnostics/control_flow_expressions.rs index cb6fe57..c9518b6 100644 --- a/rottlib/src/diagnostics/parse_error_diagnostics/control_flow_expressions.rs +++ b/rottlib/src/diagnostics/parse_error_diagnostics/control_flow_expressions.rs @@ -263,7 +263,7 @@ pub(super) fn diagnostic_for_each_iterator_expression_expected<'src>( let control_keyword_text = control_keyword_span.and_then(|span| file.token_text(span.end)); let found = found_at(file, error.blame_span.end); - let found_body_block_start = matches!(found, FoundAt::Token("{")); + let _ = matches!(found, FoundAt::Token("{")); let (header_text, primary_text) = match (control_keyword_text, found) { (Some(keyword_text), FoundAt::Token(token_text)) => { diff --git a/rottlib/src/diagnostics/parse_error_diagnostics/mod.rs b/rottlib/src/diagnostics/parse_error_diagnostics/mod.rs index 3fc5f19..9e13ecd 100644 --- a/rottlib/src/diagnostics/parse_error_diagnostics/mod.rs +++ b/rottlib/src/diagnostics/parse_error_diagnostics/mod.rs @@ -10,9 +10,14 @@ //! parser areas or grammar families. use super::{Diagnostic, DiagnosticBuilder}; +use crate::diagnostics::parse_error_diagnostics::block_items::{ + diagnostic_block_expected_item, diagnostic_block_missing_closing_brace, + diagnostic_block_missing_semicolon_after_expression, +}; use crate::lexer::{TokenPosition, TokenSpan, TokenizedFile}; use crate::parser::{ParseError, ParseErrorKind}; +mod block_items; mod control_flow_expressions; mod primary_expressions; @@ -133,6 +138,14 @@ pub(crate) fn diagnostic_from_parse_error<'src>( } ParseErrorKind::BreakValueInvalidStart => diagnostic_break_value_invalid_start(error, file), ParseErrorKind::GotoMissingLabel => diagnostic_goto_missing_label(error, file), + // block_items.rs + ParseErrorKind::BlockMissingSemicolonAfterExpression => { + diagnostic_block_missing_semicolon_after_expression(error, file) + } + ParseErrorKind::BlockMissingClosingBrace => { + diagnostic_block_missing_closing_brace(error, file) + } + ParseErrorKind::BlockExpectedItem => diagnostic_block_expected_item(error, file), _ => DiagnosticBuilder::error(format!("error {:?} while parsing", error.kind)) .primary_label(error.covered_span, "happened here") diff --git a/rottlib/src/lexer/raw_lexer.rs b/rottlib/src/lexer/raw_lexer.rs index 892f1d7..0cc441b 100644 --- a/rottlib/src/lexer/raw_lexer.rs +++ b/rottlib/src/lexer/raw_lexer.rs @@ -59,7 +59,7 @@ pub enum BraceKind { #[logos(extras = LexerState)] pub enum RawToken { // # Compiler/directive keywords - #[regex(r"(?i)#exec[^\r\n]*(?:\r\n|\n|\r)?")] + #[regex(r"(?i)#exec[^\r\n]*")] ExecDirective, #[regex("(?i)cpptext", |lex| { if is_next_nontrivia_left_brace(lex) { diff --git a/rottlib/src/parser/cursor.rs b/rottlib/src/parser/cursor.rs index 812a9ab..10fab51 100644 --- a/rottlib/src/parser/cursor.rs +++ b/rottlib/src/parser/cursor.rs @@ -363,7 +363,8 @@ impl<'src, 'arena> Parser<'src, 'arena> { .unwrap_or(TokenPosition(0)) } - /// Ensures that parsing has advanced past `old_position`. + /// Ensures that parsing has advanced past `old_position`. + // TODO: must be given peeked value! /// /// This is intended as a safeguard against infinite-loop bugs while /// recovering from invalid input. In debug builds it asserts that progress diff --git a/rottlib/src/parser/errors.rs b/rottlib/src/parser/errors.rs index 3931d3e..9eec540 100644 --- a/rottlib/src/parser/errors.rs +++ b/rottlib/src/parser/errors.rs @@ -64,6 +64,12 @@ pub enum ParseErrorKind { BreakValueInvalidStart, /// P0024 GotoMissingLabel, + /// P0025 + BlockMissingSemicolonAfterExpression, + /// P0026 + BlockMissingClosingBrace, + /// P0027 + BlockExpectedItem, // ================== Old errors to be thrown away! ================== /// Expression inside `(...)` could not be parsed and no closing `)` /// was found. @@ -83,11 +89,7 @@ pub enum ParseErrorKind { TypeSpecClassMissingInnerType, TypeSpecClassMissingClosingAngle, - /// An expression inside a block is not terminated with `;`. - BlockMissingSemicolonAfterExpression, - /// A statement inside a block is not terminated with `;`. BlockMissingSemicolonAfterStatement, - BlockMissingClosingBrace, /// `switch` has no body (missing matching braces). SwitchMissingBody, /// The first top-level item in a `switch` body is not a `case`. diff --git a/rottlib/src/parser/grammar/class.rs b/rottlib/src/parser/grammar/class.rs index 76b49d0..57be640 100644 --- a/rottlib/src/parser/grammar/class.rs +++ b/rottlib/src/parser/grammar/class.rs @@ -357,7 +357,7 @@ impl<'src, 'arena> crate::parser::Parser<'src, 'arena> { Ok(rule) => rules.push(rule), Err(error) => { self.report_error(error); - self.recover_until(SyncLevel::Statement); + self.recover_until(SyncLevel::StatementStart); let _ = self.eat(Token::Semicolon); if !self.ensure_progress_or_break(loop_start) { break; @@ -899,7 +899,7 @@ impl<'src, 'arena> crate::parser::Parser<'src, 'arena> { } Some((_, _)) if declarators.is_empty() => { self.report_error_here(ParseErrorKind::DeclBadVariableIdentifier); - self.recover_until(SyncLevel::Statement); + self.recover_until(SyncLevel::StatementStart); let _ = self.eat(Token::Semicolon); break; } diff --git a/rottlib/src/parser/grammar/declarations/enum_definition.rs b/rottlib/src/parser/grammar/declarations/enum_definition.rs index 76d7286..46597d5 100644 --- a/rottlib/src/parser/grammar/declarations/enum_definition.rs +++ b/rottlib/src/parser/grammar/declarations/enum_definition.rs @@ -104,7 +104,7 @@ impl<'src, 'arena> Parser<'src, 'arena> { variants: &mut ArenaVec<'arena, IdentifierToken>, ) -> ControlFlow<()> { self.parse_identifier(ParseErrorKind::EnumBadVariant) - .sync_error_until(self, SyncLevel::Statement) + .sync_error_until(self, SyncLevel::StatementStart) .ok_or_report(self) .map_or(ControlFlow::Break(()), |variant| { variants.push(variant); @@ -122,7 +122,7 @@ impl<'src, 'arena> Parser<'src, 'arena> { let Some(variant) = self .parse_identifier(ParseErrorKind::EnumBadVariant) .widen_error_span_from(error_start_position) - .sync_error_until(self, SyncLevel::Statement) + .sync_error_until(self, SyncLevel::StatementStart) .ok_or_report(self) else { // If we don't even get a good identifier - error is different diff --git a/rottlib/src/parser/grammar/declarations/struct_definition.rs b/rottlib/src/parser/grammar/declarations/struct_definition.rs index 235f06d..5c22c11 100644 --- a/rottlib/src/parser/grammar/declarations/struct_definition.rs +++ b/rottlib/src/parser/grammar/declarations/struct_definition.rs @@ -93,7 +93,7 @@ impl<'src, 'arena> Parser<'src, 'arena> { self.advance(); if !self.eat(Token::CppBlock) { self.report_error_here(ParseErrorKind::CppDirectiveMissingCppBlock); - self.recover_until(SyncLevel::Statement); + self.recover_until(SyncLevel::StatementStart); } StructBodyItemParseOutcome::Skip } diff --git a/rottlib/src/parser/grammar/declarations/variable_declarators.rs b/rottlib/src/parser/grammar/declarations/variable_declarators.rs index 22762c2..254d4a3 100644 --- a/rottlib/src/parser/grammar/declarations/variable_declarators.rs +++ b/rottlib/src/parser/grammar/declarations/variable_declarators.rs @@ -104,7 +104,7 @@ impl<'src, 'arena> Parser<'src, 'arena> { ) -> ControlFlow<()> { if let Some(parsed_declarator) = self .parse_variable_declarator() - .sync_error_until(self, SyncLevel::Statement) + .sync_error_until(self, SyncLevel::StatementStart) .ok_or_report(self) { declarators.push(parsed_declarator); @@ -122,7 +122,7 @@ impl<'src, 'arena> Parser<'src, 'arena> { if let Some(parsed_declarator) = self .parse_variable_declarator() .widen_error_span_from(error_start_position) - .sync_error_until(self, SyncLevel::Statement) + .sync_error_until(self, SyncLevel::StatementStart) .ok_or_report(self) { self.make_error_at_last_consumed(ParseErrorKind::DeclNoSeparatorBetweenVariableDeclarations) diff --git a/rottlib/src/parser/grammar/expression/block.rs b/rottlib/src/parser/grammar/expression/block.rs index 7b7a5c2..361b195 100644 --- a/rottlib/src/parser/grammar/expression/block.rs +++ b/rottlib/src/parser/grammar/expression/block.rs @@ -4,106 +4,187 @@ //! function, loop, state, and similar constructs after the opening `{` //! has been consumed. -use crate::arena::ArenaVec; -use crate::ast::{BlockBody, Expression, ExpressionRef, Statement, StatementRef}; +use crate::ast::{BlockBody, Expression, ExpressionRef, Statement, StatementList, StatementRef}; use crate::lexer::{Token, TokenPosition, TokenSpan}; -use crate::parser::{ParseErrorKind, Parser}; +use crate::parser::{ParseErrorKind, ParseResult, Parser, ResultRecoveryExt, SyncLevel}; impl<'src, 'arena> Parser<'src, 'arena> { - /// Parses a `{ ... }` block after the opening `{` has been consumed. + /// Parses a braced block body into an [`Expression::Block`]. /// - /// Consumes tokens until the matching `}` and returns an - /// [`Expression::Block`] whose span covers the entire block, from - /// `opening_brace_position` to the closing `}`. + /// The opening `{` must already have been consumed. The returned block's + /// span covers the whole block, from `left_brace_position` through + /// the closing `}`. /// - /// On premature end-of-file, returns a best-effort block. + /// On premature end-of-file, reports the missing `}` and returns + /// a best-effort block. #[must_use] - pub(crate) fn parse_block_tail( + pub(crate) fn parse_block_body_tail( &mut self, - opening_brace_position: TokenPosition, + left_brace_position: TokenPosition, ) -> ExpressionRef<'src, 'arena> { let BlockBody { statements, span } = - self.parse_braced_block_statements_tail(opening_brace_position); + self.parse_braced_block_statements_tail(left_brace_position); self.arena.alloc_node(Expression::Block(statements), span) } - /// Parses a `{ ... }` block after the opening `{` has been consumed. + /// Parses the statements in a braced block body. /// - /// Consumes tokens until the matching `}` and returns the contained - /// statements together with a span that covers the entire block, from - /// `opening_brace_position` to the closing `}`. + /// The opening `{` must already have been consumed. Returns the parsed + /// statements and a span covering the whole block, from + /// `left_brace_position` through the closing `}`. /// - /// On premature end-of-file, returns a best-effort statement list and span. + /// On premature end-of-file, reports the missing `}` and returns + /// a best-effort body. #[must_use] pub(crate) fn parse_braced_block_statements_tail( &mut self, - opening_brace_position: TokenPosition, + left_brace_position: TokenPosition, ) -> BlockBody<'src, 'arena> { let mut statements = self.arena.vec(); while let Some((token, token_position)) = self.peek_token_and_position() { if token == Token::RightBrace { self.advance(); // '}' - let span = TokenSpan::range(opening_brace_position, token_position); + let span = TokenSpan::range(left_brace_position, token_position); return BlockBody { statements, span }; } - self.parse_next_block_item_into(&mut statements); - self.ensure_forward_progress(token_position); + match self.parse_and_append_next_block_item(&mut statements) { + Ok(()) => { + // Guard against parser bugs that would otherwise leave + // block parsing stuck on the same token. + self.ensure_forward_progress(token_position); + } + Err(error) => { + // Item-level recovery failed, + // so escalate to the block boundary. + let error = error.sync_error_at_matching_delimiter(self, left_brace_position); + let error_statement = error.fallback(self); + statements.push(error_statement); + let span = TokenSpan::range( + left_brace_position, + self.last_consumed_position_or_start(), + ); + return BlockBody { statements, span }; + } + } } - // Reached EOF without a closing `}` - self.report_error_here(ParseErrorKind::BlockMissingClosingBrace); - let span = TokenSpan::range( - opening_brace_position, - self.last_consumed_position_or_start(), - ); + let eof_position = self.peek_position_or_eof(); + self.make_error_at(ParseErrorKind::BlockMissingClosingBrace, eof_position) + .related_token("left_brace", left_brace_position) + .report(self); + let span = TokenSpan::range(left_brace_position, self.last_consumed_position_or_start()); + BlockBody { statements, span } } - /// Parses one statement inside a `{ ... }` block and appends it to - /// `statements`. + /// Parses one statement-like item inside a `{ ... }` block and appends it + /// to `statements`. /// /// This method never consumes the closing `}` and is only meant to be - /// called while parsing inside a block. It always appends at least one - /// statement, even in the presence of syntax errors. - pub(crate) fn parse_next_block_item_into( + /// called while parsing inside a block. + /// + /// On success, it appends exactly one statement. If neither a statement nor + /// an expression statement can be recovered locally, it returns + /// an unreported error so the enclosing block parser can recover at + /// block level. + pub(crate) fn parse_and_append_next_block_item( &mut self, - statements: &mut ArenaVec<'arena, StatementRef<'src, 'arena>>, - ) { - let mut next_statement = self.parse_statement().unwrap_or_else(|| { - let next_expression = self.parse_expression(); - let next_expression_span = *next_expression.span(); - self.arena - .alloc_node(Statement::Expression(next_expression), next_expression_span) - }); - if statement_needs_semicolon(&next_statement) - && let Some((Token::Semicolon, semicolon_position)) = self.peek_token_and_position() - { - next_statement.span_mut().extend_to(semicolon_position); - self.advance(); // ';' + statements: &mut StatementList<'src, 'arena>, + ) -> ParseResult<'src, 'arena, ()> { + let mut statement = match self.parse_statement() { + Some(statement) => statement, + None => { + // Non-statement starters are parsed as expression statements + self.parse_expression_statement_in_block()? + } + }; + if block_item_requires_semicolon(&statement) { + match self.peek_token_and_position() { + Some((Token::Semicolon, semicolon_position)) => { + statement.span_mut().extend_to(semicolon_position); + self.advance(); // ';' + } + // A final expression before `}` may omit `;`; this makes it + // the block's tail value. + // + // On end-of-file, suppress the missing-`;` diagnostic as well. + // The block parser will report the missing `}`, + // and an extra semicolon error would just cascade. + None | Some((Token::RightBrace, _)) => (), + Some((_, unexpected_token_position)) => { + self.make_error_at_last_consumed( + ParseErrorKind::BlockMissingSemicolonAfterExpression, + ) + .widen_error_span_from(statement.span().start) + .blame_token(unexpected_token_position) + .related("expression_span", *statement.span()) + .report(self); + } + } } - statements.push(next_statement); + statements.push(statement); + Ok(()) + } + + fn parse_expression_statement_in_block( + &mut self, + ) -> ParseResult<'src, 'arena, StatementRef<'src, 'arena>> { + let expression_start_position = self.peek_position_or_eof(); + let expected_block_item_after_position = self.last_consumed_position_or_start(); + let expression_result = self.parse_expression_with_start_error( + ParseErrorKind::BlockExpectedItem, + expected_block_item_after_position, + expected_block_item_after_position, + ); + let expression = match expression_result { + Ok(expression) => expression, + Err(error) => { + let expression_recovery_made_no_progress = + self.peek_position_or_eof() == expression_start_position; + // Without progress, a fallback statement could leave + // the block loop stuck. + if expression_recovery_made_no_progress { + return self.recover_bad_block_item_start_as_error_statement(error); + } + error.fallback(self) + } + }; + let expression_span = *expression.span(); + Ok(self + .arena + .alloc_node(Statement::Expression(expression), expression_span)) + } + + fn recover_bad_block_item_start_as_error_statement( + &mut self, + error: crate::parser::ParseError, + ) -> ParseResult<'src, 'arena, StatementRef<'src, 'arena>> { + let position_before_statement_recovery = self.peek_position_or_eof(); + // Recover one damaged block item if possible; + // otherwise let the enclosing block recover at its own boundary. + let error = error.sync_error_at(self, SyncLevel::StatementTerminator); + if self.peek_position_or_eof() != position_before_statement_recovery { + return Ok(error.fallback(self)); + } + Err(error) } } -fn statement_needs_semicolon(statement: &Statement) -> bool { - use Statement::{Empty, Error, Expression, Function, Label, LocalVariableDeclaration}; - match statement { - Empty | Label(_) | Error | Function(_) => false, - Expression(expression) => expression_needs_semicolon(expression), - LocalVariableDeclaration { .. } => true, +fn block_item_requires_semicolon(statement: &Statement) -> bool { + // Control-flow and block expressions do not require a trailing semicolon + // when used as block items. + if let Statement::Expression(expression) = statement { + !matches!( + **expression, + Expression::Block { .. } + | Expression::If { .. } + | Expression::While { .. } + | Expression::DoUntil { .. } + | Expression::ForEach { .. } + | Expression::For { .. } + | Expression::Switch { .. } + | Expression::Error + ) + } else { + false } } - -const fn expression_needs_semicolon(expression: &Expression) -> bool { - use Expression::{Block, DoUntil, Error, For, ForEach, If, Switch, While}; - matches!( - expression, - Block { .. } - | If { .. } - | While { .. } - | DoUntil { .. } - | ForEach { .. } - | For { .. } - | Switch { .. } - | Error - ) -} diff --git a/rottlib/src/parser/grammar/expression/primary/mod.rs b/rottlib/src/parser/grammar/expression/primary/mod.rs index 38b8ced..71afce9 100644 --- a/rottlib/src/parser/grammar/expression/primary/mod.rs +++ b/rottlib/src/parser/grammar/expression/primary/mod.rs @@ -78,7 +78,7 @@ impl<'src, 'arena> Parser<'src, 'arena> { token_position, ), Token::LeftParenthesis => self.parse_parenthesized_expression_tail(token_position), - Token::LeftBrace => self.parse_block_tail(token_position), + Token::LeftBrace => self.parse_block_body_tail(token_position), Token::Keyword(keyword) => { match self.try_parse_keyword_primary(keyword, token_position) { Some(keyword_expression) => keyword_expression, diff --git a/rottlib/src/parser/grammar/expression/switch.rs b/rottlib/src/parser/grammar/expression/switch.rs index be948b4..0ca0e54 100644 --- a/rottlib/src/parser/grammar/expression/switch.rs +++ b/rottlib/src/parser/grammar/expression/switch.rs @@ -96,7 +96,7 @@ impl<'src, 'arena> crate::parser::Parser<'src, 'arena> { // at statement sync level. self.expect(Token::Colon, ParseErrorKind::SwitchCaseMissingColon) .widen_error_span_from(case_position) - .sync_error_until(self, crate::parser::SyncLevel::Statement) + .sync_error_until(self, crate::parser::SyncLevel::StatementStart) .report_error(self); } let mut body = self.arena.vec(); @@ -115,7 +115,7 @@ impl<'src, 'arena> crate::parser::Parser<'src, 'arena> { self.advance(); // 'default' self.expect(Token::Colon, ParseErrorKind::SwitchCaseMissingColon) .widen_error_span_from(default_position) - .sync_error_until(self, crate::parser::SyncLevel::Statement) + .sync_error_until(self, crate::parser::SyncLevel::StatementStart) .report_error(self); self.parse_switch_arm_body(statements); } @@ -129,7 +129,7 @@ impl<'src, 'arena> crate::parser::Parser<'src, 'arena> { match token { Token::Keyword(Keyword::Case | Keyword::Default) | Token::RightBrace => break, _ => { - self.parse_next_block_item_into(statements); + self.parse_and_append_next_block_item(statements); self.ensure_forward_progress(token_position); } } diff --git a/rottlib/src/parser/grammar/statement.rs b/rottlib/src/parser/grammar/statement.rs index 97a4d39..0e2ec59 100644 --- a/rottlib/src/parser/grammar/statement.rs +++ b/rottlib/src/parser/grammar/statement.rs @@ -11,8 +11,9 @@ impl<'src, 'arena> crate::parser::Parser<'src, 'arena> { /// Parses a single statement. /// /// Does not consume a trailing `;` except for [`Statement::Empty`]. - /// The caller handles semicolons. Returns [`Some`] if a statement is + /// The caller handles semicolons (WRONG NOW - WE MUST HANDLE THEM). Returns [`Some`] if a statement is /// recognized; otherwise [`None`]. + /// ALSO WE SPECIFICALLY DONT HANDLE EXPRESSION TYPE STATEMENTS #[must_use] pub(crate) fn parse_statement(&mut self) -> Option> { let Some((token, lexeme, position)) = self.peek_token_lexeme_and_position() else { diff --git a/rottlib/src/parser/recovery.rs b/rottlib/src/parser/recovery.rs index 18df7e7..cbf1e2a 100644 --- a/rottlib/src/parser/recovery.rs +++ b/rottlib/src/parser/recovery.rs @@ -50,7 +50,10 @@ pub enum SyncLevel { /// /// Includes `;` and keywords that begin standalone statements / /// statement-like control-flow forms. - Statement, + StatementStart, + + /// Statement terminator `;`. + StatementTerminator, /// Start of a `switch` arm. /// @@ -77,7 +80,7 @@ impl SyncLevel { use crate::lexer::Keyword; use SyncLevel::{ BlockBoundary, CloseAngleBracket, CloseBracket, CloseParenthesis, DeclarationStart, - ExpressionStart, ListSeparator, Statement, SwitchArmStart, + ExpressionStart, ListSeparator, StatementStart, StatementTerminator, SwitchArmStart, }; match token { @@ -88,8 +91,7 @@ impl SyncLevel { Token::RightBracket => Some(CloseBracket), // Statement-level boundaries - Token::Semicolon - | Token::Keyword( + Token::Keyword( Keyword::If | Keyword::Else | Keyword::Switch @@ -102,7 +104,9 @@ impl SyncLevel { | Keyword::Break | Keyword::Continue | Keyword::Local, - ) => Some(Statement), + ) => Some(StatementStart), + + Token::Semicolon => Some(StatementTerminator), // Switch-specific stronger boundary Token::Keyword(Keyword::Case | Keyword::Default) => Some(SwitchArmStart), diff --git a/rottlib/tests/parser_diagnostics/block_items.rs b/rottlib/tests/parser_diagnostics/block_items.rs new file mode 100644 index 0000000..56f434b --- /dev/null +++ b/rottlib/tests/parser_diagnostics/block_items.rs @@ -0,0 +1,455 @@ +use super::*; + +pub(super) const P0025_FIXTURES: &[Fixture] = &[ + Fixture { + label: "files/P0025_01.uc", + source: "{\n local int Count;\n Count = Count + 1 UpdateHud();\n DrawHud(CanvasRef);\n}\n", + }, + Fixture { + label: "files/P0025_02.uc", + source: "{\n local float XL;\n C.TextSize(LevelTitle, XL, YL)\n C.SetPos(0, 0);\n C.DrawText(LevelTitle);\n}\n", + }, + Fixture { + label: "files/P0025_03.uc", + source: "{\n local bool bReady;\n bReady = CheckReady()\n if (bReady) { StartMatch(); }\n NotifyReady();\n}\n", + }, + Fixture { + label: "files/P0025_04.uc", + source: "{\n local int I;\n Scores[I] = Scores[I] + 1\n I++;\n RefreshScores();\n}\n", + }, +]; + +pub(super) const P0026_FIXTURES: &[Fixture] = &[ + Fixture { + label: "files/P0026_01.uc", + source: "{\n local int Count;\n Count = 0;\n Count++;\n UpdateHud();\n", + }, + Fixture { + label: "files/P0026_02.uc", + source: "{\n local bool bReady;\n bReady = CheckReady();\n if (bReady) { StartMatch(); }\n NotifyReady();\n", + }, + Fixture { + label: "files/P0026_03.uc", + source: "{ local float XL; do { C.TextSize(LevelTitle, XL, YL); } until (XL < C.ClipX) C.SetPos(0, 0); C.DrawText(LevelTitle);", + }, + Fixture { + label: "files/P0026_04.uc", + source: "{\n local int Count;\n Count = Count + 1;\n UpdateHud();\n Count\n", + }, +]; + +pub(super) const P0025_P0026_MIXED_FIXTURES: &[Fixture] = &[ + Fixture { + label: "files/P_mixed_01.uc", + source: "{ local int Count; Count = Count + 1 UpdateHud(); DrawHud(CanvasRef);", + }, + Fixture { + label: "files/P_mixed_02.uc", + source: "{\n local bool bReady;\n bReady = CheckReady()\n if (bReady) { StartMatch(); }\n NotifyReady();\n", + }, +]; + +#[test] +fn check_p0025_fixtures() { + let runs = run_fixtures(P0025_FIXTURES); + + assert_eq!(runs.get("files/P0025_01.uc").unwrap().len(), 1); + assert_eq!(runs.get("files/P0025_02.uc").unwrap().len(), 1); + assert_eq!(runs.get("files/P0025_03.uc").unwrap().len(), 1); + assert_eq!(runs.get("files/P0025_04.uc").unwrap().len(), 1); + + assert_diagnostic( + &runs.get_any("files/P0025_01.uc"), + &ExpectedDiagnostic { + headline: "missing `;` after expression statement", + severity: Severity::Error, + code: Some("P0025"), + primary_label: Some(ExpectedLabel { + span: TokenSpan { + start: TokenPosition(21), + end: TokenPosition(21), + }, + message: "expected `;` before `UpdateHud`", + }), + secondary_labels: &[ExpectedLabel { + span: TokenSpan { + start: TokenPosition(11), + end: TokenPosition(19), + }, + message: "expression statement", + }], + help: None, + notes: &[], + }, + ); + + assert_diagnostic( + &runs.get_any("files/P0025_02.uc"), + &ExpectedDiagnostic { + headline: "missing `;` after expression statement", + severity: Severity::Error, + code: Some("P0025"), + primary_label: Some(ExpectedLabel { + span: TokenSpan { + start: TokenPosition(25), + end: TokenPosition(25), + }, + message: "expected `;` before `C`", + }), + secondary_labels: &[ExpectedLabel { + span: TokenSpan { + start: TokenPosition(22), + end: TokenPosition(22), + }, + message: "expression statement ends here", + }], + help: None, + notes: &[], + }, + ); + + assert_diagnostic( + &runs.get_any("files/P0025_03.uc"), + &ExpectedDiagnostic { + headline: "missing `;` after expression statement", + severity: Severity::Error, + code: Some("P0025"), + primary_label: Some(ExpectedLabel { + span: TokenSpan { + start: TokenPosition(20), + end: TokenPosition(20), + }, + message: "expected `;` before `if`", + }), + secondary_labels: &[ExpectedLabel { + span: TokenSpan { + start: TokenPosition(17), + end: TokenPosition(17), + }, + message: "expression statement ends here", + }], + help: None, + notes: &[], + }, + ); + + assert_diagnostic( + &runs.get_any("files/P0025_04.uc"), + &ExpectedDiagnostic { + headline: "missing `;` after expression statement", + severity: Severity::Error, + code: Some("P0025"), + primary_label: Some(ExpectedLabel { + span: TokenSpan { + start: TokenPosition(28), + end: TokenPosition(28), + }, + message: "expected `;` before `I`", + }), + secondary_labels: &[ExpectedLabel { + span: TokenSpan { + start: TokenPosition(25), + end: TokenPosition(25), + }, + message: "expression statement ends here", + }], + help: None, + notes: &[], + }, + ); +} + +#[test] +fn check_p0026_fixtures() { + let runs = run_fixtures(P0026_FIXTURES); + + assert_eq!(runs.get("files/P0026_01.uc").unwrap().len(), 1); + assert_eq!(runs.get("files/P0026_02.uc").unwrap().len(), 1); + assert_eq!(runs.get("files/P0026_03.uc").unwrap().len(), 1); + assert_eq!(runs.get("files/P0026_04.uc").unwrap().len(), 1); + + assert_diagnostic( + &runs.get_any("files/P0026_01.uc"), + &ExpectedDiagnostic { + headline: "missing `}` to close block", + severity: Severity::Error, + code: Some("P0026"), + primary_label: Some(ExpectedLabel { + span: TokenSpan { + start: TokenPosition(0), + end: TokenPosition(29), + }, + message: "expected `}` before end of file", + }), + secondary_labels: &[], + help: None, + notes: &[], + }, + ); + + assert_diagnostic( + &runs.get_any("files/P0026_02.uc"), + &ExpectedDiagnostic { + headline: "missing `}` to close block", + severity: Severity::Error, + code: Some("P0026"), + primary_label: Some(ExpectedLabel { + span: TokenSpan { + start: TokenPosition(0), + end: TokenPosition(42), + }, + message: "expected `}` before end of file", + }), + secondary_labels: &[], + help: None, + notes: &[], + }, + ); + + assert_diagnostic( + &runs.get_any("files/P0026_03.uc"), + &ExpectedDiagnostic { + headline: "missing `}` to close block", + severity: Severity::Error, + code: Some("P0026"), + primary_label: Some(ExpectedLabel { + span: TokenSpan { + start: TokenPosition(59), + end: TokenPosition(59), + }, + message: "expected `}` before end of file", + }), + secondary_labels: &[], + help: None, + notes: &[], + }, + ); + + assert_diagnostic( + &runs.get_any("files/P0026_04.uc"), + &ExpectedDiagnostic { + headline: "missing `}` to close block", + severity: Severity::Error, + code: Some("P0026"), + primary_label: Some(ExpectedLabel { + span: TokenSpan { + start: TokenPosition(0), + end: TokenPosition(31), + }, + message: "expected `}` before end of file", + }), + secondary_labels: &[], + help: None, + notes: &[], + }, + ); +} + +#[test] +fn check_p0025_mixed_fixtures() { + let runs = run_fixtures(P0025_P0026_MIXED_FIXTURES); + + assert_eq!(runs.get("files/P_mixed_01.uc").unwrap().len(), 2); + assert_eq!(runs.get("files/P_mixed_02.uc").unwrap().len(), 2); + + assert_diagnostic( + &runs.get_by_code("files/P_mixed_01.uc", "P0025"), + &ExpectedDiagnostic { + headline: "missing `;` after expression statement", + severity: Severity::Error, + code: Some("P0025"), + primary_label: Some(ExpectedLabel { + span: TokenSpan { + start: TokenPosition(19), + end: TokenPosition(19), + }, + message: "expected `;` before `UpdateHud`", + }), + secondary_labels: &[ExpectedLabel { + span: TokenSpan { + start: TokenPosition(9), + end: TokenPosition(17), + }, + message: "expression statement", + }], + help: None, + notes: &[], + }, + ); + + assert_diagnostic( + &runs.get_by_code("files/P_mixed_02.uc", "P0025"), + &ExpectedDiagnostic { + headline: "missing `;` after expression statement", + severity: Severity::Error, + code: Some("P0025"), + primary_label: Some(ExpectedLabel { + span: TokenSpan { + start: TokenPosition(20), + end: TokenPosition(20), + }, + message: "expected `;` before `if`", + }), + secondary_labels: &[ExpectedLabel { + span: TokenSpan { + start: TokenPosition(17), + end: TokenPosition(17), + }, + message: "expression statement ends here", + }], + help: None, + notes: &[], + }, + ); +} + +#[test] +fn check_p0026_mixed_fixtures() { + let runs = run_fixtures(P0025_P0026_MIXED_FIXTURES); + + assert_eq!(runs.get("files/P_mixed_01.uc").unwrap().len(), 2); + assert_eq!(runs.get("files/P_mixed_02.uc").unwrap().len(), 2); + + assert_diagnostic( + &runs.get_by_code("files/P_mixed_01.uc", "P0026"), + &ExpectedDiagnostic { + headline: "missing `}` to close block", + severity: Severity::Error, + code: Some("P0026"), + primary_label: Some(ExpectedLabel { + span: TokenSpan { + start: TokenPosition(29), + end: TokenPosition(29), + }, + message: "expected `}` before end of file", + }), + secondary_labels: &[], + help: None, + notes: &[], + }, + ); + + assert_diagnostic( + &runs.get_by_code("files/P_mixed_02.uc", "P0026"), + &ExpectedDiagnostic { + headline: "missing `}` to close block", + severity: Severity::Error, + code: Some("P0026"), + primary_label: Some(ExpectedLabel { + span: TokenSpan { + start: TokenPosition(0), + end: TokenPosition(41), + }, + message: "expected `}` before end of file", + }), + secondary_labels: &[], + help: None, + notes: &[], + }, + ); +} + +pub(super) const P0027_FIXTURES: &[Fixture] = &[ + Fixture { + label: "files/P0027_01.uc", + source: "{\n local bool bReady;\n bReady = CheckReady();\n else { StartMatch(); }\n NotifyReady();\n}\n", + }, + Fixture { + label: "files/P0027_02.uc", + source: "{ local int Count; Count = 3; case 3: Count++; UpdateHud();}", + }, + Fixture { + label: "files/P0027_03.uc", + source: "{\n local bool bDone;\n bDone = false;\n until (bDone)\n TickWork();\n}\n", + }, + Fixture { + label: "files/P0027_04.uc", + source: "{\n local int Count;\n Count = 0;\n #exec TEXTURE IMPORT NAME=Bad FILE=Bad.bmp\n Count++;\n}\n", + }, +]; + +#[test] +fn check_p0027_fixtures() { + let runs = run_fixtures(P0027_FIXTURES); + + assert_eq!(runs.get("files/P0027_01.uc").unwrap().len(), 1); + assert_eq!(runs.get("files/P0027_02.uc").unwrap().len(), 1); + assert_eq!(runs.get("files/P0027_03.uc").unwrap().len(), 1); + assert_eq!(runs.get("files/P0027_04.uc").unwrap().len(), 1); + + assert_diagnostic( + &runs.get_any("files/P0027_01.uc"), + &ExpectedDiagnostic { + headline: "expected statement or expression, found `else`", + severity: Severity::Error, + code: Some("P0027"), + primary_label: Some(ExpectedLabel { + span: TokenSpan { + start: TokenPosition(21), + end: TokenPosition(21), + }, + message: "unexpected `else`", + }), + secondary_labels: &[], + help: None, + notes: &[], + }, + ); + + assert_diagnostic( + &runs.get_any("files/P0027_02.uc"), + &ExpectedDiagnostic { + headline: "expected statement or expression, found `case`", + severity: Severity::Error, + code: Some("P0027"), + primary_label: Some(ExpectedLabel { + span: TokenSpan { + start: TokenPosition(16), + end: TokenPosition(16), + }, + message: "unexpected `case`", + }), + secondary_labels: &[], + help: None, + notes: &[], + }, + ); + + assert_diagnostic( + &runs.get_any("files/P0027_03.uc"), + &ExpectedDiagnostic { + headline: "expected statement or expression, found `until`", + severity: Severity::Error, + code: Some("P0027"), + primary_label: Some(ExpectedLabel { + span: TokenSpan { + start: TokenPosition(19), + end: TokenPosition(19), + }, + message: "unexpected `until`", + }), + secondary_labels: &[], + help: None, + notes: &[], + }, + ); + + assert_diagnostic( + &runs.get_any("files/P0027_04.uc"), + &ExpectedDiagnostic { + headline: "expected statement or expression, found `#exec` directive", + severity: Severity::Error, + code: Some("P0027"), + primary_label: Some(ExpectedLabel { + span: TokenSpan { + start: TokenPosition(19), + end: TokenPosition(19), + }, + message: "`#exec` directives are not allowed in a statement block", + }), + secondary_labels: &[], + help: None, + notes: &[], + }, + ); +} \ No newline at end of file diff --git a/rottlib/tests/parser_diagnostics/mod.rs b/rottlib/tests/parser_diagnostics/mod.rs index c61c0bb..426a3ef 100644 --- a/rottlib/tests/parser_diagnostics/mod.rs +++ b/rottlib/tests/parser_diagnostics/mod.rs @@ -5,6 +5,7 @@ use rottlib::diagnostics::{Diagnostic, Severity}; use rottlib::lexer::{TokenPosition, TokenSpan, TokenizedFile}; use rottlib::parser::Parser; +mod block_items; mod control_flow_expressions; mod primary_expressions;