Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Update: Add enforceForIndexOf option to use-isnan (fixes #12207) (#12379
)

* Update: Add enforceForIndexOf option to use-isnan (fixes #12207)

* Remove Array prototype from a sentence and fix newlines in docs
  • Loading branch information
mdjermanovic authored and platinumazure committed Oct 25, 2019
1 parent 364877b commit c6a9a3b
Show file tree
Hide file tree
Showing 3 changed files with 246 additions and 6 deletions.
78 changes: 75 additions & 3 deletions docs/rules/use-isnan.md
Expand Up @@ -43,10 +43,10 @@ if (!isNaN(foo)) {

## Options

This rule has an object option, with one option:
This rule has an object option, with two options:

* `"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"` 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)`.
* `"enforceForIndexOf"` when set to `true` disallows the use of `indexOf` and `lastIndexOf` methods with `NaN`. Default is `false`, meaning that this rule by default does not warn about `indexOf(NaN)` or `lastIndexOf(NaN)` method calls.

### enforceForSwitchCase

Expand Down Expand Up @@ -103,3 +103,75 @@ if (Number.isNaN(a)) {
baz();
} // ...
```

### enforceForIndexOf

The following methods internally use the `===` comparison to match the given value with an array element:

* [`Array.prototype.indexOf`](https://www.ecma-international.org/ecma-262/6.0/#sec-array.prototype.indexof)
* [`Array.prototype.lastIndexOf`](https://www.ecma-international.org/ecma-262/6.0/#sec-array.prototype.lastindexof)

Therefore, for any array `foo`, `foo.indexOf(NaN)` and `foo.lastIndexOf(NaN)` will always return `-1`.

Set `"enforceForIndexOf"` to `true` if you want this rule to report `indexOf(NaN)` and `lastIndexOf(NaN)` method calls.

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

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

var hasNaN = myArray.indexOf(NaN) >= 0;

var firstIndex = myArray.indexOf(NaN);

var lastIndex = myArray.lastIndexOf(NaN);
```

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

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

function myIsNaN(val) {
return typeof val === "number" && isNaN(val);
}

function indexOfNaN(arr) {
for (var i = 0; i < arr.length; i++) {
if (myIsNaN(arr[i])) {
return i;
}
}
return -1;
}

function lastIndexOfNaN(arr) {
for (var i = arr.length - 1; i >= 0; i--) {
if (myIsNaN(arr[i])) {
return i;
}
}
return -1;
}

var hasNaN = myArray.some(myIsNaN);

var hasNaN = indexOfNaN(myArray) >= 0;

var firstIndex = indexOfNaN(myArray);

var lastIndex = lastIndexOfNaN(myArray);

// ES2015
var hasNaN = myArray.some(Number.isNaN);

// ES2015
var firstIndex = myArray.findIndex(Number.isNaN);

// ES2016
var hasNaN = myArray.includes(NaN);
```

#### Known Limitations

This option checks methods with the given names, *even if* the object which has the method is *not* an array.
43 changes: 40 additions & 3 deletions lib/rules/use-isnan.js
Expand Up @@ -5,6 +5,12 @@

"use strict";

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const astUtils = require("./utils/ast-utils");

//------------------------------------------------------------------------------
// Helpers
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -40,6 +46,10 @@ module.exports = {
enforceForSwitchCase: {
type: "boolean",
default: false
},
enforceForIndexOf: {
type: "boolean",
default: false
}
},
additionalProperties: false
Expand All @@ -49,16 +59,18 @@ module.exports = {
messages: {
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."
caseNaN: "'case NaN' can never match. Use Number.isNaN before the switch.",
indexOfNaN: "Array prototype method '{{ methodName }}' cannot find NaN."
}
},

create(context) {

const enforceForSwitchCase = context.options[0] && context.options[0].enforceForSwitchCase;
const enforceForIndexOf = context.options[0] && context.options[0].enforceForIndexOf;

/**
* Checks the given `BinaryExpression` node.
* Checks the given `BinaryExpression` node for `foo === NaN` and other comparisons.
* @param {ASTNode} node The node to check.
* @returns {void}
*/
Expand All @@ -72,7 +84,7 @@ module.exports = {
}

/**
* Checks the discriminant and all case clauses of the given `SwitchStatement` node.
* Checks the discriminant and all case clauses of the given `SwitchStatement` node for `switch(NaN)` and `case NaN:`
* @param {ASTNode} node The node to check.
* @returns {void}
*/
Expand All @@ -88,6 +100,27 @@ module.exports = {
}
}

/**
* Checks the the given `CallExpression` node for `.indexOf(NaN)` and `.lastIndexOf(NaN)`.
* @param {ASTNode} node The node to check.
* @returns {void}
*/
function checkCallExpression(node) {
const callee = node.callee;

if (callee.type === "MemberExpression") {
const methodName = astUtils.getStaticPropertyName(callee);

if (
(methodName === "indexOf" || methodName === "lastIndexOf") &&
node.arguments.length === 1 &&
isNaNIdentifier(node.arguments[0])
) {
context.report({ node, messageId: "indexOfNaN", data: { methodName } });
}
}
}

const listeners = {
BinaryExpression: checkBinaryExpression
};
Expand All @@ -96,6 +129,10 @@ module.exports = {
listeners.SwitchStatement = checkSwitchStatement;
}

if (enforceForIndexOf) {
listeners.CallExpression = checkCallExpression;
}

return listeners;
}
};
131 changes: 131 additions & 0 deletions tests/lib/rules/use-isnan.js
Expand Up @@ -114,6 +114,102 @@ ruleTester.run("use-isnan", rule, {
{
code: "switch(foo) { case bar: break; case 1: break; default: break; }",
options: [{ enforceForSwitchCase: true }]
},

//------------------------------------------------------------------------------
// enforceForIndexOf
//------------------------------------------------------------------------------

"foo.indexOf(NaN)",
"foo.lastIndexOf(NaN)",
{
code: "foo.indexOf(NaN)",
options: [{}]
},
{
code: "foo.lastIndexOf(NaN)",
options: [{}]
},
{
code: "foo.indexOf(NaN)",
options: [{ enforceForIndexOf: false }]
},
{
code: "foo.lastIndexOf(NaN)",
options: [{ enforceForIndexOf: false }]
},
{
code: "indexOf(NaN)",
options: [{ enforceForIndexOf: true }]
},
{
code: "lastIndexOf(NaN)",
options: [{ enforceForIndexOf: true }]
},
{
code: "new foo.indexOf(NaN)",
options: [{ enforceForIndexOf: true }]
},
{
code: "foo.bar(NaN)",
options: [{ enforceForIndexOf: true }]
},
{
code: "foo.IndexOf(NaN)",
options: [{ enforceForIndexOf: true }]
},
{
code: "foo[indexOf](NaN)",
options: [{ enforceForIndexOf: true }]
},
{
code: "foo[lastIndexOf](NaN)",
options: [{ enforceForIndexOf: true }]
},
{
code: "indexOf.foo(NaN)",
options: [{ enforceForIndexOf: true }]
},
{
code: "foo.indexOf()",
options: [{ enforceForIndexOf: true }]
},
{
code: "foo.lastIndexOf()",
options: [{ enforceForIndexOf: true }]
},
{
code: "foo.indexOf(a)",
options: [{ enforceForIndexOf: true }]
},
{
code: "foo.lastIndexOf(Nan)",
options: [{ enforceForIndexOf: true }]
},
{
code: "foo.indexOf(a, NaN)",
options: [{ enforceForIndexOf: true }]
},
{
code: "foo.lastIndexOf(NaN, b)",
options: [{ enforceForIndexOf: true }]
},
{
code: "foo.indexOf(a, b)",
options: [{ enforceForIndexOf: true }]
},
{
code: "foo.lastIndexOf(NaN, NaN)",
options: [{ enforceForIndexOf: true }]
},
{
code: "foo.indexOf(...NaN)",
options: [{ enforceForIndexOf: true }],
parserOptions: { ecmaVersion: 6 }
},
{
code: "foo.lastIndexOf(NaN())",
options: [{ enforceForIndexOf: true }]
}
],
invalid: [
Expand Down Expand Up @@ -246,6 +342,41 @@ ruleTester.run("use-isnan", rule, {
{ messageId: "switchNaN", type: "SwitchStatement", column: 1 },
{ messageId: "caseNaN", type: "SwitchCase", column: 15 }
]
},

//------------------------------------------------------------------------------
// enforceForIndexOf
//------------------------------------------------------------------------------

{
code: "foo.indexOf(NaN)",
options: [{ enforceForIndexOf: true }],
errors: [{ messageId: "indexOfNaN", type: "CallExpression", data: { methodName: "indexOf" } }]
},
{
code: "foo.lastIndexOf(NaN)",
options: [{ enforceForIndexOf: true }],
errors: [{ messageId: "indexOfNaN", type: "CallExpression", data: { methodName: "lastIndexOf" } }]
},
{
code: "foo['indexOf'](NaN)",
options: [{ enforceForIndexOf: true }],
errors: [{ messageId: "indexOfNaN", type: "CallExpression", data: { methodName: "indexOf" } }]
},
{
code: "foo['lastIndexOf'](NaN)",
options: [{ enforceForIndexOf: true }],
errors: [{ messageId: "indexOfNaN", type: "CallExpression", data: { methodName: "lastIndexOf" } }]
},
{
code: "foo().indexOf(NaN)",
options: [{ enforceForIndexOf: true }],
errors: [{ messageId: "indexOfNaN", type: "CallExpression", data: { methodName: "indexOf" } }]
},
{
code: "foo.bar.lastIndexOf(NaN)",
options: [{ enforceForIndexOf: true }],
errors: [{ messageId: "indexOfNaN", type: "CallExpression", data: { methodName: "lastIndexOf" } }]
}
]
});

0 comments on commit c6a9a3b

Please sign in to comment.