Skip to content

Commit c7b1ddb

Browse files
committed
fix(formatter): correct comments checking for some places
1 parent 54c4ed1 commit c7b1ddb

File tree

12 files changed

+114
-72
lines changed

12 files changed

+114
-72
lines changed

crates/oxc_formatter/src/formatter/comments.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ impl<'a> Comments<'a> {
236236

237237
/// Checks if there are any comments between the given positions.
238238
pub fn has_comment_in_range(&self, start: u32, end: u32) -> bool {
239-
self.comments_before_iter(end).any(|comment| comment.span.end >= start)
239+
self.comments_before_iter(end).any(|comment| comment.span.end > start)
240240
}
241241

242242
/// Checks if there are any comments within the given span.
@@ -253,10 +253,8 @@ impl<'a> Comments<'a> {
253253

254254
/// Checks if there are any leading own-line comments before the given position.
255255
pub fn has_leading_own_line_comment(&self, start: u32) -> bool {
256-
self.comments_before_iter(start).any(|comment| {
257-
self.source_text.is_own_line_comment(comment)
258-
|| self.source_text.lines_after(comment.span.end) > 0
259-
})
256+
self.comments_before_iter(start)
257+
.any(|comment| self.source_text.lines_after(comment.span.end) > 0)
260258
}
261259

262260
/// Checks if there are leading or trailing comments around `current_span`.

crates/oxc_formatter/src/formatter/source_text.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ impl<'a> SourceText<'a> {
146146

147147
// Should skip the leading comments of the node.
148148
if let Some(comment) = comments.first()
149-
&& comment.span.end < start
149+
&& comment.span.end <= start
150150
{
151151
start = comment.span.start;
152152
}

crates/oxc_formatter/src/generated/ast_nodes.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4189,7 +4189,7 @@ impl<'a> AstNode<'a, ComputedMemberExpression<'a>> {
41894189

41904190
#[inline]
41914191
pub fn expression(&self) -> &AstNode<'a, Expression<'a>> {
4192-
let following_node = self.following_node;
4192+
let following_node = None;
41934193
self.allocator.alloc(AstNode {
41944194
inner: &self.inner.expression,
41954195
allocator: self.allocator,

crates/oxc_formatter/src/utils/assignment_like.rs

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -314,19 +314,19 @@ impl<'a> AssignmentLike<'a, '_> {
314314
}
315315

316316
let right_expression = self.get_right_expression();
317+
if let Some(expr) = right_expression {
318+
if let Some(layout) = self.chain_formatting_layout(expr) {
319+
return layout;
320+
}
317321

318-
if let Some(layout) = right_expression.and_then(|expr| self.chain_formatting_layout(expr)) {
319-
return layout;
320-
}
321-
322-
if let Some(Expression::CallExpression(call_expression)) =
323-
&right_expression.map(AsRef::as_ref)
324-
&& call_expression
325-
.callee
326-
.get_identifier_reference()
327-
.is_some_and(|ident| ident.name == "require")
328-
{
329-
return AssignmentLikeLayout::NeverBreakAfterOperator;
322+
if let Expression::CallExpression(call_expression) = expr.as_ref()
323+
&& call_expression
324+
.callee
325+
.get_identifier_reference()
326+
.is_some_and(|ident| ident.name == "require")
327+
{
328+
return AssignmentLikeLayout::NeverBreakAfterOperator;
329+
}
330330
}
331331

332332
if self.should_break_after_operator(right_expression, f) {
@@ -359,10 +359,8 @@ impl<'a> AssignmentLike<'a, '_> {
359359
return AssignmentLikeLayout::BreakAfterOperator;
360360
}
361361

362-
let is_poorly_breakable = match &right_expression {
363-
Some(expression) => is_poorly_breakable_member_or_call_chain(expression, f),
364-
None => false,
365-
};
362+
let is_poorly_breakable =
363+
right_expression.is_some_and(|expr| is_poorly_breakable_member_or_call_chain(expr, f));
366364

367365
if is_poorly_breakable {
368366
return AssignmentLikeLayout::BreakAfterOperator;
@@ -581,17 +579,19 @@ fn should_break_after_operator<'a>(
581579
) -> bool {
582580
let is_jsx = matches!(right.as_ref(), Expression::JSXElement(_) | Expression::JSXFragment(_));
583581

582+
if is_jsx {
583+
return false;
584+
}
585+
586+
let comments = f.comments();
584587
let source_text = f.source_text();
585-
for comment in f.comments().comments_before(right.span().start) {
586-
if !is_jsx
587-
&& (source_text.lines_after(comment.span.end) > 0
588-
|| source_text.is_own_line_comment(comment))
589-
{
588+
for comment in comments.comments_before(right.span().start) {
589+
if source_text.lines_after(comment.span.end) > 0 {
590590
return true;
591591
}
592592

593593
// Needs to wrap a parenthesis for the node, so it won't break.
594-
if f.comments().is_type_cast_comment(comment) {
594+
if comments.is_type_cast_comment(comment) {
595595
return false;
596596
}
597597
}

crates/oxc_formatter/src/write/member_expression.rs

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,18 @@
11
use std::ops::Deref;
22

33
use oxc_ast::ast::*;
4+
use oxc_span::GetSpan;
45

56
use crate::{
67
JsLabels, format_args,
78
formatter::{
8-
Buffer, Format, FormatResult, Formatter, prelude::*, trivia::format_dangling_comments,
9+
Buffer, Format, FormatResult, Formatter,
10+
buffer::RemoveSoftLinesBuffer,
11+
prelude::*,
12+
trivia::{
13+
DanglingIndentMode, FormatDanglingComments, FormatLeadingComments,
14+
FormatTrailingComments, format_dangling_comments,
15+
},
916
},
1017
generated::ast_nodes::{AstNode, AstNodes},
1118
options::Expand,
@@ -30,7 +37,14 @@ impl<'a> FormatWrite<'a> for AstNode<'a, StaticMemberExpression<'a>> {
3037
recording.stop().has_label(LabelId::of(JsLabels::MemberChain))
3138
};
3239

33-
match layout(self, is_member_chain) {
40+
let comments = f.context().comments().block_comments_before(self.property.span.start);
41+
if !comments.is_empty() {
42+
write!(f, [space()])?;
43+
f.context_mut().comments_mut().increase_printed_count_by(comments.len());
44+
f.join_with(space()).entries(comments.iter()).finish()?;
45+
}
46+
47+
match layout(self, is_member_chain, f) {
3448
StaticMemberLayout::NoBreak => {
3549
let format_no_break =
3650
format_with(|f| write!(f, [operator_token(self.optional()), self.property()]));
@@ -46,6 +60,17 @@ impl<'a> FormatWrite<'a> for AstNode<'a, StaticMemberExpression<'a>> {
4660
f,
4761
[group(&indent(&format_args!(
4862
soft_line_break(),
63+
&format_once(|f| {
64+
let comments =
65+
f.context().comments().comments_before(self.property.span.start);
66+
if !comments.is_empty() {
67+
write!(
68+
f,
69+
[FormatLeadingComments::Comments(comments), soft_line_break()]
70+
)?;
71+
}
72+
Ok(())
73+
}),
4974
operator_token(self.optional()),
5075
self.property(),
5176
)))]
@@ -71,7 +96,15 @@ fn operator_token(optional: bool) -> &'static str {
7196
fn layout<'a>(
7297
node: &AstNode<'a, StaticMemberExpression<'a>>,
7398
is_member_chain: bool,
99+
f: &Formatter<'_, 'a>,
74100
) -> StaticMemberLayout {
101+
if f.comments()
102+
.comments_before_iter(node.property.span.start)
103+
.any(|c| f.source_text().is_own_line_comment(c))
104+
{
105+
return StaticMemberLayout::BreakAfterObject;
106+
}
107+
75108
// `a.b.c!` and `a.b?.c`
76109
// `TSNonNullExpression` is a wrapper node for `!`, and `ChainExpression` is a wrapper node for `?.`,
77110
// so we need to skip them to find the real parent node.

crates/oxc_formatter/src/write/mod.rs

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ use self::{
8383
object_like::ObjectLike,
8484
object_pattern_like::ObjectPatternLike,
8585
parameters::{ParameterLayout, ParameterList},
86+
return_or_throw_statement::FormatAdjacentArgument,
8687
semicolon::OptionalSemicolon,
8788
type_parameters::{FormatTSTypeParameters, FormatTSTypeParametersOptions},
8889
utils::{
@@ -659,7 +660,7 @@ impl<'a> FormatWrite<'a> for AstNode<'a, ForStatement<'a>> {
659660

660661
impl<'a> FormatWrite<'a> for AstNode<'a, ForInStatement<'a>> {
661662
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
662-
let comments = f.context().comments().own_line_comments_before(self.body.span().start);
663+
let comments = f.context().comments().own_line_comments_before(self.right.span().start);
663664
write!(
664665
f,
665666
[
@@ -683,7 +684,7 @@ impl<'a> FormatWrite<'a> for AstNode<'a, ForInStatement<'a>> {
683684

684685
impl<'a> FormatWrite<'a> for AstNode<'a, ForOfStatement<'a>> {
685686
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
686-
let comments = f.context().comments().own_line_comments_before(self.body.span().start);
687+
let comments = f.context().comments().own_line_comments_before(self.right.span().start);
687688

688689
let r#await = self.r#await();
689690
let left = self.left();
@@ -857,12 +858,17 @@ impl<'a> FormatWrite<'a> for AstNode<'a, BindingPattern<'a>> {
857858

858859
impl<'a> FormatWrite<'a> for AstNode<'a, AssignmentPattern<'a>> {
859860
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
860-
let comments = f.context().comments().own_line_comments_before(self.right.span().start);
861+
let mut left = self.left().memoized();
862+
left.inspect(f)?;
861863
write!(
862864
f,
863865
[
864-
FormatLeadingComments::Comments(comments),
865-
self.left(),
866+
format_once(|f| {
867+
let comments =
868+
f.context().comments().own_line_comments_before(self.right.span().start);
869+
FormatLeadingComments::Comments(comments).fmt(f)
870+
}),
871+
left,
866872
space(),
867873
"=",
868874
space(),
@@ -938,12 +944,9 @@ impl<'a> FormatWrite<'a, FormatJsArrowFunctionExpressionOptions>
938944

939945
impl<'a> FormatWrite<'a> for AstNode<'a, YieldExpression<'a>> {
940946
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
941-
write!(f, "yield")?;
942-
if self.delegate() {
943-
write!(f, "*")?;
944-
}
947+
write!(f, ["yield", self.delegate().then_some("*")])?;
945948
if let Some(argument) = &self.argument() {
946-
write!(f, [space(), argument])?;
949+
write!(f, [space(), FormatAdjacentArgument(argument)])?;
947950
}
948951
Ok(())
949952
}

crates/oxc_formatter/src/write/parameters.rs

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ impl<'a> FormatWrite<'a> for AstNode<'a, FormalParameters<'a>> {
4343
let has_any_decorated_parameter =
4444
self.items.iter().any(|param| !param.decorators.is_empty());
4545

46-
let can_hug = should_hug_function_parameters(self, this_param, parentheses_not_needed, f)
47-
&& !has_any_decorated_parameter;
46+
let can_hug = !has_any_decorated_parameter
47+
&& should_hug_function_parameters(self, this_param, parentheses_not_needed, f);
4848

4949
let layout = if !self.has_parameter() && this_param.is_none() {
5050
ParameterLayout::NoParameters
@@ -77,13 +77,7 @@ impl<'a> FormatWrite<'a> for AstNode<'a, FormalParameters<'a>> {
7777
ParameterLayout::Default => {
7878
write!(
7979
f,
80-
soft_block_indent(&format_args!(
81-
&ParameterList::with_layout(self, this_param, layout),
82-
format_once(|f| {
83-
let comments = f.context().comments().comments_before(self.span.end);
84-
write!(f, [FormatTrailingComments::Comments(comments)])
85-
})
86-
))
80+
soft_block_indent(&&ParameterList::with_layout(self, this_param, layout))
8781
);
8882
}
8983
}
@@ -341,6 +335,11 @@ pub fn should_hug_function_parameters<'a>(
341335
Expression::Identifier(_) => true,
342336
_ => false,
343337
}
338+
&& !f
339+
.comments()
340+
.comments_in_range(assignment.left.span().end, assignment.right.span().start)
341+
.iter()
342+
.any(|c| f.source_text().is_own_line_comment(c))
344343
}
345344
BindingPatternKind::ArrayPattern(_) | BindingPatternKind::ObjectPattern(_) => true,
346345
BindingPatternKind::BindingIdentifier(_) => {

crates/oxc_formatter/src/write/return_or_throw_statement.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ impl<'a> Format<'a> for ReturnAndThrowStatement<'a, '_> {
6060
write!(f, self.keyword())?;
6161

6262
if let Some(argument) = self.argument() {
63-
write!(f, [space(), FormatReturnOrThrowArgument(argument)])?;
63+
write!(f, [space(), FormatAdjacentArgument(argument)])?;
6464
}
6565

6666
let dangling_comments = f.context().comments().comments_before(self.span().end);
@@ -84,9 +84,9 @@ impl<'a> Format<'a> for ReturnAndThrowStatement<'a, '_> {
8484
}
8585
}
8686

87-
pub struct FormatReturnOrThrowArgument<'a, 'b>(&'b AstNode<'a, Expression<'a>>);
87+
pub struct FormatAdjacentArgument<'a, 'b>(pub &'b AstNode<'a, Expression<'a>>);
8888

89-
impl<'a> Format<'a> for FormatReturnOrThrowArgument<'a, '_> {
89+
impl<'a> Format<'a> for FormatAdjacentArgument<'a, '_> {
9090
fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
9191
let argument = self.0;
9292

crates/oxc_formatter/src/write/try_statement.rs

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@ use crate::{
99
Formatter,
1010
prelude::*,
1111
separated::FormatSeparatedIter,
12-
trivia::{FormatLeadingComments, FormatTrailingComments},
12+
trivia::{
13+
DanglingIndentMode, FormatDanglingComments, FormatLeadingComments,
14+
FormatTrailingComments,
15+
},
1316
},
1417
generated::ast_nodes::{AstNode, AstNodes},
1518
write,
@@ -40,15 +43,27 @@ impl<'a> FormatWrite<'a> for AstNode<'a, TryStatement<'a>> {
4043

4144
impl<'a> FormatWrite<'a> for AstNode<'a, CatchClause<'a>> {
4245
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);
46+
let comments = f.context().comments().comments_before(self.span.start);
47+
let has_line_comment = comments.iter().any(|comment| {
48+
comment.is_line()
49+
|| f.source_text().is_own_line_comment(comment)
50+
|| f.source_text().is_end_of_line_comment(comment)
51+
});
52+
53+
if has_line_comment {
54+
// `try {} /* comment */\n catch (e) {}`
55+
// should be formatted like:
56+
// `try {} catch (e) { /* comment */ }`
57+
//
58+
// Comments before the catch clause should be printed in the block statement.
59+
// We cache them here to avoid the `params` printing them accidentally.
60+
let printed_comments = f.intern(&FormatLeadingComments::Comments(comments));
61+
if let Ok(Some(comments)) = printed_comments {
62+
f.context_mut().cache_element(&self.span, comments);
63+
}
64+
} else if !comments.is_empty() {
65+
// otherwise, print them before `catch`
66+
write!(f, [FormatTrailingComments::Comments(comments), space()]);
5267
}
5368

5469
write!(f, ["catch", space(), self.param(), space()])?;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ const FORMATTER_CRATE_PATH: &str = "crates/oxc_formatter";
2121
/// Based on the printing comments algorithm, the last child of these AST nodes don't need to print comments.
2222
/// Without following nodes could lead to only print comments that before the end of the node, which is what we want.
2323
const AST_NODE_WITHOUT_FOLLOWING_NODE_LIST: &[&str] =
24-
&["AssignmentExpression", "FormalParameters", "StaticMemberExpression", "ObjectProperty"];
24+
&["AssignmentExpression", "FormalParameters", "StaticMemberExpression", "ObjectProperty", "ComputedMemberExpression"];
2525

2626
const AST_NODE_WITH_FOLLOWING_NODE_LIST: &[&str] = &["Function", "Class"];
2727

0 commit comments

Comments
 (0)