From b788d704f6d0c8aaecb1bed21d03dcef2846aa77 Mon Sep 17 00:00:00 2001 From: Denis Bezrukov <6227442+denbezrukov@users.noreply.github.com> Date: Wed, 9 Nov 2022 18:52:52 +0200 Subject: [PATCH] fix(rome_js_formatter): Trailing comma inside import #3600 (#3624) Fix https://github.com/rome/tools/issues/3600 --- .../src/js/expressions/call_arguments.rs | 21 +++- .../js/expressions/import_call_expression.rs | 2 +- .../js/module/import/import_call.js.snap | 2 +- .../trailing-comma/import_trailing_comma.js | 20 ++++ .../import_trailing_comma.js.snap | 107 ++++++++++++++++++ .../js/comments/dynamic_imports.js.snap | 69 ----------- .../js/trailing-comma/dynamic-import.js.snap | 39 ------- 7 files changed, 145 insertions(+), 115 deletions(-) delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/js/comments/dynamic_imports.js.snap delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/js/trailing-comma/dynamic-import.js.snap diff --git a/crates/rome_js_formatter/src/js/expressions/call_arguments.rs b/crates/rome_js_formatter/src/js/expressions/call_arguments.rs index 606a68ffae6..78de1d9591d 100644 --- a/crates/rome_js_formatter/src/js/expressions/call_arguments.rs +++ b/crates/rome_js_formatter/src/js/expressions/call_arguments.rs @@ -12,7 +12,8 @@ use rome_formatter::{format_args, format_element, write, VecBuffer}; use rome_js_syntax::{ JsAnyCallArgument, JsAnyExpression, JsAnyFunctionBody, JsAnyLiteralExpression, JsAnyStatement, JsCallArgumentList, JsCallArguments, JsCallArgumentsFields, JsCallExpression, - JsExpressionStatement, JsFunctionExpression, JsLanguage, TsAnyReturnType, TsType, + JsExpressionStatement, JsFunctionExpression, JsImportCallExpression, JsLanguage, + TsAnyReturnType, TsType, }; use rome_rowan::{AstSeparatedElement, AstSeparatedList, SyntaxResult}; @@ -99,6 +100,7 @@ impl FormatNodeRule for FormatJsCallArguments { l_paren: &l_paren_token.format(), args: &arguments, r_paren: &r_paren_token.format(), + node, expand: true, }] ); @@ -124,7 +126,8 @@ impl FormatNodeRule for FormatJsCallArguments { l_paren: &l_paren_token.format(), args: &arguments, r_paren: &r_paren_token.format(), - expand: false + node, + expand: false, }] ) } @@ -344,7 +347,8 @@ fn write_grouped_arguments( l_paren: &l_paren_token.format(), args: &arguments, r_paren: &r_paren_token.format(), - expand: true + node: call_arguments, + expand: true, }] ); } @@ -385,7 +389,8 @@ fn write_grouped_arguments( l_paren: &l_paren, args: &arguments, r_paren: &r_paren, - expand: true + node: call_arguments, + expand: true, }] )?; buffer.write_element(FormatElement::Tag(Tag::EndEntry))?; @@ -703,10 +708,13 @@ struct FormatAllArgsBrokenOut<'a> { args: &'a [FormatCallArgument], r_paren: &'a dyn Format, expand: bool, + node: &'a JsCallArguments, } impl<'a> Format for FormatAllArgsBrokenOut<'a> { fn fmt(&self, f: &mut Formatter) -> FormatResult<()> { + let is_inside_import = self.node.parent::().is_some(); + write!( f, [group(&format_args![ @@ -723,7 +731,10 @@ impl<'a> Format for FormatAllArgsBrokenOut<'a> { write!(f, [entry])?; } - write!(f, [FormatTrailingComma::All]) + if !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 index a277f6eb54b..53b434934f7 100644 --- a/crates/rome_js_formatter/src/js/expressions/import_call_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/import_call_expression.rs @@ -15,7 +15,7 @@ impl FormatNodeRule for FormatJsImportCallExpression { arguments, } = node.as_fields(); - write![f, [import_token.format(), arguments.format(),]] + write![f, [import_token.format(), arguments.format()]] } 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 index 5bee5b9992c..f201dc9ee35 100644 --- 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/crates/rome_js_formatter/tests/specs/js/module/import/trailing-comma/import_trailing_comma.js b/crates/rome_js_formatter/tests/specs/js/module/import/trailing-comma/import_trailing_comma.js index cd5638771a6..876513ec4a9 100644 --- a/crates/rome_js_formatter/tests/specs/js/module/import/trailing-comma/import_trailing_comma.js +++ b/crates/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/crates/rome_js_formatter/tests/specs/js/module/import/trailing-comma/import_trailing_comma.js.snap b/crates/rome_js_formatter/tests/specs/js/module/import/trailing-comma/import_trailing_comma.js.snap index 7d1b9c0f0aa..997a27d8f40 100644 --- a/crates/rome_js_formatter/tests/specs/js/module/import/trailing-comma/import_trailing_comma.js.snap +++ b/crates/rome_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 deleted file mode 100644 index bed6e52e3a1..00000000000 --- a/crates/rome_js_formatter/tests/specs/prettier/js/comments/dynamic_imports.js.snap +++ /dev/null @@ -1,69 +0,0 @@ ---- -source: crates/rome_js_formatter/tests/prettier_tests.rs -info: - test_file: js/comments/dynamic_imports.js ---- - -# Input - -```js -import(/* Hello */ 'something') - -import('something' /* Hello */) - -import(/* Hello */ 'something' /* Hello */) - -import('something' /* Hello */ + 'else') - -import( - /* Hello */ - 'something' - /* Hello */ -) - -wrap( - import(/* Hello */ - 'something' - ) -) -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Rome -@@ -8,7 +8,7 @@ - - import( - /* Hello */ -- "something" -+ "something", - /* Hello */ - ); - -``` - -# Output - -```js -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/crates/rome_js_formatter/tests/specs/prettier/js/trailing-comma/dynamic-import.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/trailing-comma/dynamic-import.js.snap deleted file mode 100644 index b3d1674092f..00000000000 --- a/crates/rome_js_formatter/tests/specs/prettier/js/trailing-comma/dynamic-import.js.snap +++ /dev/null @@ -1,39 +0,0 @@ ---- -source: crates/rome_js_formatter/tests/prettier_tests.rs ---- - -# Input - -```js -import( - 'myreallylongdynamicallyloadedmodulenamemyreallylongdynamicallyloadedmodulename' -); -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Rome -@@ -1,3 +1,3 @@ - import( -- "myreallylongdynamicallyloadedmodulenamemyreallylongdynamicallyloadedmodulename" -+ "myreallylongdynamicallyloadedmodulenamemyreallylongdynamicallyloadedmodulename", - ); -``` - -# Output - -```js -import( - "myreallylongdynamicallyloadedmodulenamemyreallylongdynamicallyloadedmodulename", -); -``` - - -# Lines exceeding max width of 80 characters -``` - 2: "myreallylongdynamicallyloadedmodulenamemyreallylongdynamicallyloadedmodulename", -``` -