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

feature(rome_js_formatter): Static Member Expression formatting #2727

Merged
merged 2 commits into from
Jun 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
2 changes: 0 additions & 2 deletions crates/rome_formatter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
//! * [`format_args!`]: Concatenates a sequence of Format objects.
//! * [`write!`]: Writes a sequence of formatable objects into an output buffer.

extern crate core;

mod arguments;
mod buffer;
mod builders;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
use crate::prelude::*;

use rome_formatter::{format_args, write};
use rome_js_syntax::{JsStaticMemberExpression, JsStaticMemberExpressionFields};
use rome_js_syntax::{
JsAnyExpression, JsAssignmentExpression, JsStaticMemberExpression,
JsStaticMemberExpressionFields, JsVariableDeclarator,
};
use rome_rowan::AstNode;

#[derive(Debug, Clone, Default)]
pub struct FormatJsStaticMemberExpression;
Expand All @@ -14,12 +18,85 @@ impl FormatNodeRule<JsStaticMemberExpression> for FormatJsStaticMemberExpression
member,
} = node.as_fields();

write!(
f,
[
object.format(),
group_elements(&format_args![operator_token.format(), member.format()])
]
)
write!(f, [object.format()])?;

let layout = compute_member_layout(node)?;

match layout {
StaticMemberExpressionLayout::NoBreak => {
write!(f, [operator_token.format(), member.format()])
}
StaticMemberExpressionLayout::BreakAfterObject => {
write!(
f,
[group_elements(&indent(&format_args![
soft_line_break(),
operator_token.format(),
member.format(),
]))]
)
}
}
}
}

enum StaticMemberExpressionLayout {
/// Forces that there's no line break between the object, operator, and member
NoBreak,

/// Breaks the static member expression after the object if the whole expression doesn't fit on a single line
BreakAfterObject,
}

fn compute_member_layout(
member: &JsStaticMemberExpression,
) -> FormatResult<StaticMemberExpressionLayout> {
let parent = member.syntax().parent();

let nested = parent
.as_ref()
.map_or(false, |p| JsStaticMemberExpression::can_cast(p.kind()));

if let Some(parent) = &parent {
if JsAssignmentExpression::can_cast(parent.kind())
|| JsVariableDeclarator::can_cast(parent.kind())
{
let no_break = match member.object()? {
JsAnyExpression::JsCallExpression(call_expression) => {
!call_expression.arguments()?.args().is_empty()
}
JsAnyExpression::TsNonNullAssertionExpression(non_null_assertion) => {
match non_null_assertion.expression()? {
JsAnyExpression::JsCallExpression(call_expression) => {
!call_expression.arguments()?.args().is_empty()
}
_ => false,
}
}
_ => false,
};

if no_break {
return Ok(StaticMemberExpressionLayout::NoBreak);
}
}
};

if !nested && matches!(member.object()?, JsAnyExpression::JsIdentifierExpression(_)) {
return Ok(StaticMemberExpressionLayout::NoBreak);
}

let first_non_static_member_ancestor = member
.syntax()
.ancestors()
.find(|parent| !JsStaticMemberExpression::can_cast(parent.kind()));

if matches!(
first_non_static_member_ancestor.and_then(JsAnyExpression::cast),
Some(JsAnyExpression::JsNewExpression(_))
) {
return Ok(StaticMemberExpressionLayout::NoBreak);
}

Ok(StaticMemberExpressionLayout::BreakAfterObject)
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
---
source: crates/rome_js_formatter/tests/spec_test.rs
assertion_line: 257
expression: call.js
---
# Input
Expand Down Expand Up @@ -107,11 +106,7 @@ const composition = (ViewComponent, ContainerComponent) =>

romise.then(
(result) =>
result.veryLongVariable.veryLongPropertyName > someOtherVariable ? "ok" : "fail",
result.veryLongVariable
.veryLongPropertyName > someOtherVariable ? "ok" : "fail",
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 formatting doesn't yet match Prettier's because Rome formats conditional expressions differently. Same for all other tests that involve conditional expressions.

);


## Lines exceeding width of 80 characters

51: result.veryLongVariable.veryLongPropertyName > someOtherVariable ? "ok" : "fail",

Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,8 @@ Quote style: Double Quotes
}

