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

fix(rome_js_analyze): improve the diagnostics emitted by noUnreachable #3348

Merged
merged 2 commits into from
Oct 6, 2022
+797 −344
Merged

fix(rome_js_analyze): improve the diagnostics emitted by noUnreachable #3348

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
3f12972
fix(rome_js_analyze): improve the range merging logic in `noDeadCode`
leops Aug 2, 2022
eb39b80
fix the order and wording of advices
leops Oct 5, 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
11 changes: 6 additions & 5 deletions crates/rome_cli/tests/snapshots/main_check/upgrade_severity.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,19 @@ errors where emitted while running checks
# Emitted Messages

```block
file.js:3:9 lint/nursery/noUnreachable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
file.js:2:5 lint/nursery/noUnreachable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× This code is unreachable

1 │ function f() {
2 │ for (;;) {
> 2 │ for (;;) {
│ ^^^^^^^^^^
> 3 │ continue;
│ ^^^^^^^^^
> 4 │ break;
│ ^^^^^^
5 │ }
> 5 │ }
│ ^
6 │ }
7 │


```
Expand Down
338 changes: 297 additions & 41 deletions crates/rome_js_analyze/src/analyzers/nursery/no_unreachable.rs
View file
Open in desktop

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions crates/rome_js_analyze/src/control_flow/nodes/switch_stmt.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ impl NodeVisitor for SwitchVisitor {
builder: &mut FunctionBuilder,
_: StatementStack,
) -> SyntaxResult<Self> {
// Execute the discriminant expression as a side-effect
builder.append_statement().with_node(node.discriminant()?);

let entry_block = builder.cursor();
let break_block = builder.append_block();

Expand Down
12 changes: 4 additions & 8 deletions crates/rome_js_analyze/tests/specs/nursery/noUnreachable/JsBreakStatement.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@ function JsBreakStatement2() {
```
JsBreakStatement.js:4:9 lint/nursery/noUnreachable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This code is unreachable
! ... before it can reach this code
! This code will never be reached ...
2 │ while (true) {
3 │ break;
Expand All @@ -35,7 +33,7 @@ JsBreakStatement.js:4:9 lint/nursery/noUnreachable ━━━━━━━━━
5 │ }
6 │ }
i This statement will break the flow of the code ...
i ... because this statement will break the flow of the code beforehand
1 │ function JsBreakStatement1() {
2 │ while (true) {
Expand All @@ -50,9 +48,7 @@ JsBreakStatement.js:4:9 lint/nursery/noUnreachable ━━━━━━━━━
```
JsBreakStatement.js:11:9 lint/nursery/noUnreachable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This code is unreachable
! ... before it can reach this code
! This code will never be reached ...
9 │ while (true) {
10 │ break;
Expand All @@ -61,7 +57,7 @@ JsBreakStatement.js:11:9 lint/nursery/noUnreachable ━━━━━━━━━
12 │ }
13 │ }
i This statement will break the flow of the code ...
i ... because this statement will break the flow of the code beforehand
8 │ function JsBreakStatement2() {
9 │ while (true) {
Expand Down
12 changes: 4 additions & 8 deletions crates/rome_js_analyze/tests/specs/nursery/noUnreachable/JsContinueStatement.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@ function JsContinueStatement2() {
```
JsContinueStatement.js:4:9 lint/nursery/noUnreachable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This code is unreachable
! ... before it can reach this code
! This code will never be reached ...
2 │ while (true) {
3 │ continue;
Expand All @@ -35,7 +33,7 @@ JsContinueStatement.js:4:9 lint/nursery/noUnreachable ━━━━━━━━
5 │ }
6 │ }
i This statement will continue the loop ...
i ... because this statement will continue the loop beforehand
1 │ function JsContinueStatement1() {
2 │ while (true) {
Expand All @@ -50,9 +48,7 @@ JsContinueStatement.js:4:9 lint/nursery/noUnreachable ━━━━━━━━
```
JsContinueStatement.js:11:9 lint/nursery/noUnreachable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This code is unreachable
! ... before it can reach this code
! This code will never be reached ...
9 │ while (true) {
10 │ continue;
Expand All @@ -61,7 +57,7 @@ JsContinueStatement.js:11:9 lint/nursery/noUnreachable ━━━━━━━━
12 │ }
13 │ }
i This statement will continue the loop ...
i ... because this statement will continue the loop beforehand
8 │ function JsContinueStatement2() {
9 │ while (true) {
Expand Down
12 changes: 4 additions & 8 deletions crates/rome_js_analyze/tests/specs/nursery/noUnreachable/JsDoWhileStatement.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,7 @@ function JsDoWhileStatement2() {
```
JsDoWhileStatement.js:4:14 lint/nursery/noUnreachable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This code is unreachable
! ... before it can reach this code
! This code will never be reached ...
2 │ do {
3 │ break;
Expand All @@ -34,7 +32,7 @@ JsDoWhileStatement.js:4:14 lint/nursery/noUnreachable ━━━━━━━━
5 │ }
6 │
i This statement will break the flow of the code ...
i ... because this statement will break the flow of the code beforehand
1 │ function JsDoWhileStatement1() {
2 │ do {
Expand All @@ -49,9 +47,7 @@ JsDoWhileStatement.js:4:14 lint/nursery/noUnreachable ━━━━━━━━
```
JsDoWhileStatement.js:10:9 lint/nursery/noUnreachable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This code is unreachable
! ... before it can reach this code
! This code will never be reached ...
8 │ do {
9 │ continue;
Expand All @@ -60,7 +56,7 @@ JsDoWhileStatement.js:10:9 lint/nursery/noUnreachable ━━━━━━━━
11 │ } while (true);
12 │ }
i This statement will continue the loop ...
i ... because this statement will continue the loop beforehand
7 │ function JsDoWhileStatement2() {
8 │ do {
Expand Down
12 changes: 4 additions & 8 deletions crates/rome_js_analyze/tests/specs/nursery/noUnreachable/JsForInStatement.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@ function JsForInStatement2() {
```
JsForInStatement.js:4:9 lint/nursery/noUnreachable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This code is unreachable
! ... before it can reach this code
! This code will never be reached ...
2 │ for (const key in value) {
3 │ break;
Expand All @@ -35,7 +33,7 @@ JsForInStatement.js:4:9 lint/nursery/noUnreachable ━━━━━━━━━
5 │ }
6 │ }
i This statement will break the flow of the code ...
i ... because this statement will break the flow of the code beforehand
1 │ function JsForInStatement1() {
2 │ for (const key in value) {
Expand All @@ -50,9 +48,7 @@ JsForInStatement.js:4:9 lint/nursery/noUnreachable ━━━━━━━━━
```
JsForInStatement.js:11:9 lint/nursery/noUnreachable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This code is unreachable
! ... before it can reach this code
! This code will never be reached ...
9 │ for (const key in value) {
10 │ continue;
Expand All @@ -61,7 +57,7 @@ JsForInStatement.js:11:9 lint/nursery/noUnreachable ━━━━━━━━━
12 │ }
13 │ }
i This statement will continue the loop ...
i ... because this statement will continue the loop beforehand
8 │ function JsForInStatement2() {
9 │ for (const key in value) {
Expand Down
12 changes: 4 additions & 8 deletions crates/rome_js_analyze/tests/specs/nursery/noUnreachable/JsForOfStatement.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@ function JsForOfStatement2() {
```
JsForOfStatement.js:4:9 lint/nursery/noUnreachable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This code is unreachable
! ... before it can reach this code
! This code will never be reached ...
2 │ for (const key of value) {
3 │ break;
Expand All @@ -35,7 +33,7 @@ JsForOfStatement.js:4:9 lint/nursery/noUnreachable ━━━━━━━━━
5 │ }
6 │ }
i This statement will break the flow of the code ...
i ... because this statement will break the flow of the code beforehand
1 │ function JsForOfStatement1() {
2 │ for (const key of value) {
Expand All @@ -50,9 +48,7 @@ JsForOfStatement.js:4:9 lint/nursery/noUnreachable ━━━━━━━━━
```
JsForOfStatement.js:11:9 lint/nursery/noUnreachable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This code is unreachable
! ... before it can reach this code
! This code will never be reached ...
9 │ for (const key of value) {
10 │ continue;
Expand All @@ -61,7 +57,7 @@ JsForOfStatement.js:11:9 lint/nursery/noUnreachable ━━━━━━━━━
12 │ }
13 │ }
i This statement will continue the loop ...
i ... because this statement will continue the loop beforehand
8 │ function JsForOfStatement2() {
9 │ for (const key of value) {
Expand Down
2 changes: 0 additions & 2 deletions crates/rome_js_analyze/tests/specs/nursery/noUnreachable/JsForStatement.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ function JsForStatement1() {
```
JsForStatement.js:2:29 lint/nursery/noUnreachable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This code is unreachable
! This code will never be reached ...
1 │ function JsForStatement1() {
Expand Down
8 changes: 3 additions & 5 deletions crates/rome_js_analyze/tests/specs/nursery/noUnreachable/JsIfStatement.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@ function JsIfStatement1() {
```
JsIfStatement.js:8:5 lint/nursery/noUnreachable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This code is unreachable
! ... before it can reach this code
! This code will never be reached ...
6 │ }
7 │
Expand All @@ -31,7 +29,7 @@ JsIfStatement.js:8:5 lint/nursery/noUnreachable ━━━━━━━━━━
9 │ }
10 │
i Either this statement ...
i ... because either this statement ...
1 │ function JsIfStatement1() {
2 │ if (true) {
Expand All @@ -40,7 +38,7 @@ JsIfStatement.js:8:5 lint/nursery/noUnreachable ━━━━━━━━━━
4 │ } else {
5 │ return;
i ... or this statement will return from the function ...
i ... or this statement will return from the function beforehand
3 │ return;
4 │ } else {
Expand Down
14 changes: 5 additions & 9 deletions crates/rome_js_analyze/tests/specs/nursery/noUnreachable/JsLabeledStatement.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,7 @@ function JsLabeledStatement2() {
```
JsLabeledStatement.js:9:9 lint/nursery/noUnreachable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! This code is unreachable

! ... before it can reach this code
! This code will never be reached ...

7 │ }
8 │
Expand All @@ -43,7 +41,7 @@ JsLabeledStatement.js:9:9 lint/nursery/noUnreachable ━━━━━━━━━
10 │ }
11 │ }

i Either this statement will continue the loop ...
i ... because either this statement will continue the loop ...

2 │ label: while (true) {
3 │ if (true) {
Expand All @@ -52,7 +50,7 @@ JsLabeledStatement.js:9:9 lint/nursery/noUnreachable ━━━━━━━━━
5 │ } else {
6 │ break label;

i ... or this statement will break the flow of the code ...
i ... or this statement will break the flow of the code beforehand

4 │ continue label;
5 │ } else {
Expand All @@ -67,9 +65,7 @@ JsLabeledStatement.js:9:9 lint/nursery/noUnreachable ━━━━━━━━━
```
JsLabeledStatement.js:17:9 lint/nursery/noUnreachable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! This code is unreachable

! ... before it can reach this code
! This code will never be reached ...

15 │ beforeBreak();
16 │ break label;
Expand All @@ -78,7 +74,7 @@ JsLabeledStatement.js:17:9 lint/nursery/noUnreachable ━━━━━━━━
18 │ }
19 │

i This statement will break the flow of the code ...
i ... because this statement will break the flow of the code beforehand

14 │ label: {
15 │ beforeBreak();
Expand Down
12 changes: 4 additions & 8 deletions crates/rome_js_analyze/tests/specs/nursery/noUnreachable/JsReturnStatement.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@ function JsReturnStatement2() {
```
JsReturnStatement.js:3:5 lint/nursery/noUnreachable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! This code is unreachable

! ... before it can reach this code
! This code will never be reached ...

1 │ function JsReturnStatement1() {
2 │ return;
Expand All @@ -31,7 +29,7 @@ JsReturnStatement.js:3:5 lint/nursery/noUnreachable ━━━━━━━━━
4 │ }
5 │

i This statement will return from the function ...
i ... because this statement will return from the function beforehand

1 │ function JsReturnStatement1() {
> 2 │ return;
Expand All @@ -45,9 +43,7 @@ JsReturnStatement.js:3:5 lint/nursery/noUnreachable ━━━━━━━━━
```
JsReturnStatement.js:8:5 lint/nursery/noUnreachable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! This code is unreachable

! ... before it can reach this code
! This code will never be reached ...

6 │ function JsReturnStatement2() {
7 │ return;
Expand All @@ -56,7 +52,7 @@ JsReturnStatement.js:8:5 lint/nursery/noUnreachable ━━━━━━━━━
9 │ }
10 │

i This statement will return from the function ...
i ... because this statement will return from the function beforehand

6 │ function JsReturnStatement2() {
> 7 │ return;
Expand Down
5 changes: 5 additions & 0 deletions crates/rome_js_analyze/tests/specs/nursery/noUnreachable/JsSwitchStatement.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,8 @@ function JsSwitchStatement2() {
afterBreak();
}
}

function JsSwitchStatement3() {
return;
switch (value) {}
}
Loading