feat(rome_js_formatter): Computed member assignment formatting

This PR improves the formatting of computed member assignment to match the formatting of computed member expressions.
rome · Aug 15, 2022 · e63306b · e63306b
1 parent 8358d25
commit e63306b
Show file tree

Hide file tree

Showing 5 changed files with 103 additions and 117 deletions.
diff --git a/crates/rome_js_formatter/src/js/assignments/computed_member_assignment.rs b/crates/rome_js_formatter/src/js/assignments/computed_member_assignment.rs
@@ -1,8 +1,7 @@
 use crate::prelude::*;
-use rome_formatter::write;
 
+use crate::js::expressions::computed_member_expression::JsAnyComputedMemberLike;
 use rome_js_syntax::JsComputedMemberAssignment;
-use rome_js_syntax::JsComputedMemberAssignmentFields;
 
 #[derive(Debug, Clone, Default)]
 pub struct FormatJsComputedMemberAssignment;
@@ -13,21 +12,6 @@ impl FormatNodeRule<JsComputedMemberAssignment> for FormatJsComputedMemberAssign
         node: &JsComputedMemberAssignment,
         f: &mut JsFormatter,
     ) -> FormatResult<()> {
-        let JsComputedMemberAssignmentFields {
-            object,
-            l_brack_token,
-            member,
-            r_brack_token,
-        } = node.as_fields();
-
-        write!(
-            f,
-            [
-                object.format(),
-                l_brack_token.format(),
-                member.format(),
-                r_brack_token.format(),
-            ]
-        )
+        JsAnyComputedMemberLike::from(node.clone()).fmt(f)
     }
 }
diff --git a/crates/rome_js_formatter/src/js/expressions/computed_member_expression.rs b/crates/rome_js_formatter/src/js/expressions/computed_member_expression.rs
@@ -1,8 +1,11 @@
 use crate::prelude::*;
 
 use rome_formatter::{format_args, write};
-use rome_js_syntax::JsComputedMemberExpression;
-use rome_js_syntax::JsComputedMemberExpressionFields;
+use rome_js_syntax::JsSyntaxToken;
+use rome_js_syntax::{
+    JsAnyExpression, JsAnyLiteralExpression, JsComputedMemberAssignment, JsComputedMemberExpression,
+};
+use rome_rowan::{declare_node_union, SyntaxResult};
 
 #[derive(Debug, Clone, Default)]
 pub struct FormatJsComputedMemberExpression;
@@ -13,26 +16,91 @@ impl FormatNodeRule<JsComputedMemberExpression> for FormatJsComputedMemberExpres
         node: &JsComputedMemberExpression,
         f: &mut JsFormatter,
     ) -> FormatResult<()> {
-        let JsComputedMemberExpressionFields {
-            object,
-            optional_chain_token,
-            l_brack_token,
-            member,
-            r_brack_token,
-        } = node.as_fields();
-
-        write![
-            f,
-            [
-                object.format(),
-                group(&format_args![
-                    optional_chain_token.format(),
-                    l_brack_token.format(),
-                    soft_line_break(),
-                    soft_block_indent(&member.format()),
-                    r_brack_token.format()
-                ]),
-            ]
-        ]
+        JsAnyComputedMemberLike::from(node.clone()).fmt(f)
+    }
+}
+
+declare_node_union! {
+    pub(crate) JsAnyComputedMemberLike = JsComputedMemberExpression | JsComputedMemberAssignment
+}
+
+impl Format<JsFormatContext> for JsAnyComputedMemberLike {
+    fn fmt(&self, f: &mut Formatter<JsFormatContext>) -> FormatResult<()> {
+        write!(f, [self.object().format()])?;
+
+        match self.member()? {
+            JsAnyExpression::JsAnyLiteralExpression(
+                JsAnyLiteralExpression::JsNumberLiteralExpression(literal),
+            ) => {
+                write!(
+                    f,
+                    [
+                        self.optional_chain_token().format(),
+                        self.l_brack_token().format(),
+                        literal.format(),
+                        self.r_brack_token().format()
+                    ]
+                )
+            }
+            member => {
+                write![
+                    f,
+                    [group(&format_args![
+                        self.optional_chain_token().format(),
+                        self.l_brack_token().format(),
+                        soft_line_break(),
+                        soft_block_indent(&member.format()),
+                        self.r_brack_token().format()
+                    ]),]
+                ]
+            }
+        }
+    }
+}
+
+impl JsAnyComputedMemberLike {
+    fn object(&self) -> SyntaxResult<JsAnyExpression> {
+        match self {
+            JsAnyComputedMemberLike::JsComputedMemberExpression(expression) => expression.object(),
+            JsAnyComputedMemberLike::JsComputedMemberAssignment(assignment) => assignment.object(),
+        }
+    }
+
+    fn l_brack_token(&self) -> SyntaxResult<JsSyntaxToken> {
+        match self {
+            JsAnyComputedMemberLike::JsComputedMemberExpression(expression) => {
+                expression.l_brack_token()
+            }
+            JsAnyComputedMemberLike::JsComputedMemberAssignment(assignment) => {
+                assignment.l_brack_token()
+            }
+        }
+    }
+
+    fn optional_chain_token(&self) -> Option<JsSyntaxToken> {
+        match self {
+            JsAnyComputedMemberLike::JsComputedMemberExpression(expression) => {
+                expression.optional_chain_token()
+            }
+            JsAnyComputedMemberLike::JsComputedMemberAssignment(_) => None,
+        }
+    }
+
+    fn member(&self) -> SyntaxResult<JsAnyExpression> {
+        match self {
+            JsAnyComputedMemberLike::JsComputedMemberExpression(expression) => expression.member(),
+            JsAnyComputedMemberLike::JsComputedMemberAssignment(assignment) => assignment.member(),
+        }
+    }
+
+    fn r_brack_token(&self) -> SyntaxResult<JsSyntaxToken> {
+        match self {
+            JsAnyComputedMemberLike::JsComputedMemberExpression(expression) => {
+                expression.r_brack_token()
+            }
+            JsAnyComputedMemberLike::JsComputedMemberAssignment(assignment) => {
+                assignment.r_brack_token()
+            }
+        }
     }
 }
