Skip to content

Commit 248fd94

Browse files
committed
feat(formatter): correct formatting for TryStatement (#12862)
1 parent 95b0230 commit 248fd94

File tree

7 files changed

+141
-46
lines changed

7 files changed

+141
-46
lines changed

crates/oxc_formatter/src/formatter/comments/mod.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,9 @@ impl<'a> Comments<'a> {
9595

9696
pub(crate) fn comments_before_character(&self, mut start: u32, character: u8) -> &'a [Comment] {
9797
let mut index = 0;
98-
let comments = self.unprinted_comments();
98+
let comments = self.comments_after(start);
9999
while index < comments.len() {
100100
let comment = &comments[index];
101-
102101
if self.source_text[start as usize..comment.span.end as usize]
103102
.contains(character as char)
104103
{

crates/oxc_formatter/src/generated/format.rs

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -787,19 +787,13 @@ impl<'a> Format<'a> for AstNode<'a, TryStatement<'a>> {
787787

788788
impl<'a> Format<'a> for AstNode<'a, CatchClause<'a>> {
789789
fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
790-
self.format_leading_comments(f)?;
791-
let result = self.write(f);
792-
self.format_trailing_comments(f)?;
793-
result
790+
self.write(f)
794791
}
795792
}
796793

797794
impl<'a> Format<'a> for AstNode<'a, CatchParameter<'a>> {
798795
fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
799-
self.format_leading_comments(f)?;
800-
let result = self.write(f);
801-
self.format_trailing_comments(f)?;
802-
result
796+
self.write(f)
803797
}
804798
}
805799

crates/oxc_formatter/src/write/block_statement.rs

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use oxc_span::GetSpan;
44

55
use super::FormatWrite;
66
use crate::{
7+
format_args,
78
formatter::{
89
Buffer, FormatResult, Formatter,
910
prelude::*,
@@ -16,19 +17,46 @@ use crate::{
1617
impl<'a> FormatWrite<'a> for AstNode<'a, BlockStatement<'a>> {
1718
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
1819
write!(f, "{")?;
20+
21+
let comments_before_catch_clause = if let AstNodes::CatchClause(catch) = self.parent {
22+
f.context().get_cached_element(&catch.span)
23+
} else {
24+
None
25+
};
26+
let has_comments_before_catch_clause = comments_before_catch_clause.is_some();
27+
// See reason in `[AstNode<'a, CatchClause<'a>>::write]`
28+
let formatted_comments_before_catch_clause = format_once(|f| {
29+
if let Some(comments) = comments_before_catch_clause {
30+
f.write_element(comments)
31+
} else {
32+
Ok(())
33+
}
34+
});
35+
1936
if is_empty_block(&self.body, f) {
2037
// `if (a) /* comment */ {}`
2138
// should be formatted like:
2239
// `if (a) { /* comment */ }`
2340
//
2441
// Some comments are not inside the block, but we need to print them inside the block.
25-
if f.context().comments().has_comments_before(self.span.end) {
26-
write!(f, format_dangling_comments(self.span).with_block_indent())?;
42+
if has_comments_before_catch_clause
43+
|| f.context().comments().has_comments_before(self.span.end)
44+
{
45+
write!(
46+
f,
47+
block_indent(&format_args!(
48+
&formatted_comments_before_catch_clause,
49+
format_dangling_comments(self.span)
50+
))
51+
)?;
2752
} else if is_non_collapsible(self.parent) {
2853
write!(f, hard_line_break())?;
2954
}
3055
} else {
31-
write!(f, block_indent(&self.body()))?;
56+
write!(
57+
f,
58+
block_indent(&format_args!(&formatted_comments_before_catch_clause, self.body()))
59+
)?;
3260
}
3361
write!(f, "}")
3462
}

crates/oxc_formatter/src/write/mod.rs

Lines changed: 1 addition & 28 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 try_statement;
1819
mod type_parameters;
1920
mod utils;
2021
mod variable_declaration;
@@ -982,34 +983,6 @@ impl<'a> FormatWrite<'a> for AstNode<'a, LabeledStatement<'a>> {
982983
}
983984
}
984985

985-
impl<'a> FormatWrite<'a> for AstNode<'a, TryStatement<'a>> {
986-
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
987-
let block = self.block();
988-
let handler = self.handler();
989-
let finalizer = self.finalizer();
990-
write!(f, ["try", space(), block])?;
991-
if let Some(handler) = handler {
992-
write!(f, [space(), handler])?;
993-
}
994-
if let Some(finalizer) = finalizer {
995-
write!(f, [space(), "finally", space(), finalizer])?;
996-
}
997-
Ok(())
998-
}
999-
}
1000-
1001-
impl<'a> FormatWrite<'a> for AstNode<'a, CatchClause<'a>> {
1002-
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
1003-
write!(f, ["catch", space(), self.param(), space(), self.body()])
1004-
}
1005-
}
1006-
1007-
impl<'a> FormatWrite<'a> for AstNode<'a, CatchParameter<'a>> {
1008-
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
1009-
write!(f, ["(", self.pattern(), ")"])
1010-
}
1011-
}
1012-
1013986
impl<'a> FormatWrite<'a> for AstNode<'a, DebuggerStatement> {
1014987
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
1015988
write!(f, ["debugger", OptionalSemicolon])
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
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, FormatTrailingCommas, QuoteProperties, TrailingSeparator,
8+
formatter::{
9+
Formatter,
10+
prelude::*,
11+
separated::FormatSeparatedIter,
12+
trivia::{FormatLeadingComments, FormatTrailingComments},
13+
},
14+
generated::ast_nodes::{AstNode, AstNodes},
15+
write,
16+
write::semicolon::OptionalSemicolon,
17+
};
18+
19+
use super::FormatWrite;
20+
21+
impl<'a> FormatWrite<'a> for AstNode<'a, TryStatement<'a>> {
22+
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
23+
let block = self.block();
24+
let handler = self.handler();
25+
let finalizer = self.finalizer();
26+
write!(f, ["try", space()])?;
27+
28+
// Use `write` rather than `write!` in order to avoid printing leading comments for `block`.
29+
block.write(f)?;
30+
if let Some(handler) = handler {
31+
write!(f, [space(), handler])?;
32+
}
33+
if let Some(finalizer) = finalizer {
34+
write!(f, [space(), "finally", space()])?;
35+
finalizer.write(f)?;
36+
}
37+
Ok(())
38+
}
39+
}
40+
41+
impl<'a> FormatWrite<'a> for AstNode<'a, CatchClause<'a>> {
42+
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
43+
// `try {} /* comment */ catch (e) {}`
44+
// should be formatted like:
45+
// `try {} catch (e) { /* comment */ }`
46+
//
47+
// Comments before the catch clause should be printed in the block statement.
48+
// We cache them here to avoid the `params` printing them accidentally.
49+
let printed_comments = f.intern(&format_leading_comments(self.span));
50+
if let Ok(Some(comments)) = printed_comments {
51+
f.context_mut().cache_element(&self.span, comments);
52+
}
53+
54+
write!(f, ["catch", space(), self.param(), space()])?;
55+
56+
// Use `write` rather than `write!` in order to avoid printing leading comments for `block`.
57+
self.body().write(f)
58+
}
59+
}
60+
61+
impl<'a> FormatWrite<'a> for AstNode<'a, CatchParameter<'a>> {
62+
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
63+
write!(f, "(")?;
64+
65+
let span = self.pattern.span();
66+
67+
let leading_comments = f.context().comments().comments_before(span.start);
68+
let leading_comment_with_break = leading_comments.iter().any(|comment| {
69+
comment.is_line() || get_lines_after(comment.span.end, f.source_text()) > 0
70+
});
71+
72+
let trailing_comments =
73+
f.context().comments().comments_before_character(self.span().end, b')');
74+
let trailing_comment_with_break = trailing_comments
75+
.iter()
76+
.any(|comment| comment.is_line() || get_lines_before(comment.span, f) > 0);
77+
78+
if leading_comment_with_break || trailing_comment_with_break {
79+
write!(
80+
f,
81+
soft_block_indent(&format_once(|f| {
82+
write!(f, [FormatLeadingComments::Comments(leading_comments)])?;
83+
let printed_len = f.context().comments().printed_comments().len();
84+
write!(f, self.pattern())?;
85+
if trailing_comments.is_empty() ||
86+
// The `pattern` cannot print comments that are below it, so we need to check whether there
87+
// are any trailing comments that haven't been printed yet. If there are, print them.
88+
f.context().comments().printed_comments().len() - printed_len
89+
== trailing_comments.len()
90+
{
91+
Ok(())
92+
} else {
93+
write!(f, FormatTrailingComments::Comments(trailing_comments))
94+
}
95+
}))
96+
)?;
97+
} else {
98+
write!(f, self.pattern())?;
99+
}
100+
101+
write!(f, ")")
102+
}
103+
}

