diff --git a/crates/oxc_formatter/src/write/call_arguments.rs b/crates/oxc_formatter/src/write/call_arguments.rs index 4a37121696d1e..a7d46afcddc5f 100644 --- a/crates/oxc_formatter/src/write/call_arguments.rs +++ b/crates/oxc_formatter/src/write/call_arguments.rs @@ -59,27 +59,21 @@ impl<'a> Format<'a> for AstNode<'a, ArenaVec<'a, Argument<'a>>> { ); } - let call_expression = - if let AstNodes::CallExpression(call) = self.parent { Some(call) } else { None }; + let is_simple_module_import = is_simple_module_import(self, f.comments()); - let (is_commonjs_or_amd_call, is_test_call) = call_expression - .map(|call| (is_commonjs_or_amd_call(self, call), is_test_call_expression(call))) - .unwrap_or_default(); + let call_expression = + if !is_simple_module_import && let AstNodes::CallExpression(call) = self.parent { + Some(call) + } else { + None + }; - if is_commonjs_or_amd_call + if is_simple_module_import + || call_expression.is_some_and(|call| { + is_commonjs_or_amd_call(self, call, f) || is_test_call_expression(call) + }) || is_multiline_template_only_args(self, f.source_text()) || is_react_hook_with_deps_array(self, f.comments()) - || (is_test_call && { - self.len() != 2 - || matches!( - arguments.first(), - Some( - Argument::StringLiteral(_) - | Argument::TemplateLiteral(_) - | Argument::TaggedTemplateExpression(_) - ) - ) - }) { return write!( f, @@ -929,11 +923,59 @@ fn function_has_only_simple_parameters(params: &FormalParameters<'_>) -> bool { has_only_simple_parameters(params, false) } -/// Tests if this is a call to commonjs [`require`](https://nodejs.org/api/modules.html#requireid) -/// or amd's [`define`](https://github.com/amdjs/amdjs-api/wiki/AMD#define-function-) function. +/// Tests if this a simple module import like `import("module-name")` or `require("module-name")`. +pub fn is_simple_module_import( + arguments: &AstNode<'_, ArenaVec<'_, Argument<'_>>>, + comments: &Comments, +) -> bool { + if arguments.len() != 1 { + return false; + } + + match arguments.parent { + AstNodes::ImportExpression(_) => {} + AstNodes::CallExpression(call) => { + match &call.callee { + Expression::StaticMemberExpression(member) => match member.property.name.as_str() { + "resolve" => { + match &member.object { + Expression::Identifier(ident) if ident.name.as_str() == "require" => { + // `require.resolve("foo")` + } + Expression::MetaProperty(_) => { + // `import.meta.resolve("foo")` + } + _ => return false, + } + } + "paths" => { + if !matches!( + &member.object, Expression::StaticMemberExpression(member) + if matches!(&member.object, Expression::Identifier(ident) + if ident.name == "require") && member.property.name.as_str() == "resolve" + ) { + return false; + } + } + _ => return false, + }, + _ => { + return false; + } + } + } + _ => return false, + } + + matches!(arguments.as_ref()[0], Argument::StringLiteral(_)) + && !comments.has_comment_before(arguments.parent.span().end) +} + +/// Tests if amd's [`define`](https://github.com/amdjs/amdjs-api/wiki/AMD#define-function-) function. fn is_commonjs_or_amd_call( arguments: &[Argument<'_>], call: &AstNode<'_, CallExpression<'_>>, + f: &Formatter<'_, '_>, ) -> bool { let Expression::Identifier(ident) = &call.callee else { return false; @@ -941,8 +983,11 @@ fn is_commonjs_or_amd_call( match ident.name.as_str() { "require" => { + let first_argument = &arguments[0]; + if f.comments().has_comment_before(first_argument.span().start) { + return false; + } match arguments.len() { - 0 => false, // `require` can be called with any expression that resolves to a // string. This check is only an escape hatch to allow a complex // expression to break rather than group onto the previous line. @@ -956,7 +1001,9 @@ fn is_commonjs_or_amd_call( // require( // path.join(__dirname, 'relative/path') // ); - 1 => matches!(arguments.first(), Some(Argument::StringLiteral(_))), + 1 => { + matches!(first_argument, Argument::StringLiteral(_)) + } _ => true, } } diff --git a/crates/oxc_formatter/src/write/import_declaration.rs b/crates/oxc_formatter/src/write/import_declaration.rs index 5027c88551d8a..e31f51baa2e91 100644 --- a/crates/oxc_formatter/src/write/import_declaration.rs +++ b/crates/oxc_formatter/src/write/import_declaration.rs @@ -186,7 +186,7 @@ impl<'a> FormatWrite<'a> for AstNode<'a, WithClause<'a>> { WithClauseKeyword::Assert => "assert", }, space(), - self.with_entries(), + self.with_entries() ] ) } @@ -194,18 +194,67 @@ impl<'a> FormatWrite<'a> for AstNode<'a, WithClause<'a>> { impl<'a> Format<'a> for AstNode<'a, Vec<'a, ImportAttribute<'a>>> { fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { - write!(f, "{")?; - if !self.is_empty() { - let maybe_space = maybe_space(f.options().bracket_spacing.value()); - write!(f, [maybe_space])?; + if self.is_empty() { + return write!(f, "{}"); + } - f.join_with(space()) - .entries_with_trailing_separator(self.iter(), ",", TrailingSeparator::Disallowed) - .finish()?; + let format_inner = format_once(|f| { + let should_insert_space_around_brackets = f.options().bracket_spacing.value(); - write!(f, [maybe_space])?; - } - write!(f, "}") + write!(f, "{")?; + + if self.len() > 1 + || self.first().is_some_and(|attribute| attribute.key.as_atom().as_str() != "type") + || f.comments().has_comment_before(self.parent.span().end) + { + write!( + f, + [soft_block_indent_with_maybe_space( + &format_once(|f| { + let trailing_separator = + FormatTrailingCommas::ES5.trailing_separator(f.options()); + + f.join_with(soft_line_break()) + .entries_with_trailing_separator( + self.iter(), + ",", + trailing_separator, + ) + .finish() + },), + should_insert_space_around_brackets + )] + )?; + } else { + write!( + f, + [format_once(|f| { + let maybe_space = maybe_space(f.options().bracket_spacing.value()); + write!(f, [maybe_space])?; + + f.join_with(space()) + .entries_with_trailing_separator( + self.iter(), + ",", + TrailingSeparator::Disallowed, + ) + .finish()?; + + write!(f, [maybe_space]) + })] + )?; + } + + write!(f, "}") + }); + + let first = self.as_ref().first().unwrap(); + + write!( + f, + group(&format_inner) + .should_expand(f.source_text().has_newline_before(first.span.start)) + ) } } @@ -222,7 +271,16 @@ impl<'a> FormatWrite<'a> for AstNode<'a, ImportAttribute<'a>> { } else { write!(f, self.key())?; } - write!(f, [":", space(), self.value()]) + write!(f, [":", space()])?; + + let has_leading_own_line_comment = + f.comments().has_leading_own_line_comment(self.value.span.start); + + if has_leading_own_line_comment { + write!(f, [group(&indent(&format_args!(soft_line_break(), self.value())))]) + } else { + write!(f, [self.value()]) + } } } @@ -247,6 +305,14 @@ impl<'a> FormatWrite<'a> for AstNode<'a, TSImportEqualsDeclaration<'a>> { impl<'a> FormatWrite<'a> for AstNode<'a, TSExternalModuleReference<'a>> { fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { - write!(f, ["require(", self.expression(), ")"]) + write!(f, ["require("])?; + + if f.comments().has_comment_in_span(self.span) { + write!(f, [block_indent(self.expression())])?; + } else { + write!(f, [self.expression()])?; + } + + write!(f, [")"]) } } diff --git a/crates/oxc_formatter/src/write/mod.rs b/crates/oxc_formatter/src/write/mod.rs index fc8c35524d93c..e0fd943bc8b52 100644 --- a/crates/oxc_formatter/src/write/mod.rs +++ b/crates/oxc_formatter/src/write/mod.rs @@ -83,6 +83,7 @@ use crate::{ use self::{ array_expression::FormatArrayExpression, arrow_function_expression::is_multiline_template_starting_on_same_line, + call_arguments::is_simple_module_import, class::format_grouped_parameters_with_return_type_for_method, function::should_group_function_parameters, object_like::ObjectLike, @@ -222,7 +223,8 @@ impl<'a> FormatWrite<'a> for AstNode<'a, CallExpression<'a>> { is_multiline_template_starting_on_same_line(expr, f.source_text()) }); - if !is_template_literal_single_arg + if !is_simple_module_import(self.arguments(), f.comments()) + && !is_template_literal_single_arg && callee.as_member_expression().is_some_and(|e| { matches!( e, @@ -1659,10 +1661,39 @@ impl<'a> FormatWrite<'a> for AstNode<'a, TSTypeQuery<'a>> { impl<'a> FormatWrite<'a> for AstNode<'a, TSImportType<'a>> { fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { - write!(f, ["import(", self.argument()])?; - if let Some(options) = &self.options() { - write!(f, [",", space(), options])?; + write!(f, ["import("])?; + + let has_comment = f.context().comments().has_comment_before(self.argument.span().start); + + let format_argument = self.argument().memoized(); + let format_options = self.options().memoized(); + + if has_comment || self.options().is_some() { + let format_inner = format_with(|f| { + write!( + f, + [ + format_argument, + &format_once(|f| { + if self.options.is_some() { + write!(f, [",", soft_line_break_or_space(), format_options])?; + } + Ok(()) + }), + ] + ) + }); + if has_comment { + write!(f, [&soft_block_indent(&format_inner)])?; + } else if self.options().is_some() { + write!(f, [best_fitting!(format_inner, &soft_block_indent(&format_inner))])?; + } else { + write!(f, [format_inner])?; + } + } else { + write!(f, self.argument())?; } + write!(f, ")")?; if let Some(qualified_name) = &self.qualifier() { write!(f, [".", qualified_name])?; diff --git a/tasks/prettier_conformance/snapshots/prettier.js.snap.md b/tasks/prettier_conformance/snapshots/prettier.js.snap.md index 01ce8925209d1..1dc962a7a827b 100644 --- a/tasks/prettier_conformance/snapshots/prettier.js.snap.md +++ b/tasks/prettier_conformance/snapshots/prettier.js.snap.md @@ -1,4 +1,4 @@ -js compatibility: 685/749 (91.46%) +js compatibility: 693/749 (92.52%) # Failed @@ -34,11 +34,6 @@ js compatibility: 685/749 (91.46%) | js/if/expr_and_same_line_comments.js | 💥 | 97.73% | | js/if/if_comments.js | 💥 | 76.00% | | js/if/trailing_comment.js | 💥 | 91.43% | -| js/import/long-module-name/import-defer.js | 💥 | 66.67% | -| js/import/long-module-name/import-expression.js | 💥 | 47.06% | -| js/import/long-module-name/import-source.js | 💥 | 66.67% | -| js/import-attributes/long-sources.js | 💥 | 64.58% | -| js/import-attributes/multiple.js | 💥 | 15.38% | | js/label/empty_label.js | 💥 | 66.67% | | js/last-argument-expansion/dangling-comment-in-arrow-function.js | 💥 | 22.22% | | js/logical-expressions/multiple-comments/17192.js | 💥 | 60.00% | @@ -47,8 +42,6 @@ js compatibility: 685/749 (91.46%) | js/quote-props/objects.js | 💥💥✨✨ | 45.10% | | js/quote-props/with_numbers.js | 💥💥✨✨ | 46.43% | | js/quotes/objects.js | 💥💥 | 80.00% | -| js/require/comments.js | 💥 | 81.25% | -| js/require/long-module-name.js | 💥 | 18.18% | | js/sequence-expression/ignored.js | 💥 | 25.00% | | js/strings/template-literals.js | 💥💥 | 98.01% | | js/ternaries/binary.js | 💥💥💥💥✨✨✨✨ | 18.42% | @@ -62,7 +55,6 @@ js compatibility: 685/749 (91.46%) | js/ternaries/parenthesis/await-expression.js | 💥✨ | 14.29% | | js/test-declarations/angularjs_inject.js | 💥💥 | 91.53% | | js/test-declarations/test_declarations.js | 💥💥 | 95.88% | -| js/trailing-comma/dynamic-import.js | 💥💥💥 | 0.00% | | jsx/fbt/test.js | 💥 | 84.06% | | jsx/jsx/quotes.js | 💥💥💥💥 | 79.41% | | jsx/optional-chaining/optional-chaining.jsx | 💥 | 85.96% | diff --git a/tasks/prettier_conformance/snapshots/prettier.ts.snap.md b/tasks/prettier_conformance/snapshots/prettier.ts.snap.md index 0592a29936b8a..e9f4b2b1f2cee 100644 --- a/tasks/prettier_conformance/snapshots/prettier.ts.snap.md +++ b/tasks/prettier_conformance/snapshots/prettier.ts.snap.md @@ -1,4 +1,4 @@ -ts compatibility: 538/598 (89.97%) +ts compatibility: 542/598 (90.64%) # Failed @@ -35,10 +35,6 @@ ts compatibility: 538/598 (89.97%) | typescript/conditional-types/new-ternary-spec.ts | 💥✨ | 10.67% | | typescript/conditional-types/parentheses.ts | 💥✨ | 15.22% | | typescript/decorators-ts/angular.ts | 💥 | 87.50% | -| typescript/import-require/comments.ts | 💥 | 33.33% | -| typescript/import-type/long-module-name/long-module-name2.ts | 💥 | 25.00% | -| typescript/import-type/long-module-name/long-module-name4.ts | 💥 | 89.29% | -| typescript/import-type/long-module-name/long-module-name5.ts | 💥 | 33.33% | | typescript/instantiation-expression/17714.ts | 💥 | 0.00% | | typescript/interface/comments-generic.ts | 💥💥 | 41.94% | | typescript/interface/long-extends.ts | 💥💥 | 83.64% |