fix(rome_js_formatter): Trailing comma inside import #3600

rome · Nov 9, 2022 · 4a7954c · 4a7954c
1 parent 99f59bf
commit 4a7954c
Show file tree

Hide file tree

Showing 7 changed files with 174 additions and 117 deletions.
diff --git a/crates/rome_js_formatter/src/js/expressions/call_arguments.rs b/crates/rome_js_formatter/src/js/expressions/call_arguments.rs
@@ -8,7 +8,7 @@ use crate::prelude::*;
 use crate::utils::function_body::FunctionBodyCacheMode;
 use crate::utils::test_call::is_test_call_expression;
 use crate::utils::{is_long_curried_call, write_arguments_multi_line};
-use rome_formatter::{format_args, format_element, write, VecBuffer};
+use rome_formatter::{format_args, format_element, write, FormatRuleWithOptions, VecBuffer};
 use rome_js_syntax::{
     JsAnyCallArgument, JsAnyExpression, JsAnyFunctionBody, JsAnyLiteralExpression, JsAnyStatement,
     JsCallArgumentList, JsCallArguments, JsCallArgumentsFields, JsCallExpression,
@@ -17,7 +17,23 @@ use rome_js_syntax::{
 use rome_rowan::{AstSeparatedElement, AstSeparatedList, SyntaxResult};
 
 #[derive(Debug, Clone, Default)]
-pub struct FormatJsCallArguments;
+pub struct FormatJsCallArguments {
+    options: FormatJsCallArgumentsOptions,
+}
+
+impl FormatRuleWithOptions<JsCallArguments> for FormatJsCallArguments {
+    type Options = FormatJsCallArgumentsOptions;
+
+    fn with_options(mut self, options: Self::Options) -> Self {
+        self.options = options;
+        self
+    }
+}
+
+#[derive(Debug, Copy, Clone, Default)]
+pub struct FormatJsCallArgumentsOptions {
+    pub(crate) is_inside_import: bool,
+}
 
 impl FormatNodeRule<JsCallArguments> for FormatJsCallArguments {
     fn fmt_fields(&self, node: &JsCallArguments, f: &mut JsFormatter) -> FormatResult<()> {
@@ -27,6 +43,8 @@ impl FormatNodeRule<JsCallArguments> for FormatJsCallArguments {
             r_paren_token,
         } = node.as_fields();
 
+        let is_inside_import = self.options.is_inside_import;
+
         if args.is_empty() {
             return write!(
                 f,
@@ -99,13 +117,14 @@ impl FormatNodeRule<JsCallArguments> for FormatJsCallArguments {
                     l_paren: &l_paren_token.format(),
                     args: &arguments,
                     r_paren: &r_paren_token.format(),
+                    is_inside_import,
                     expand: true,
                 }]
             );
         }
 
         if let Some(group_layout) = arguments_grouped_layout(&args, f.comments()) {
-            write_grouped_arguments(node, arguments, group_layout, f)
+            write_grouped_arguments(node, arguments, group_layout, is_inside_import, f)
         } else if is_long_curried_call(call_expression.as_ref()) {
             write!(
                 f,
@@ -124,7 +143,8 @@ impl FormatNodeRule<JsCallArguments> for FormatJsCallArguments {
                     l_paren: &l_paren_token.format(),
                     args: &arguments,
                     r_paren: &r_paren_token.format(),
-                    expand: false
+                    is_inside_import,
+                    expand: false,
                 }]
             )
         }
@@ -315,6 +335,7 @@ fn write_grouped_arguments(
     call_arguments: &JsCallArguments,
     mut arguments: Vec<FormatCallArgument>,
     group_layout: GroupedCallArgumentLayout,
+    is_inside_import: bool,
     f: &mut JsFormatter,
 ) -> FormatResult<()> {
     let l_paren_token = call_arguments.l_paren_token();
@@ -344,7 +365,8 @@ fn write_grouped_arguments(
                     l_paren: &l_paren_token.format(),
                     args: &arguments,
                     r_paren: &r_paren_token.format(),
-                    expand: true
+                    is_inside_import,
+                    expand: true,
                 }]
             );
         }
