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

Commit

Permalink
fix the order and wording of advices
Browse files Browse the repository at this point in the history
  • Loading branch information
@leops
leops committed Oct 5, 2022
1 parent 7abec69 commit 004b599
Show file tree
Hide file tree
Showing 19 changed files with 118 additions and 190 deletions.
78 changes: 40 additions & 38 deletions crates/rome_js_analyze/src/analyzers/nursery/no_unreachable.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use std::{cmp::Ordering, collections::VecDeque, num::NonZeroU32, vec::IntoIter};

use roaring::bitmap::RoaringBitmap;
use rome_analyze::{context::RuleContext, declare_rule, Rule, RuleDiagnostic};
use rome_console::markup;
use rome_control_flow::{builder::BlockId, ExceptionHandler, Instruction, InstructionKind};
use rome_js_syntax::{
JsBlockStatement, JsCaseClause, JsDefaultClause, JsDoWhileStatement, JsForInStatement,
Expand Down Expand Up @@ -75,17 +74,15 @@ impl Rule for NoUnreachable {
let mut diagnostic = RuleDiagnostic::new(
rule_category!(),
state.text_trimmed_range,
markup! {
if state.terminators.is_empty() {
"This code is unreachable"
} else {
"This code will never be reached ..."
},
)
.summary("This code is unreachable")
.unnecessary();

/// Primary label of the diagnostic if it comes earlier in the source than its secondary labels
const PRIMARY_LABEL_BEFORE: &str = "This code will never be reached ...";
/// Primary label of the diagnostic if it comes later in the source than its secondary labels
const PRIMARY_LABEL_AFTER: &str = "... before it can reach this code";

// Pluralize and adapt the error message accordingly based on the
// number and position of secondary labels
match state.terminators.as_slice() {
Expand All @@ -94,44 +91,42 @@ impl Rule for NoUnreachable {
[] => {}
// A single node is responsible for this range being unreachable
[node] => {
if node.range.start() < state.text_trimmed_range.start() {
diagnostic = diagnostic
.secondary(
node.range,
format_args!("This statement will {} ...", node.reason()),
)
.primary(PRIMARY_LABEL_AFTER);
} else {
diagnostic = diagnostic.primary(PRIMARY_LABEL_BEFORE).secondary(
node.range,
format_args!(
"... because this statement will {} beforehand",
node.reason()
),
);
}
diagnostic = diagnostic.secondary(
node.range,
format_args!(
"... because this statement will {} beforehand",
node.reason()
),
);
}
// The range has two dominating terminator instructions
[node_a, node_b] => {
if node_a.kind == node_b.kind {
diagnostic = diagnostic
.secondary(node_a.range, "Either this statement ...")
.secondary(node_a.range, "... because either this statement ...")
.secondary(
node_b.range,
format_args!("... or this statement will {} ...", node_b.reason()),
)
.primary(PRIMARY_LABEL_AFTER);
format_args!(
"... or this statement will {} beforehand",
node_b.reason()
),
);
} else {
diagnostic = diagnostic
.secondary(
node_a.range,
format_args!("Either this statement will {} ...", node_a.reason()),
format_args!(
"... because either this statement will {} ...",
node_a.reason()
),
)
.secondary(
node_b.range,
format_args!("... or this statement will {} ...", node_b.reason()),
)
.primary(PRIMARY_LABEL_AFTER);
format_args!(
"... or this statement will {} beforehand",
node_b.reason()
),
);
}
}
// The range has three or more dominating terminator instructions
Expand All @@ -153,15 +148,18 @@ impl Rule for NoUnreachable {
if has_homogeneous_kind {
for (index, node) in terminators.iter().enumerate() {
if index == 0 {
diagnostic =
diagnostic.secondary(node.range, "Either this statement, ...");
diagnostic = diagnostic
.secondary(node.range, "... because either this statement, ...");
} else if index < last {
diagnostic =
diagnostic.secondary(node.range, "... this statement, ...");
} else {
diagnostic = diagnostic.secondary(
node.range,
format_args!("... or this statement will {} ...", node.reason()),
format_args!(
"... or this statement will {} beforehand",
node.reason()
),
);
}
}
Expand All @@ -170,7 +168,10 @@ impl Rule for NoUnreachable {
if index == 0 {
diagnostic = diagnostic.secondary(
node.range,
format_args!("Either this statement will {}, ...", node.reason()),
format_args!(
"... because either this statement will {}, ...",
node.reason()
),
);
} else if index < last {
diagnostic = diagnostic.secondary(
Expand All @@ -180,13 +181,14 @@ impl Rule for NoUnreachable {
} else {
diagnostic = diagnostic.secondary(
node.range,
format_args!("... or this statement will {} ...", node.reason()),
format_args!(
"... or this statement will {} beforehand",
node.reason()
),
);
}
}
}

diagnostic = diagnostic.primary(PRIMARY_LABEL_AFTER);
}
}

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
Loading

0 comments on commit 004b599

Please sign in to comment.