From e2057a4b250440d5c568e2f56cf7479e357f4bef Mon Sep 17 00:00:00 2001 From: StyleShit Date: Tue, 16 Jan 2024 20:56:59 +0200 Subject: [PATCH 01/10] feat: add suggestions to `use-isnan` in binary expressions --- lib/rules/use-isnan.js | 30 +++- tests/lib/rules/use-isnan.js | 295 +++++++++++++++++++++++++++++++---- 2 files changed, 289 insertions(+), 36 deletions(-) diff --git a/lib/rules/use-isnan.js b/lib/rules/use-isnan.js index 21dc3952902..7968815c6e9 100644 --- a/lib/rules/use-isnan.js +++ b/lib/rules/use-isnan.js @@ -34,6 +34,7 @@ function isNaNIdentifier(node) { /** @type {import('../shared/types').Rule} */ module.exports = { meta: { + hasSuggestions: true, type: "problem", docs: { @@ -63,7 +64,8 @@ module.exports = { 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.", - indexOfNaN: "Array prototype method '{{ methodName }}' cannot find NaN." + indexOfNaN: "Array prototype method '{{ methodName }}' cannot find NaN.", + replaceWithIsNaN: "Replace with Number.isNaN." } }, @@ -71,6 +73,7 @@ module.exports = { const enforceForSwitchCase = !context.options[0] || context.options[0].enforceForSwitchCase; const enforceForIndexOf = context.options[0] && context.options[0].enforceForIndexOf; + const sourceCode = context.getSourceCode(); /** * Checks the given `BinaryExpression` node for `foo === NaN` and other comparisons. @@ -82,7 +85,30 @@ module.exports = { /^(?:[<>]|[!=]=)=?$/u.test(node.operator) && (isNaNIdentifier(node.left) || isNaNIdentifier(node.right)) ) { - context.report({ node, messageId: "comparisonWithNaN" }); + context.report({ + node, + messageId: "comparisonWithNaN", + suggest: [ + { + messageId: "replaceWithIsNaN", + fix(fixer) { + const comparedValue = isNaNIdentifier(node.left) ? node.right : node.left; + let shouldNegate = ["!=", "!=="].includes(node.operator); + + if (comparedValue === node.right) { + shouldNegate ||= ["<", "<="].includes(node.operator); + } else { + shouldNegate ||= [">", ">="].includes(node.operator); + } + + const negation = shouldNegate ? "!" : ""; + const comparedValueText = sourceCode.getText(comparedValue); + + return fixer.replaceText(node, `${negation}Number.isNaN(${comparedValueText})`); + } + } + ] + }); } } diff --git a/tests/lib/rules/use-isnan.js b/tests/lib/rules/use-isnan.js index 8c036bc3971..0414ac56edc 100644 --- a/tests/lib/rules/use-isnan.js +++ b/tests/lib/rules/use-isnan.js @@ -349,140 +349,367 @@ ruleTester.run("use-isnan", rule, { invalid: [ { code: "123 == NaN;", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: "Number.isNaN(123);" + }] + }] }, { code: "123 === NaN;", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: "Number.isNaN(123);" + }] + }] }, { code: "NaN === \"abc\";", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: 'Number.isNaN("abc");' + }] + }] }, { code: "NaN == \"abc\";", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: 'Number.isNaN("abc");' + }] + }] }, { code: "123 != NaN;", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: "!Number.isNaN(123);" + }] + }] }, { code: "123 !== NaN;", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: "!Number.isNaN(123);" + }] + }] }, { code: "NaN !== \"abc\";", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: '!Number.isNaN("abc");' + }] + }] }, { code: "NaN != \"abc\";", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: '!Number.isNaN("abc");' + }] + }] }, { code: "NaN < \"abc\";", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: '!Number.isNaN("abc");' + }] + }] }, { code: "\"abc\" < NaN;", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: 'Number.isNaN("abc");' + }] + }] }, { code: "NaN > \"abc\";", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: 'Number.isNaN("abc");' + }] + }] }, { code: "\"abc\" > NaN;", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: '!Number.isNaN("abc");' + }] + }] }, { code: "NaN <= \"abc\";", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: '!Number.isNaN("abc");' + }] + }] }, { code: "\"abc\" <= NaN;", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: 'Number.isNaN("abc");' + }] + }] }, { code: "NaN >= \"abc\";", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: 'Number.isNaN("abc");' + }] + }] }, { code: "\"abc\" >= NaN;", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: '!Number.isNaN("abc");' + }] + }] }, { code: "123 == Number.NaN;", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: "Number.isNaN(123);" + }] + }] }, { code: "123 === Number.NaN;", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: "Number.isNaN(123);" + }] + }] }, { code: "Number.NaN === \"abc\";", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: "Number.isNaN(\"abc\");" + }] + }] }, { code: "Number.NaN == \"abc\";", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: 'Number.isNaN("abc");' + }] + }] }, { code: "123 != Number.NaN;", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: "!Number.isNaN(123);" + }] + }] }, { code: "123 !== Number.NaN;", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: "!Number.isNaN(123);" + }] + }] }, { code: "Number.NaN !== \"abc\";", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: '!Number.isNaN("abc");' + }] + }] }, { code: "Number.NaN != \"abc\";", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: '!Number.isNaN("abc");' + }] + }] }, { code: "Number.NaN < \"abc\";", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: '!Number.isNaN("abc");' + }] + }] }, { code: "\"abc\" < Number.NaN;", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: 'Number.isNaN("abc");' + }] + }] }, { code: "Number.NaN > \"abc\";", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: 'Number.isNaN("abc");' + }] + }] }, { code: "\"abc\" > Number.NaN;", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: '!Number.isNaN("abc");' + }] + }] }, { code: "Number.NaN <= \"abc\";", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: '!Number.isNaN("abc");' + }] + }] }, { code: "\"abc\" <= Number.NaN;", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: 'Number.isNaN("abc");' + }] + }] }, { code: "Number.NaN >= \"abc\";", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: 'Number.isNaN("abc");' + }] + }] }, { code: "\"abc\" >= Number.NaN;", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: '!Number.isNaN("abc");' + }] + }] }, { code: "x === Number?.NaN;", languageOptions: { ecmaVersion: 2020 }, - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: "Number.isNaN(x);" + }] + }] + }, + { + code: "x !== Number?.NaN;", + languageOptions: { ecmaVersion: 2020 }, + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: "!Number.isNaN(x);" + }] + }] }, { code: "x === Number['NaN'];", - errors: [comparisonError] + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: "Number.isNaN(x);" + }] + }] + }, + { + code: `/* just + adding */ x /* some */ === /* comments */ NaN; // here`, + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: `/* just + adding */ Number.isNaN(x); // here` + }] + }] }, //------------------------------------------------------------------------------ From 8f9d2777421daa4a582c05d0d4459bfd46331f8f Mon Sep 17 00:00:00 2001 From: StyleShit Date: Wed, 17 Jan 2024 19:08:34 +0200 Subject: [PATCH 02/10] remove `>` `<` from fixes --- lib/rules/use-isnan.js | 45 ++++++++++---------- tests/lib/rules/use-isnan.js | 80 ++++++++---------------------------- 2 files changed, 39 insertions(+), 86 deletions(-) diff --git a/lib/rules/use-isnan.js b/lib/rules/use-isnan.js index 7968815c6e9..8c5b8cf325b 100644 --- a/lib/rules/use-isnan.js +++ b/lib/rules/use-isnan.js @@ -85,30 +85,31 @@ module.exports = { /^(?:[<>]|[!=]=)=?$/u.test(node.operator) && (isNaNIdentifier(node.left) || isNaNIdentifier(node.right)) ) { - context.report({ - node, - messageId: "comparisonWithNaN", - suggest: [ - { - messageId: "replaceWithIsNaN", - fix(fixer) { - const comparedValue = isNaNIdentifier(node.left) ? node.right : node.left; - let shouldNegate = ["!=", "!=="].includes(node.operator); - - if (comparedValue === node.right) { - shouldNegate ||= ["<", "<="].includes(node.operator); - } else { - shouldNegate ||= [">", ">="].includes(node.operator); + const fixableOperators = ["==", "===", "!=", "!=="]; + const isFixable = fixableOperators.includes(node.operator); + + if (isFixable) { + context.report({ + node, + messageId: "comparisonWithNaN", + suggest: [ + { + messageId: "replaceWithIsNaN", + fix(fixer) { + const comparedValue = isNaNIdentifier(node.left) ? node.right : node.left; + const shouldNegate = node.operator[0] === "!"; + + const negation = shouldNegate ? "!" : ""; + const comparedValueText = sourceCode.getText(comparedValue); + + return fixer.replaceText(node, `${negation}Number.isNaN(${comparedValueText})`); } - - const negation = shouldNegate ? "!" : ""; - const comparedValueText = sourceCode.getText(comparedValue); - - return fixer.replaceText(node, `${negation}Number.isNaN(${comparedValueText})`); } - } - ] - }); + ] + }); + } else { + context.report({ node, messageId: "comparisonWithNaN" }); + } } } diff --git a/tests/lib/rules/use-isnan.js b/tests/lib/rules/use-isnan.js index 0414ac56edc..d3484185f68 100644 --- a/tests/lib/rules/use-isnan.js +++ b/tests/lib/rules/use-isnan.js @@ -431,80 +431,56 @@ ruleTester.run("use-isnan", rule, { code: "NaN < \"abc\";", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: '!Number.isNaN("abc");' - }] + suggestions: [] }] }, { code: "\"abc\" < NaN;", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: 'Number.isNaN("abc");' - }] + suggestions: [] }] }, { code: "NaN > \"abc\";", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: 'Number.isNaN("abc");' - }] + suggestions: [] }] }, { code: "\"abc\" > NaN;", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: '!Number.isNaN("abc");' - }] + suggestions: [] }] }, { code: "NaN <= \"abc\";", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: '!Number.isNaN("abc");' - }] + suggestions: [] }] }, { code: "\"abc\" <= NaN;", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: 'Number.isNaN("abc");' - }] + suggestions: [] }] }, { code: "NaN >= \"abc\";", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: 'Number.isNaN("abc");' - }] + suggestions: [] }] }, { code: "\"abc\" >= NaN;", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: '!Number.isNaN("abc");' - }] + suggestions: [] }] }, { @@ -591,80 +567,56 @@ ruleTester.run("use-isnan", rule, { code: "Number.NaN < \"abc\";", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: '!Number.isNaN("abc");' - }] + suggestions: [] }] }, { code: "\"abc\" < Number.NaN;", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: 'Number.isNaN("abc");' - }] + suggestions: [] }] }, { code: "Number.NaN > \"abc\";", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: 'Number.isNaN("abc");' - }] + suggestions: [] }] }, { code: "\"abc\" > Number.NaN;", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: '!Number.isNaN("abc");' - }] + suggestions: [] }] }, { code: "Number.NaN <= \"abc\";", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: '!Number.isNaN("abc");' - }] + suggestions: [] }] }, { code: "\"abc\" <= Number.NaN;", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: 'Number.isNaN("abc");' - }] + suggestions: [] }] }, { code: "Number.NaN >= \"abc\";", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: 'Number.isNaN("abc");' - }] + suggestions: [] }] }, { code: "\"abc\" >= Number.NaN;", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: '!Number.isNaN("abc");' - }] + suggestions: [] }] }, { From bf1965df61cbe369bc25836f651da10d5deab507 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Thu, 18 Jan 2024 08:10:52 +0200 Subject: [PATCH 03/10] move the array up --- lib/rules/use-isnan.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/rules/use-isnan.js b/lib/rules/use-isnan.js index 8c5b8cf325b..3825ac2eb91 100644 --- a/lib/rules/use-isnan.js +++ b/lib/rules/use-isnan.js @@ -75,6 +75,9 @@ module.exports = { const enforceForIndexOf = context.options[0] && context.options[0].enforceForIndexOf; const sourceCode = context.getSourceCode(); + // eslint-disable-next-line unicorn/prefer-set-has -- Array.includes is faster in this case. + const fixableOperators = ["==", "===", "!=", "!=="]; + /** * Checks the given `BinaryExpression` node for `foo === NaN` and other comparisons. * @param {ASTNode} node The node to check. @@ -85,7 +88,6 @@ module.exports = { /^(?:[<>]|[!=]=)=?$/u.test(node.operator) && (isNaNIdentifier(node.left) || isNaNIdentifier(node.right)) ) { - const fixableOperators = ["==", "===", "!=", "!=="]; const isFixable = fixableOperators.includes(node.operator); if (isFixable) { From f735887328230523c4237d9dce3daeefda8366f4 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Fri, 19 Jan 2024 07:18:56 +0200 Subject: [PATCH 04/10] use Set --- lib/rules/use-isnan.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/rules/use-isnan.js b/lib/rules/use-isnan.js index 3825ac2eb91..c3854ae6206 100644 --- a/lib/rules/use-isnan.js +++ b/lib/rules/use-isnan.js @@ -74,9 +74,7 @@ module.exports = { const enforceForSwitchCase = !context.options[0] || context.options[0].enforceForSwitchCase; const enforceForIndexOf = context.options[0] && context.options[0].enforceForIndexOf; const sourceCode = context.getSourceCode(); - - // eslint-disable-next-line unicorn/prefer-set-has -- Array.includes is faster in this case. - const fixableOperators = ["==", "===", "!=", "!=="]; + const fixableOperators = new Set(["==", "===", "!=", "!=="]); /** * Checks the given `BinaryExpression` node for `foo === NaN` and other comparisons. @@ -88,7 +86,7 @@ module.exports = { /^(?:[<>]|[!=]=)=?$/u.test(node.operator) && (isNaNIdentifier(node.left) || isNaNIdentifier(node.right)) ) { - const isFixable = fixableOperators.includes(node.operator); + const isFixable = fixableOperators.has(node.operator); if (isFixable) { context.report({ From 9d02fbefdf30226ac0a5b3ff32c46789f51fcb0b Mon Sep 17 00:00:00 2001 From: StyleShit Date: Sun, 21 Jan 2024 18:12:12 +0200 Subject: [PATCH 05/10] wip --- lib/rules/use-isnan.js | 9 +++++++-- tests/lib/rules/use-isnan.js | 10 ++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/lib/rules/use-isnan.js b/lib/rules/use-isnan.js index c3854ae6206..ceb7480b287 100644 --- a/lib/rules/use-isnan.js +++ b/lib/rules/use-isnan.js @@ -73,7 +73,7 @@ module.exports = { const enforceForSwitchCase = !context.options[0] || context.options[0].enforceForSwitchCase; const enforceForIndexOf = context.options[0] && context.options[0].enforceForIndexOf; - const sourceCode = context.getSourceCode(); + const sourceCode = context.sourceCode; const fixableOperators = new Set(["==", "===", "!=", "!=="]); /** @@ -97,10 +97,15 @@ module.exports = { messageId: "replaceWithIsNaN", fix(fixer) { const comparedValue = isNaNIdentifier(node.left) ? node.right : node.left; + const shouldWrap = comparedValue.type === "SequenceExpression"; const shouldNegate = node.operator[0] === "!"; const negation = shouldNegate ? "!" : ""; - const comparedValueText = sourceCode.getText(comparedValue); + let comparedValueText = sourceCode.getText(comparedValue); + + if (shouldWrap) { + comparedValueText = `(${comparedValueText})`; + } return fixer.replaceText(node, `${negation}Number.isNaN(${comparedValueText})`); } diff --git a/tests/lib/rules/use-isnan.js b/tests/lib/rules/use-isnan.js index d3484185f68..5bec5759b9f 100644 --- a/tests/lib/rules/use-isnan.js +++ b/tests/lib/rules/use-isnan.js @@ -663,6 +663,16 @@ ruleTester.run("use-isnan", rule, { }] }] }, + { + code: "(1, 2) === NaN;", + errors: [{ + ...comparisonError, + suggestions: [{ + messageId: "replaceWithIsNaN", + output: "Number.isNaN((1, 2));" + }] + }] + }, //------------------------------------------------------------------------------ // enforceForSwitchCase From d844e0352128bd2392ed4b3a2e399f2146ba3bc4 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Sun, 21 Jan 2024 18:13:59 +0200 Subject: [PATCH 06/10] small refactor --- lib/rules/use-isnan.js | 52 ++++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/lib/rules/use-isnan.js b/lib/rules/use-isnan.js index ceb7480b287..d41d53ddc5c 100644 --- a/lib/rules/use-isnan.js +++ b/lib/rules/use-isnan.js @@ -88,33 +88,35 @@ module.exports = { ) { const isFixable = fixableOperators.has(node.operator); - if (isFixable) { - context.report({ - node, - messageId: "comparisonWithNaN", - suggest: [ - { - messageId: "replaceWithIsNaN", - fix(fixer) { - const comparedValue = isNaNIdentifier(node.left) ? node.right : node.left; - const shouldWrap = comparedValue.type === "SequenceExpression"; - const shouldNegate = node.operator[0] === "!"; - - const negation = shouldNegate ? "!" : ""; - let comparedValueText = sourceCode.getText(comparedValue); - - if (shouldWrap) { - comparedValueText = `(${comparedValueText})`; - } - - return fixer.replaceText(node, `${negation}Number.isNaN(${comparedValueText})`); - } - } - ] - }); - } else { + if (!isFixable) { context.report({ node, messageId: "comparisonWithNaN" }); + return; + } + + context.report({ + node, + messageId: "comparisonWithNaN", + suggest: [ + { + messageId: "replaceWithIsNaN", + fix(fixer) { + const comparedValue = isNaNIdentifier(node.left) ? node.right : node.left; + const shouldWrap = comparedValue.type === "SequenceExpression"; + const shouldNegate = node.operator[0] === "!"; + + const negation = shouldNegate ? "!" : ""; + let comparedValueText = sourceCode.getText(comparedValue); + + if (shouldWrap) { + comparedValueText = `(${comparedValueText})`; + } + + return fixer.replaceText(node, `${negation}Number.isNaN(${comparedValueText})`); + } + } + ] + }); } } From 24c4238ed64db6eec98558dfa708be9c89919e35 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Sun, 21 Jan 2024 18:14:36 +0200 Subject: [PATCH 07/10] remove empty line --- lib/rules/use-isnan.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/rules/use-isnan.js b/lib/rules/use-isnan.js index d41d53ddc5c..25a14d215ec 100644 --- a/lib/rules/use-isnan.js +++ b/lib/rules/use-isnan.js @@ -91,7 +91,6 @@ module.exports = { if (!isFixable) { context.report({ node, messageId: "comparisonWithNaN" }); return; - } context.report({ From 7266bad50b0f058c7308f7f2a78b724c5338e9f6 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Tue, 23 Jan 2024 20:34:49 +0200 Subject: [PATCH 08/10] add multiple suggestions --- lib/rules/use-isnan.js | 47 ++++-- tests/lib/rules/use-isnan.js | 295 +++++++++++++++++++++++++---------- 2 files changed, 243 insertions(+), 99 deletions(-) diff --git a/lib/rules/use-isnan.js b/lib/rules/use-isnan.js index 25a14d215ec..1077fc95c89 100644 --- a/lib/rules/use-isnan.js +++ b/lib/rules/use-isnan.js @@ -65,7 +65,8 @@ module.exports = { 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.", indexOfNaN: "Array prototype method '{{ methodName }}' cannot find NaN.", - replaceWithIsNaN: "Replace with Number.isNaN." + replaceWithIsNaN: "Replace with Number.isNaN.", + replaceWithCastingAndIsNaN: "Replace with a cast to number and Number.isNaN." } }, @@ -76,6 +77,31 @@ module.exports = { const sourceCode = context.sourceCode; const fixableOperators = new Set(["==", "===", "!=", "!=="]); + /** + * Get a fixer for a binary expression that compares to NaN. + * @param {ASTNode} node The node to fix. + * @param {function(string): string} wrapValue A function that wraps the compared value with a fix. + * @returns {function(Fixer): Fix} The fixer function. + */ + function getBinaryExpressionFixer(node, wrapValue) { + return fixer => { + const comparedValue = isNaNIdentifier(node.left) ? node.right : node.left; + const shouldWrap = comparedValue.type === "SequenceExpression"; + const shouldNegate = node.operator[0] === "!"; + + const negation = shouldNegate ? "!" : ""; + let comparedValueText = sourceCode.getText(comparedValue); + + if (shouldWrap) { + comparedValueText = `(${comparedValueText})`; + } + + const fixedValue = wrapValue(comparedValueText); + + return fixer.replaceText(node, `${negation}${fixedValue}`); + }; + } + /** * Checks the given `BinaryExpression` node for `foo === NaN` and other comparisons. * @param {ASTNode} node The node to check. @@ -99,20 +125,11 @@ module.exports = { suggest: [ { messageId: "replaceWithIsNaN", - fix(fixer) { - const comparedValue = isNaNIdentifier(node.left) ? node.right : node.left; - const shouldWrap = comparedValue.type === "SequenceExpression"; - const shouldNegate = node.operator[0] === "!"; - - const negation = shouldNegate ? "!" : ""; - let comparedValueText = sourceCode.getText(comparedValue); - - if (shouldWrap) { - comparedValueText = `(${comparedValueText})`; - } - - return fixer.replaceText(node, `${negation}Number.isNaN(${comparedValueText})`); - } + fix: getBinaryExpressionFixer(node, value => `Number.isNaN(${value})`) + }, + { + messageId: "replaceWithCastingAndIsNaN", + fix: getBinaryExpressionFixer(node, value => `Number.isNaN(Number(${value}))`) } ] }); diff --git a/tests/lib/rules/use-isnan.js b/tests/lib/rules/use-isnan.js index 5bec5759b9f..7e046b0df57 100644 --- a/tests/lib/rules/use-isnan.js +++ b/tests/lib/rules/use-isnan.js @@ -351,80 +351,128 @@ ruleTester.run("use-isnan", rule, { code: "123 == NaN;", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: "Number.isNaN(123);" - }] + suggestions: [ + { + messageId: "replaceWithIsNaN", + output: "Number.isNaN(123);" + }, + { + messageId: "replaceWithCastingAndIsNaN", + output: "Number.isNaN(Number(123));" + } + ] }] }, { code: "123 === NaN;", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: "Number.isNaN(123);" - }] + suggestions: [ + { + messageId: "replaceWithIsNaN", + output: "Number.isNaN(123);" + }, + { + messageId: "replaceWithCastingAndIsNaN", + output: "Number.isNaN(Number(123));" + } + ] }] }, { code: "NaN === \"abc\";", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: 'Number.isNaN("abc");' - }] + suggestions: [ + { + messageId: "replaceWithIsNaN", + output: 'Number.isNaN("abc");' + }, + { + messageId: "replaceWithCastingAndIsNaN", + output: 'Number.isNaN(Number("abc"));' + } + ] }] }, { code: "NaN == \"abc\";", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: 'Number.isNaN("abc");' - }] + suggestions: [ + { + messageId: "replaceWithIsNaN", + output: 'Number.isNaN("abc");' + }, + { + messageId: "replaceWithCastingAndIsNaN", + output: 'Number.isNaN(Number("abc"));' + } + ] }] }, { code: "123 != NaN;", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: "!Number.isNaN(123);" - }] + suggestions: [ + { + messageId: "replaceWithIsNaN", + output: "!Number.isNaN(123);" + }, + { + messageId: "replaceWithCastingAndIsNaN", + output: "!Number.isNaN(Number(123));" + } + ] }] }, { code: "123 !== NaN;", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: "!Number.isNaN(123);" - }] + suggestions: [ + { + messageId: "replaceWithIsNaN", + output: "!Number.isNaN(123);" + }, + { + messageId: "replaceWithCastingAndIsNaN", + output: "!Number.isNaN(Number(123));" + } + ] }] }, { code: "NaN !== \"abc\";", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: '!Number.isNaN("abc");' - }] + suggestions: [ + { + messageId: "replaceWithIsNaN", + output: '!Number.isNaN("abc");' + }, + { + messageId: "replaceWithCastingAndIsNaN", + output: '!Number.isNaN(Number("abc"));' + } + ] }] }, { code: "NaN != \"abc\";", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: '!Number.isNaN("abc");' - }] + suggestions: [ + { + messageId: "replaceWithIsNaN", + output: '!Number.isNaN("abc");' + }, + { + messageId: "replaceWithCastingAndIsNaN", + output: '!Number.isNaN(Number("abc"));' + } + ] }] }, { @@ -487,80 +535,128 @@ ruleTester.run("use-isnan", rule, { code: "123 == Number.NaN;", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: "Number.isNaN(123);" - }] + suggestions: [ + { + messageId: "replaceWithIsNaN", + output: "Number.isNaN(123);" + }, + { + messageId: "replaceWithCastingAndIsNaN", + output: "Number.isNaN(Number(123));" + } + ] }] }, { code: "123 === Number.NaN;", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: "Number.isNaN(123);" - }] + suggestions: [ + { + messageId: "replaceWithIsNaN", + output: "Number.isNaN(123);" + }, + { + messageId: "replaceWithCastingAndIsNaN", + output: "Number.isNaN(Number(123));" + } + ] }] }, { code: "Number.NaN === \"abc\";", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: "Number.isNaN(\"abc\");" - }] + suggestions: [ + { + messageId: "replaceWithIsNaN", + output: "Number.isNaN(\"abc\");" + }, + { + messageId: "replaceWithCastingAndIsNaN", + output: 'Number.isNaN(Number("abc"));' + } + ] }] }, { code: "Number.NaN == \"abc\";", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: 'Number.isNaN("abc");' - }] + suggestions: [ + { + messageId: "replaceWithIsNaN", + output: 'Number.isNaN("abc");' + }, + { + messageId: "replaceWithCastingAndIsNaN", + output: 'Number.isNaN(Number("abc"));' + } + ] }] }, { code: "123 != Number.NaN;", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: "!Number.isNaN(123);" - }] + suggestions: [ + { + messageId: "replaceWithIsNaN", + output: "!Number.isNaN(123);" + }, + { + messageId: "replaceWithCastingAndIsNaN", + output: "!Number.isNaN(Number(123));" + } + ] }] }, { code: "123 !== Number.NaN;", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: "!Number.isNaN(123);" - }] + suggestions: [ + { + messageId: "replaceWithIsNaN", + output: "!Number.isNaN(123);" + }, + { + messageId: "replaceWithCastingAndIsNaN", + output: "!Number.isNaN(Number(123));" + } + ] }] }, { code: "Number.NaN !== \"abc\";", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: '!Number.isNaN("abc");' - }] + suggestions: [ + { + messageId: "replaceWithIsNaN", + output: '!Number.isNaN("abc");' + }, + { + messageId: "replaceWithCastingAndIsNaN", + output: '!Number.isNaN(Number("abc"));' + } + ] }] }, { code: "Number.NaN != \"abc\";", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: '!Number.isNaN("abc");' - }] + suggestions: [ + { + messageId: "replaceWithIsNaN", + output: '!Number.isNaN("abc");' + }, + { + messageId: "replaceWithCastingAndIsNaN", + output: '!Number.isNaN(Number("abc"));' + } + ] }] }, { @@ -624,10 +720,16 @@ ruleTester.run("use-isnan", rule, { languageOptions: { ecmaVersion: 2020 }, errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: "Number.isNaN(x);" - }] + suggestions: [ + { + messageId: "replaceWithIsNaN", + output: "Number.isNaN(x);" + }, + { + messageId: "replaceWithCastingAndIsNaN", + output: "Number.isNaN(Number(x));" + } + ] }] }, { @@ -635,20 +737,32 @@ ruleTester.run("use-isnan", rule, { languageOptions: { ecmaVersion: 2020 }, errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: "!Number.isNaN(x);" - }] + suggestions: [ + { + messageId: "replaceWithIsNaN", + output: "!Number.isNaN(x);" + }, + { + messageId: "replaceWithCastingAndIsNaN", + output: "!Number.isNaN(Number(x));" + } + ] }] }, { code: "x === Number['NaN'];", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: "Number.isNaN(x);" - }] + suggestions: [ + { + messageId: "replaceWithIsNaN", + output: "Number.isNaN(x);" + }, + { + messageId: "replaceWithCastingAndIsNaN", + output: "Number.isNaN(Number(x));" + } + ] }] }, { @@ -656,21 +770,34 @@ ruleTester.run("use-isnan", rule, { adding */ x /* some */ === /* comments */ NaN; // here`, errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: `/* just + suggestions: [ + { + messageId: "replaceWithIsNaN", + output: `/* just adding */ Number.isNaN(x); // here` - }] + }, + { + messageId: "replaceWithCastingAndIsNaN", + output: `/* just + adding */ Number.isNaN(Number(x)); // here` + } + ] }] }, { code: "(1, 2) === NaN;", errors: [{ ...comparisonError, - suggestions: [{ - messageId: "replaceWithIsNaN", - output: "Number.isNaN((1, 2));" - }] + suggestions: [ + { + messageId: "replaceWithIsNaN", + output: "Number.isNaN((1, 2));" + }, + { + messageId: "replaceWithCastingAndIsNaN", + output: "Number.isNaN(Number((1, 2)));" + } + ] }] }, From a64c253e56cd6790f2fe5b008407f7b1b7c5e3fe Mon Sep 17 00:00:00 2001 From: StyleShit Date: Tue, 23 Jan 2024 20:37:57 +0200 Subject: [PATCH 09/10] change wording --- lib/rules/use-isnan.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/use-isnan.js b/lib/rules/use-isnan.js index 1077fc95c89..64d9f535e02 100644 --- a/lib/rules/use-isnan.js +++ b/lib/rules/use-isnan.js @@ -66,7 +66,7 @@ module.exports = { caseNaN: "'case NaN' can never match. Use Number.isNaN before the switch.", indexOfNaN: "Array prototype method '{{ methodName }}' cannot find NaN.", replaceWithIsNaN: "Replace with Number.isNaN.", - replaceWithCastingAndIsNaN: "Replace with a cast to number and Number.isNaN." + replaceWithCastingAndIsNaN: "Replace with Number.isNaN cast to a Number." } }, From 0481f3d19ef08305fa83aea72cc3b422159154f0 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Wed, 24 Jan 2024 16:29:25 +0200 Subject: [PATCH 10/10] suggest casting only in `==` and `!=` --- lib/rules/use-isnan.js | 30 +++++++++++--------- tests/lib/rules/use-isnan.js | 53 ------------------------------------ 2 files changed, 17 insertions(+), 66 deletions(-) diff --git a/lib/rules/use-isnan.js b/lib/rules/use-isnan.js index 64d9f535e02..b00a701c6bd 100644 --- a/lib/rules/use-isnan.js +++ b/lib/rules/use-isnan.js @@ -75,7 +75,9 @@ module.exports = { const enforceForSwitchCase = !context.options[0] || context.options[0].enforceForSwitchCase; const enforceForIndexOf = context.options[0] && context.options[0].enforceForIndexOf; const sourceCode = context.sourceCode; + const fixableOperators = new Set(["==", "===", "!=", "!=="]); + const castableOperators = new Set(["==", "!="]); /** * Get a fixer for a binary expression that compares to NaN. @@ -112,26 +114,28 @@ module.exports = { /^(?:[<>]|[!=]=)=?$/u.test(node.operator) && (isNaNIdentifier(node.left) || isNaNIdentifier(node.right)) ) { + const suggestedFixes = []; const isFixable = fixableOperators.has(node.operator); + const isCastable = castableOperators.has(node.operator); + + if (isFixable) { + suggestedFixes.push({ + messageId: "replaceWithIsNaN", + fix: getBinaryExpressionFixer(node, value => `Number.isNaN(${value})`) + }); + } - if (!isFixable) { - context.report({ node, messageId: "comparisonWithNaN" }); - return; + if (isCastable) { + suggestedFixes.push({ + messageId: "replaceWithCastingAndIsNaN", + fix: getBinaryExpressionFixer(node, value => `Number.isNaN(Number(${value}))`) + }); } context.report({ node, messageId: "comparisonWithNaN", - suggest: [ - { - messageId: "replaceWithIsNaN", - fix: getBinaryExpressionFixer(node, value => `Number.isNaN(${value})`) - }, - { - messageId: "replaceWithCastingAndIsNaN", - fix: getBinaryExpressionFixer(node, value => `Number.isNaN(Number(${value}))`) - } - ] + suggest: suggestedFixes }); } } diff --git a/tests/lib/rules/use-isnan.js b/tests/lib/rules/use-isnan.js index 7e046b0df57..26c887a1975 100644 --- a/tests/lib/rules/use-isnan.js +++ b/tests/lib/rules/use-isnan.js @@ -371,10 +371,6 @@ ruleTester.run("use-isnan", rule, { { messageId: "replaceWithIsNaN", output: "Number.isNaN(123);" - }, - { - messageId: "replaceWithCastingAndIsNaN", - output: "Number.isNaN(Number(123));" } ] }] @@ -387,10 +383,6 @@ ruleTester.run("use-isnan", rule, { { messageId: "replaceWithIsNaN", output: 'Number.isNaN("abc");' - }, - { - messageId: "replaceWithCastingAndIsNaN", - output: 'Number.isNaN(Number("abc"));' } ] }] @@ -435,10 +427,6 @@ ruleTester.run("use-isnan", rule, { { messageId: "replaceWithIsNaN", output: "!Number.isNaN(123);" - }, - { - messageId: "replaceWithCastingAndIsNaN", - output: "!Number.isNaN(Number(123));" } ] }] @@ -451,10 +439,6 @@ ruleTester.run("use-isnan", rule, { { messageId: "replaceWithIsNaN", output: '!Number.isNaN("abc");' - }, - { - messageId: "replaceWithCastingAndIsNaN", - output: '!Number.isNaN(Number("abc"));' } ] }] @@ -555,10 +539,6 @@ ruleTester.run("use-isnan", rule, { { messageId: "replaceWithIsNaN", output: "Number.isNaN(123);" - }, - { - messageId: "replaceWithCastingAndIsNaN", - output: "Number.isNaN(Number(123));" } ] }] @@ -571,10 +551,6 @@ ruleTester.run("use-isnan", rule, { { messageId: "replaceWithIsNaN", output: "Number.isNaN(\"abc\");" - }, - { - messageId: "replaceWithCastingAndIsNaN", - output: 'Number.isNaN(Number("abc"));' } ] }] @@ -619,10 +595,6 @@ ruleTester.run("use-isnan", rule, { { messageId: "replaceWithIsNaN", output: "!Number.isNaN(123);" - }, - { - messageId: "replaceWithCastingAndIsNaN", - output: "!Number.isNaN(Number(123));" } ] }] @@ -635,10 +607,6 @@ ruleTester.run("use-isnan", rule, { { messageId: "replaceWithIsNaN", output: '!Number.isNaN("abc");' - }, - { - messageId: "replaceWithCastingAndIsNaN", - output: '!Number.isNaN(Number("abc"));' } ] }] @@ -724,10 +692,6 @@ ruleTester.run("use-isnan", rule, { { messageId: "replaceWithIsNaN", output: "Number.isNaN(x);" - }, - { - messageId: "replaceWithCastingAndIsNaN", - output: "Number.isNaN(Number(x));" } ] }] @@ -741,10 +705,6 @@ ruleTester.run("use-isnan", rule, { { messageId: "replaceWithIsNaN", output: "!Number.isNaN(x);" - }, - { - messageId: "replaceWithCastingAndIsNaN", - output: "!Number.isNaN(Number(x));" } ] }] @@ -757,10 +717,6 @@ ruleTester.run("use-isnan", rule, { { messageId: "replaceWithIsNaN", output: "Number.isNaN(x);" - }, - { - messageId: "replaceWithCastingAndIsNaN", - output: "Number.isNaN(Number(x));" } ] }] @@ -775,11 +731,6 @@ ruleTester.run("use-isnan", rule, { messageId: "replaceWithIsNaN", output: `/* just adding */ Number.isNaN(x); // here` - }, - { - messageId: "replaceWithCastingAndIsNaN", - output: `/* just - adding */ Number.isNaN(Number(x)); // here` } ] }] @@ -792,10 +743,6 @@ ruleTester.run("use-isnan", rule, { { messageId: "replaceWithIsNaN", output: "Number.isNaN((1, 2));" - }, - { - messageId: "replaceWithCastingAndIsNaN", - output: "Number.isNaN(Number((1, 2)));" } ] }]