Skip to content

Commit fedb0bc

Browse files
DunqingDunqing
committed
feat(formatter): align import-like formatting the same as Prettier
1 parent 53f09b5 commit fedb0bc

File tree

5 files changed

+114
-38
lines changed

5 files changed

+114
-38
lines changed

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

Lines changed: 68 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -59,27 +59,21 @@ impl<'a> Format<'a> for AstNode<'a, ArenaVec<'a, Argument<'a>>> {
5959
);
6060
}
6161

62-
let call_expression =
63-
if let AstNodes::CallExpression(call) = self.parent { Some(call) } else { None };
62+
let is_simple_module_import = is_simple_module_import(self, f.comments());
6463

65-
let (is_commonjs_or_amd_call, is_test_call) = call_expression
66-
.map(|call| (is_commonjs_or_amd_call(self, call), is_test_call_expression(call)))
67-
.unwrap_or_default();
64+
let call_expression =
65+
if !is_simple_module_import && let AstNodes::CallExpression(call) = self.parent {
66+
Some(call)
67+
} else {
68+
None
69+
};
6870

69-
if is_commonjs_or_amd_call
71+
if is_simple_module_import
72+
|| call_expression.is_some_and(|call| {
73+
is_commonjs_or_amd_call(self, call, f) || is_test_call_expression(call)
74+
})
7075
|| is_multiline_template_only_args(self, f.source_text())
7176
|| is_react_hook_with_deps_array(self, f.comments())
72-
|| (is_test_call && {
73-
self.len() != 2
74-
|| matches!(
75-
arguments.first(),
76-
Some(
77-
Argument::StringLiteral(_)
78-
| Argument::TemplateLiteral(_)
79-
| Argument::TaggedTemplateExpression(_)
80-
)
81-
)
82-
})
8377
{
8478
return write!(
8579
f,
@@ -929,20 +923,71 @@ fn function_has_only_simple_parameters(params: &FormalParameters<'_>) -> bool {
929923
has_only_simple_parameters(params, false)
930924
}
931925

932-
/// Tests if this is a call to commonjs [`require`](https://nodejs.org/api/modules.html#requireid)
933-
/// or amd's [`define`](https://github.com/amdjs/amdjs-api/wiki/AMD#define-function-) function.
926+
/// Tests if this a simple module import like `import("module-name")` or `require("module-name")`.
927+
pub fn is_simple_module_import(
928+
arguments: &AstNode<'_, ArenaVec<'_, Argument<'_>>>,
929+
comments: &Comments,
930+
) -> bool {
931+
if arguments.len() != 1 {
932+
return false;
933+
}
934+
935+
match arguments.parent {
936+
AstNodes::ImportExpression(_) => {}
937+
AstNodes::CallExpression(call) => {
938+
match &call.callee {
939+
Expression::StaticMemberExpression(member) => match member.property.name.as_str() {
940+
"resolve" => {
941+
match &member.object {
942+
Expression::Identifier(ident) if ident.name.as_str() == "require" => {
943+
// `require.resolve("foo")`
944+
}
945+
Expression::MetaProperty(_) => {
946+
// `import.meta.resolve("foo")`
947+
}
948+
_ => return false,
949+
}
950+
}
951+
"paths" => {
952+
if !matches!(
953+
&member.object, Expression::StaticMemberExpression(member)
954+
if matches!(&member.object, Expression::Identifier(ident)
955+
if ident.name == "require") && member.property.name.as_str() == "resolve"
956+
) {
957+
return false;
958+
}
959+
}
960+
_ => return false,
961+
},
962+
_ => {
963+
return false;
964+
}
965+
}
966+
}
967+
_ => return false,
968+
}
969+
970+
matches!(arguments.as_ref()[0], Argument::StringLiteral(_))
971+
&& !comments.has_comment_before(arguments.parent.span().end)
972+
}
973+
974+
/// Tests if amd's [`define`](https://github.com/amdjs/amdjs-api/wiki/AMD#define-function-) function.
934975
fn is_commonjs_or_amd_call(
935976
arguments: &[Argument<'_>],
936977
call: &AstNode<'_, CallExpression<'_>>,
978+
f: &Formatter<'_, '_>,
937979
) -> bool {
938980
let Expression::Identifier(ident) = &call.callee else {
939981
return false;
940982
};
941983

942984
match ident.name.as_str() {
943985
"require" => {
986+
let first_argument = &arguments[0];
987+
if f.comments().has_comment_before(first_argument.span().start) {
988+
return false;
989+
}
944990
match arguments.len() {
945-
0 => false,
946991
// `require` can be called with any expression that resolves to a
947992
// string. This check is only an escape hatch to allow a complex
948993
// expression to break rather than group onto the previous line.
@@ -956,7 +1001,9 @@ fn is_commonjs_or_amd_call(
9561001
// require(
9571002
// path.join(__dirname, 'relative/path')
9581003
// );
959-
1 => matches!(arguments.first(), Some(Argument::StringLiteral(_))),
1004+
1 => {
1005+
matches!(first_argument, Argument::StringLiteral(_))
1006+
}
9601007
_ => true,
9611008
}
9621009
}

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,14 @@ impl<'a> FormatWrite<'a> for AstNode<'a, TSImportEqualsDeclaration<'a>> {
247247

248248
impl<'a> FormatWrite<'a> for AstNode<'a, TSExternalModuleReference<'a>> {
249249
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
250-
write!(f, ["require(", self.expression(), ")"])
250+
write!(f, ["require("])?;
251+
252+
if f.comments().has_comment_in_span(self.span) {
253+
write!(f, [block_indent(self.expression())])?;
254+
} else {
255+
write!(f, [self.expression()])?;
256+
}
257+
258+
write!(f, [")"])
251259
}
252260
}

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

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ use crate::{
8383
use self::{
8484
array_expression::FormatArrayExpression,
8585
arrow_function_expression::is_multiline_template_starting_on_same_line,
86+
call_arguments::is_simple_module_import,
8687
class::format_grouped_parameters_with_return_type_for_method,
8788
function::should_group_function_parameters,
8889
object_like::ObjectLike,
@@ -222,7 +223,8 @@ impl<'a> FormatWrite<'a> for AstNode<'a, CallExpression<'a>> {
222223
is_multiline_template_starting_on_same_line(expr, f.source_text())
223224
});
224225

225-
if !is_template_literal_single_arg
226+
if !is_simple_module_import(self.arguments(), f.comments())
227+
&& !is_template_literal_single_arg
226228
&& callee.as_member_expression().is_some_and(|e| {
227229
matches!(
228230
e,
@@ -1659,10 +1661,39 @@ impl<'a> FormatWrite<'a> for AstNode<'a, TSTypeQuery<'a>> {
16591661

16601662
impl<'a> FormatWrite<'a> for AstNode<'a, TSImportType<'a>> {
16611663
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
1662-
write!(f, ["import(", self.argument()])?;
1663-
if let Some(options) = &self.options() {
1664-
write!(f, [",", space(), options])?;
1664+
write!(f, ["import("])?;
1665+
1666+
let has_comment = f.context().comments().has_comment_before(self.argument.span().start);
1667+
1668+
let format_argument = self.argument().memoized();
1669+
let format_options = self.options().memoized();
1670+
1671+
if has_comment || self.options().is_some() {
1672+
let format_inner = format_with(|f| {
1673+
write!(
1674+
f,
1675+
[
1676+
format_argument,
1677+
&format_once(|f| {
1678+
if self.options.is_some() {
1679+
write!(f, [",", soft_line_break_or_space(), format_options])?;
1680+
}
1681+
Ok(())
1682+
}),
1683+
]
1684+
)
1685+
});
1686+
if has_comment {
1687+
write!(f, [&soft_block_indent(&format_inner)])?;
1688+
} else if self.options().is_some() {
1689+
write!(f, [best_fitting!(format_inner, &soft_block_indent(&format_inner))])?;
1690+
} else {
1691+
write!(f, [format_inner])?;
1692+
}
1693+
} else {
1694+
write!(f, self.argument())?;
16651695
}
1696+
16661697
write!(f, ")")?;
16671698
if let Some(qualified_name) = &self.qualifier() {
16681699
write!(f, [".", qualified_name])?;

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

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
js compatibility: 685/749 (91.46%)
1+
js compatibility: 691/749 (92.26%)
22

33
# Failed
44

@@ -34,9 +34,6 @@ js compatibility: 685/749 (91.46%)
3434
| js/if/expr_and_same_line_comments.js | 💥 | 97.73% |
3535
| js/if/if_comments.js | 💥 | 76.00% |
3636
| js/if/trailing_comment.js | 💥 | 91.43% |
37-
| js/import/long-module-name/import-defer.js | 💥 | 66.67% |
38-
| js/import/long-module-name/import-expression.js | 💥 | 47.06% |
39-
| js/import/long-module-name/import-source.js | 💥 | 66.67% |
4037
| js/import-attributes/long-sources.js | 💥 | 64.58% |
4138
| js/import-attributes/multiple.js | 💥 | 15.38% |
4239
| js/label/empty_label.js | 💥 | 66.67% |
@@ -47,8 +44,6 @@ js compatibility: 685/749 (91.46%)
4744
| js/quote-props/objects.js | 💥💥✨✨ | 45.10% |
4845
| js/quote-props/with_numbers.js | 💥💥✨✨ | 46.43% |
4946
| js/quotes/objects.js | 💥💥 | 80.00% |
50-
| js/require/comments.js | 💥 | 81.25% |
51-
| js/require/long-module-name.js | 💥 | 18.18% |
5247
| js/sequence-expression/ignored.js | 💥 | 25.00% |
5348
| js/strings/template-literals.js | 💥💥 | 98.01% |
5449
| js/ternaries/binary.js | 💥💥💥💥✨✨✨✨ | 18.42% |
@@ -62,7 +57,6 @@ js compatibility: 685/749 (91.46%)
6257
| js/ternaries/parenthesis/await-expression.js | 💥✨ | 14.29% |
6358
| js/test-declarations/angularjs_inject.js | 💥💥 | 91.53% |
6459
| js/test-declarations/test_declarations.js | 💥💥 | 95.88% |
65-
| js/trailing-comma/dynamic-import.js | 💥💥💥 | 0.00% |
6660
| jsx/fbt/test.js | 💥 | 84.06% |
6761
| jsx/jsx/quotes.js | 💥💥💥💥 | 79.41% |
6862
| jsx/optional-chaining/optional-chaining.jsx | 💥 | 85.96% |

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
ts compatibility: 538/598 (89.97%)
1+
ts compatibility: 542/598 (90.64%)
22

33
# Failed
44

@@ -35,10 +35,6 @@ ts compatibility: 538/598 (89.97%)
3535
| typescript/conditional-types/new-ternary-spec.ts | 💥✨ | 10.67% |
3636
| typescript/conditional-types/parentheses.ts | 💥✨ | 15.22% |
3737
| typescript/decorators-ts/angular.ts | 💥 | 87.50% |
38-
| typescript/import-require/comments.ts | 💥 | 33.33% |
39-
| typescript/import-type/long-module-name/long-module-name2.ts | 💥 | 25.00% |
40-
| typescript/import-type/long-module-name/long-module-name4.ts | 💥 | 89.29% |
41-
| typescript/import-type/long-module-name/long-module-name5.ts | 💥 | 33.33% |
4238
| typescript/instantiation-expression/17714.ts | 💥 | 0.00% |
4339
| typescript/interface/comments-generic.ts | 💥💥 | 41.94% |
4440
| typescript/interface/long-extends.ts | 💥💥 | 83.64% |

0 commit comments

Comments
 (0)