diff --git a/docs/rules/use-isnan.md b/docs/rules/use-isnan.md index 426eaca5e6d..0d75251e440 100644 --- a/docs/rules/use-isnan.md +++ b/docs/rules/use-isnan.md @@ -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 @@ -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. diff --git a/lib/rules/use-isnan.js b/lib/rules/use-isnan.js index b2eb84b7b37..cd9ccdbaf89 100644 --- a/lib/rules/use-isnan.js +++ b/lib/rules/use-isnan.js @@ -5,6 +5,12 @@ "use strict"; +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const astUtils = require("./utils/ast-utils"); + //------------------------------------------------------------------------------ // Helpers //------------------------------------------------------------------------------ @@ -40,6 +46,10 @@ module.exports = { enforceForSwitchCase: { type: "boolean", default: false + }, + enforceForIndexOf: { + type: "boolean", + default: false } }, additionalProperties: false @@ -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} */ @@ -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} */ @@ -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 }; @@ -96,6 +129,10 @@ module.exports = { listeners.SwitchStatement = checkSwitchStatement; } + if (enforceForIndexOf) { + listeners.CallExpression = checkCallExpression; + } + return listeners; } }; diff --git a/tests/lib/rules/use-isnan.js b/tests/lib/rules/use-isnan.js index 82bad95cb61..e494fc014ad 100644 --- a/tests/lib/rules/use-isnan.js +++ b/tests/lib/rules/use-isnan.js @@ -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: [ @@ -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" } }] } ] });