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

Commit

Permalink
fix(rome_js_analyze): improve the diagnostics emitted by `noUnreachab…
Browse files Browse the repository at this point in the history 
…le` (#3348)

* fix(rome_js_analyze): improve the range merging logic in `noDeadCode`

* fix the order and wording of advices
  • Loading branch information
@leops
leops authored Oct 6, 2022
1 parent bc4393e commit c854ce1
Show file tree
Hide file tree
Showing 24 changed files with 797 additions and 344 deletions.
11 changes: 6 additions & 5 deletions crates/rome_cli/tests/snapshots/main_check/upgrade_severity.snap
View file
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

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
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
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
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
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
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
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
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
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
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
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
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

0 comments on commit c854ce1

Please sign in to comment.