fix(rome_js_analyzer): add newline if statement has comments in `useB…

…lockStatements` (#3276)
rome · Sep 27, 2022 · 107c9f9 · 107c9f9
1 parent 1022663
commit 107c9f9
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 12 deletions.
diff --git a/crates/rome_js_analyze/src/analyzers/correctness/use_block_statements.rs b/crates/rome_js_analyze/src/analyzers/correctness/use_block_statements.rs
@@ -216,8 +216,24 @@ impl Rule for UseBlockStatements {
 
                     r_curly_token.with_leading_trivia(leading_trivia)
                 } else {
-                    r_curly_token
-                        .with_leading_trivia(iter::once((TriviaPieceKind::Whitespace, " ")))
+                    let has_trailing_single_line_comments = stmt
+                        .syntax()
+                        .last_trailing_trivia()
+                        .map(|trivia| {
+                            trivia
+                                .pieces()
+                                .any(|trivia| trivia.kind() == TriviaPieceKind::SingleLineComment)
+                        })
+                        .unwrap_or(false);
+                    // if the node we have to enclose has some trailing comments, then we add a new line
+                    // to the leading trivia of the right curly brace
+                    if !has_trailing_single_line_comments {
+                        r_curly_token
+                            .with_leading_trivia(iter::once((TriviaPieceKind::Whitespace, " ")))
+                    } else {
+                        r_curly_token
+                            .with_leading_trivia(iter::once((TriviaPieceKind::Newline, "\n")))
+                    }
                 };
 
                 mutation.replace_node_discard_trivia(

diff --git a/crates/rome_js_analyze/src/lib.rs b/crates/rome_js_analyze/src/lib.rs
@@ -112,12 +112,10 @@ mod tests {
     #[test]
     fn quick_test() {
         const SOURCE: &str = r#"
-import AwesomeReact, { Fragment as AwesomeFragment } from "react";
-
-<>
-    <AwesomeFragment>foo</AwesomeFragment>
-    <AwesomeReact.Fragment>foo</AwesomeReact.Fragment>
-</>
+if (true) {
+  console.log("true");
+} else
+  console.log("false"); // comment
 
         "#;
 
@@ -131,13 +129,16 @@ import AwesomeReact, { Fragment as AwesomeFragment } from "react";
             |signal| {
                 if let Some(diag) = signal.diagnostic() {
                     let diag = diag.into_diagnostic(Severity::Warning);
-                    dbg!(&diag);
                     let primary = diag.primary.as_ref().unwrap();
 
                     error_ranges.push(primary.span.range);
                 }
 
-                dbg!(signal.action());
+                if let Some(action) = signal.action() {
+                    let new_code = action.mutation.commit();
+
+                    eprintln!("{new_code}");
+                }
 
                 ControlFlow::<Never>::Continue(())
             },

diff --git a/crates/rome_js_analyze/tests/specs/correctness/useBlockStatements.js b/crates/rome_js_analyze/tests/specs/correctness/useBlockStatements.js
@@ -38,3 +38,7 @@ else
     bar
   else
     bar
+
+if (test) {
+  correct;
+} else console.log("false") // comment
diff --git a/crates/rome_js_analyze/tests/specs/correctness/useBlockStatements.js.snap b/crates/rome_js_analyze/tests/specs/correctness/useBlockStatements.js.snap
@@ -45,6 +45,10 @@ else
   else
     bar
 
+if (test) {
+  correct;
+} else console.log("false") // comment
+
 ```
 
 # Diagnostics
@@ -546,7 +550,7 @@ useBlockStatements.js:37:8 lint/correctness/useBlockStatements  FIXABLE  ━━
   
   i Suggested fix: Wrap the statement with a `JsBlockStatement`
   
-        | @@ -34,7 +34,8 @@
+        | @@ -34,8 +34,9 @@
   33 33 |   
   34 34 |     if (test)
   35 35 |       bar
@@ -556,6 +560,7 @@ useBlockStatements.js:37:8 lint/correctness/useBlockStatements  FIXABLE  ━━
      38 | +   }
   38 39 |     else
   39 40 |       bar
+  40 41 |   
   
 
 ```
@@ -573,14 +578,40 @@ useBlockStatements.js:39:3 lint/correctness/useBlockStatements  FIXABLE  ━━
   
   i Suggested fix: Wrap the statement with a `JsBlockStatement`
   
-        | @@ -36,5 +36,6 @@
+        | @@ -36,8 +36,9 @@
   35 35 |       bar
   36 36 |     else if(test)
   37 37 |       bar
   38    | -   else
      38 | +   else {
   39 39 |       bar
      40 | +   }
+  40 41 |   
+  41 42 |   if (test) {
+  42 43 |     correct;
+  
+
+```
+
+```
+useBlockStatements.js:44:3 lint/correctness/useBlockStatements  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Block statements are preferred in this position.
+  
+     ┌─ useBlockStatements.js:44:3
+     │
+  44 │ } else console.log("false") // comment
+     │   ^^^^^^^^^^^^^^^^^^^^^^^^^
+  
+  i Suggested fix: Wrap the statement with a `JsBlockStatement`
+  
+        | @@ -41,4 +41,5 @@
+  40 40 |   
+  41 41 |   if (test) {
+  42 42 |     correct;
+  43    | - } else console.log("false") // comment
+     43 | + } else { console.log("false") // comment
+     44 | + }
   
 
 ```