Skip to content

Commit c9ccdbe

Browse files
DunqingDunqing
committed
feat(formatter): finish todo part in binary like expression (#11914)
1 parent b2cb2fa commit c9ccdbe

File tree

3 files changed

+87
-70
lines changed

3 files changed

+87
-70
lines changed

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

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,39 @@ impl<'a> Comments<'a> {
3535
&self.comments[self.printed_count..]
3636
}
3737

38+
/// Returns the comments that after the given `start` position, even if they were already printed.
39+
pub fn comments_after(&self, start: u32) -> &'a [Comment] {
40+
let mut index = self.printed_count;
41+
42+
// No comments or all are printed already
43+
if index == self.comments.len() {
44+
return &[];
45+
}
46+
47+
let forward = self.comments[index].span.end < start;
48+
// Skip comments that before start
49+
if forward {
50+
while index < self.comments.len() - 1 {
51+
if self.comments[index + 1].span.end <= start {
52+
index += 1;
53+
continue;
54+
}
55+
break;
56+
}
57+
} else {
58+
// Find comments that after start
59+
while index > 0 {
60+
if self.comments[index - 1].span.start > start {
61+
index -= 1;
62+
continue;
63+
}
64+
break;
65+
}
66+
}
67+
68+
&self.comments[index..]
69+
}
70+
3871
#[inline]
3972
pub fn filter_comments_in_span(&self, span: Span) -> impl Iterator<Item = &Comment> {
4073
self.comments
@@ -132,7 +165,8 @@ impl<'a> Comments<'a> {
132165
}
133166

134167
pub fn has_trailing_comments(&self, current_end: u32, following_start: u32) -> bool {
135-
let comments = self.unprinted_comments();
168+
let comments = &self.comments_after(current_end);
169+
136170
let mut comment_index = 0;
137171
while let Some(comment) = comments.get(comment_index) {
138172
// Check if the comment is before the following node's span
@@ -167,6 +201,22 @@ impl<'a> Comments<'a> {
167201
false
168202
}
169203

204+
pub fn has_trailing_line_comments(&self, current_end: u32, following_start: u32) -> bool {
205+
for comment in self.comments_after(current_end) {
206+
if comment.span.start > following_start {
207+
return false;
208+
}
209+
210+
if is_own_line_comment(comment, self.source_text) {
211+
return false;
212+
} else if is_end_of_line_comment(comment, self.source_text) {
213+
return true;
214+
}
215+
}
216+
217+
false
218+
}
219+
170220
/// Leading comments are between the `previous_span` and the `current_span`.
171221
/// Trailing comments are between the `current_span` and the `following_span`.
172222
#[inline]

‎crates/oxc_formatter/src/write/binary_like_expression.rs‎

Lines changed: 28 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -284,13 +284,6 @@ enum BinaryLeftOrRightSide<'a, 'b> {
284284
parent: BinaryLikeExpression<'a, 'b>,
285285
/// Is the parent the condition of a `if` / `while` / `do-while` / `for` statement?
286286
inside_condition: bool,
287-
288-
/// Indicates if the comments of the parent should be printed or not.
289-
/// Must be true if `parent` isn't the root `BinaryLikeExpression` for which `format` is called.
290-
print_parent_comments: bool,
291-
292-
/// Indicates if the parent has the same kind as the current binary expression.
293-
parent_has_same_kind: bool,
294287
},
295288
}
296289

@@ -301,8 +294,6 @@ impl<'a> Format<'a> for BinaryLeftOrRightSide<'a, '_> {
301294
Self::Right {
302295
parent: binary_like_expression,
303296
inside_condition: inside_parenthesis,
304-
print_parent_comments,
305-
parent_has_same_kind,
306297
} => {
307298
// // It's only possible to suppress the formatting of the whole binary expression formatting OR
308299
// // the formatting of the right hand side value but not of a nested binary expression.
@@ -319,52 +310,37 @@ impl<'a> Format<'a> for BinaryLeftOrRightSide<'a, '_> {
319310
write!(f, [soft_line_break_or_space()])?;
320311
}
321312

322-
write!(f, right)?;
323-
324-
Ok(())
313+
write!(f, right)
325314
});
326315

316+
let parent = binary_like_expression.parent();
317+
327318
// Doesn't match prettier that only distinguishes between logical and binary
328-
let left_has_same_kind = is_same_binary_expression_kind(
329-
binary_like_expression,
330-
binary_like_expression.left(),
331-
);
332-
333-
let right_has_same_kind =
334-
is_same_binary_expression_kind(binary_like_expression, right);
335-
336-
// let should_break = f
337-
// .context()
338-
// .comments()
339-
// .trailing_comments(binary_like_expression.left()?.syntax())
340-
// .iter()
341-
// .any(|comment| comment.kind().is_line());
342-
let should_break = false;
343-
344-
let should_group = !(*parent_has_same_kind
345-
|| left_has_same_kind
346-
|| right_has_same_kind
347-
|| (*inside_parenthesis
348-
&& matches!(
319+
let should_group =
320+
!(is_same_binary_expression_kind(binary_like_expression, parent)
321+
|| is_same_binary_expression_kind(
349322
binary_like_expression,
350-
BinaryLikeExpression::LogicalExpression(_)
351-
)));
352-
353-
// if *print_parent_comments {
354-
// write!(f, binary_like_expression)?;
355-
// }
323+
binary_like_expression.left().as_ast_nodes(),
324+
)
325+
|| is_same_binary_expression_kind(
326+
binary_like_expression,
327+
right.as_ast_nodes(),
328+
)
329+
|| (*inside_parenthesis
330+
&& matches!(
331+
binary_like_expression,
332+
BinaryLikeExpression::LogicalExpression(_)
333+
)));
356334

357335
if should_group {
358-
write!(f, [group(&operator_and_right_expression).should_expand(should_break)])?;
336+
let should_break = f.context().comments().has_trailing_line_comments(
337+
binary_like_expression.left().span().end,
338+
right.span().start,
339+
);
340+
write!(f, [group(&operator_and_right_expression).should_expand(should_break)])
359341
} else {
360-
write!(f, [operator_and_right_expression])?;
342+
write!(f, [operator_and_right_expression])
361343
}
362-
363-
// if *print_parent_comments {
364-
// write!(f, [format_trailing_comments(binary_like_expression.syntax())])?;
365-
// }
366-
367-
Ok(())
368344
}
369345
}
370346
}
@@ -403,7 +379,6 @@ fn split_into_left_and_right_sides<'a, 'b>(
403379
fn split_into_left_and_right_sides_inner<'a, 'b>(
404380
binary: BinaryLikeExpression<'a, 'b>,
405381
inside_condition: bool,
406-
parent_has_same_kind: bool,
407382
items: &mut Vec<BinaryLeftOrRightSide<'a, 'b>>,
408383
) {
409384
let left = binary.left();
@@ -416,28 +391,20 @@ fn split_into_left_and_right_sides<'a, 'b>(
416391
// SAFETY: `left` is guaranteed to be a valid binary like expression in `can_flatten()`.
417392
BinaryLikeExpression::try_from(left).unwrap(),
418393
inside_condition,
419-
is_same_binary_expression_kind(&binary, left),
420394
items,
421395
);
422396
} else {
423397
items.push(BinaryLeftOrRightSide::Left { parent: binary });
424398
}
425399

426-
items.push(BinaryLeftOrRightSide::Right {
427-
parent: binary,
428-
inside_condition,
429-
// TODO:
430-
// print_parent_comments: expression.syntax() != root.syntax(),
431-
print_parent_comments: false,
432-
parent_has_same_kind,
433-
});
400+
items.push(BinaryLeftOrRightSide::Right { parent: binary, inside_condition });
434401
}
435402