tasks/ast_tools/src/generators/formatter/format.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ use crate::{
1414
const FORMATTER_CRATE_PATH: &str = "crates/oxc_formatter";
1515

1616
/// Based on the prettier printing comments algorithm, these nodes don't need to print comments.
17-
const AST_NODE_WITHOUT_PRINTING_COMMENTS_LIST: &[&str] = &["FormalParameters", "FunctionBody"];
17+
const AST_NODE_WITHOUT_PRINTING_COMMENTS_LIST: &[&str] =
18+
&["FormalParameters", "FunctionBody", "CatchParameter", "CatchClause"];
1819

1920
const NEEDS_PARENTHESES: &[&str] = &[
2021
"Class",

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
js compatibility: 498/699 (71.24%)
1+
js compatibility: 501/699 (71.67%)
22

33
# Failed
44

@@ -47,7 +47,6 @@ js compatibility: 498/699 (71.24%)
4747
| js/comments/tagged-template-literal.js | 💥💥 | 69.23% |
4848
| js/comments/template-literal.js | 💥💥 | 30.43% |
4949
| js/comments/trailing_space.js | 💥💥 | 60.00% |
50-
| js/comments/try.js | 💥💥 | 71.43% |
5150
| js/comments/variable_declarator.js | 💥✨ | 49.31% |
5251
| js/comments/flow-types/inline.js | 💥 | 62.50% |
5352
| js/comments/html-like/comment.js | 💥 | 0.00% |
@@ -154,8 +153,6 @@ js compatibility: 498/699 (71.24%)
154153
| js/test-declarations/test_declarations.js | 💥💥 | 75.81% |
155154
| js/trailing-comma/dynamic-import.js | 💥💥💥 | 0.00% |
156155
| js/trailing-comma/jsx.js | 💥💥💥 | 0.00% |
157-
| js/try/catch.js | 💥 | 52.63% |
158-
| js/try/try.js | 💥 | 50.00% |
159156
| js/unicode/nbsp-jsx.js | 💥 | 22.22% |
160157
| js/yield/jsx-without-parenthesis.js | 💥 | 50.00% |
161158
| js/yield/jsx.js | 💥 | 50.00% |

0 commit comments

Comments
 (0)