diff --git a/crates/rome_js_formatter/src/js/expressions/static_member_expression.rs b/crates/rome_js_formatter/src/js/expressions/static_member_expression.rs
@@ -1,5 +1,6 @@
 use crate::prelude::*;
 
+use crate::js::expressions::computed_member_expression::JsAnyComputedMemberLike;
 use rome_formatter::{format_args, write};
 use rome_js_syntax::{
     JsAnyAssignment, JsAnyAssignmentPattern, JsAnyExpression, JsAnyName, JsAssignmentExpression,
@@ -16,6 +17,7 @@ impl FormatNodeRule<JsStaticMemberExpression> for FormatJsStaticMemberExpression
     }
 }
 
+#[derive(Debug, Copy, Clone)]
 enum StaticMemberLikeLayout {
     /// Forces that there's no line break between the object, operator, and member
     NoBreak,
@@ -106,6 +108,7 @@ impl JsAnyStaticMemberLike {
                 }
 
                 JsAnyStaticMemberLike::can_cast(parent.kind())
+                    || JsAnyComputedMemberLike::can_cast(parent.kind())
             }
             None => false,
         };
@@ -114,10 +117,10 @@ impl JsAnyStaticMemberLike {
             return Ok(StaticMemberLikeLayout::NoBreak);
         }
 
-        let first_non_static_member_ancestor = self
-            .syntax()
-            .ancestors()
-            .find(|parent| !JsAnyStaticMemberLike::can_cast(parent.kind()));
+        let first_non_static_member_ancestor = self.syntax().ancestors().find(|parent| {
+            !JsAnyStaticMemberLike::can_cast(parent.kind())
+                || JsAnyComputedMemberLike::can_cast(parent.kind())
+        });
 
         let layout = match first_non_static_member_ancestor.and_then(JsAnyExpression::cast) {
             Some(JsAnyExpression::JsNewExpression(_)) => StaticMemberLikeLayout::NoBreak,

diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/method-chain/bracket_0.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/method-chain/bracket_0.js.snap
diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/method-chain/break-last-member.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/method-chain/break-last-member.js.snap
@@ -33,18 +33,7 @@ const {
 ```diff
 --- Prettier
 +++ Rome
-@@ -3,22 +3,17 @@
- weNeedToReachTheEightyCharacterLimitXXXXXXXXXXXXXXXXX.someNode
-   .childrenInAnArray[0];
- superSupersuperSupersuperSupersuperSupersuperSuperLong.exampleOfOrderOfGetterAndSetterReordered;
--superSupersuperSupersuperSupersuperSupersuperSuperLong
--  .exampleOfOrderOfGetterAndSetterReordered[0];
-+superSupersuperSupersuperSupersuperSupersuperSuperLong.exampleOfOrderOfGetterAndSetterReordered[
-+  0
-+];
-
- expect(
-   findDOMNode(component.instance()).getElementsByClassName(styles.inner)[0]
+@@ -11,14 +11,8 @@
      .style.paddingRight,
  ).toBe("1000px");
 
@@ -71,9 +60,8 @@ SomeVeryLongUpperCaseConstant.someVeryLongCallExpression()
 weNeedToReachTheEightyCharacterLimitXXXXXXXXXXXXXXXXX.someNode
   .childrenInAnArray[0];
 superSupersuperSupersuperSupersuperSupersuperSuperLong.exampleOfOrderOfGetterAndSetterReordered;
-superSupersuperSupersuperSupersuperSupersuperSuperLong.exampleOfOrderOfGetterAndSetterReordered[
-  0
-];
+superSupersuperSupersuperSupersuperSupersuperSuperLong
+  .exampleOfOrderOfGetterAndSetterReordered[0];
 
 expect(
   findDOMNode(component.instance()).getElementsByClassName(styles.inner)[0]
@@ -104,6 +92,5 @@ const {
 # Lines exceeding max width of 80 characters
 ```
     5: superSupersuperSupersuperSupersuperSupersuperSuperLong.exampleOfOrderOfGetterAndSetterReordered;
-    6: superSupersuperSupersuperSupersuperSupersuperSuperLong.exampleOfOrderOfGetterAndSetterReordered[
 ```