Skip to content

Commit

Permalink
fix(lint/noUnreachableSuper): handle complex CFG
Browse files Browse the repository at this point in the history
  • Loading branch information
@Conaclos
Conaclos committed Sep 4, 2023
1 parent 9649616 commit 059ebaf
Show file tree
Hide file tree
Showing 13 changed files with 658 additions and 599 deletions.
97 changes: 51 additions & 46 deletions CHANGELOG.md
View file

Large diffs are not rendered by default.

475 changes: 134 additions & 341 deletions crates/rome_js_analyze/src/analyzers/correctness/no_unreachable_super.rs
View file

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion crates/rome_js_analyze/src/control_flow.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use rome_js_syntax::JsLanguage;
use rome_js_syntax::TextRange;

pub type JsControlFlowGraph = rome_control_flow::ControlFlowGraph<JsLanguage>;
pub(crate) type BasicBlock = rome_control_flow::BasicBlock<JsLanguage>;
pub(crate) type FunctionBuilder = rome_control_flow::builder::FunctionBuilder<JsLanguage>;

mod nodes;
Expand Down
23 changes: 22 additions & 1 deletion crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/duplicateSuper.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ class D extends A {
if (cond) {
super(true);
}

super();
}
}
Expand Down Expand Up @@ -59,3 +58,25 @@ class G extends A {
}
}
}

// invalid
class G extends A {
constructor(condA, condB) {
try {
super()
} catch {
super()
}
}
}

// invalid
class G extends A {
constructor(condA, condB) {
try {
super()
} finally {
super()
}
}
}
200 changes: 137 additions & 63 deletions crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/duplicateSuper.js.snap
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
---
source: crates/rome_js_analyze/tests/spec_tests.rs
assertion_line: 96
expression: duplicateSuper.js
---
# Input
Expand Down Expand Up @@ -30,7 +29,6 @@ class D extends A {
if (cond) {
super(true);
}

super();
}
}
Expand Down Expand Up @@ -67,6 +65,28 @@ class G extends A {
}
}

// invalid
class G extends A {
constructor(condA, condB) {
try {
super()
} catch {
super()
}
}
}

// invalid
class G extends A {
constructor(condA, condB) {
try {
super()
} finally {
super()
}
}
}

```

# Diagnostics
Expand Down Expand Up @@ -117,12 +137,13 @@ duplicateSuper.js:22:5 lint/correctness/noUnreachableSuper ━━━━━━━
> 22 │ constructor(cond) {
│ ^^^^^^^^^^^^^^^^^^^
> 23 │ if (cond) {
...
> 27 │ super();
> 28 │ }
> 24 │ super(true);
> 25 │ }
> 26 │ super();
> 27 │ }
│ ^
29 │ }
30 │
28 │ }
29 │
i `super()` is first called here:
Expand All @@ -131,112 +152,165 @@ duplicateSuper.js:22:5 lint/correctness/noUnreachableSuper ━━━━━━━
> 24 │ super(true);
│ ^^^^^
25 │ }
26 │
26 │ super();
i `super()` is then called again here:
24 │ super(true);
25 │ }
26 │
> 27 │ super();
> 26 │ super();
│ ^^^^^
28 │ }
29 │ }
27 │ }
28 │ }
```

```
duplicateSuper.js:33:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
duplicateSuper.js:32:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This constructor calls `super()` in a loop.
31 │ // invalid
32 │ class E extends A {
> 33 │ constructor(cond) {
30 │ // invalid
31 │ class E extends A {
> 32 │ constructor(cond) {
│ ^^^^^^^^^^^^^^^^^^^
> 34 │ do {
> 35 │ super();
> 36 │ } while (cond);
> 37 │ }
> 33 │ do {
> 34 │ super();
> 35 │ } while (cond);
> 36 │ }
│ ^
38 │ }
39 │
37 │ }
38 │
i `super()` is called here:
33 │ constructor(cond) {
34 │ do {
> 35 │ super();
32 │ constructor(cond) {
33 │ do {
> 34 │ super();
│ ^^^^^
36 │ } while (cond);
37 │ }
35 │ } while (cond);
36 │ }
```

```
duplicateSuper.js:41:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This constructor has code paths that return without calling `super()`.
39 │ // invalid
40 │ class F extends A {
> 41 │ constructor(condA, condB) {
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 42 │ if (condA) {
...
> 47 │ }
> 48 │ }
│ ^
49 │ }
50 │
i If this is intentional, add an explicit throw statement in unsupported paths.
```

```
duplicateSuper.js:53:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This constructor has code paths that return without calling `super()`.
51 │ // invalid
52 │ class G extends A {
> 53 │ constructor(condA, condB) {
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 54 │ while (condA) {
...
> 58 │ }
> 59 │ }
│ ^
60 │ }
61 │
i If this is intentional, add an explicit throw statement in unsupported paths.
```

```
duplicateSuper.js:42:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
duplicateSuper.js:64:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This constructor has code paths where `super()` is called more than once.
40 │ // invalid
41 │ class F extends A {
> 42 │ constructor(condA, condB) {
62 │ // invalid
63 │ class G extends A {
> 64 │ constructor(condA, condB) {
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 43 │ if (condA) {
> 65 │ try {
...
> 48 │ }
> 49 │ }
> 69 │ }
> 70 │ }
│ ^
50 │ }
51 │
71 │ }
72 │
i `super()` is first called here:
42 │ constructor(condA, condB) {
43 │ if (condA) {
> 44 │ super(true);
64 │ constructor(condA, condB) {
65 │ try {
> 66 │ super()
│ ^^^^^
45 │ }
46 │ if (condB) {
67 │ } catch {
68 │ super()
i `super()` is then called again here:
45 │ }
46 │ if (condB) {
> 47 │ super(true);
66 │ super()
67 │ } catch {
> 68 │ super()
│ ^^^^^
48 │ }
49 │ }
69 │ }
70 │ }
```

```
duplicateSuper.js:54:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
duplicateSuper.js:75:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This constructor calls `super()` in a loop.
! This constructor has code paths where `super()` is called more than once.
52 │ // invalid
53 │ class G extends A {
> 54 │ constructor(condA, condB) {
73 │ // invalid
74 │ class G extends A {
> 75 │ constructor(condA, condB) {
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 55 │ while (condA) {
> 76 │ try {
...
> 59 │ }
> 60 │ }
> 80 │ }
> 81 │ }
│ ^
61 │ }
62 │
82 │ }
83 │
i `super()` is called here:
i `super()` is first called here:
75 │ constructor(condA, condB) {
76 │ try {
> 77 │ super()
│ ^^^^^
78 │ } finally {
79 │ super()
55 │ while (condA) {
56 │ if (condB) {
> 57 │ super();
│ ^^^^^
58 │ }
59 │ }
i `super()` is then called again here:
77 │ super()
78 │ } finally {
> 79 │ super()
│ ^^^^^
80 │ }
81 │ }
```
Expand Down
11 changes: 8 additions & 3 deletions crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/missingSuper.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ class E extends A {
if (cond) {
return;
}

super(true);
}
}
Expand All @@ -67,7 +66,6 @@ class G extends A {
} else {
throw new Error();
}

this.field = "value";
}
}
Expand All @@ -94,4 +92,11 @@ class I extends A {
}
}
}
}
}

// invalid
class I extends A {
constructor() {
super.method();
}
}
Loading

0 comments on commit 059ebaf

Please sign in to comment.