Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Browse files
Browse the repository at this point in the history
* New: Add no-unreachable-loop rule (fixes #12381) * Add fallthrough switch test
- Loading branch information
1 parent
13999d2
commit b550330
Showing
5 changed files
with
773 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,190 @@ | ||
# Disallow loops with a body that allows only one iteration (no-unreachable-loop) | ||
|
||
A loop that can never reach the second iteration is a possible error in the code. | ||
|
||
```js | ||
for (let i = 0; i < arr.length; i++) { | ||
if (arr[i].name === myName) { | ||
doSomething(arr[i]); | ||
// break was supposed to be here | ||
} | ||
break; | ||
} | ||
``` | ||
|
||
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. | ||
|
||
## Rule Details | ||
|
||
This rule aims to detect and disallow loops that can have at most one iteration, by performing static code path analysis on loop bodies. | ||
|
||
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. | ||
|
||
This rule checks `while`, `do-while`, `for`, `for-in` and `for-of` loops. You can optionally disable checks for each of these constructs. | ||
|
||
Examples of **incorrect** code for this rule: | ||
|
||
```js | ||
/*eslint no-unreachable-loop: "error"*/ | ||
|
||
while (foo) { | ||
doSomething(foo); | ||
foo = foo.parent; | ||
break; | ||
} | ||
|
||
function verifyList(head) { | ||
let item = head; | ||
do { | ||
if (verify(item)) { | ||
return true; | ||
} else { | ||
return false; | ||
} | ||
} while (item); | ||
} | ||
|
||
function findSomething(arr) { | ||
for (var i = 0; i < arr.length; i++) { | ||
if (isSomething(arr[i])) { | ||
return arr[i]; | ||
} else { | ||
throw new Error("Doesn't exist."); | ||
} | ||
} | ||
} | ||
|
||
for (key in obj) { | ||
if (key.startsWith("_")) { | ||
break; | ||
} | ||
firstKey = key; | ||
firstValue = obj[key]; | ||
break; | ||
} | ||
|
||
for (foo of bar) { | ||
if (foo.id === id) { | ||
doSomething(foo); | ||
} | ||
break; | ||
} | ||
``` | ||
|
||
Examples of **correct** code for this rule: | ||
|
||
```js | ||
/*eslint no-unreachable-loop: "error"*/ | ||
|
||
while (foo) { | ||
doSomething(foo); | ||
foo = foo.parent; | ||
} | ||
|
||
function verifyList(head) { | ||
let item = head; | ||
do { | ||
if (verify(item)) { | ||
item = item.next; | ||
} else { | ||
return false; | ||
} | ||
} while (item); | ||
|
||
return true; | ||
} | ||
|
||
function findSomething(arr) { | ||
for (var i = 0; i < arr.length; i++) { | ||
if (isSomething(arr[i])) { | ||
return arr[i]; | ||
} | ||
} | ||
throw new Error("Doesn't exist."); | ||
} | ||
|
||
for (key in obj) { | ||
if (key.startsWith("_")) { | ||
continue; | ||
} | ||
firstKey = key; | ||
firstValue = obj[key]; | ||
break; | ||
} | ||
|
||
for (foo of bar) { | ||
if (foo.id === id) { | ||
doSomething(foo); | ||
break; | ||
} | ||
} | ||
``` | ||
|
||
Please note that this rule is not designed to check loop conditions, and will not warn in cases such as the following examples. | ||
|
||
Examples of additional **correct** code for this rule: | ||
|
||
```js | ||
/*eslint no-unreachable-loop: "error"*/ | ||
|
||
do { | ||
doSomething(); | ||
} while (false) | ||
|
||
for (let i = 0; i < 1; i++) { | ||
doSomething(i); | ||
} | ||
|
||
for (const a of [1]) { | ||
doSomething(a); | ||
} | ||
``` | ||
|
||
## Options | ||
|
||
This rule has an object option, with one option: | ||
|
||
* `"ignore"` - an optional array of loop types that will be ignored by this rule. | ||
|
||
## ignore | ||
|
||
You can specify up to 5 different elements in the `"ignore"` array: | ||
|
||
* `"WhileStatement"` - to ignore all `while` loops. | ||
* `"DoWhileStatement"` - to ignore all `do-while` loops. | ||
* `"ForStatement"` - to ignore all `for` loops (does not apply to `for-in` and `for-of` loops). | ||
* `"ForInStatement"` - to ignore all `for-in` loops. | ||
* `"ForOfStatement"` - to ignore all `for-of` loops. | ||
|
||
Examples of **correct** code for this rule with the `"ignore"` option: | ||
|
||
```js | ||
/*eslint no-unreachable-loop: ["error", { "ignore": ["ForInStatement", "ForOfStatement"] }]*/ | ||
|
||
for (var key in obj) { | ||
hasEnumerableProperties = true; | ||
break; | ||
} | ||
|
||
for (const a of b) break; | ||
``` | ||
|
||
## Known Limitations | ||
|
||
Static code path analysis, in general, does not evaluate conditions. Due to this fact, this rule might miss reporting cases such as the following: | ||
|
||
```js | ||
for (let i = 0; i < 10; i++) { | ||
doSomething(i); | ||
if (true) { | ||
break; | ||
} | ||
} | ||
``` | ||
|
||
## Related Rules | ||
|
||
* [no-unreachable](no-unreachable.md) | ||
* [no-constant-condition](no-constant-condition.md) | ||
* [no-unmodified-loop-condition](no-unmodified-loop-condition.md) | ||
* [for-direction](for-direction.md) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,150 @@ | ||
/** | ||
* @fileoverview Rule to disallow loops with a body that allows only one iteration | ||
* @author Milos Djermanovic | ||
*/ | ||
|
||
"use strict"; | ||
|
||
//------------------------------------------------------------------------------ | ||
// Helpers | ||
//------------------------------------------------------------------------------ | ||
|
||
const allLoopTypes = ["WhileStatement", "DoWhileStatement", "ForStatement", "ForInStatement", "ForOfStatement"]; | ||
|
||
/** | ||
* Determines whether the given node is the first node in the code path to which a loop statement | ||
* 'loops' for the next iteration. | ||
* @param {ASTNode} node The node to check. | ||
* @returns {boolean} `true` if the node is a looping target. | ||
*/ | ||
function isLoopingTarget(node) { | ||
const parent = node.parent; | ||
|
||
if (parent) { | ||
switch (parent.type) { | ||
case "WhileStatement": | ||
return node === parent.test; | ||
case "DoWhileStatement": | ||
return node === parent.body; | ||
case "ForStatement": | ||
return node === (parent.update || parent.test || parent.body); | ||
case "ForInStatement": | ||
case "ForOfStatement": | ||
return node === parent.left; | ||
|
||
// no default | ||
} | ||
} | ||
|
||
return false; | ||
} | ||
|
||
/** | ||
* Creates an array with elements from the first given array that are not included in the second given array. | ||
* @param {Array} arrA The array to compare from. | ||
* @param {Array} arrB The array to compare against. | ||
* @returns {Array} a new array that represents `arrA \ arrB`. | ||
*/ | ||
function getDifference(arrA, arrB) { | ||
return arrA.filter(a => !arrB.includes(a)); | ||
} | ||
|
||
//------------------------------------------------------------------------------ | ||
// Rule Definition | ||
//------------------------------------------------------------------------------ | ||
|
||
module.exports = { | ||
meta: { | ||
type: "problem", | ||
|
||
docs: { | ||
description: "disallow loops with a body that allows only one iteration", | ||
category: "Possible Errors", | ||
recommended: false, | ||
url: "https://eslint.org/docs/rules/no-unreachable-loop" | ||
}, | ||
|
||
schema: [{ | ||
type: "object", | ||
properties: { | ||
ignore: { | ||
type: "array", | ||
items: { | ||
enum: allLoopTypes | ||
}, | ||
uniqueItems: true | ||
} | ||
}, | ||
additionalProperties: false | ||
}], | ||
|
||
messages: { | ||
invalid: "Invalid loop. Its body allows only one iteration." | ||
} | ||
}, | ||
|
||
create(context) { | ||
const ignoredLoopTypes = context.options[0] && context.options[0].ignore || [], | ||
loopTypesToCheck = getDifference(allLoopTypes, ignoredLoopTypes), | ||
loopSelector = loopTypesToCheck.join(","), | ||
loopsByTargetSegments = new Map(), | ||
loopsToReport = new Set(); | ||
|
||
let currentCodePath = null; | ||
|
||
return { | ||
onCodePathStart(codePath) { | ||
currentCodePath = codePath; | ||
}, | ||
|
||
onCodePathEnd() { | ||
currentCodePath = currentCodePath.upper; | ||
}, | ||
|
||
[loopSelector](node) { | ||
|
||
/** | ||
* Ignore unreachable loop statements to avoid unnecessary complexity in the implementation, or false positives otherwise. | ||
* For unreachable segments, the code path analysis does not raise events required for this implementation. | ||
*/ | ||
if (currentCodePath.currentSegments.some(segment => segment.reachable)) { | ||
loopsToReport.add(node); | ||
} | ||
}, | ||
|
||
onCodePathSegmentStart(segment, node) { | ||
if (isLoopingTarget(node)) { | ||
const loop = node.parent; | ||
|
||
loopsByTargetSegments.set(segment, loop); | ||
} | ||
}, | ||
|
||
onCodePathSegmentLoop(_, toSegment, node) { | ||
const loop = loopsByTargetSegments.get(toSegment); | ||
|
||
/** | ||
* The second iteration is reachable, meaning that the loop is valid by the logic of this rule, | ||
* only if there is at least one loop event with the appropriate target (which has been already | ||
* determined in the `loopsByTargetSegments` map), raised from either: | ||
* | ||
* - the end of the loop's body (in which case `node === loop`) | ||
* - a `continue` statement | ||
* | ||
* This condition skips loop events raised from `ForInStatement > .right` and `ForOfStatement > .right` nodes. | ||
*/ | ||
if (node === loop || node.type === "ContinueStatement") { | ||
|
||
// Removes loop if it exists in the set. Otherwise, `Set#delete` has no effect and doesn't throw. | ||
loopsToReport.delete(loop); | ||
} | ||
}, | ||
|
||
"Program:exit"() { | ||
loopsToReport.forEach( | ||
node => context.report({ node, messageId: "invalid" }) | ||
); | ||
} | ||
}; | ||
} | ||
}; |
Oops, something went wrong.