436403
// Stores the left and right parts of the binary expression in sequence (rather than nested as they
437404
// appear in the tree).
438405
let mut items = Vec::new();
439406

440-
split_into_left_and_right_sides_inner(root, inside_condition, false, &mut items);
407+
split_into_left_and_right_sides_inner(root, inside_condition, &mut items);
441408

442409
items
443410
}
@@ -464,14 +431,14 @@ fn should_indent_if_parent_inlines(parent: &AstNodes<'_>) -> bool {
464431

465432
fn is_same_binary_expression_kind(
466433
binary: &BinaryLikeExpression<'_, '_>,
467-
other: &Expression<'_>,
434+
other: &AstNodes<'_>,
468435
) -> bool {
469436
match binary {
470437
BinaryLikeExpression::LogicalExpression(_) => {
471-
matches!(other, Expression::LogicalExpression(_))
438+
matches!(other, AstNodes::LogicalExpression(_))
472439
}
473440
BinaryLikeExpression::BinaryExpression(_) => {
474-
matches!(other, Expression::BinaryExpression(_))
441+
matches!(other, AstNodes::BinaryExpression(_))
475442
}
476443
}
477444
}

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ js compatibility: 323/699 (46.21%)
3737
| js/assignment/issue-15534.js | 💥 | 30.77% |
3838
| js/assignment/issue-1966.js | 💥 | 30.77% |
3939
| js/assignment/issue-2482-1.js | 💥 | 33.33% |
40-
| js/assignment/issue-2482-2.js | 💥 | 53.33% |
40+
| js/assignment/issue-2482-2.js | 💥 | 50.00% |
4141
| js/assignment/issue-3819.js | 💥 | 0.00% |
4242
| js/assignment/issue-7091.js | 💥 | 0.00% |
4343
| js/assignment/issue-7572.js | 💥 | 72.73% |
@@ -93,8 +93,8 @@ js compatibility: 323/699 (46.21%)
9393
| js/classes-private-fields/with_comments.js | 💥💥 | 61.54% |
9494
| js/comments/15661.js | 💥💥 | 68.35% |
9595
| js/comments/16398.js | 💥💥 | 80.00% |
96-
| js/comments/binary-expressions-block-comments.js | 💥💥 | 47.06% |
97-
| js/comments/binary-expressions-single-comments.js | 💥💥 | 29.41% |
96+
| js/comments/binary-expressions-block-comments.js | 💥💥 | 38.64% |
97+
| js/comments/binary-expressions-single-comments.js | 💥💥 | 25.64% |
9898
| js/comments/blank.js | 💥💥 | 95.24% |
9999
| js/comments/call_comment.js | 💥💥 | 90.91% |
100100
| js/comments/dangling.js | 💥💥 | 59.26% |
@@ -116,7 +116,7 @@ js compatibility: 323/699 (46.21%)
116116
| js/comments/multi-comments-2.js | 💥💥 | 90.91% |
117117
| js/comments/multi-comments-on-same-line.js | 💥✨ | 42.78% |
118118
| js/comments/multi-comments.js | 💥✨ | 36.84% |
119-
| js/comments/return-statement.js | 💥💥 | 52.49% |
119+
| js/comments/return-statement.js | 💥💥 | 52.32% |
120120
| js/comments/tagged-template-literal.js | 💥💥 | 69.23% |
121121
| js/comments/template-literal.js | 💥💥 | 47.83% |
122122
| js/comments/trailing-jsdocs.js | 💥💥 | 91.30% |
@@ -149,7 +149,7 @@ js compatibility: 323/699 (46.21%)
149149
| js/conditional/no-confusing-arrow.js | 💥💥 | 66.67% |
150150
| js/conditional/postfix-ternary-regressions.js | 💥💥 | 34.71% |
151151
| js/decorators/classes.js | 💥 | 47.06% |
152-
| js/decorators/comments.js | 💥 | 52.46% |
152+
| js/decorators/comments.js | 💥 | 71.19% |
153153
| js/decorators/member-expression.js | 💥 | 80.60% |
154154
| js/decorators/mobx.js | 💥 | 70.45% |
155155
| js/decorators/multiline.js | 💥 | 44.44% |
@@ -215,7 +215,7 @@ js compatibility: 323/699 (46.21%)
215215
| js/last-argument-expansion/assignment-pattern.js | 💥 | 28.57% |
216216
| js/last-argument-expansion/break-parent.js | 💥 | 57.14% |
217217
| js/last-argument-expansion/dangling-comment-in-arrow-function.js | 💥 | 0.00% |
218-
| js/last-argument-expansion/edge_case.js | 💥 | 76.34% |
218+
| js/last-argument-expansion/edge_case.js | 💥 | 41.74% |
219219
| js/last-argument-expansion/empty-object.js | 💥 | 82.76% |
220220
| js/last-argument-expansion/function-expression-issue-2239.js | 💥 | 0.00% |
221221
| js/last-argument-expansion/function-expression.js | 💥 | 26.32% |
@@ -224,7 +224,7 @@ js compatibility: 323/699 (46.21%)
224224
| js/last-argument-expansion/jsx.js | 💥 | 16.67% |
225225
| js/last-argument-expansion/object.js | 💥 | 65.00% |
226226
| js/last-argument-expansion/overflow.js | 💥 | 72.55% |
227-
| js/line-suffix-boundary/boundary.js | 💥 | 43.48% |
227+
| js/line-suffix-boundary/boundary.js | 💥 | 42.55% |
228228
| js/logical-assignment/logical-assignment.js | 💥 | 81.48% |
229229
| js/logical_expressions/issue-7024.js | 💥 | 0.00% |
230230
| js/member/conditional.js | 💥 | 0.00% |
@@ -274,7 +274,7 @@ js compatibility: 323/699 (46.21%)
274274
| js/preserve-line/member-chain.js | 💥 | 19.20% |
275275
| js/preserve-line/parameter-list.js | 💥 | 79.31% |
276276
| js/quote-props/classes.js | 💥💥✨✨ | 47.06% |
277-
| js/quote-props/objects.js | 💥💥✨✨ | 45.10% |
277+
| js/quote-props/objects.js | 💥💥💥💥 | 71.36% |
278278
| js/quote-props/with_numbers.js | 💥💥✨✨ | 46.43% |
279279
| js/quotes/objects.js | 💥💥 | 80.00% |
280280
| js/require/require.js | 💥 | 90.67% |

0 commit comments

Comments
 (0)