Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update: Add enforceForSwitchCase option to use-isnan #12106

Merged
merged 2 commits into from Sep 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
63 changes: 63 additions & 0 deletions docs/rules/use-isnan.md
Expand Up @@ -40,3 +40,66 @@ if (!isNaN(foo)) {
// ...
}
```

## Options

This rule has an object option, with one option:

* `"enforceForSwitchCase"` when set to `true` disallows `case NaN` and `switch(NaN)` in `switch` statements. Default is `false`, meaning
that this rule by default does not warn about `case NaN` or `switch(NaN)`.

### enforceForSwitchCase

The `switch` statement internally uses the `===` comparison to match the expression's value to a case clause.
Therefore, it can never match `case NaN`. Also, `switch(NaN)` can never match a case clause.

Set `"enforceForSwitchCase"` to `true` if you want this rule to report `case NaN` and `switch(NaN)` in `switch` statements.

Examples of **incorrect** code for this rule with `"enforceForSwitchCase"` option set to `true`:

```js
/*eslint use-isnan: ["error", {"enforceForSwitchCase": true}]*/

switch (foo) {
case NaN:
bar();
break;
case 1:
baz();
break;
// ...
}

switch (NaN) {
case a:
bar();
break;
case b:
baz();
break;
// ...
}
```

Examples of **correct** code for this rule with `"enforceForSwitchCase"` option set to `true`:

```js
/*eslint use-isnan: ["error", {"enforceForSwitchCase": true}]*/

if (Number.isNaN(foo)) {
bar();
} else {
switch (foo) {
case 1:
baz();
break;
// ...
}
}

if (Number.isNaN(a)) {
bar();
} else if (Number.isNaN(b)) {
baz();
} // ...
```
73 changes: 67 additions & 6 deletions lib/rules/use-isnan.js
Expand Up @@ -5,6 +5,19 @@

"use strict";

//------------------------------------------------------------------------------
// Helpers
//------------------------------------------------------------------------------

/**
* Determines if the given node is a NaN `Identifier` node.
* @param {ASTNode|null} node The node to check.
* @returns {boolean} `true` if the node is 'NaN' identifier.
*/
function isNaNIdentifier(node) {
return Boolean(node) && node.type === "Identifier" && node.name === "NaN";
}

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand All @@ -20,21 +33,69 @@ module.exports = {
url: "https://eslint.org/docs/rules/use-isnan"
},

schema: [],
schema: [
{
type: "object",
properties: {
enforceForSwitchCase: {
type: "boolean",
default: false
}
},
additionalProperties: false
}
],

messages: {
useIsNaN: "Use the isNaN function to compare with NaN."
comparisonWithNaN: "Use the isNaN function to compare with NaN.",
switchNaN: "'switch(NaN)' can never match a case clause. Use Number.isNaN instead of the switch.",
caseNaN: "'case NaN' can never match. Use Number.isNaN before the switch."
}
},

create(context) {

return {
BinaryExpression(node) {
if (/^(?:[<>]|[!=]=)=?$/u.test(node.operator) && (node.left.name === "NaN" || node.right.name === "NaN")) {
context.report({ node, messageId: "useIsNaN" });
const enforceForSwitchCase = context.options[0] && context.options[0].enforceForSwitchCase;

/**
* Checks the given `BinaryExpression` node.
* @param {ASTNode} node The node to check.
* @returns {void}
*/
function checkBinaryExpression(node) {
if (
/^(?:[<>]|[!=]=)=?$/u.test(node.operator) &&
(isNaNIdentifier(node.left) || isNaNIdentifier(node.right))
) {
context.report({ node, messageId: "comparisonWithNaN" });
}
}

/**
* Checks the discriminant and all case clauses of the given `SwitchStatement` node.
* @param {ASTNode} node The node to check.
* @returns {void}
*/
function checkSwitchStatement(node) {
if (isNaNIdentifier(node.discriminant)) {
context.report({ node, messageId: "switchNaN" });
}

for (const switchCase of node.cases) {
if (isNaNIdentifier(switchCase.test)) {
context.report({ node: switchCase, messageId: "caseNaN" });
}
}
}

const listeners = {
BinaryExpression: checkBinaryExpression
};

if (enforceForSwitchCase) {
listeners.SwitchStatement = checkSwitchStatement;
}

return listeners;
}
};