Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

feat(rome_js_formatter): Hard line breaks in object expressions. #2458

Merged
merged 1 commit into from
May 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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?)
Expand Down
30 changes: 21 additions & 9 deletions crates/rome_js_formatter/src/js/module/export_named_from_clause.rs
Original file line number Diff line number Diff line change
@@ -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(
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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(
Expand All @@ -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?,
Expand Down
28 changes: 20 additions & 8 deletions crates/rome_js_formatter/src/ts/types/object_type.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::prelude::*;
use crate::utils::has_leading_newline;
use crate::FormatNodeFields;
use rome_js_syntax::{TsObjectType, TsObjectTypeFields};

Expand All @@ -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()
}
}
}
12 changes: 12 additions & 0 deletions crates/rome_js_formatter/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
///
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
);

Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
// ...
},
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {},
};
Comment on lines +21 to +26
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This matches the input in getter_setter.js.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That file is the source code, you should check the file that finishes with .prettier-snap.

Check their playground


11 changes: 11 additions & 0 deletions crates/rome_js_formatter/tests/specs/js/module/object/object.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'});
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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" });

Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},
};

Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
---
source: crates/rome_js_formatter/tests/prettier_tests.rs
assertion_line: 57
expression: call-with-template.js

---
# Input
```js
Expand All @@ -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([]),
});

```

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
---
source: crates/rome_js_formatter/tests/prettier_tests.rs
assertion_line: 57
expression: lone-arg.js

---
# Input
```js
Expand Down Expand Up @@ -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 })();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
---
source: crates/rome_js_formatter/tests/prettier_tests.rs
assertion_line: 57
expression: async-shorthand-method.js

---
# Input
```js
Expand All @@ -15,7 +13,10 @@ expression: async-shorthand-method.js

# Output
```js
({ async get() {}, async set() {} });
({
async get() {},
async set() {},
});

```

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
---
source: crates/rome_js_formatter/tests/prettier_tests.rs
assertion_line: 121
expression: optional-chaining.js

---
# Input
```js
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down
Loading