Skip to content

Commit 0374b44

Browse files
committed
fix(formatter): add same-line trailing comment check to ensure idempotent if/elseif formatting
closes #813 Signed-off-by: azjezz <[email protected]>
1 parent faf2535 commit 0374b44

File tree

6 files changed

+49
-1
lines changed

6 files changed

+49
-1
lines changed

crates/formatter/src/internal/comment/format.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,33 @@ impl<'arena> FormatterState<'_, 'arena> {
7474
|| (remaining_content.starts_with('#') && !remaining_content.starts_with("#["))
7575
}
7676

77+
/// Checks if a node has a trailing line comment on the same line.
78+
///
79+
/// This is different from `is_followed_by_comment_on_next_line` which checks
80+
/// for comments on the subsequent line. This method detects trailing comments
81+
/// that are on the same line as the node (e.g., `} // comment`).
82+
///
83+
/// # Arguments
84+
///
85+
/// * `span` - The span of the node to check for same-line trailing comments.
86+
///
87+
/// # Returns
88+
///
89+
/// `true` if there's a line comment on the same line after the node, `false` otherwise.
90+
pub(crate) fn has_same_line_trailing_comment(&self, span: Span) -> bool {
91+
let Some(first_char_offset) = self.skip_spaces(Some(span.end_offset()), false) else {
92+
return false;
93+
};
94+
95+
// If there's a newline before the next content, the comment is on the next line, not same line
96+
if self.has_newline(first_char_offset, /* backwards */ true) {
97+
return false;
98+
}
99+
100+
let remaining = &self.source_text[first_char_offset as usize..];
101+
remaining.starts_with("//") || (remaining.starts_with('#') && !remaining.starts_with("#["))
102+
}
103+
77104
pub(crate) fn has_leading_own_line_comment(&self, range: Span) -> bool {
78105
self.has_comment_with_filter(range, CommentFlags::Leading, |comment| {
79106
self.has_newline(comment.end, /* backwards */ false)

crates/formatter/src/internal/format/misc.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -565,7 +565,10 @@ pub(super) fn adjust_clause<'arena>(
565565
};
566566

567567
if has_trailing_segment {
568-
if !is_block || f.is_followed_by_comment_on_next_line(node.span()) {
568+
if !is_block
569+
|| f.is_followed_by_comment_on_next_line(node.span())
570+
|| f.has_same_line_trailing_comment(node.span())
571+
{
569572
Document::Array(vec![in f.arena; clause, Document::Line(Line::hard())])
570573
} else {
571574
Document::Array(vec![in f.arena; clause, Document::space()])
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
3+
if ($x) {
4+
} // foo
5+
elseif (
6+
$aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
7+
) {
8+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
<?php
2+
3+
if ($x) {
4+
} // foo
5+
elseif ($aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) {
6+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
FormatSettings {
2+
..Default::default()
3+
}

crates/formatter/tests/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,7 @@ test_case!(issue_738);
308308
test_case!(issue_788);
309309
test_case!(issue_796);
310310
test_case!(issue_812);
311+
test_case!(issue_813);
311312

312313
#[test]
313314
fn test_all_test_cases_are_ran() {

0 commit comments

Comments
 (0)