Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
New: Add no-dupe-else-if rule (fixes #12469)
- Loading branch information
1 parent
8ac71a3
commit 9088993
Showing
5 changed files
with
358 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,130 @@ | ||
# Disallow duplicate conditions in `if-else-if` chains (no-dupe-else-if) | ||
|
||
`if-else-if` chains are commonly used when there is a need to execute only one branch or at most one branch out of several possible branches, based on certain conditions. | ||
|
||
```js | ||
if (a) { | ||
foo(); | ||
} else if (b) { | ||
bar(); | ||
} else if (c) { | ||
baz(); | ||
} | ||
``` | ||
|
||
Two identical test conditions in the same chain are almost always a mistake in the code. Unless there are side effects in the expressions, a duplicate will evaluate to the same `true` or `false` value as the identical expression earlier in the chain, meaning that its branch can never execute. | ||
|
||
```js | ||
if (a) { | ||
foo(); | ||
} else if (b) { | ||
bar(); | ||
} else if (b) { | ||
baz(); | ||
} | ||
``` | ||
|
||
In the above example, `baz()` can never execute. Obviously, `baz()` could be executed only when `b` evaluates to `true`, but in that case `bar()` would be executed instead, since it's earlier in the chain. | ||
|
||
## Rule Details | ||
|
||
This rule disallows duplicate conditions in the same `if-else-if` chain. | ||
|
||
Examples of **incorrect** code for this rule: | ||
|
||
```js | ||
/*eslint no-dupe-else-if: "error"*/ | ||
|
||
if (isSomething(x)) { | ||
foo(); | ||
} else if (isSomething(x)) { | ||
bar(); | ||
} | ||
|
||
if (a) { | ||
foo(); | ||
} else if (b) { | ||
bar(); | ||
} else if (c && d) { | ||
baz(); | ||
} else if (c && d) { | ||
quux(); | ||
} else { | ||
quuux(); | ||
} | ||
|
||
if (n === 1) { | ||
foo(); | ||
} else if (n === 2) { | ||
bar(); | ||
} else if (n === 3) { | ||
baz(); | ||
} else if (n === 2) { | ||
quux(); | ||
} else if (n === 5) { | ||
quuux(); | ||
} | ||
``` | ||
|
||
Examples of **correct** code for this rule: | ||
|
||
```js | ||
/*eslint no-dupe-else-if: "error"*/ | ||
|
||
if (isSomething(x)) { | ||
foo(); | ||
} else if (isSomethingElse(x)) { | ||
bar(); | ||
} | ||
|
||
if (a) { | ||
foo(); | ||
} else if (b) { | ||
bar(); | ||
} else if (c && d) { | ||
baz(); | ||
} else if (c && e) { | ||
quux(); | ||
} else { | ||
quuux(); | ||
} | ||
|
||
if (n === 1) { | ||
foo(); | ||
} else if (n === 2) { | ||
bar(); | ||
} else if (n === 3) { | ||
baz(); | ||
} else if (n === 4) { | ||
quux(); | ||
} else if (n === 5) { | ||
quuux(); | ||
} | ||
``` | ||
|
||
Please note that this rule does not compare conditions from the chain with conditions inside statements, and will not warn in the cases such as follows: | ||
|
||
```js | ||
if (a) { | ||
if (a) { | ||
foo(); | ||
} | ||
} | ||
|
||
if (a) { | ||
foo(); | ||
} else { | ||
if (a) { | ||
bar(); | ||
} | ||
} | ||
``` | ||
|
||
## When Not To Use It | ||
|
||
In rare cases where you really need identical test conditions in the same chain, which necessarily means that the expressions in the chain are causing and relying on side effects, you will have to turn this rule off. | ||
|
||
## Related Rules | ||
|
||
* [no-duplicate-case](no-duplicate-case.md) | ||
* [no-lonely-if](no-lonely-if.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,54 @@ | ||
/** | ||
* @fileoverview Rule to disallow duplicate conditions in if-else-if chains | ||
* @author Milos Djermanovic | ||
*/ | ||
|
||
"use strict"; | ||
|
||
//------------------------------------------------------------------------------ | ||
// Requirements | ||
//------------------------------------------------------------------------------ | ||
|
||
const astUtils = require("./utils/ast-utils"); | ||
|
||
//------------------------------------------------------------------------------ | ||
// Rule Definition | ||
//------------------------------------------------------------------------------ | ||
|
||
module.exports = { | ||
meta: { | ||
type: "problem", | ||
|
||
docs: { | ||
description: "disallow duplicate conditions in if-else-if chains", | ||
category: "Possible Errors", | ||
recommended: false, | ||
url: "https://eslint.org/docs/rules/no-dupe-else-if" | ||
}, | ||
|
||
schema: [], | ||
|
||
messages: { | ||
unexpected: "Duplicate condition in if-else-if chain." | ||
} | ||
}, | ||
|
||
create(context) { | ||
const sourceCode = context.getSourceCode(); | ||
|
||
return { | ||
IfStatement(node) { | ||
let current = node; | ||
|
||
while (current.parent && current.parent.type === "IfStatement" && current.parent.alternate === current) { | ||
current = current.parent; | ||
|
||
if (astUtils.equalTokens(node.test, current.test, sourceCode)) { | ||
context.report({ node: node.test, messageId: "unexpected" }); | ||
break; | ||
} | ||
} | ||
} | ||
}; | ||
} | ||
}; |
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,172 @@ | ||
/** | ||
* @fileoverview Tests for the no-dupe-else-if rule | ||
* @author Milos Djermanovic | ||
*/ | ||
|
||
"use strict"; | ||
|
||
//------------------------------------------------------------------------------ | ||
// Requirements | ||
//------------------------------------------------------------------------------ | ||
|
||
const rule = require("../../../lib/rules/no-dupe-else-if"); | ||
const { RuleTester } = require("../../../lib/rule-tester"); | ||
|
||
//------------------------------------------------------------------------------ | ||
// Tests | ||
//------------------------------------------------------------------------------ | ||
|
||
const ruleTester = new RuleTester(); | ||
|
||
ruleTester.run("no-dupe-else-if", rule, { | ||
valid: [ | ||
|
||
// different test conditions | ||
"if (a) {} else if (b) {}", | ||
"if (a); else if (b); else if (c);", | ||
"if (true) {} else if (false) {} else {}", | ||
"if (1) {} else if (2) {}", | ||
"if (f) {} else if (f()) {}", | ||
"if (f(a)) {} else if (g(a)) {}", | ||
"if (f(a)) {} else if (f(b)) {}", | ||
"if (a === 1) {} else if (a === 2) {}", | ||
"if (a === 1) {} else if (b === 1) {}", | ||
|
||
// not an if-else-if chain | ||
"if (a) {}", | ||
"if (a);", | ||
"if (a) {} else {}", | ||
"if (a) if (a) {}", | ||
"if (a) if (a);", | ||
"if (a) { if (a) {} }", | ||
"if (a) {} else { if (a) {} }", | ||
"if (a) {} if (a) {}", | ||
"if (a); if (a);", | ||
"while (a) if (a);", | ||
"if (a); else a ? a : a;", | ||
|
||
// not same conditions in the chain | ||
"if (a) { if (b) {} } else if (b) {}", | ||
"if (a) if (b); else if (a);", | ||
|
||
// not equal tokens | ||
"if (a) {} else if (!!a) {}", | ||
"if (a === 1) {} else if (a === (1)) {}" | ||
], | ||
|
||
invalid: [ | ||
|
||
// basic tests | ||
{ | ||
code: "if (a) {} else if (a) {}", | ||
errors: [{ messageId: "unexpected", type: "Identifier", column: 20 }] | ||
}, | ||
{ | ||
code: "if (a); else if (a);", | ||
errors: [{ messageId: "unexpected", type: "Identifier", column: 18 }] | ||
}, | ||
{ | ||
code: "if (a) {} else if (a) {} else {}", | ||
errors: [{ messageId: "unexpected", type: "Identifier", column: 20 }] | ||
}, | ||
{ | ||
code: "if (a) {} else if (b) {} else if (a) {} else if (c) {}", | ||
errors: [{ messageId: "unexpected", type: "Identifier", column: 35 }] | ||
}, | ||
{ | ||
code: "if (a) {} else if (b) {} else if (a) {}", | ||
errors: [{ messageId: "unexpected", type: "Identifier", column: 35 }] | ||
}, | ||
{ | ||
code: "if (a) {} else if (b) {} else if (c) {} else if (a) {}", | ||
errors: [{ messageId: "unexpected", type: "Identifier", column: 50 }] | ||
}, | ||
{ | ||
code: "if (a) {} else if (b) {} else if (b) {}", | ||
errors: [{ messageId: "unexpected", type: "Identifier", column: 35 }] | ||
}, | ||
{ | ||
code: "if (a) {} else if (b) {} else if (b) {} else {}", | ||
errors: [{ messageId: "unexpected", type: "Identifier", column: 35 }] | ||
}, | ||
{ | ||
code: "if (a) {} else if (b) {} else if (c) {} else if (b) {}", | ||
errors: [{ messageId: "unexpected", type: "Identifier", column: 50 }] | ||
}, | ||
{ | ||
code: "if (a); else if (b); else if (c); else if (b); else if (d); else;", | ||
errors: [{ messageId: "unexpected", type: "Identifier", column: 44 }] | ||
}, | ||
{ | ||
code: "if (a); else if (b); else if (c); else if (d); else if (b); else if (e);", | ||
errors: [{ messageId: "unexpected", type: "Identifier", column: 57 }] | ||
}, | ||
|
||
// multiple duplicates of the same condition | ||
{ | ||
code: "if (a) {} else if (a) {} else if (a) {}", | ||
errors: [ | ||
{ messageId: "unexpected", type: "Identifier", column: 20 }, | ||
{ messageId: "unexpected", type: "Identifier", column: 35 } | ||
] | ||
}, | ||
|
||
// multiple duplicates of different conditions | ||
{ | ||
code: "if (a) {} else if (b) {} else if (a) {} else if (b) {} else if (a) {}", | ||
errors: [ | ||
{ messageId: "unexpected", type: "Identifier", column: 35 }, | ||
{ messageId: "unexpected", type: "Identifier", column: 50 }, | ||
{ messageId: "unexpected", type: "Identifier", column: 65 } | ||
] | ||
}, | ||
|
||
// inner if statements do not affect chain | ||
{ | ||
code: "if (a) { if (b) {} } else if (a) {}", | ||
errors: [{ messageId: "unexpected", type: "Identifier", column: 31 }] | ||
}, | ||
|
||
// various kinds of test conditions | ||
{ | ||
code: "if (a === 1) {} else if (a === 1) {}", | ||
errors: [{ messageId: "unexpected", type: "BinaryExpression", column: 26 }] | ||
}, | ||
{ | ||
code: "if (1 < a) {} else if (1 < a) {}", | ||
errors: [{ messageId: "unexpected", type: "BinaryExpression", column: 24 }] | ||
}, | ||
{ | ||
code: "if (true) {} else if (true) {}", | ||
errors: [{ messageId: "unexpected", type: "Literal", column: 23 }] | ||
}, | ||
{ | ||
code: "if (a && b) {} else if (a && b) {}", | ||
errors: [{ messageId: "unexpected", type: "LogicalExpression", column: 25 }] | ||
}, | ||
{ | ||
code: "if (a && b || c) {} else if (a && b || c) {}", | ||
errors: [{ messageId: "unexpected", type: "LogicalExpression", column: 31 }] | ||
}, | ||
{ | ||
code: "if (f(a)) {} else if (f(a)) {}", | ||
errors: [{ messageId: "unexpected", type: "CallExpression", column: 23 }] | ||
}, | ||
|
||
// spaces and comments do not affect comparison | ||
{ | ||
code: "if (a === 1) {} else if (a===1) {}", | ||
errors: [{ messageId: "unexpected", type: "BinaryExpression", column: 26 }] | ||
}, | ||
{ | ||
code: "if (a === 1) {} else if (a === /* comment */ 1) {}", | ||
errors: [{ messageId: "unexpected", type: "BinaryExpression", column: 26 }] | ||
}, | ||
|
||
// extra parens around the whole test condition do not affect comparison | ||
{ | ||
code: "if (a === 1) {} else if ((a === 1)) {}", | ||
errors: [{ messageId: "unexpected", type: "BinaryExpression", column: 27 }] | ||
} | ||
] | ||
}); |
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