//fluid layout
const otherBrandsWithThisAdjacencyCount123 = Object.values(
edge.to.edges,
).length;
const otherBrandsWithThisAdjacencyCount123 = Object.values(edge.to.edges)
.length;
let vgChannel = pointPositionDefaultRef({ model, defaultPos, channel })();
const bifornCringerMoshedPerplexSawderGlyphsHb = someBigFunctionName(
`foo
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,29 +16,19 @@ Line width: 80
Quote style: Double Quotes
-----
console.log(import.meta);
import.meta.field =
obj.aReallyLongVariableName.andAnotherReallyLongVariableName.andAnotherReallyLongVariableName.andAnotherReallyLongVariable;
import.meta.aReallyLongVariableName.andAnotherReallyLongVariableName.andAnotherReallyLongVariableName.andAnotherReallyLongVariable;


## Lines exceeding width of 80 characters

3: obj.aReallyLongVariableName.andAnotherReallyLongVariableName.andAnotherReallyLongVariableName.andAnotherReallyLongVariable;
4: import.meta.aReallyLongVariableName.andAnotherReallyLongVariableName.andAnotherReallyLongVariableName.andAnotherReallyLongVariable;
import.meta.field = obj.aReallyLongVariableName.andAnotherReallyLongVariableName
.andAnotherReallyLongVariableName.andAnotherReallyLongVariable;
import.meta.aReallyLongVariableName.andAnotherReallyLongVariableName
.andAnotherReallyLongVariableName.andAnotherReallyLongVariable;
## Output 2
-----
Indent style: Spaces, size: 4
Line width: 120
Quote style: Double Quotes
-----
console.log(import.meta);
import.meta.field =
obj.aReallyLongVariableName.andAnotherReallyLongVariableName.andAnotherReallyLongVariableName.andAnotherReallyLongVariable;
import.meta.aReallyLongVariableName.andAnotherReallyLongVariableName.andAnotherReallyLongVariableName.andAnotherReallyLongVariable;


## Lines exceeding width of 120 characters

3: obj.aReallyLongVariableName.andAnotherReallyLongVariableName.andAnotherReallyLongVariableName.andAnotherReallyLongVariable;
4: import.meta.aReallyLongVariableName.andAnotherReallyLongVariableName.andAnotherReallyLongVariableName.andAnotherReallyLongVariable;
import.meta.field = obj.aReallyLongVariableName.andAnotherReallyLongVariableName.andAnotherReallyLongVariableName
.andAnotherReallyLongVariable;
import.meta.aReallyLongVariableName.andAnotherReallyLongVariableName.andAnotherReallyLongVariableName
.andAnotherReallyLongVariable;

Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,9 @@ lorem()[0]().ipsum().looooooooooooooooooooooooooong().looooooooooooooooooooooooo

something()[1]()[3]().items.item.what_else[3]().something().something().then().catcht().else().what_the_hell();

some.member.with.
// rome-ignore format: Verify that formatting calls into right.format()
some.member.with
.
// rome-ignore format: Verify that formatting calls into right.format()
rather.hard.to.test.because.name.doesnt.format.being.ignored;


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ Quote style: Double Quotes
-----
(123).toString;
(123)
/****/.toString;
/****/.toString;
(123) /**/.toString;
(123).toString;

Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ const breakAfterColonObject = {
33333333333333331, "dsadsadasdsadas", 3, "dsadsadasdsadasdsadsadasdsadas", 5
),
"conditional-expression-1":
this.state.longLongLongLongLongLongLongLongLongTooLongProp === true ? {} : {},
this.state
.longLongLongLongLongLongLongLongLongTooLongProp === true ? {} : {},
"conditional-expression-2":
longLongLongLongLongLongLongLongLongLongLongLongLongTooLongVar || 1337 ? {} : {},
"conditional-expression-3":
Expand All @@ -162,7 +163,8 @@ const breakAfterColonObject = {
} in "long-key",

1:
this.state.longLongLongLongLongLlongLongLongLongLongLongLongLongLongTooLongPropongLongLongLongTooLongProp === true,
this.state
.longLongLongLongLongLlongLongLongLongLongLongLongLongLongTooLongPropongLongLongLongTooLongProp === true,
2:
longLongLongLongLongLongLongLongLonlongLongLongLongLongLongLongLongLongTooLongPropgLongLongLongLongTooLongVar || 1337,
11:
Expand All @@ -179,7 +181,8 @@ const breakAfterColonObject = {
),

a:
this.state.longLongLongLongLongLongLongLongLongTooLongProp === true ? {} : {},
this.state
.longLongLongLongLongLongLongLongLongTooLongProp === true ? {} : {},
b:
longLongLongLongLongLongLongLongLongLongLongLongLongTooLongVar || 1337 ? {} : {},
c:
Expand Down Expand Up @@ -249,12 +252,12 @@ const fluidObject = {
28: "12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890",
36: 13321321312312321311332132131231232131232132132132232132132132 + 1332132131231232131232132132132,
38: 1332132131231232131232132132132 - 13321321312312321312321321321321332132131231232131232132132132,
51: longLongLongLongLongLongLongLongLongLongLongLongLongTooLongVar || 1337 ? {} : {},
53: 13321321312312321311332132131231232131232132132132232132132132 + 1332132131231232131232132132132 ? {} : {},
62: this.state.longLongLongLongLongLlongLongLongLongLongLongLongLongLongTooLongPropongLongLongLongTooLongProp === true,
64: longLongLongLongLongLongLongLongLonlongLongLongLongLongLongLongLongLongTooLongPropgLongLongLongLongTooLongVar || 1337,
66: 13321321312312321311332132131231232131232132132132232132132132 + 1332132131231232131232132132132,
68: 1332132131231232131232132132132 - 13321321312312321312321321321321332132131231232131232132132132,
81: longLongLongLongLongLongLongLongLongLongLongLongLongTooLongVar || 1337 ? {} : {},
83: 13321321312312321311332132131231232131232132132132232132132132 + 1332132131231232131232132132132 ? {} : {},
52: longLongLongLongLongLongLongLongLongLongLongLongLongTooLongVar || 1337 ? {} : {},
54: 13321321312312321311332132131231232131232132132132232132132132 + 1332132131231232131232132132132 ? {} : {},
64: .longLongLongLongLongLlongLongLongLongLongLongLongLongLongTooLongPropongLongLongLongTooLongProp === true,
66: longLongLongLongLongLongLongLongLonlongLongLongLongLongLongLongLongLongTooLongPropgLongLongLongLongTooLongVar || 1337,
68: 13321321312312321311332132131231232131232132132132232132132132 + 1332132131231232131232132132132,
70: 1332132131231232131232132132132 - 13321321312312321312321321321321332132131231232131232132132132,
84: longLongLongLongLongLongLongLongLongLongLongLongLongTooLongVar || 1337 ? {} : {},
86: 13321321312312321311332132131231232131232132132132232132132132 + 1332132131231232131232132132132 ? {} : {},

Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
---
source: crates/rome_js_formatter/tests/prettier_tests.rs
assertion_line: 182
expression: arrow_call.js
---
# Input
Expand Down Expand Up @@ -97,13 +96,10 @@ func(

promise.then(
(result) =>
result.veryLongVariable.veryLongPropertyName > someOtherVariable ? "ok" : "fail",
result.veryLongVariable
.veryLongPropertyName > someOtherVariable ? "ok" : "fail",
);

```

# Lines exceeding max width of 80 characters
```
49: result.veryLongVariable.veryLongPropertyName > someOtherVariable ? "ok" : "fail",
```

Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,15 @@ const {_id:id3} = data.createTestMessageWithAReallyLongName.someVeryLongProperty

# Output
```js
const _id1 =
data.createTestMessageWithAReallyLongName.someVeryLongProperty.thisIsAlsoALongProperty._id;
const _id1 = data.createTestMessageWithAReallyLongName.someVeryLongProperty
.thisIsAlsoALongProperty._id;

const { _id2 } =
data.createTestMessageWithAReallyLongName.someVeryLongProperty.thisIsAlsoALongProperty;
const { _id2 } = data.createTestMessageWithAReallyLongName.someVeryLongProperty
.thisIsAlsoALongProperty;

const { _id: id3 } =
data.createTestMessageWithAReallyLongName.someVeryLongProperty.thisIsAlsoALongProperty;
const { _id: id3 } = data.createTestMessageWithAReallyLongName
.someVeryLongProperty.thisIsAlsoALongProperty;

```

# Lines exceeding max width of 80 characters
```
2: data.createTestMessageWithAReallyLongName.someVeryLongProperty.thisIsAlsoALongProperty._id;
5: data.createTestMessageWithAReallyLongName.someVeryLongProperty.thisIsAlsoALongProperty;
8: data.createTestMessageWithAReallyLongName.someVeryLongProperty.thisIsAlsoALongProperty;
```

Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ const aVeryLongNameThatGoesOnAndOn =
this.someObject.someOtherNestedObject =
this.someOtherObject.whyNotNestAnotherOne.someLongFunctionName();

this.isaverylongmethodexpression.withmultiplelevels =
this.isanotherverylongexpression.thatisalsoassigned = 0;
this.isaverylongmethodexpression.withmultiplelevels = this
.isanotherverylongexpression.thatisalsoassigned = 0;

```

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@ if (something) {
# Output
```js
if (something) {
const otherBrandsWithThisAdjacencyCount123 = Object.values(
edge.to.edges,
).length;
const otherBrandsWithThisAdjacencyCount123 = Object.values(edge.to.edges)
.length;
}

```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ something.veeeeeery.looooooooooooooooooooooooooong = some.other.rather.long.chai
# Output
```js
// works as expected
something.veeeeeery.looooooooooooooooooooooooooong =
some.other.rather.long.chain;
something.veeeeeery.looooooooooooooooooooooooooong = some.other.rather.long
.chain;

// does not work if it ends with a function call
something.veeeeeery.looooooooooooooooooooooooooong =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ const pendingIndicatorz =

# Output
```js
const pendingIndicators =
shield.alarmGeneratorConfiguration.getPendingVersionColumnValues;
const pendingIndicators = shield.alarmGeneratorConfiguration
.getPendingVersionColumnValues;

const pendingIndicatorz =
shield.alarmGeneratorConfiguration.getPendingVersionColumnValues();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,20 @@ const loooooooooooooooooooooooooong8 = !!"looooooooooooooooooooooooooooooooooooo

# Output
```js
const loooooooooooooooooooooooooong1 =
void looooooooooooooong.looooooooooooooong.loooooong;
const loooooooooooooooooooooooooong1 = void looooooooooooooong
.looooooooooooooong.loooooong;
const loooooooooooooooooooooooooong2 =
void "looooooooooooooooooooooooooooooooooooooooooog";
const loooooooooooooooooooooooooong3 =
!looooooooooooooong.looooooooooooooong.loooooong;
const loooooooooooooooooooooooooong3 = !looooooooooooooong.looooooooooooooong
.loooooong;
const loooooooooooooooooooooooooong4 =
!"looooooooooooooooooooooooooooooooooooooooooog";
const loooooooooooooooooooooooooong5 =
void void looooooooooooooong.looooooooooooooong.loooooong;
const loooooooooooooooooooooooooong5 = void void looooooooooooooong
.looooooooooooooong.loooooong;
const loooooooooooooooooooooooooong6 =
void void "looooooooooooooooooooooooooooooooooooooooooog";
const loooooooooooooooooooooooooong7 =
!!looooooooooooooong.looooooooooooooong.loooooong;
const loooooooooooooooooooooooooong7 = !!looooooooooooooong.looooooooooooooong
.loooooong;
const loooooooooooooooooooooooooong8 =
!!"looooooooooooooooooooooooooooooooooooooooooog";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ obj?.foo.bar?.baz; // Only access `foo` if `obj` exists, and `baz` if
// `bar` exists

// Example usage with bracket notation:
// rome-ignore format: https://github.com/rome/tools/issues/2768
obj?.['foo']?.bar?.baz // 42

const obj2 = {
Expand Down
Loading