Skip to content

Commit b550330

Browse files
mdjermanovicmdjermanovic
authored
New: Add no-unreachable-loop rule (fixes #12381) (#12660)
* New: Add no-unreachable-loop rule (fixes #12381) * Add fallthrough switch test
1 parent 13999d2 commit b550330

5 files changed

Lines changed: 773 additions & 0 deletions

File tree

‎docs/rules/no-unreachable-loop.md‎

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
# Disallow loops with a body that allows only one iteration (no-unreachable-loop)
2+
3+
A loop that can never reach the second iteration is a possible error in the code.
4+
5+
```js
6+
for (let i = 0; i < arr.length; i++) {
7+
if (arr[i].name === myName) {
8+
doSomething(arr[i]);
9+
// break was supposed to be here
10+
}
11+
break;
12+
}
13+
```
14+
15+
In rare cases where only one iteration (or at most one iteration) is intended behavior, the code should be refactored to use `if` conditionals instead of `while`, `do-while` and `for` loops. It's considered a best practice to avoid using loop constructs for such cases.
16+
17+
## Rule Details
18+
19+
This rule aims to detect and disallow loops that can have at most one iteration, by performing static code path analysis on loop bodies.
20+
21+
In particular, this rule will disallow a loop with a body that exits the loop in all code paths. If all code paths in the loop's body will end with either a `break`, `return` or a `throw` statement, the second iteration of such loop is certainly unreachable, regardless of the loop's condition.
22+
23+
This rule checks `while`, `do-while`, `for`, `for-in` and `for-of` loops. You can optionally disable checks for each of these constructs.
24+
25+
Examples of **incorrect** code for this rule:
26+
27+
```js
28+
/*eslint no-unreachable-loop: "error"*/
29+
30+
while (foo) {
31+
doSomething(foo);
32+
foo = foo.parent;
33+
break;
34+
}
35+
36+
function verifyList(head) {
37+
let item = head;
38+
do {
39+
if (verify(item)) {
40+
return true;
41+
} else {
42+
return false;
43+
}
44+
} while (item);
45+
}
46+
47+
function findSomething(arr) {
48+
for (var i = 0; i < arr.length; i++) {
49+
if (isSomething(arr[i])) {
50+
return arr[i];
51+
} else {
52+
throw new Error("Doesn't exist.");
53+
}
54+
}
55+
}
56+
57+
for (key in obj) {
58+
if (key.startsWith("_")) {
59+
break;
60+
}
61+
firstKey = key;
62+
firstValue = obj[key];
63+
break;
64+
}
65+
66+
for (foo of bar) {
67+
if (foo.id === id) {
68+
doSomething(foo);
69+
}
70+
break;
71+
}
72+
```
73+
74+
Examples of **correct** code for this rule:
75+
76+
```js
77+
/*eslint no-unreachable-loop: "error"*/
78+
79+
while (foo) {
80+
doSomething(foo);
81+
foo = foo.parent;
82+
}
83+
84+
function verifyList(head) {
85+
let item = head;
86+
do {
87+
if (verify(item)) {
88+
item = item.next;
89+
} else {
90+
return false;
91+
}
92+
} while (item);
93+
94+
return true;
95+
}
96+
97+
function findSomething(arr) {
98+
for (var i = 0; i < arr.length; i++) {
99+
if (isSomething(arr[i])) {
100+
return arr[i];
101+
}
102+
}
103+
throw new Error("Doesn't exist.");
104+
}
105+
106+
for (key in obj) {
107+
if (key.startsWith("_")) {
108+
continue;
109+
}
110+
firstKey = key;
111+
firstValue = obj[key];
112+
break;
113+
}
114+
115+
for (foo of bar) {
116+
if (foo.id === id) {
117+
doSomething(foo);
118+
break;
119+
}
120+
}
121+
```
122+
123+
Please note that this rule is not designed to check loop conditions, and will not warn in cases such as the following examples.
124+
125+
Examples of additional **correct** code for this rule:
126+
127+
```js
128+
/*eslint no-unreachable-loop: "error"*/
129+
130+
do {
131+
doSomething();
132+
} while (false)
133+
134+
for (let i = 0; i < 1; i++) {
135+
doSomething(i);
136+
}
137+
138+
for (const a of [1]) {
139+
doSomething(a);
140+
}
141+
```
142+
143+
## Options
144+
145+
This rule has an object option, with one option:
146+
147+
* `"ignore"` - an optional array of loop types that will be ignored by this rule.
148+
149+
## ignore
150+
151+
You can specify up to 5 different elements in the `"ignore"` array:
152+
153+
* `"WhileStatement"` - to ignore all `while` loops.
154+
* `"DoWhileStatement"` - to ignore all `do-while` loops.
155+
* `"ForStatement"` - to ignore all `for` loops (does not apply to `for-in` and `for-of` loops).
156+
* `"ForInStatement"` - to ignore all `for-in` loops.
157+
* `"ForOfStatement"` - to ignore all `for-of` loops.
158+
159+
Examples of **correct** code for this rule with the `"ignore"` option:
160+
161+
```js
162+
/*eslint no-unreachable-loop: ["error", { "ignore": ["ForInStatement", "ForOfStatement"] }]*/
163+
164+
for (var key in obj) {
165+
hasEnumerableProperties = true;
166+
break;
167+
}
168+
169+
for (const a of b) break;
170+
```
171+
172+
## Known Limitations
173+
174+
Static code path analysis, in general, does not evaluate conditions. Due to this fact, this rule might miss reporting cases such as the following:
175+
176+
```js
177+
for (let i = 0; i < 10; i++) {
178+
doSomething(i);
179+
if (true) {
180+
break;
181+
}
182+
}
183+
```
184+
185+
## Related Rules
186+
187+
* [no-unreachable](no-unreachable.md)
188+
* [no-constant-condition](no-constant-condition.md)
189+
* [no-unmodified-loop-condition](no-unmodified-loop-condition.md)
190+
* [for-direction](for-direction.md)

‎lib/rules/index.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,7 @@ module.exports = new LazyLoadingRuleMap(Object.entries({
212212
"no-unmodified-loop-condition": () => require("./no-unmodified-loop-condition"),
213213
"no-unneeded-ternary": () => require("./no-unneeded-ternary"),
214214
"no-unreachable": () => require("./no-unreachable"),
215+
"no-unreachable-loop": () => require("./no-unreachable-loop"),
215216
"no-unsafe-finally": () => require("./no-unsafe-finally"),
216217
"no-unsafe-negation": () => require("./no-unsafe-negation"),
217218
"no-unused-expressions": () => require("./no-unused-expressions"),

‎lib/rules/no-unreachable-loop.js‎

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
/**
2+
* @fileoverview Rule to disallow loops with a body that allows only one iteration
3+
* @author Milos Djermanovic
4+
*/
5+
6+
"use strict";
7+
8+
//------------------------------------------------------------------------------
9+
// Helpers
10+
//------------------------------------------------------------------------------
11+
12+
const allLoopTypes = ["WhileStatement", "DoWhileStatement", "ForStatement", "ForInStatement", "ForOfStatement"];
13+
14+
/**
15+
* Determines whether the given node is the first node in the code path to which a loop statement
16+
* 'loops' for the next iteration.
17+
* @param {ASTNode} node The node to check.
18+
* @returns {boolean} `true` if the node is a looping target.
19+
*/
20+
function isLoopingTarget(node) {
21+
const parent = node.parent;
22+
23+
if (parent) {
24+
switch (parent.type) {
25+
case "WhileStatement":
26+
return node === parent.test;
27+
case "DoWhileStatement":
28+
return node === parent.body;
29+
case "ForStatement":
30+
return node === (parent.update || parent.test || parent.body);
31+
case "ForInStatement":
32+
case "ForOfStatement":
33+
return node === parent.left;
34+
35+
// no default
36+
}
37+
}
38+
39+
return false;
40+
}
41+
42+
/**
43+
* Creates an array with elements from the first given array that are not included in the second given array.
44+
* @param {Array} arrA The array to compare from.
45+
* @param {Array} arrB The array to compare against.
46+
* @returns {Array} a new array that represents `arrA \ arrB`.
47+
*/
48+
function getDifference(arrA, arrB) {
49+
return arrA.filter(a => !arrB.includes(a));
50+
}
51+
52+
//------------------------------------------------------------------------------
53+
// Rule Definition
54+
//------------------------------------------------------------------------------
55+
56+
module.exports = {
57+
meta: {
58+
type: "problem",
59+
60+
docs: {
61+
description: "disallow loops with a body that allows only one iteration",
62+
category: "Possible Errors",
63+
recommended: false,
64+
url: "https://eslint.org/docs/rules/no-unreachable-loop"
65+
},
66+
67+
schema: [{
68+
type: "object",
69+
properties: {
70+
ignore: {
71+
type: "array",
72+
items: {
73+
enum: allLoopTypes
74+
},
75+
uniqueItems: true
76+
}
77+
},
78+
additionalProperties: false
79+
}],
80+
81+
messages: {
82+
invalid: "Invalid loop. Its body allows only one iteration."
83+
}
84+
},
85+
86+
create(context) {
87+
const ignoredLoopTypes = context.options[0] && context.options[0].ignore || [],
88+
loopTypesToCheck = getDifference(allLoopTypes, ignoredLoopTypes),
89+
loopSelector = loopTypesToCheck.join(","),
90+
loopsByTargetSegments = new Map(),
91+
loopsToReport = new Set();
92+
93+
let currentCodePath = null;
94+
95+
return {
96+
onCodePathStart(codePath) {
97+
currentCodePath = codePath;
98+
},
99+
100+
onCodePathEnd() {
101+
currentCodePath = currentCodePath.upper;
102+
},
103+
104+
[loopSelector](node) {
105+
106+
/**
107+
* Ignore unreachable loop statements to avoid unnecessary complexity in the implementation, or false positives otherwise.
108+
* For unreachable segments, the code path analysis does not raise events required for this implementation.
109+
*/
110+
if (currentCodePath.currentSegments.some(segment => segment.reachable)) {
111+
loopsToReport.add(node);
112+
}
113+
},
114+
115+
onCodePathSegmentStart(segment, node) {
116+
if (isLoopingTarget(node)) {
117+
const loop = node.parent;
118+
119+
loopsByTargetSegments.set(segment, loop);
120+
}
121+
},
122+
123+
onCodePathSegmentLoop(_, toSegment, node) {
124+
const loop = loopsByTargetSegments.get(toSegment);
125+
126+
/**
127+
* The second iteration is reachable, meaning that the loop is valid by the logic of this rule,
128+
* only if there is at least one loop event with the appropriate target (which has been already
129+
* determined in the `loopsByTargetSegments` map), raised from either:
130+
*
131+
* - the end of the loop's body (in which case `node === loop`)
132+
* - a `continue` statement
133+
*
134+
* This condition skips loop events raised from `ForInStatement > .right` and `ForOfStatement > .right` nodes.
135+
*/
136+
if (node === loop || node.type === "ContinueStatement") {
137+
138+
// Removes loop if it exists in the set. Otherwise, `Set#delete` has no effect and doesn't throw.
139+
loopsToReport.delete(loop);
140+
}
141+
},
142+
143+
"Program:exit"() {
144+
loopsToReport.forEach(
145+
node => context.report({ node, messageId: "invalid" })
146+
);
147+
}
148+
};
149+
}
150+
};

0 commit comments

Comments
 (0)