feat(formatter): Initial version of hard line breaks in object expres…

…sions. (#2458) This is the initial draft for respecting a newline in an object expression (or import specifier) in between an opening curly brace { and the first item in the member list. This should make Rome match prettier in this situation.
rome · May 23, 2022 · e044cfc · e044cfc
1 parent b3de775
commit e044cfc
Show file tree

Hide file tree

Showing 100 changed files with 692 additions and 243 deletions.
diff --git a/crates/rome_js_formatter/src/js/expressions/object_expression.rs b/crates/rome_js_formatter/src/js/expressions/object_expression.rs
@@ -1,5 +1,5 @@
 use crate::prelude::*;
-
+use crate::utils::has_leading_newline;
 use crate::FormatNodeFields;
 use rome_js_syntax::JsObjectExpression;
 use rome_js_syntax::JsObjectExpressionFields;
@@ -15,13 +15,19 @@ impl FormatNodeFields<JsObjectExpression> for FormatNodeRule<JsObjectExpression>
             r_curly_token,
         } = node.as_fields();
 
+        let has_newline = has_leading_newline(members.syntax());
         let members_content = formatted![formatter, [members.format()]]?;
 
         if members.is_empty() {
             formatter
                 .delimited(&l_curly_token?, members_content, &r_curly_token?)
                 .soft_block_indent()
                 .finish()
+        } else if has_newline {
+            formatter
+                .delimited(&l_curly_token?, members_content, &r_curly_token?)
+                .block_indent()
+                .finish()
         } else {
             formatter
                 .delimited(&l_curly_token?, members_content, &r_curly_token?)

diff --git a/crates/rome_js_formatter/src/js/module/export_named_from_clause.rs b/crates/rome_js_formatter/src/js/module/export_named_from_clause.rs
@@ -1,10 +1,11 @@
 use crate::prelude::*;
-
 use crate::utils::format_with_semicolon;
+use crate::utils::has_leading_newline;
 
 use crate::FormatNodeFields;
 use rome_js_syntax::JsExportNamedFromClause;
 use rome_js_syntax::JsExportNamedFromClauseFields;
+use rome_rowan::AstNode;
 
 impl FormatNodeFields<JsExportNamedFromClause> for FormatNodeRule<JsExportNamedFromClause> {
     fn format_fields(
@@ -22,14 +23,25 @@ impl FormatNodeFields<JsExportNamedFromClause> for FormatNodeRule<JsExportNamedF
             semicolon_token,
         } = node.as_fields();
 
-        let list = formatter
-            .delimited(
-                &l_curly_token?,
-                formatted![formatter, [specifiers.format()]]?,
-                &r_curly_token?,
-            )
-            .soft_block_spaces()
-            .finish()?;
+        let list = if has_leading_newline(specifiers.syntax()) {
+            formatter
+                .delimited(
+                    &l_curly_token?,
+                    formatted![formatter, [specifiers.format()]]?,
+                    &r_curly_token?,
+                )
+                .block_indent()
+                .finish()?
+        } else {
+            formatter
+                .delimited(
+                    &l_curly_token?,
+                    formatted![formatter, [specifiers.format()]]?,
+                    &r_curly_token?,
+                )
+                .soft_block_spaces()
+                .finish()?
+        };
 
         format_with_semicolon(
             formatter,

diff --git a/crates/rome_js_formatter/src/ts/declarations/enum_declaration.rs b/crates/rome_js_formatter/src/ts/declarations/enum_declaration.rs
@@ -1,6 +1,8 @@
 use crate::prelude::*;
+use crate::utils::has_leading_newline;
 use crate::FormatNodeFields;
 use rome_js_syntax::{TsEnumDeclaration, TsEnumDeclarationFields};
+use rome_rowan::AstNode;
 
 impl FormatNodeFields<TsEnumDeclaration> for FormatNodeRule<TsEnumDeclaration> {
     fn format_fields(
@@ -16,11 +18,16 @@ impl FormatNodeFields<TsEnumDeclaration> for FormatNodeRule<TsEnumDeclaration> {
             r_curly_token,
         } = node.as_fields();
 
+        let has_newline = has_leading_newline(members.syntax());
         let list = formatter
             .delimited(
                 &l_curly_token?,
                 join_elements(
-                    soft_line_break_or_space(),
+                    if has_newline {
+                        hard_line_break()
+                    } else {
+                        soft_line_break_or_space()
+                    },
                     formatter.format_separated(&members, || token(","))?,
                 ),
                 &r_curly_token?,

diff --git a/crates/rome_js_formatter/src/ts/types/object_type.rs b/crates/rome_js_formatter/src/ts/types/object_type.rs
@@ -1,4 +1,5 @@
 use crate::prelude::*;
+use crate::utils::has_leading_newline;
 use crate::FormatNodeFields;
 use rome_js_syntax::{TsObjectType, TsObjectTypeFields};
 
@@ -13,13 +14,24 @@ impl FormatNodeFields<TsObjectType> for FormatNodeRule<TsObjectType> {
             r_curly_token,
         } = node.as_fields();
 
-        formatter
-            .delimited(
-                &l_curly_token?,
-                formatted![formatter, [members.format()]]?,
-                &r_curly_token?,
-            )
-            .soft_block_spaces()
-            .finish()
+        if has_leading_newline(members.syntax()) {
+            formatter
+                .delimited(
+                    &l_curly_token?,
+                    formatted![formatter, [members.format()]]?,
+                    &r_curly_token?,
+                )
+                .block_indent()
+                .finish()
+        } else {
+            formatter
+                .delimited(
+                    &l_curly_token?,
+                    formatted![formatter, [members.format()]]?,
+                    &r_curly_token?,
+                )
+                .soft_block_spaces()
+                .finish()
+        }
     }
 }
diff --git a/crates/rome_js_formatter/src/utils/mod.rs b/crates/rome_js_formatter/src/utils/mod.rs
@@ -105,6 +105,18 @@ pub(crate) fn has_formatter_trivia(node: &JsSyntaxNode) -> bool {
     false
 }
 
+/// Returns true if this node contains newlines in trivias.
+pub(crate) fn has_leading_newline(node: &JsSyntaxNode) -> bool {
+    if let Some(leading_trivia) = node.first_leading_trivia() {
+        for piece in leading_trivia.pieces() {
+            if piece.is_newline() {
+                return true;
+            }
+        }
+    }
+    false
+}
+
 /// Format an element with a single line head and a body that might
 /// be either a block or a single statement
 ///

diff --git a/crates/rome_js_formatter/tests/specs/js/module/expression/member-chain/computed.js.snap b/crates/rome_js_formatter/tests/specs/js/module/expression/member-chain/computed.js.snap
@@ -19,5 +19,10 @@ Quote style: Double Quotes
 -----
 nock(/test/)
 	.matchHeader("Accept", "application/json")[httpMethodNock(method)]("/foo")
-	.reply(200, { foo: "bar" });
+	.reply(
+		200,
+		{
+			foo: "bar",
+		},
+	);
 
diff --git a/crates/rome_js_formatter/tests/specs/js/module/expression/member-chain/inline-merge.js.snap b/crates/rome_js_formatter/tests/specs/js/module/expression/member-chain/inline-merge.js.snap
@@ -28,7 +28,11 @@ _.flatMap(this.visibilityHandlers, (fn) => fn())
 	.concat(this.record.resolved_legacy_visrules)
 	.filter(Boolean);
 
-Object.keys(availableLocales({ test: true })).forEach(
+Object.keys(
+	availableLocales({
+		test: true,
+	}),
+).forEach(
 	(locale) => {
 		// ...
 	},

diff --git a/crates/rome_js_formatter/tests/specs/js/module/import/import_specifiers.js b/crates/rome_js_formatter/tests/specs/js/module/import/import_specifiers.js
@@ -1,5 +1,8 @@
 import { hey } from "hey"
 import { hey } from "hey";
+import {
+    apple,
+banana } from "fruits";
 import {test} from "foo.json" assert { for: "for" }
 import { // some funky comment
     loooooooooooooooooooong as moreeeeeeloooooooooooooooooooong,

diff --git a/crates/rome_js_formatter/tests/specs/js/module/import/import_specifiers.js.snap b/crates/rome_js_formatter/tests/specs/js/module/import/import_specifiers.js.snap
@@ -5,6 +5,9 @@ expression: import_specifiers.js
 # Input
 import { hey } from "hey"
 import { hey } from "hey";
+import {
+    apple,
+banana } from "fruits";
 import {test} from "foo.json" assert { for: "for" }
 import { // some funky comment
     loooooooooooooooooooong as moreeeeeeloooooooooooooooooooong,
@@ -30,6 +33,7 @@ Quote style: Double Quotes
 -----
 import { hey } from "hey";
 import { hey } from "hey";
+import { apple, banana } from "fruits";
 import { test } from "foo.json" assert { for: "for" };
 import {
 	// some funky comment

diff --git a/crates/rome_js_formatter/tests/specs/js/module/object/getter_setter.js.snap b/crates/rome_js_formatter/tests/specs/js/module/object/getter_setter.js.snap
@@ -18,6 +18,10 @@ Indent style: Tab
 Line width: 80
 Quote style: Double Quotes
 -----
-let a = { get foo() {} };
-let b = { set foo(a) {} };
+let a = {
+	get foo() {},
+};
+let b = {
+	set foo(a) {},
+};
 
diff --git a/crates/rome_js_formatter/tests/specs/js/module/object/object.js b/crates/rome_js_formatter/tests/specs/js/module/object/object.js
@@ -19,3 +19,14 @@ let a = {
 
 	...spread,
 }
+
+const x = {apple: "banana"};
+
+const y = {
+	apple: "banana",
+};
+
+({a, b, c} = {a: 'apple', b: 'banana', c: 'coconut'});
+
+({
+	a, b, c} = {a: 'apple', b: 'banana', c: 'coconut'});
diff --git a/crates/rome_js_formatter/tests/specs/js/module/object/object.js.snap b/crates/rome_js_formatter/tests/specs/js/module/object/object.js.snap
@@ -25,6 +25,17 @@ let a = {
 	...spread,
 }
 
+const x = {apple: "banana"};
+
+const y = {
+	apple: "banana",
+};
+
+({a, b, c} = {a: 'apple', b: 'banana', c: 'coconut'});
+
+({
+	a, b, c} = {a: 'apple', b: 'banana', c: 'coconut'});
+
 =============================
 # Outputs
 ## Output 1
@@ -53,3 +64,13 @@ let a = {
 	...spread,
 };
 
+const x = { apple: "banana" };
+
+const y = {
+	apple: "banana",
+};
+
+({ a, b, c } = { a: "apple", b: "banana", c: "coconut" });
+
+({ a, b, c } = { a: "apple", b: "banana", c: "coconut" });
+
diff --git a/crates/rome_js_formatter/tests/specs/js/module/object/property_key.js.snap b/crates/rome_js_formatter/tests/specs/js/module/object/property_key.js.snap
@@ -22,6 +22,10 @@ Quote style: Double Quotes
 -----
 const foo = {
 	"foo-bar": true,
-	"bar": { "lorem_ispsum": { "lorem-ipsum": true } },
+	"bar": {
+		"lorem_ispsum": {
+			"lorem-ipsum": true,
+		},
+	},
 };
 
diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/assignment/call-with-template.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/assignment/call-with-template.js.snap
@@ -1,8 +1,6 @@
 ---
 source: crates/rome_js_formatter/tests/prettier_tests.rs
-assertion_line: 57
 expression: call-with-template.js
-
 ---
 # Input
 ```js
@@ -25,9 +23,13 @@ const result = template(
   `
   if (SOME_VAR === "") {}
 `,
-)({ SOME_VAR: value });
+)({
+  SOME_VAR: value,
+});
 
-const output = template(`function f() %%A%%`)({ A: t.blockStatement([]) });
+const output = template(`function f() %%A%%`)({
+  A: t.blockStatement([]),
+});
 
 ```
 

diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/assignment/lone-arg.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/assignment/lone-arg.js.snap
@@ -1,8 +1,6 @@
 ---
 source: crates/rome_js_formatter/tests/prettier_tests.rs
-assertion_line: 57
 expression: lone-arg.js
-
 ---
 # Input
 ```js
@@ -31,7 +29,11 @@ const bifornCringerMoshedPerplexSawderGlyphsHb = someBigFunctionName(`foo
 
 # Output
 ```js
-let vgChannel = pointPositionDefaultRef({ model, defaultPos, channel })();
+let vgChannel = pointPositionDefaultRef({
+  model,
+  defaultPos,
+  channel,
+})();
 
 let vgChannel2 = pointPositionDefaultRef({ model, defaultPos, channel })();
 

diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/async/async-shorthand-method.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/async/async-shorthand-method.js.snap
@@ -1,8 +1,6 @@
 ---
 source: crates/rome_js_formatter/tests/prettier_tests.rs
-assertion_line: 57
 expression: async-shorthand-method.js
-
 ---
 # Input
 ```js
@@ -15,7 +13,10 @@ expression: async-shorthand-method.js
 
 # Output
 ```js
-({ async get() {}, async set() {} });
+({
+  async get() {},
+  async set() {},
+});
 
 ```
 

diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/babel-plugins/optional-chaining.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/babel-plugins/optional-chaining.js.snap
@@ -1,8 +1,6 @@
 ---
 source: crates/rome_js_formatter/tests/prettier_tests.rs
-assertion_line: 121
 expression: optional-chaining.js
-
 ---
 # Input
 ```js
@@ -75,7 +73,13 @@ const ret = delete obj?.foo?.bar?.baz; // true
 ```js
 // https://babeljs.io/docs/en/babel-plugin-proposal-optional-chaining
 
-const obj = { foo: { bar: { baz: 42 } } };
+const obj = {
+  foo: {
+    bar: {
+      baz: 42,
+    },
+  },
+};
 
 const baz = obj?.foo?.bar?.baz; // 42
 
@@ -113,9 +117,19 @@ test?.(); // 42
 
 exists?.(); // undefined
 
-const obj3 = { foo: { bar: { baz: class {} } } };
+const obj3 = {
+  foo: {
+    bar: {
+      baz: class {},
+    },
+  },
+};
 
-const obj4 = { foo: { bar: {} } };
+const obj4 = {
+  foo: {
+    bar: {},
+  },
+};
 
 const ret = delete obj?.foo?.bar?.baz; // true