Skip to content

Commit 79dd4be

Browse files
DunqingDunqing
committed
feat(formatter): correct comments printing for ConditionalExpression
1 parent 8f566e6 commit 79dd4be

File tree

7 files changed

+335
-21
lines changed

7 files changed

+335
-21
lines changed

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

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -353,16 +353,6 @@ impl<'a> Comments<'a> {
353353
return &[];
354354
}
355355

356-
// No need to print trailing comments for the most right side for `BinaryExpression` and `LogicalExpression`,
357-
// instead, print trailing comments for expression itself.
358-
if matches!(
359-
enclosing_node,
360-
SiblingNode::BinaryExpression(_) | SiblingNode::LogicalExpression(_)
361-
) && matches!(following_node, Some(SiblingNode::ExpressionStatement(_)))
362-
{
363-
return &[];
364-
}
365-
366356
let comments = self.unprinted_comments();
367357
if comments.is_empty() {
368358
return &[];

‎crates/oxc_formatter/src/utils/conditional.rs

Lines changed: 76 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,9 @@ use oxc_span::{GetSpan, Span};
33

44
use crate::{
55
Format, FormatResult, FormatWrite,
6-
formatter::{Formatter, prelude::*},
6+
formatter::{Formatter, prelude::*, trivia::FormatTrailingComments},
77
generated::ast_nodes::{AstNode, AstNodes},
8+
utils::expression::FormatExpressionWithoutTrailingComments,
89
write,
910
};
1011

@@ -112,6 +113,61 @@ impl ConditionalLayout {
112113
}
113114
}
114115

