fix(rome_js_analyze): keep tokens when simplifying logical expression

rome · Oct 28, 2022 · ec795b5 · ec795b5
1 parent a9db0f0
commit ec795b5
Show file tree

Hide file tree

Showing 6 changed files with 114 additions and 14 deletions.
diff --git a/crates/rome_js_analyze/src/analyzers/correctness/use_simplified_logic_expression.rs b/crates/rome_js_analyze/src/analyzers/correctness/use_simplified_logic_expression.rs
@@ -217,15 +217,29 @@ fn simplify_de_morgan(node: &JsLogicalExpression) -> Option<JsUnaryExpression> {
     let left = node.left().ok()?;
     let right = node.right().ok()?;
     let operator_token = node.operator_token().ok()?;
+    let right_argument_trivia = right
+        .syntax()
+        .first_leading_trivia()
+        .map(|trivia| trivia.pieces());
     match (left, right) {
         (JsAnyExpression::JsUnaryExpression(left), JsAnyExpression::JsUnaryExpression(right)) => {
             let mut next_logic_expression = match operator_token.kind() {
-                T![||] => node
-                    .clone()
-                    .replace_token(operator_token, make::token(T![&&])),
-                T![&&] => node
-                    .clone()
-                    .replace_token(operator_token, make::token(T![||])),
+                T![||] => {
+                    let mut token = make::token(T![&&]);
+                    if let Some(right_argument_trivia) = right_argument_trivia {
+                        token = token.with_leading_trivia_pieces(right_argument_trivia);
+                    }
+                    node.clone()
+                        .replace_token_discard_trivia(operator_token, token)
+                }
+                T![&&] => {
+                    let mut token = make::token(T![||]);
+                    if let Some(right_argument_trivia) = right_argument_trivia {
+                        token = token.with_leading_trivia_pieces(right_argument_trivia);
+                    }
+                    node.clone()
+                        .replace_token_discard_trivia(operator_token, token)
+                }
                 _ => return None,
             }?;
             next_logic_expression = next_logic_expression.with_left(left.argument().ok()?);

diff --git a/crates/rome_js_analyze/src/lib.rs b/crates/rome_js_analyze/src/lib.rs
@@ -141,7 +141,11 @@ mod tests {
             String::from_utf8(buffer).unwrap()
         }
 
-        const SOURCE: &str = r#"<input disabled />
+        const SOURCE: &str = r#"if (
+            !firstCondition &&
+            // comment
+            !secondCondition
+        ) {}
         "#;
 
         let parsed = parse(SOURCE, FileId::zero(), SourceType::jsx());

diff --git a/crates/rome_js_analyze/tests/specs/correctness/useSimplifiedLogicExpression.js b/crates/rome_js_analyze/tests/specs/correctness/useSimplifiedLogicExpression.js
@@ -16,3 +16,13 @@ const r3 = null ?? nonNullExp;
 const boolExpr1 = true;
 const boolExpr2 = false;
 const r4 = !boolExpr1 || !boolExpr2;
+if (
+    !boolExpr1 ||
+    // comment
+    !boolExpr2
+) {}
+if (
+    !boolExpr1 ||
+    // comment
+    !boolExpr2
+) {}
diff --git a/crates/rome_js_analyze/tests/specs/correctness/useSimplifiedLogicExpression.js.snap b/crates/rome_js_analyze/tests/specs/correctness/useSimplifiedLogicExpression.js.snap
@@ -22,6 +22,16 @@ const r3 = null ?? nonNullExp;
 const boolExpr1 = true;
 const boolExpr2 = false;
 const r4 = !boolExpr1 || !boolExpr2;
+if (
+    !boolExpr1 ||
+    // comment
+    !boolExpr2
+) {}
+if (
+    !boolExpr1 ||
+    // comment
+    !boolExpr2
+) {}
 
 ```
 
@@ -92,15 +102,77 @@ useSimplifiedLogicExpression.js:18:12 lint/correctness/useSimplifiedLogicExpress
     17 │ const boolExpr2 = false;
   > 18 │ const r4 = !boolExpr1 || !boolExpr2;
        │            ^^^^^^^^^^^^^^^^^^^^^^^^
-    19 │ 
+    19 │ if (
+    20 │     !boolExpr1 ||
   
   i Suggested fix: Reduce the complexity of the logical expression.
   
     16 16 │   const boolExpr1 = true;
     17 17 │   const boolExpr2 = false;
     18    │ - const·r4·=·!boolExpr1·||·!boolExpr2;
-       18 │ + const·r4·=·!(boolExpr1·&&·boolExpr2);
-    19 19 │   
+       18 │ + const·r4·=·!(boolExpr1·&&boolExpr2);
+    19 19 │   if (
+    20 20 │       !boolExpr1 ||
+  
+
+```
+
+```
+useSimplifiedLogicExpression.js:20:5 lint/correctness/useSimplifiedLogicExpression  FIXABLE  ━━━━━━━━━━
+
+  ! Logical expression contains unnecessary complexity.
+  
+    18 │ const r4 = !boolExpr1 || !boolExpr2;
+    19 │ if (
+  > 20 │     !boolExpr1 ||
+       │     ^^^^^^^^^^^^^
+  > 21 │     // comment
+  > 22 │     !boolExpr2
+       │     ^^^^^^^^^^
+    23 │ ) {}
+    24 │ if (
+  
+  i Suggested fix: Reduce the complexity of the logical expression.
+  
+    18 18 │   const r4 = !boolExpr1 || !boolExpr2;
+    19 19 │   if (
+    20    │ - ····!boolExpr1·||
+       20 │ + ····!(boolExpr1·
+    21 21 │       // comment
+    22    │ - ····!boolExpr2
+       22 │ + ····&&boolExpr2)
+    23 23 │   ) {}
+    24 24 │   if (
+  
+
+```
+
+```
+useSimplifiedLogicExpression.js:25:5 lint/correctness/useSimplifiedLogicExpression  FIXABLE  ━━━━━━━━━━
+
+  ! Logical expression contains unnecessary complexity.
+  
+    23 │ ) {}
+    24 │ if (
+  > 25 │     !boolExpr1 ||
+       │     ^^^^^^^^^^^^^
+  > 26 │     // comment
+  > 27 │     !boolExpr2
+       │     ^^^^^^^^^^
+    28 │ ) {}
+    29 │ 
+  
+  i Suggested fix: Reduce the complexity of the logical expression.
+  
+    23 23 │   ) {}
+    24 24 │   if (
+    25    │ - ····!boolExpr1·||
+       25 │ + ····!(boolExpr1·
+    26 26 │       // comment
+    27    │ - ····!boolExpr2
+       27 │ + ····&&boolExpr2)
+    28 28 │   ) {}
+    29 29 │   
   
 
 ```

diff --git a/crates/rome_rowan/src/ast/mutation.rs b/crates/rome_rowan/src/ast/mutation.rs
@@ -14,7 +14,7 @@ pub trait AstNodeExt: AstNode {
         Self: Sized;
 
     /// Return a new version of this node with the node `prev_node` replaced with `next_node`,
-    /// transfering the leading and trailing trivia of `prev_node` to `next_node`
+    /// transferring the leading and trailing trivia of `prev_node` to `next_node`
     ///
     /// `prev_node` can be a direct child of this node, or an indirect child through any descendant node
     ///
@@ -38,7 +38,7 @@ pub trait AstNodeExt: AstNode {
         Self: Sized;
 
     /// Return a new version of this node with the token `prev_token` replaced with `next_token`,
-    /// transfering the leading and trailing trivia of `prev_token` to `next_token`
+    /// transferring the leading and trailing trivia of `prev_token` to `next_token`
     ///
     /// `prev_token` can be a direct child of this node, or an indirect child through any descendant node
     ///

diff --git a/website/src/docs/lint/rules/useSimplifiedLogicExpression.md b/website/src/docs/lint/rules/useSimplifiedLogicExpression.md