fix(rome_js_formatter): breaking logic for member chains
MODIFY
crates/rome_js_formatter/src/utils/member_chain/flatten_item.rs
crates/rome_js_formatter/src/utils/member_chain/flatten_item.rs
@@ -73,6 +73,17 @@ impl FlattenItem {
}
}
+ pub(crate) fn has_leading_comments(&self) -> SyntaxResult<bool> {
+ Ok(match self {
+ FlattenItem::StaticMember(node) => {
+ node.syntax().has_comments_direct() || node.operator_token()?.has_leading_comments()
+ }
+ FlattenItem::CallExpression(node) => node.syntax().has_leading_comments(),
+ FlattenItem::ComputedMember(node) => node.syntax().has_leading_comments(),
+ FlattenItem::Node(node) => node.has_leading_comments(),
+ })
+ }
+
/// There are cases like Object.keys(), Observable.of(), _.values() where
/// they are the subject of all the chained calls and therefore should
/// be kept on the same line:

MODIFY
crates/rome_js_formatter/src/utils/member_chain/groups.rs
crates/rome_js_formatter/src/utils/member_chain/groups.rs
+use crate::context::TabWidth;
use crate::prelude::*;
use crate::utils::member_chain::flatten_item::FlattenItem;
use crate::utils::member_chain::simple_argument::SimpleArgument;
-
-use crate::context::TabWidth;
use rome_js_syntax::JsCallExpression;
use rome_rowan::{AstSeparatedList, SyntaxResult};
use std::mem;
@@ -78,7 +77,11 @@ impl Groups {
}
/// It tells if the groups should be break on multiple lines
- pub fn groups_should_break(&self, calls_count: usize) -> SyntaxResult<bool> {
+ pub fn groups_should_break(
+ &self,
+ calls_count: usize,
+ head_group: &HeadGroup,
+ ) -> FormatResult<bool> {
// Do not allow the group to break if it only contains a single call expression
if calls_count <= 1 {
return Ok(false);
@@ -90,7 +93,50 @@ impl Groups {
let call_expressions_are_not_simple =
calls_count > 2 && self.call_expressions_are_not_simple()?;
- Ok(call_expressions_are_not_simple)
+ // TODO: add here will_break logic
+
+ let node_has_comments = self.has_comments()? || head_group.has_comments();
+
+ let should_break = node_has_comments || call_expressions_are_not_simple;
+
+ Ok(should_break)
+ }
+
+ /// Checks if the groups contain comments.
+ pub fn has_comments(&self) -> SyntaxResult<bool> {
+ let mut has_leading_comments = false;
+
+ let flat_groups = self.groups.iter().flat_map(|item| item.iter());
+ for item in flat_groups {
+ if item.has_leading_comments()? {
+ has_leading_comments = true;
+ break;
+ }
+ }
+
+ let has_trailing_comments = self
+ .groups
+ .iter()
+ .flat_map(|item| item.iter())
+ .any(|item| item.has_trailing_comments());
+
+ let cutoff_has_leading_comments = if self.groups.len() >= self.cutoff as usize {
+ let group = self.groups.get(self.cutoff as usize);
+ if let Some(group) = group {
+ let first_item = group.first();
+ if let Some(first_item) = first_item {
+ first_item.has_leading_comments()?
+ } else {
+ false
+ }
+ } else {
+ false
+ }
+ } else {
+ false
+ };
+
+ Ok(has_leading_comments || has_trailing_comments || cutoff_has_leading_comments)
}
/// Format groups on multiple lines
@@ -109,6 +155,10 @@ impl Groups {
///
/// It's up to the printer to decide which one to use.
pub fn write(&self, f: &mut JsFormatter) -> FormatResult<()> {
+ if self.groups.is_empty() {
+ return Ok(());
+ }
+
f.join()
.entries(self.groups.iter().flat_map(|group| group.iter()))
.finish()
@@ -219,6 +269,10 @@ impl HeadGroup {
pub fn expand_group(&mut self, group: Vec<FlattenItem>) {
self.items.extend(group)
}
+
+ fn has_comments(&self) -> bool {
+ self.items.iter().any(|item| item.has_trailing_comments())
+ }
}
impl Format<JsFormatContext> for HeadGroup {

MODIFY
crates/rome_js_formatter/src/utils/member_chain/mod.rs
crates/rome_js_formatter/src/utils/member_chain/mod.rs
@@ -294,7 +294,7 @@ fn write_groups(
// TODO use Alternatives once available
write!(f, [head_group])?;
- if groups.groups_should_break(calls_count)? {
+ if groups.groups_should_break(calls_count, &head_group)? {
write!(
f,
[indent(&format_args!(

MODIFY
crates/rome_js_formatter/tests/specs/js/module/arrow/call.js.snap
crates/rome_js_formatter/tests/specs/js/module/arrow/call.js.snap
---
source: crates/rome_js_formatter/tests/spec_test.rs
+assertion_line: 256
expression: call.js
---
# Input
@@ -77,9 +78,8 @@ expect(
);
// .toThrowError(/Required parameter/);
-expect(
- () => asyncRequest({ type: "foo", url: "/test-endpoint" }),
-).not.toThrowError();
+expect(() => asyncRequest({ type: "foo", url: "/test-endpoint" })).not
+ .toThrowError();
expect(
() => asyncRequest({ type: "foo", url: "/test-endpoint-but-with-a-long-url" }),

MODIFY
crates/rome_js_formatter/tests/specs/prettier/js/function-first-param/function_expression.js.snap
crates/rome_js_formatter/tests/specs/prettier/js/function-first-param/function_expression.js.snap
@@ -35,7 +35,9 @@ db.collection('indexOptionDefault').createIndex({ a: 1 }, {
# Output
```js
//https://github.com/prettier/prettier/issues/3002
-beep.boop().baz("foo", {
+beep
+ .boop()
+ .baz("foo", {
some: {
thing: {
nested: true,
@@ -44,7 +46,9 @@ beep.boop().baz("foo", {
}, { another: { thing: true } }, () => {});
//https://github.com/prettier/prettier/issues/2984
-db.collection("indexOptionDefault").createIndex({ a: 1 }, {
+db
+ .collection("indexOptionDefault")
+ .createIndex({ a: 1 }, {
indexOptionDefaults: true,
w: 2,
wtimeout: 1000,

MODIFY
crates/rome_js_formatter/tests/specs/prettier/js/method-chain/comment.js.snap
crates/rome_js_formatter/tests/specs/prettier/js/method-chain/comment.js.snap
@@ -76,44 +76,43 @@ _.a(a)
_.a(
a,
-) /* very very very very very very very long such that it is longer than 80 columns */.a();
+) /* very very very very very very very long such that it is longer than 80 columns */
+ .a();
_.a(
a,
-) /* very very very very very very very long such that it is longer than 80 columns */.a();
+) /* very very very very very very very long such that it is longer than 80 columns */
+ .a();
Something
// $FlowFixMe(>=0.41.0)
.getInstance(this.props.dao).getters();
// Warm-up first
-measure().then(() => {
+measure()
+ .then(() => {
SomethingLong();
});
-measure() // Warm-up first
+measure()
+ // Warm-up first
.then(() => {
SomethingLong();
});
-const configModel = this.baseConfigurationService.getCache().consolidated.merge(
- // global/default values (do NOT modify)
- this.cachedWorkspaceConfig,
-);
+const configModel = this.baseConfigurationService
+ .getCache()
+ .consolidated // global/default values (do NOT modify)
+ .merge(this.cachedWorkspaceConfig);
-this.doWriteConfiguration(
- target,
- value,
- options,
-) // queue up writes to prevent race conditions
+this.doWriteConfiguration(target, value, options)
+ // queue up writes to prevent race conditions
.then(
() => null,
(error) => {
- return options.donotNotifyError ? TPromise.wrapError(error) : this.onError(
+ return options.donotNotifyError ? TPromise.wrapError(
error,
- target,
- value,
- );
+ ) : this.onError(error, target, value);
},
);
@@ -121,7 +120,8 @@ ret = __DEV__ ?
// $FlowFixMe: this type differs according to the env
vm.runInContext(source, ctx) : a;
-angular.module("AngularAppModule")
+angular
+ .module("AngularAppModule")
// Hello, I am comment.
.constant("API_URL", "http://localhost:8080/api");
@@ -131,7 +131,7 @@ angular.module("AngularAppModule")
```
3: // Debounce manually rather than using editor.onDidStopChanging so that the debounce time is
9: /* very very very very very very very long such that it is longer than 80 columns */
- 14: ) /* very very very very very very very long such that it is longer than 80 columns */.a();
- 18: ) /* very very very very very very very long such that it is longer than 80 columns */.a();
+ 14: ) /* very very very very very very very long such that it is longer than 80 columns */
+ 19: ) /* very very very very very very very long such that it is longer than 80 columns */
```

MODIFY
crates/rome_js_formatter/tests/specs/prettier/js/method-chain/issue-4125.js.snap
crates/rome_js_formatter/tests/specs/prettier/js/method-chain/issue-4125.js.snap
@@ -180,8 +180,11 @@ somePromise
// you can still force multi-line chaining with a comment:
const sha256_2 = (data) =>
- crypto // breakme
- .createHash("sha256").update(data).digest("hex");
+ crypto
+ // breakme
+ .createHash("sha256")
+ .update(data)
+ .digest("hex");
// examples from https://github.com/prettier/prettier/pull/4765
@@ -241,10 +244,10 @@ const date = moment.utc(userInput).hour(0).minute(0).second(0);
fetchUser(id).then(fetchAccountForUser).catch(handleFetchError);
-fetchUser(
- id,
-) //
-.then(fetchAccountForUser).catch(handleFetchError);
+fetchUser(id)
+ //
+ .then(fetchAccountForUser)
+ .catch(handleFetchError);
// examples from https://github.com/prettier/prettier/issues/3107

MODIFY
crates/rome_js_formatter/tests/specs/prettier/js/optional-chaining/comments.js.snap
crates/rome_js_formatter/tests/specs/prettier/js/optional-chaining/comments.js.snap
---
source: crates/rome_js_formatter/tests/prettier_tests.rs
+assertion_line: 182
expression: comments.js
---
# Input
@@ -56,7 +57,9 @@ foo.bar.baz
# Output
```js
function foo() {
- return a.b().c()
+ return a
+ .b()
+ .c()
// Comment
?.d();
}

MODIFY
crates/rome_js_formatter/tests/specs/prettier/typescript/method-chain/comment.ts.snap
crates/rome_js_formatter/tests/specs/prettier/typescript/method-chain/comment.ts.snap
@@ -19,7 +19,8 @@ this.firebase.object(`/shops/${shopLocation.shop}`)
# Output
```js
-this.firebase.object(`/shops/${shopLocation.shop}`)
+this.firebase
+ .object(`/shops/${shopLocation.shop}`)
// keep distance info
.first((
shop: ShopQueryResult,

fix(rome_js_formatter): breaking logic for member chains (#2800)