116+
fn format_trailing_comments<'a>(
117+
mut start: u32,
118+
end: u32,
119+
operator: u8,
120+
f: &mut Formatter<'_, 'a>,
121+
) -> FormatResult<()> {
122+
let mut get_comments = |f: &mut Formatter<'_, 'a>| -> &'a [Comment] {
123+
let comments = f.context().comments().unprinted_comments();
124+
if comments.is_empty() {
125+
return &[];
126+
}
127+
128+
let source_text = f.context().source_text();
129+
let mut index_before_operator = None;
130+
for (index, comment) in comments.iter().enumerate() {
131+
// This comment is after the `end` position, so we stop here and return the comments before this comment
132+
if comment.span.end > end {
133+
return &comments[..index_before_operator.unwrap_or(index)];
134+
}
135+
136+
// `a /* c1 */ /* c2 */ ? b : c`
137+
// ^ ^ ^
138+
// | | |
139+
// | | |
140+
// these are the gaps between comments
141+
let gap_str = &source_text.as_bytes()[start as usize..comment.span.start as usize];
142+
143+
// If this comment is in a new line, we stop here and return the comments before this comment
144+
if gap_str.contains(&b'\n') {
145+
return &comments[..index];
146+
}
147+
// If this comment is a line comment, then it is a end of line comment, so we stop here and return the comments with this comment
148+
else if comment.is_line() {
149+
return &comments[..=index];
150+
}
151+
// Store the index of the comment before the operator, if no line comment or no new line is found, then return all comments before operator
152+
else if gap_str.contains(&operator) {
153+
index_before_operator = Some(index + 1);
154+
}
155+
156+
// Update the start position for the next iteration
157+
start = comment.span.end;
158+
}
159+
160+
&comments[..index_before_operator.unwrap_or(comments.len())]
161+
};
162+
163+
let comments = get_comments(f);
164+
if !comments.is_empty() {
165+
FormatTrailingComments::Comments(comments).fmt(f)?;
166+
}
167+
168+
Ok(())
169+
}
170+
115171
impl<'a> ConditionalLike<'a, '_> {
116172
/// Determines the layout of this conditional based on its parent
117173
fn layout(&self) -> ConditionalLayout {
@@ -303,7 +359,13 @@ impl<'a> ConditionalLike<'a, '_> {
303359
) -> FormatResult<()> {
304360
let format_inner = format_with(|f| match self {
305361
Self::ConditionalExpression(conditional) => {
306-
write!(f, [conditional.test()])
362+
write!(f, FormatExpressionWithoutTrailingComments(conditional.test()))?;
363+
format_trailing_comments(
364+
conditional.test.span().end,
365+
conditional.consequent.span().start,
366+
b'?',
367+
f,
368+
)
307369
}
308370
Self::TSConditionalType(conditional) => {
309371
write!(
@@ -346,6 +408,16 @@ impl<'a> ConditionalLike<'a, '_> {
346408
}
347409
};
348410

411+
let format_consequent = format_once(|f| {
412+
write!(f, FormatExpressionWithoutTrailingComments(conditional.consequent()))?;
413+
format_trailing_comments(
414+
conditional.consequent.span().end,
415+
conditional.alternate.span().start,
416+
b':',
417+
f,
418+
)
419+
});
420+
349421
if is_consequent_nested && !layout.is_jsx_chain() {
350422
// Add parentheses around the consequent if it is a conditional expression and fits on the same line
351423
// so that it's easier to identify the parts that belong to a conditional expression.
@@ -354,12 +426,12 @@ impl<'a> ConditionalLike<'a, '_> {
354426
f,
355427
[
356428
if_group_fits_on_line(&text("(")),
357-
conditional.consequent(),
429+
format_consequent,
358430
if_group_fits_on_line(&text(")"))
359431
]
360432
)
361433
} else {
362-
write!(f, [conditional.consequent()])
434+
write!(f, format_consequent)
363435
}
364436
}
365437
Self::TSConditionalType(conditional) => {

‎crates/oxc_formatter/src/utils/expression.rs

Lines changed: 245 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,245 @@
1+
use oxc_ast::ast::Expression;
2+
3+
use crate::{
4+
Format,
5+
formatter::{FormatResult, Formatter, prelude::*},
6+
generated::ast_nodes::{AstNode, AstNodes},
7+
parentheses::NeedsParentheses,
8+
write,
9+
write::FormatWrite,
10+
};
11+
12+
pub struct FormatExpressionWithoutTrailingComments<'a, 'b>(pub &'b AstNode<'a, Expression<'a>>);
13+
14+
impl<'a> Format<'a> for FormatExpressionWithoutTrailingComments<'a, '_> {
15+
fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
16+
let needs_parentheses = self.0.needs_parentheses(f);
17+
let print_left_paren =
18+
|f: &mut Formatter<'_, 'a>| write!(f, needs_parentheses.then_some("("));
19+
20+
match self.0.as_ast_nodes() {
21+
AstNodes::BooleanLiteral(n) => {
22+
n.format_leading_comments(f)?;
23+
print_left_paren(f)?;
24+
n.write(f)
25+
}
26+
AstNodes::NullLiteral(n) => {
27+
n.format_leading_comments(f)?;
28+
print_left_paren(f)?;
29+
n.write(f)
30+
}
31+
AstNodes::NumericLiteral(n) => {
32+
n.format_leading_comments(f)?;
33+
print_left_paren(f)?;
34+
n.write(f)
35+
}
36+
AstNodes::BigIntLiteral(n) => {
37+
n.format_leading_comments(f)?;
38+
print_left_paren(f)?;
39+
n.write(f)
40+
}
41+
AstNodes::RegExpLiteral(n) => {
42+
n.format_leading_comments(f)?;
43+
print_left_paren(f)?;
44+
n.write(f)
45+
}
46+
AstNodes::StringLiteral(n) => {
47+
n.format_leading_comments(f)?;
48+
print_left_paren(f)?;
49+
n.write(f)
50+
}
51+
AstNodes::TemplateLiteral(n) => {
52+
n.format_leading_comments(f)?;
53+
print_left_paren(f)?;
54+
n.write(f)
55+
}
56+
AstNodes::IdentifierReference(n) => {
57+
n.format_leading_comments(f)?;
58+
print_left_paren(f)?;
59+
n.write(f)
60+
}
61+
AstNodes::MetaProperty(n) => {
62+
n.format_leading_comments(f)?;
63+
print_left_paren(f)?;
64+
n.write(f)
65+
}
66+
AstNodes::Super(n) => {
67+
n.format_leading_comments(f)?;
68+
print_left_paren(f)?;
69+
n.write(f)
70+
}
71+
AstNodes::ArrayExpression(n) => {
72+
n.format_leading_comments(f)?;
73+
print_left_paren(f)?;
74+
n.write(f)
75+
}
76+
AstNodes::ArrowFunctionExpression(n) => {
77+
n.format_leading_comments(f)?;
78+
print_left_paren(f)?;
79+
n.write(f)
80+
}
81+
AstNodes::AssignmentExpression(n) => {
82+
n.format_leading_comments(f)?;
83+
print_left_paren(f)?;
84+
n.write(f)
85+
}
86+
AstNodes::AwaitExpression(n) => {
87+
n.format_leading_comments(f)?;
88+
print_left_paren(f)?;
89+
n.write(f)
90+
}
91+
AstNodes::BinaryExpression(n) => {
92+
n.format_leading_comments(f)?;
93+
print_left_paren(f)?;
94+
n.write(f)
95+
}
96+
AstNodes::CallExpression(n) => {
97+
n.format_leading_comments(f)?;
98+
print_left_paren(f)?;
99+
n.write(f)
100+
}
101+
AstNodes::ChainExpression(n) => {
102+
n.format_leading_comments(f)?;
103+
print_left_paren(f)?;
104+
n.write(f)
105+
}
106+
AstNodes::Class(n) => {
107+
n.format_leading_comments(f)?;
108+
print_left_paren(f)?;
109+
n.write(f)
110+
}
111+
AstNodes::ConditionalExpression(n) => {
112+
n.format_leading_comments(f)?;
113+
print_left_paren(f)?;
114+
n.write(f)
115+
}
116+
AstNodes::Function(n) => {
117+
n.format_leading_comments(f)?;
118+
print_left_paren(f)?;
119+
n.write(f)
120+
}
121+
AstNodes::ImportExpression(n) => {
122+
n.format_leading_comments(f)?;
123+
print_left_paren(f)?;
124+
n.write(f)
125+
}
126+
AstNodes::LogicalExpression(n) => {
127+
n.format_leading_comments(f)?;
128+
print_left_paren(f)?;
129+
n.write(f)
130+
}
131+
AstNodes::NewExpression(n) => {
132+
n.format_leading_comments(f)?;
133+
print_left_paren(f)?;
134+
n.write(f)
135+
}
136+
AstNodes::ObjectExpression(n) => {
137+
n.format_leading_comments(f)?;
138+
print_left_paren(f)?;
139+
n.write(f)
140+
}
141+
AstNodes::ParenthesizedExpression(n) => {
142+
n.format_leading_comments(f)?;
143+
print_left_paren(f)?;
144+
n.write(f)
145+
}
146+
AstNodes::SequenceExpression(n) => {
147+
n.format_leading_comments(f)?;
148+
print_left_paren(f)?;
149+
n.write(f)
150+
}
151+
AstNodes::TaggedTemplateExpression(n) => {
152+
n.format_leading_comments(f)?;
153+
print_left_paren(f)?;
154+
n.write(f)
155+
}
156+
AstNodes::ThisExpression(n) => {
157+
n.format_leading_comments(f)?;
158+
print_left_paren(f)?;
159+
n.write(f)
160+
}
161+
AstNodes::UnaryExpression(n) => {
162+
n.format_leading_comments(f)?;
163+
print_left_paren(f)?;
164+
n.write(f)
165+
}
166+
AstNodes::UpdateExpression(n) => {
167+
n.format_leading_comments(f)?;
168+
print_left_paren(f)?;
169+
n.write(f)
170+
}
171+
AstNodes::YieldExpression(n) => {
172+
n.format_leading_comments(f)?;
173+
print_left_paren(f)?;
174+
n.write(f)
175+
}
176+
AstNodes::PrivateInExpression(n) => {
177+
n.format_leading_comments(f)?;
178+
print_left_paren(f)?;
179+
n.write(f)
180+
}
181+
AstNodes::JSXElement(n) => {
182+
n.format_leading_comments(f)?;
183+
print_left_paren(f)?;
184+
n.write(f)
185+
}
186+
AstNodes::JSXFragment(n) => {
187+
n.format_leading_comments(f)?;
188+
print_left_paren(f)?;
189+
n.write(f)
190+
}
191+
AstNodes::TSAsExpression(n) => {
192+
n.format_leading_comments(f)?;
193+
print_left_paren(f)?;
194+
n.write(f)
195+
}
196+
AstNodes::TSSatisfiesExpression(n) => {
197+
n.format_leading_comments(f)?;
198+
print_left_paren(f)?;
199+
n.write(f)
200+
}
201+
AstNodes::TSTypeAssertion(n) => {
202+
n.format_leading_comments(f)?;
203+
print_left_paren(f)?;
204+
n.write(f)
205+
}
206+
AstNodes::TSNonNullExpression(n) => {
207+
n.format_leading_comments(f)?;
208+
print_left_paren(f)?;
209+
n.write(f)
210+
}
211+
AstNodes::TSInstantiationExpression(n) => {
212+
n.format_leading_comments(f)?;
213+
print_left_paren(f)?;
214+
n.write(f)
215+
}
216+
AstNodes::V8IntrinsicExpression(n) => {
217+
n.format_leading_comments(f)?;
218+
print_left_paren(f)?;
219+
n.write(f)
220+
}
221+
AstNodes::StaticMemberExpression(n) => {
222+
n.format_leading_comments(f)?;
223+
print_left_paren(f)?;
224+
n.write(f)
225+
}
226+
AstNodes::ComputedMemberExpression(n) => {
227+
n.format_leading_comments(f)?;
228+
print_left_paren(f)?;
229+
n.write(f)
230+
}
231+
AstNodes::PrivateFieldExpression(n) => {
232+
n.format_leading_comments(f)?;
233+
print_left_paren(f)?;
234+
n.write(f)
235+
}
236+
_ => unreachable!(),
237+
}?;
238+
239+
if needs_parentheses {
240+
write!(f, [")"])?;
241+
}
242+
243+
Ok(())
244+
}
245+
}

‎crates/oxc_formatter/src/utils/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
pub mod assignment_like;
22
pub mod call_expression;
33
pub mod conditional;
4+
pub mod expression;
45
pub mod member_chain;
56

67
use oxc_allocator::Address;

0 commit comments

Comments
 (0)