Skip to content

Commit 5302c8e

Browse files
committed
feat(formatter): correct comments printing for SwitchStatement (#12881)
1 parent 9e69e1a commit 5302c8e

File tree

3 files changed

+147
-101
lines changed

3 files changed

+147
-101
lines changed

crates/oxc_formatter/src/write/mod.rs

Lines changed: 1 addition & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ mod object_pattern_like;
1515
mod parameter_list;
1616
mod return_or_throw_statement;
1717
mod semicolon;
18+
mod switch_statement;
1819
mod try_statement;
1920
mod type_parameters;
2021
mod utils;
@@ -871,104 +872,6 @@ impl<'a> FormatWrite<'a> for AstNode<'a, WithStatement<'a>> {
871872
}
872873
}
873874

874-
impl<'a> FormatWrite<'a> for AstNode<'a, SwitchStatement<'a>> {
875-
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
876-
let discriminant = self.discriminant();
877-
let cases = self.cases();
878-
let format_cases =
879-
format_with(|f| if cases.is_empty() { hard_line_break().fmt(f) } else { cases.fmt(f) });
880-
write!(
881-
f,
882-
[
883-
"switch",
884-
space(),
885-
"(",
886-
group(&soft_block_indent(&discriminant)),
887-
")",
888-
space(),
889-
"{",
890-
block_indent(&format_cases),
891-
"}"
892-
]
893-
)
894-
}
895-
}
896-
897-
impl<'a> Format<'a> for AstNode<'a, Vec<'a, SwitchCase<'a>>> {
898-
fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
899-
let source_text = f.source_text();
900-
let mut join = f.join_nodes_with_hardline();
901-
for case in self {
902-
join.entry(case.span(), case);
903-
}
904-
join.finish()
905-
}
906-
}
907-
908-
impl<'a> FormatWrite<'a> for AstNode<'a, SwitchCase<'a>> {
909-
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
910-
if let Some(test) = self.test() {
911-
write!(f, ["case", space(), test, ":"])?;
912-
} else {
913-
write!(f, ["default", ":"])?;
914-
}
915-
916-
let consequent = self.consequent();
917-
// Whether the first statement in the clause is a BlockStatement, and
918-
// there are no other non-empty statements. Empties may show up when
919-
// parsing depending on if the input code includes certain newlines.
920-
let is_single_block_statement =
921-
matches!(consequent.as_ref().first(), Some(Statement::BlockStatement(_)))
922-
&& consequent
923-
.iter()
924-
.filter(|statement| !matches!(statement.as_ref(), Statement::EmptyStatement(_)))
925-
.count()
926-
== 1;
927-
// When the case block is empty, the case becomes a fallthrough, so it
928-
// is collapsed directly on top of the next case (just a single
929-
// hardline).
930-
// When the block is a single statement _and_ it's a block statement,
931-
// then the opening brace of the block can hug the same line as the
932-
// case. But, if there's more than one statement, then the block
933-
// _cannot_ hug. This distinction helps clarify that the case continues
934-
// past the end of the block statement, despite the braces making it
935-
// seem like it might end.
936-
// Lastly, the default case is just to break and indent the body.
937-
//
938-
// switch (key) {
939-
// case fallthrough: // trailing comment
940-
// case normalBody:
941-
// someWork();
942-
// break;
943-
//
944-
// case blockBody: {
945-
// const a = 1;
946-
// break;
947-
// }
948-
//
949-
// case separateBlockBody:
950-
// {
951-
// breakIsNotInsideTheBlock();
952-
// }
953-
// break;
954-
//
955-
// default:
956-
// break;
957-
// }
958-
if consequent.is_empty() {
959-
// Print nothing to ensure that trailing comments on the same line
960-
// are printed on the same line. The parent list formatter takes
961-
// care of inserting a hard line break between cases.
962-
Ok(())
963-
} else if is_single_block_statement {
964-
write!(f, [space(), consequent])
965-
} else {
966-
// no line break needed after because it is added by the indent in the switch statement
967-
write!(f, indent(&format_args!(hard_line_break(), consequent)))
968-
}
969-
}
970-
}
971-
972875
impl<'a> FormatWrite<'a> for AstNode<'a, LabeledStatement<'a>> {
973876
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
974877
let label = self.label();
Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
use oxc_allocator::Vec;
2+
use oxc_ast::ast::*;
3+
use oxc_span::GetSpan;
4+
use oxc_syntax::identifier::is_identifier_name;
5+
6+
use crate::{
7+
Format, FormatResult, format_args,
8+
formatter::{
9+
Formatter,
10+
comments::{is_end_of_line_comment, is_own_line_comment},
11+
prelude::*,
12+
trivia::{DanglingIndentMode, FormatDanglingComments, FormatTrailingComments},
13+
},
14+
generated::ast_nodes::{AstNode, AstNodes},
15+
write,
16+
write::{semicolon::OptionalSemicolon, utils::statement_body::FormatStatementBody},
17+
};
18+
19+
use super::FormatWrite;
20+
21+
impl<'a> FormatWrite<'a> for AstNode<'a, SwitchStatement<'a>> {
22+
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
23+
let discriminant = self.discriminant();
24+
let cases = self.cases();
25+
let format_cases =
26+
format_with(|f| if cases.is_empty() { hard_line_break().fmt(f) } else { cases.fmt(f) });
27+
write!(
28+
f,
29+
[
30+
"switch",
31+
space(),
32+
"(",
33+
group(&soft_block_indent(&discriminant)),
34+
")",
35+
space(),
36+
"{",
37+
block_indent(&format_cases),
38+
"}"
39+
]
40+
)
41+
}
42+
}
43+
44+
impl<'a> Format<'a> for AstNode<'a, Vec<'a, SwitchCase<'a>>> {
45+
fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
46+
let source_text = f.source_text();
47+
let mut join = f.join_nodes_with_hardline();
48+
for case in self {
49+
join.entry(case.span(), case);
50+
}
51+
join.finish()
52+
}
53+
}
54+
55+
impl<'a> FormatWrite<'a> for AstNode<'a, SwitchCase<'a>> {
56+
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
57+
let is_default = if let Some(test) = self.test() {
58+
write!(f, ["case", space(), test, ":"])?;
59+
false
60+
} else {
61+
write!(f, ["default", ":"])?;
62+
true
63+
};
64+
65+
let consequent = self.consequent();
66+
// When the case block is empty, the case becomes a fallthrough, so it
67+
// is collapsed directly on top of the next case (just a single
68+
// hardline).
69+
// When the block is a single statement _and_ it's a block statement,
70+
// then the opening brace of the block can hug the same line as the
71+
// case. But, if there's more than one statement, then the block
72+
// _cannot_ hug. This distinction helps clarify that the case continues
73+
// past the end of the block statement, despite the braces making it
74+
// seem like it might end.
75+
// Lastly, the default case is just to break and indent the body.
76+
//
77+
// switch (key) {
78+
// case fallthrough: // trailing comment
79+
// case normalBody:
80+
// someWork();
81+
// break;
82+
//
83+
// case blockBody: {
84+
// const a = 1;
85+
// break;
86+
// }
87+
//
88+
// case separateBlockBody:
89+
// {
90+
// breakIsNotInsideTheBlock();
91+
// }
92+
// break;
93+
//
94+
// default:
95+
// break;
96+
// }
97+
if consequent.is_empty() {
98+
// Print nothing to ensure that trailing comments on the same line
99+
// are printed on the same line. The parent list formatter takes
100+
// care of inserting a hard line break between cases.
101+
return Ok(());
102+
}
103+
104+
// Whether the first statement in the clause is a BlockStatement, and
105+
// there are no other non-empty statements. Empties may show up when
106+
// parsing depending on if the input code includes certain newlines.
107+
let first_statement = consequent.first().unwrap();
108+
let is_single_block_statement =
109+
matches!(first_statement.as_ref(), Statement::BlockStatement(_))
110+
&& consequent
111+
.iter()
112+
.skip(1)
113+
.all(|statement| matches!(statement.as_ref(), Statement::EmptyStatement(_)));
114+
115+
// Format dangling comments before default case body.
116+
if is_default {
117+
let comments = f.context().comments();
118+
let comments = if is_single_block_statement {
119+
comments.block_comments_before(first_statement.span().start)
120+
} else {
121+
comments.comments_before_character(self.span.start, b'\n')
122+
};
123+
124+
if !comments.is_empty() {
125+
write!(
126+
f,
127+
[
128+
space(),
129+
FormatDanglingComments::Comments {
130+
comments,
131+
indent: DanglingIndentMode::None
132+
},
133+
]
134+
)?;
135+
}
136+
}
137+
138+
if is_single_block_statement {
139+
write!(f, [FormatStatementBody::new(first_statement)])
140+
} else {
141+
// no line break needed after because it is added by the indent in the switch statement
142+
write!(f, indent(&format_args!(hard_line_break(), consequent)))
143+
}
144+
}
145+
}

tasks/prettier_conformance/snapshots/prettier.js.snap.md

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
js compatibility: 503/699 (71.96%)
1+
js compatibility: 505/699 (72.25%)
22

33
# Failed
44

@@ -124,8 +124,6 @@ js compatibility: 503/699 (71.96%)
124124
| js/sequence-expression/ignore.js | 💥 | 42.86% |
125125
| js/strings/escaped.js | 💥💥 | 73.68% |
126126
| js/strings/template-literals.js | 💥💥 | 51.49% |
127-
| js/switch/comments.js | 💥 | 90.37% |
128-
| js/switch/comments2.js | 💥 | 84.21% |
129127
| js/template/comment.js | 💥 | 23.08% |
130128
| js/template/graphql.js | 💥 | 81.25% |
131129
| js/template/indent.js | 💥 | 85.71% |

0 commit comments

Comments
 (0)