@@ -385,7 +407,8 @@ fn write_grouped_arguments(
                 l_paren: &l_paren,
                 args: &arguments,
                 r_paren: &r_paren,
-                expand: true
+                is_inside_import,
+                expand: true,
             }]
         )?;
         buffer.write_element(FormatElement::Tag(Tag::EndEntry))?;
@@ -703,6 +726,7 @@ struct FormatAllArgsBrokenOut<'a> {
     args: &'a [FormatCallArgument],
     r_paren: &'a dyn Format<JsFormatContext>,
     expand: bool,
+    is_inside_import: bool,
 }
 
 impl<'a> Format<JsFormatContext> for FormatAllArgsBrokenOut<'a> {
@@ -723,7 +747,10 @@ impl<'a> Format<JsFormatContext> for FormatAllArgsBrokenOut<'a> {
                         write!(f, [entry])?;
                     }
 
-                    write!(f, [FormatTrailingComma::All])
+                    if !self.is_inside_import {
+                        write!(f, [FormatTrailingComma::All])?;
+                    }
+                    Ok(())
                 })),
                 self.r_paren,
             ])

diff --git a/crates/rome_js_formatter/src/js/expressions/import_call_expression.rs b/crates/rome_js_formatter/src/js/expressions/import_call_expression.rs
@@ -1,5 +1,6 @@
 use crate::prelude::*;
 
+use crate::js::expressions::call_arguments::FormatJsCallArgumentsOptions;
 use crate::parentheses::NeedsParentheses;
 use rome_formatter::write;
 use rome_js_syntax::JsImportCallExpressionFields;
@@ -15,7 +16,17 @@ impl FormatNodeRule<JsImportCallExpression> for FormatJsImportCallExpression {
             arguments,
         } = node.as_fields();
 
