From 1b678934bd74d0655a8d0bde37ed6b2ca9d66aec Mon Sep 17 00:00:00 2001 From: Joshua Gardner Date: Thu, 6 Aug 2026 17:17:35 +1000 Subject: [PATCH] Fix formatting of comments after statements without semicolons --- CHANGELOG.md | 1 + core/CHANGELOG.md | 1 + .../generators/logical_line_parser.rs | 141 +++++++++++++ .../generators/optimising_line_formatter.rs | 191 ++++++++++++++++++ core/src/defaults/parser.rs | 132 +++++++----- core/src/lang.rs | 1 + .../rules/optimising_line_formatter/mod.rs | 18 ++ 7 files changed, 439 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bef3e93..416c8f45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Fixed parsing of logical lines with sequential `<` and `>` comparisons. - Fixed formatting of `interface` as a generic constraint. - Fixed formatting of comments in directive blocks. +- Fixed formatting of comments after statements without semicolons. ### Added diff --git a/core/CHANGELOG.md b/core/CHANGELOG.md index c30deed1..65895a38 100644 --- a/core/CHANGELOG.md +++ b/core/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fixed parsing of logical lines with sequential `<` and `>` comparisons. - Fixed formatting of `interface` as a generic constraint. - Fixed formatting of comments in directive blocks. +- Fixed formatting of comments after statements without semicolons. ### Added diff --git a/core/datatests/generators/logical_line_parser.rs b/core/datatests/generators/logical_line_parser.rs index 3172923d..917a265b 100644 --- a/core/datatests/generators/logical_line_parser.rs +++ b/core/datatests/generators/logical_line_parser.rs @@ -195,6 +195,7 @@ mod comments { pub fn generate(root_dir: &Path) { inline::generate(root_dir); individual::generate(root_dir); + before_context_end::generate(root_dir); } mod inline { @@ -509,6 +510,146 @@ mod comments { ); } } + + mod before_context_end { + use super::*; + + pub fn generate(root_dir: &Path) { + generate_test_cases!( + root_dir, + compound = " + _|begin + _| A := B + C + _| // + _|end + ", + compound_with_conditional = " + _|begin + _|{$IFDEF A} + _| A := B + C + _|{$ENDIF} + _| // + _|end + ", + anonymous = " + _1 |A := procedure + _1 |begin{1} + _^1 | A := B + C + _^1 | // + _1 |end; + ", + if_else = " + _ |begin + _1 | if A then{1} + _^1 | A := B + C + _^1 | // + _1 | else{2} + _^2 | B := C + D + _ | // + _ |end + ", + if_else_begin = " + _ |begin + _1 | if A then{1} + _^1 | // + _^1 | begin + _^1 | // + _^1 | end + _1 | else{2} + _^2 | B := C + D + _ | // + _ |end + ", + for_in = " + _ |begin + _1 | for A in B do{1} + _^1 | A := B + C + _ | // + _ |end + ", + for_to = " + _ |begin + _1 | for A := B to C do{1} + _^1 | A := B + C + _ | // + _ |end + ", + while_do = " + _ |begin + _1 | while A do{1} + _^1 | A := B + C + _ | // + _ |end + ", + initialization_finalization = " + _|initialization + _| A := B + C + _| // + _|finalization + _| A := B + C + _| // + _|end. + ", + initialization = " + _|initialization + _| A := B + C + _| // + _|end. + ", + case = " + _ |case A of + _1 | B:{1} + _^1| A := B + C + _ | // + _ |end. + ", + case_else = " + _ |case A of + _1 | B:{1} + _^1| A := B + C + _ | // + _ |else + _ | A := B + C + _ | // + _ |end. + ", + repeat = " + _|repeat + _| A := B + C + _| // + _|until False; + ", + try_except = " + _ |try + _ | A := B + C + _ | // + _ |except + _1 | on A do{1} + _^1| A := B + C + _ | // + _ |end; + ", + try_bare_except = " + _|try + _| A := B + C + _| // + _|except + _| A := B + C + _| // + _|end; + ", + try_finally = " + _|try + _| A := B + C + _| // + _|finally + _| A := B + C + _| // + _|end; + ", + ); + } + } } mod child_lines { diff --git a/core/datatests/generators/optimising_line_formatter.rs b/core/datatests/generators/optimising_line_formatter.rs index f2abad3f..d227c425 100644 --- a/core/datatests/generators/optimising_line_formatter.rs +++ b/core/datatests/generators/optimising_line_formatter.rs @@ -362,6 +362,7 @@ mod comments { conditional_directives::generate(root_dir); individual_block::generate(root_dir); compound_operators::generate(root_dir); + before_context_end::generate(root_dir); } mod midline_line { @@ -559,6 +560,25 @@ mod comments { end ); ", + anonymous_content = " + A( + procedure + begin + // + end, + procedure + begin + A + // + end, + procedure + begin + A; + // + A + end, + ); + ", if_else = " if AAAAAA then // BBBBBBB; @@ -579,6 +599,32 @@ mod comments { else // begin end; + if A then + // + begin + // + end + // + ; + if A then + // + begin + A + // + end + // + ; + if A then begin + // + end + // + ; + if A then begin + A + // + end + // + ; ", after_do = " try @@ -744,6 +790,151 @@ mod comments { ); } } + + mod before_context_end { + use super::*; + + pub fn generate(root_dir: &Path) { + generate_test_cases!( + root_dir, + compound = " + begin + A := B + C + // + end + ", + compound_with_conditional = " + begin + {$IFDEF A} + A := B + C + {$ENDIF} + // + end + ", + if_else = " + begin + if A then begin + A := B + C + end + + // + else + B := C + D; + if A then // + begin + A := B + C + end + + // + else + B := C + D; + if A then + // + begin + A := B + C + end + + // + else + B := C + D; + if A then + A := B + C + + // + else + B := C + D + + // + end + ", + for_in = " + begin + for A in B do + A := B + C + // + end + ", + for_to = " + begin + for A := B to C do + A := B + C + // + end + ", + while_do = " + begin + while A do + A := B + C + // + end + ", + initialization_finalization = " + initialization + A := B + C + // + finalization + A := B + C + // + end. + ", + initialization = " + initialization + A := B + C + // + end. + ", + case = " + case A of + B: A := B + C + // + end. + ", + case_else = " + case A of + B: A := B + C + // + else + A := B + C + // + end. + ", + repeat = " + repeat + A := B + C + // + until False; + ", + try_except = " + try + A := B + C + // + except + on A do + A := B + C + // + end; + ", + try_bare_except = " + try + A := B + C + // + except + A := B + C + // + end; + ", + try_finally = " + try + A := B + C + // + finally + A := B + C + // + end; + ", + ); + } + } } mod anonymous { diff --git a/core/src/defaults/parser.rs b/core/src/defaults/parser.rs index 9b8d9dc5..3b719da9 100644 --- a/core/src/defaults/parser.rs +++ b/core/src/defaults/parser.rs @@ -226,8 +226,8 @@ impl<'a, 'b> InternalDelphiLogicalLineParser<'a, 'b> { while let Some(token_type) = self.get_current_token_type() { if let Some(ending_context) = self.context.get_ending_context_idx(self) { trace!( - "Context ended, returning from parse_structures with {:?}", - token_type + "Context {:?} ended, returning from parse_structures with {:?}", + self.context.contexts[ending_context].context_type, token_type ); self.context.update_statuses(ending_context); return; @@ -371,7 +371,7 @@ impl<'a, 'b> InternalDelphiLogicalLineParser<'a, 'b> { self.next_token(); self.parse_statement_list_block(ParserContext { context_type: ContextType::StatementBlock(BlockKind::Begin), - context_ending_predicate: CEP::Opaque(end), + context_ending_predicate: CEP::Opaque(statement_list_end), level: ParserContextLevel::Level(1), }); if let Some(TT::Keyword(KK::End)) = self.get_current_token_type() { @@ -438,7 +438,7 @@ impl<'a, 'b> InternalDelphiLogicalLineParser<'a, 'b> { self.next_token(); // Else self.parse_statement_list_block(ParserContext { context_type: ContextType::StatementBlock(BlockKind::Else), - context_ending_predicate: CEP::Opaque(end), + context_ending_predicate: CEP::Opaque(statement_list_end), level: ParserContextLevel::Level(1), }); } @@ -611,13 +611,19 @@ impl<'a, 'b> InternalDelphiLogicalLineParser<'a, 'b> { trace!("Parse `then` statement"); let mut level = ParserContextLevel::Parent(parent, 1); - self.parse_block(ParserContext { - context_type: ContextType::Statement(StatementKind::Normal), - context_ending_predicate: CEP::Transparent(|llp| { - matches!(llp.get_current_token_type(), Some(TT::Keyword(KK::Else))) - }), - level, - }); + self.do_with_context( + ParserContext { + context_type: ContextType::Statement(StatementKind::Normal), + context_ending_predicate: CEP::Transparent(kw_else), + level, + }, + |parser| { + parser.parse_structures(); + parser.finish_logical_line(); + // Interpret trailing comments as referring to the next line + parser.take_individual_comments(Some(LLT::ParentLineChildComment)); + }, + ); if self.context.is_ended.last() == Some(&false) && let Some(KK::Else) = self.get_current_keyword_kind() @@ -692,12 +698,7 @@ impl<'a, 'b> InternalDelphiLogicalLineParser<'a, 'b> { let context = ParserContext { context_type: ContextType::Statement(StatementKind::Case), - context_ending_predicate: CEP::Opaque(|parser: &LLP| { - matches!( - parser.get_current_token_type(), - Some(TT::Keyword(KK::End | KK::Else)) - ) - }), + context_ending_predicate: CEP::Opaque(else_end), level: ParserContextLevel::Level(1), }; self.parse_statement_block_with_kind(context, StatementKind::Case); @@ -707,7 +708,7 @@ impl<'a, 'b> InternalDelphiLogicalLineParser<'a, 'b> { self.finish_logical_line(); self.parse_statement_list_block(ParserContext { context_type: ContextType::StatementBlock(BlockKind::Else), - context_ending_predicate: CEP::Opaque(end), + context_ending_predicate: CEP::Opaque(statement_list_end), level: ParserContextLevel::Level(1), }); } @@ -818,6 +819,23 @@ impl<'a, 'b> InternalDelphiLogicalLineParser<'a, 'b> { self.current_line.pop(); } + fn take_individual_comments(&mut self, line_type: Option) { + trace!("Take individual comments"); + while let Some(TT::Comment( + CommentKind::IndividualBlock + | CommentKind::IndividualLine + | CommentKind::MultilineBlock, + )) = self.get_current_token_type() + { + trace!("Taking individual comment line"); + self.next_token(); + if let Some(line_type) = line_type { + self.set_logical_line_type(line_type); + } + self.finish_logical_line(); + } + } + fn parse_comment_lines(&mut self) { self.parse_block(ParserContext { context_type: ContextType::Utility, @@ -864,8 +882,8 @@ impl<'a, 'b> InternalDelphiLogicalLineParser<'a, 'b> { if let Some(context) = self.get_last_context() { if let Some(ending_context) = self.context.get_ending_context_idx(self) { trace!( - "Context ended, returning from parse_statement with {:?}", - token_type + "Context {:?} ended, returning from parse_statement with {:?}", + self.context.contexts[ending_context].context_type, token_type ); self.context.update_statuses(ending_context); return; @@ -1214,14 +1232,17 @@ impl<'a, 'b> InternalDelphiLogicalLineParser<'a, 'b> { self.finish_logical_line(); // With no unfinished line, this will add the separators to the last child line self.take_separators_on_last_line(level); + if self.context.get_ending_context_idx(self).is_some() || self.get_current_token_type().is_none() { // If the parent context above the `Statement` is over, return. // Otherwise, there are more statements to consume. - return; + break; } } + + self.take_individual_comments(None); } fn parse_block(&mut self, context: ParserContext) { self.do_with_context(context, |parser| { @@ -1709,7 +1730,7 @@ impl<'a, 'b> InternalDelphiLogicalLineParser<'a, 'b> { self.next_token(); // Begin self.parse_statement_list_block(ParserContext { context_type: ContextType::StatementBlock(BlockKind::Begin), - context_ending_predicate: CEP::Opaque(end), + context_ending_predicate: CEP::Opaque(statement_list_end), level: context_level, }); self.next_token(); // End @@ -2222,18 +2243,6 @@ fn of(parser: &LLP) -> bool { matches!(parser.get_current_keyword_kind(), Some(KK::Of)) } -fn section_headings(parser: &LLP) -> bool { - match parser.get_current_token_type() { - Some(TT::Keyword(KK::Implementation | KK::Initialization | KK::Finalization | KK::End)) => { - true - } - Some(TT::Keyword(KK::Interface)) => { - !matches!(parser.get_token_type::<-1>(), Some(TT::Op(OK::Equal(_)))) - } - _ => false, - } -} - fn visibility_specifier(parser: &LLP) -> bool { matches!( parser.get_current_keyword_kind(), @@ -2251,27 +2260,58 @@ fn begin_asm(parser: &LLP) -> bool { Some(TT::Keyword(KK::Begin | KK::Asm)) ) } + +macro_rules! statement_list_predicate { + ($parser: expr, $token_type_pat: pat) => {{ + ($parser.pass_index..) + .map(|pass_index| $parser.get_token_type_for_index(pass_index)) + .take_while(|token| !matches!(token, None | Some($token_type_pat))) + .all(|token| { + matches!( + token, + Some(TT::Comment( + CommentKind::IndividualLine + | CommentKind::MultilineBlock + | CommentKind::IndividualBlock + )) + ) + }) + }}; +} + +fn section_headings(parser: &LLP) -> bool { + match parser.get_current_token_type() { + Some(TT::Keyword(KK::Interface)) => { + !matches!(parser.get_token_type::<-1>(), Some(TT::Op(OK::Equal(_)))) + } + _ => { + statement_list_predicate!( + parser, + TT::Keyword(KK::Implementation | KK::Initialization | KK::Finalization | KK::End) + ) + } + } +} + fn else_end(parser: &LLP) -> bool { - matches!( - parser.get_current_token_type(), - Some(TT::Keyword(KK::End | KK::Else)) - ) + statement_list_predicate!(parser, TT::Keyword(KK::End | KK::Else)) +} + +fn kw_else(parser: &LLP) -> bool { + statement_list_predicate!(parser, TT::Keyword(KK::Else)) +} +fn statement_list_end(parser: &LLP) -> bool { + statement_list_predicate!(parser, TT::Keyword(KK::End)) } fn end(parser: &LLP) -> bool { matches!(parser.get_current_token_type(), Some(TT::Keyword(KK::End))) } fn until(parser: &LLP) -> bool { - matches!( - parser.get_current_token_type(), - Some(TT::Keyword(KK::Until)) - ) + statement_list_predicate!(parser, TT::Keyword(KK::Until)) } fn except_finally(parser: &LLP) -> bool { - matches!( - parser.get_current_token_type(), - Some(TT::Keyword(KK::Except | KK::Finally)) - ) + statement_list_predicate!(parser, TT::Keyword(KK::Except | KK::Finally)) } fn not_comment_or_directive(parser: &LLP) -> bool { diff --git a/core/src/lang.rs b/core/src/lang.rs index 28b39cfa..f74c82a2 100644 --- a/core/src/lang.rs +++ b/core/src/lang.rs @@ -461,6 +461,7 @@ pub enum LogicalLineType { CaseArm, Declaration, VariantRecordCaseArm, + ParentLineChildComment, Unknown, Voided, } diff --git a/core/src/rules/optimising_line_formatter/mod.rs b/core/src/rules/optimising_line_formatter/mod.rs index 26b3a062..13fa9fa5 100644 --- a/core/src/rules/optimising_line_formatter/mod.rs +++ b/core/src/rules/optimising_line_formatter/mod.rs @@ -1063,6 +1063,23 @@ impl<'this> InternalOptimisingLineFormatter<'this, '_> { }, ChildLineOption::BreakAll(ws) | ChildLineOption::ContinueThenBreak(ws) => ws, }; + /* + Lines with the `LLT::ParentLineChildComment` line type are + formatted to the parent's level. + + E.g., + ``` + if A then + B + // This comment + else + C; + ``` + */ + let get_child_starting_ws = |line_type| match line_type { + LLT::ParentLineChildComment => parent_base_ws, + _ => child_starting_ws, + }; let get_first_token_decision = |index, line_length| match option { ChildLineOption::ContinueAll => FirstDecision::Continue { line_length, @@ -1093,6 +1110,7 @@ impl<'this> InternalOptimisingLineFormatter<'this, '_> { .map(|&child_line| (child_line, &self.lines[child_line])) .enumerate() { + let child_starting_ws = get_child_starting_ws(line.1.get_line_type()); let mut child_whitespace = child_starting_ws.whitespace; child_whitespace.indentations += line.1.get_level(); child_whitespace.indentations = child_whitespace