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

fix(rome_js_analyzer): add newline if statement has comments in useBlockStatements #3276

Merged
merged 2 commits into from
Sep 27, 2022
Merged

fix(rome_js_analyzer): add newline if statement has comments in useBlockStatements #3276

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6a0c0cf
fix(rome_js_analyzer): add newline if statement has comments
ematipico Sep 26, 2022
d57be36
chore: code review
ematipico Sep 27, 2022
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
20 changes: 18 additions & 2 deletions crates/rome_js_analyze/src/analyzers/correctness/use_block_statements.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
17 changes: 9 additions & 8 deletions crates/rome_js_analyze/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

"#;

Expand All @@ -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(())
},
Expand Down
4 changes: 4 additions & 0 deletions crates/rome_js_analyze/tests/specs/correctness/useBlockStatements.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,7 @@ else
bar
else
bar

if (test) {
correct;
} else console.log("false") // comment
35 changes: 33 additions & 2 deletions crates/rome_js_analyze/tests/specs/correctness/useBlockStatements.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ else
else
bar

if (test) {
correct;
} else console.log("false") // comment

```

# Diagnostics
Expand Down Expand Up @@ -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
Expand All @@ -556,6 +560,7 @@ useBlockStatements.js:37:8 lint/correctness/useBlockStatements FIXABLE ━━
38 | + }
38 39 | else
39 40 | bar
40 41 |


```
Expand All @@ -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 | + }


```
Expand Down