-        write![f, [import_token.format(), arguments.format(),]]
+        write![
+            f,
+            [
+                import_token.format(),
+                arguments?
+                    .format()
+                    .with_options(FormatJsCallArgumentsOptions {
+                        is_inside_import: true,
+                    })
+            ]
+        ]
     }
 
     fn needs_parentheses(&self, item: &JsImportCallExpression) -> bool {

diff --git a/crates/rome_js_formatter/tests/specs/js/module/import/import_call.js.snap b/crates/rome_js_formatter/tests/specs/js/module/import/import_call.js.snap
@@ -32,7 +32,7 @@ import(x);
 import("x");
 import(
 	aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,
-	{ assert: { type: "json" } },
+	{ assert: { type: "json" } }
 );
 
 

diff --git a/...es/rome_js_formatter/tests/specs/js/module/import/trailing-comma/import_trailing_comma.js b/...es/rome_js_formatter/tests/specs/js/module/import/trailing-comma/import_trailing_comma.js
@@ -8,3 +8,23 @@ import {
 	adsadasdasdasdasdasdasdasdasdasdas,
 	dsadsadasdasdasdasdasdasdasd,
 	dsadsadasdasdasdasdasdasdasd, } from "W";
+
+const StandaloneBackendWASMModule = await import(/* webpackChunkName: "standalone" */'./StandaloneBackendWASM');
+
+const StandaloneBackendWASMModule = await import(/* webpackChunkName: "standalone" */'./StandaloneBackendWASM',
+	/* webpackChunkName: "standalone" */'./StandaloneBackendWASM');
+
+import(
+	'myreallylongdynamicallyloadedmodulenamemyreallylongdynamicallyloadedmodulename'
+	);
+
+import(/* Hello */ "something");
+import("something" /* Hello */);
+import(/* Hello */ "something" /* Hello */);
+import("something" /* Hello */ + "else");
+import(
+	/* Hello */
+	"something",
+	/* Hello */
+	);
+wrap(import(/* Hello */ "something"));
diff --git a/...me_js_formatter/tests/specs/js/module/import/trailing-comma/import_trailing_comma.js.snap b/...me_js_formatter/tests/specs/js/module/import/trailing-comma/import_trailing_comma.js.snap
@@ -17,6 +17,26 @@ import {
 	dsadsadasdasdasdasdasdasdasd,
 	dsadsadasdasdasdasdasdasdasd, } from "W";
 
+const StandaloneBackendWASMModule = await import(/* webpackChunkName: "standalone" */'./StandaloneBackendWASM');
+
+const StandaloneBackendWASMModule = await import(/* webpackChunkName: "standalone" */'./StandaloneBackendWASM',
+	/* webpackChunkName: "standalone" */'./StandaloneBackendWASM');
+
+import(
+	'myreallylongdynamicallyloadedmodulenamemyreallylongdynamicallyloadedmodulename'
+	);
+
+import(/* Hello */ "something");
+import("something" /* Hello */);
+import(/* Hello */ "something" /* Hello */);
+import("something" /* Hello */ + "else");
+import(
+	/* Hello */
+	"something",
+	/* Hello */
+	);
+wrap(import(/* Hello */ "something"));
+
 ```
 
 
@@ -46,6 +66,35 @@ import {
 	dsadsadasdasdasdasdasdasdasd,
 	dsadsadasdasdasdasdasdasdasd,
 } from "W";
+
+const StandaloneBackendWASMModule = await import(
+	/* webpackChunkName: "standalone" */ "./StandaloneBackendWASM"
+);
+
+const StandaloneBackendWASMModule = await import(
+	/* webpackChunkName: "standalone" */ "./StandaloneBackendWASM",
+	/* webpackChunkName: "standalone" */ "./StandaloneBackendWASM"
+);
+
+import(
+	"myreallylongdynamicallyloadedmodulenamemyreallylongdynamicallyloadedmodulename"
+);
+
+import(/* Hello */ "something");
+import("something" /* Hello */);
+import(/* Hello */ "something" /* Hello */);
+import("something" /* Hello */ + "else");
+import(
+	/* Hello */
+	"something"
+	/* Hello */
+);
+wrap(import(/* Hello */ "something"));
+
+
+## Lines exceeding width of 80 characters
+
+   23: 	"myreallylongdynamicallyloadedmodulenamemyreallylongdynamicallyloadedmodulename"
 ```
 
 ## Output 2
@@ -70,6 +119,35 @@ import {
 	dsadsadasdasdasdasdasdasdasd,
 	dsadsadasdasdasdasdasdasdasd,
 } from "W";
+
+const StandaloneBackendWASMModule = await import(
+	/* webpackChunkName: "standalone" */ "./StandaloneBackendWASM"
+);
+
+const StandaloneBackendWASMModule = await import(
+	/* webpackChunkName: "standalone" */ "./StandaloneBackendWASM",
+	/* webpackChunkName: "standalone" */ "./StandaloneBackendWASM"
+);
+
+import(
+	"myreallylongdynamicallyloadedmodulenamemyreallylongdynamicallyloadedmodulename"
+);
+
+import(/* Hello */ "something");
+import("something" /* Hello */);
+import(/* Hello */ "something" /* Hello */);
+import("something" /* Hello */ + "else");
+import(
+	/* Hello */
+	"something"
+	/* Hello */
+);
+wrap(import(/* Hello */ "something"));
+
+
+## Lines exceeding width of 80 characters
+
+   23: 	"myreallylongdynamicallyloadedmodulenamemyreallylongdynamicallyloadedmodulename"
 ```
 
 ## Output 3
@@ -94,6 +172,35 @@ import {
 	dsadsadasdasdasdasdasdasdasd,
 	dsadsadasdasdasdasdasdasdasd
 } from "W";
+
+const StandaloneBackendWASMModule = await import(
+	/* webpackChunkName: "standalone" */ "./StandaloneBackendWASM"
+);
+
+const StandaloneBackendWASMModule = await import(
+	/* webpackChunkName: "standalone" */ "./StandaloneBackendWASM",
+	/* webpackChunkName: "standalone" */ "./StandaloneBackendWASM"
+);
+
+import(
+	"myreallylongdynamicallyloadedmodulenamemyreallylongdynamicallyloadedmodulename"
+);
+
+import(/* Hello */ "something");
+import("something" /* Hello */);
+import(/* Hello */ "something" /* Hello */);
+import("something" /* Hello */ + "else");
+import(
+	/* Hello */
+	"something"
+	/* Hello */
+);
+wrap(import(/* Hello */ "something"));
+
+
+## Lines exceeding width of 80 characters
+
+   23: 	"myreallylongdynamicallyloadedmodulenamemyreallylongdynamicallyloadedmodulename"
 ```
 
 
diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/comments/dynamic_imports.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/comments/dynamic_imports.js.snap