From 5471e435d12bf5add9869d81534b147e445a2368 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrg=C3=BCn=20Day=C4=B1o=C4=9Flu?= Date: Sat, 20 Jan 2024 20:46:09 +0100 Subject: [PATCH] feat: convert unsafe autofixes to suggestions in `no-implicit-coercion` (#17985) * don't autofix except when !! is used * fixup * temporary solution: remove the erroneous line * Revert "temporary solution: remove the erroneous line" * remove rule * use suggestions * refactor: more elegant * simplify * apply suggestions * apply suggestions * only fix when boolean exists * add test case * minor * change jsdoc * remove unnecessary boolean * do not suggest for ~indexOf --- lib/rules/no-implicit-coercion.js | 76 ++-- .../left-pad-expected-quiet.js | 3 +- .../autofix-integration/left-pad-expected.js | 3 +- .../fixtures/autofix-integration/left-pad.js | 1 - tests/lib/rules/no-implicit-coercion.js | 326 ++++++++++++++---- 5 files changed, 310 insertions(+), 99 deletions(-) diff --git a/lib/rules/no-implicit-coercion.js b/lib/rules/no-implicit-coercion.js index eb42ca85d59..3f8a7c0f941 100644 --- a/lib/rules/no-implicit-coercion.js +++ b/lib/rules/no-implicit-coercion.js @@ -188,6 +188,7 @@ function getNonEmptyOperand(node) { /** @type {import('../shared/types').Rule} */ module.exports = { meta: { + hasSuggestions: true, type: "suggestion", docs: { @@ -229,7 +230,8 @@ module.exports = { }], messages: { - useRecommendation: "use `{{recommendation}}` instead." + implicitCoercion: "Unexpected implicit coercion encountered. Use `{{recommendation}}` instead.", + useRecommendation: "Use `{{recommendation}}` instead." } }, @@ -241,32 +243,54 @@ module.exports = { * Reports an error and autofixes the node * @param {ASTNode} node An ast node to report the error on. * @param {string} recommendation The recommended code for the issue + * @param {bool} shouldSuggest Whether this report should offer a suggestion * @param {bool} shouldFix Whether this report should fix the node * @returns {void} */ - function report(node, recommendation, shouldFix) { + function report(node, recommendation, shouldSuggest, shouldFix) { + + /** + * Fix function + * @param {RuleFixer} fixer The fixer to fix. + * @returns {Fix} The fix object. + */ + function fix(fixer) { + const tokenBefore = sourceCode.getTokenBefore(node); + + if ( + tokenBefore?.range[1] === node.range[0] && + !astUtils.canTokensBeAdjacent(tokenBefore, recommendation) + ) { + return fixer.replaceText(node, ` ${recommendation}`); + } + + return fixer.replaceText(node, recommendation); + } + context.report({ node, - messageId: "useRecommendation", - data: { - recommendation - }, + messageId: "implicitCoercion", + data: { recommendation }, fix(fixer) { if (!shouldFix) { return null; } - const tokenBefore = sourceCode.getTokenBefore(node); - - if ( - tokenBefore && - tokenBefore.range[1] === node.range[0] && - !astUtils.canTokensBeAdjacent(tokenBefore, recommendation) - ) { - return fixer.replaceText(node, ` ${recommendation}`); + return fix(fixer); + }, + suggest: [ + { + messageId: "useRecommendation", + data: { recommendation }, + fix(fixer) { + if (shouldFix || !shouldSuggest) { + return null; + } + + return fix(fixer); + } } - return fixer.replaceText(node, recommendation); - } + ] }); } @@ -278,8 +302,10 @@ module.exports = { operatorAllowed = options.allow.includes("!!"); if (!operatorAllowed && options.boolean && isDoubleLogicalNegating(node)) { const recommendation = `Boolean(${sourceCode.getText(node.argument.argument)})`; + const variable = astUtils.getVariableByName(sourceCode.getScope(node), "Boolean"); + const booleanExists = variable?.identifiers.length === 0; - report(node, recommendation, true); + report(node, recommendation, true, booleanExists); } // ~foo.indexOf(bar) @@ -290,7 +316,7 @@ module.exports = { const comparison = node.argument.type === "ChainExpression" ? ">= 0" : "!== -1"; const recommendation = `${sourceCode.getText(node.argument)} ${comparison}`; - report(node, recommendation, false); + report(node, recommendation, false, false); } // +foo @@ -298,7 +324,7 @@ module.exports = { if (!operatorAllowed && options.number && node.operator === "+" && !isNumeric(node.argument)) { const recommendation = `Number(${sourceCode.getText(node.argument)})`; - report(node, recommendation, true); + report(node, recommendation, true, false); } // -(-foo) @@ -306,7 +332,7 @@ module.exports = { if (!operatorAllowed && options.number && node.operator === "-" && node.argument.type === "UnaryExpression" && node.argument.operator === "-" && !isNumeric(node.argument.argument)) { const recommendation = `Number(${sourceCode.getText(node.argument.argument)})`; - report(node, recommendation, false); + report(node, recommendation, true, false); } }, @@ -322,7 +348,7 @@ module.exports = { if (nonNumericOperand) { const recommendation = `Number(${sourceCode.getText(nonNumericOperand)})`; - report(node, recommendation, true); + report(node, recommendation, true, false); } // foo - 0 @@ -330,7 +356,7 @@ module.exports = { if (!operatorAllowed && options.number && node.operator === "-" && node.right.type === "Literal" && node.right.value === 0 && !isNumeric(node.left)) { const recommendation = `Number(${sourceCode.getText(node.left)})`; - report(node, recommendation, true); + report(node, recommendation, true, false); } // "" + foo @@ -338,7 +364,7 @@ module.exports = { if (!operatorAllowed && options.string && isConcatWithEmptyString(node)) { const recommendation = `String(${sourceCode.getText(getNonEmptyOperand(node))})`; - report(node, recommendation, true); + report(node, recommendation, true, false); } }, @@ -351,7 +377,7 @@ module.exports = { const code = sourceCode.getText(getNonEmptyOperand(node)); const recommendation = `${code} = String(${code})`; - report(node, recommendation, true); + report(node, recommendation, true, false); } }, @@ -389,7 +415,7 @@ module.exports = { const code = sourceCode.getText(node.expressions[0]); const recommendation = `String(${code})`; - report(node, recommendation, true); + report(node, recommendation, true, false); } }; } diff --git a/tests/fixtures/autofix-integration/left-pad-expected-quiet.js b/tests/fixtures/autofix-integration/left-pad-expected-quiet.js index b189698bfa0..33346d4ee4b 100644 --- a/tests/fixtures/autofix-integration/left-pad-expected-quiet.js +++ b/tests/fixtures/autofix-integration/left-pad-expected-quiet.js @@ -1,7 +1,6 @@ /* eslint dot-notation: 2 */ /* eslint indent: [2, 2] */ /* eslint no-extra-parens: 2 */ -/* eslint no-implicit-coercion: 2 */ /* eslint no-whitespace-before-property: 2 */ /* eslint quotes: [2, "single"] */ /* eslint semi: 2 */ @@ -17,7 +16,7 @@ module.exports = leftpad; function leftpad (str, len, ch){ - str = String(str); + str = '' + str; var i = -1; diff --git a/tests/fixtures/autofix-integration/left-pad-expected.js b/tests/fixtures/autofix-integration/left-pad-expected.js index 24da3ce1729..c3b501f4975 100644 --- a/tests/fixtures/autofix-integration/left-pad-expected.js +++ b/tests/fixtures/autofix-integration/left-pad-expected.js @@ -1,7 +1,6 @@ /* eslint dot-notation: 2 */ /* eslint indent: [2, 2] */ /* eslint no-extra-parens: 2 */ -/* eslint no-implicit-coercion: 2 */ /* eslint no-whitespace-before-property: 2 */ /* eslint quotes: [2, "single"] */ /* eslint semi: 2 */ @@ -17,7 +16,7 @@ module.exports = leftpad; function leftpad (str, len, ch) { - str = String(str); + str = '' + str; var i = -1; diff --git a/tests/fixtures/autofix-integration/left-pad.js b/tests/fixtures/autofix-integration/left-pad.js index 7b673577e8a..672eff362b6 100644 --- a/tests/fixtures/autofix-integration/left-pad.js +++ b/tests/fixtures/autofix-integration/left-pad.js @@ -1,7 +1,6 @@ /* eslint dot-notation: 2 */ /* eslint indent: [2, 2] */ /* eslint no-extra-parens: 2 */ -/* eslint no-implicit-coercion: 2 */ /* eslint no-whitespace-before-property: 2 */ /* eslint quotes: [2, "single"] */ /* eslint semi: 2 */ diff --git a/tests/lib/rules/no-implicit-coercion.js b/tests/lib/rules/no-implicit-coercion.js index 679089aff28..88be4cf9a57 100644 --- a/tests/lib/rules/no-implicit-coercion.js +++ b/tests/lib/rules/no-implicit-coercion.js @@ -126,7 +126,7 @@ ruleTester.run("no-implicit-coercion", rule, { code: "!!foo", output: "Boolean(foo)", errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "Boolean(foo)" }, type: "UnaryExpression" }] @@ -135,16 +135,49 @@ ruleTester.run("no-implicit-coercion", rule, { code: "!!(foo + bar)", output: "Boolean(foo + bar)", errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "Boolean(foo + bar)" }, type: "UnaryExpression" }] }, + { + code: "!!(foo + bar); var Boolean = null", + output: null, + errors: [{ + messageId: "implicitCoercion", + data: { recommendation: "Boolean(foo + bar)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "Boolean(foo + bar)" }, + output: "Boolean(foo + bar); var Boolean = null" + }], + type: "UnaryExpression" + }] + }, + { + code: "!!(foo + bar)", + output: null, + languageOptions: { + globals: { + Boolean: "off" + } + }, + errors: [{ + messageId: "implicitCoercion", + data: { recommendation: "Boolean(foo + bar)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "Boolean(foo + bar)" }, + output: "Boolean(foo + bar)" + }], + type: "UnaryExpression" + }] + }, { code: "~foo.indexOf(1)", output: null, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "foo.indexOf(1) !== -1" }, type: "UnaryExpression" }] @@ -153,17 +186,22 @@ ruleTester.run("no-implicit-coercion", rule, { code: "~foo.bar.indexOf(2)", output: null, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "foo.bar.indexOf(2) !== -1" }, type: "UnaryExpression" }] }, { code: "+foo", - output: "Number(foo)", + output: null, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "Number(foo)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "Number(foo)" }, + output: "Number(foo)" + }], type: "UnaryExpression" }] }, @@ -171,181 +209,276 @@ ruleTester.run("no-implicit-coercion", rule, { code: "-(-foo)", output: null, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "Number(foo)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "Number(foo)" }, + output: "Number(foo)" + }], type: "UnaryExpression" }] }, { code: "+foo.bar", - output: "Number(foo.bar)", + output: null, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "Number(foo.bar)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "Number(foo.bar)" }, + output: "Number(foo.bar)" + }], type: "UnaryExpression" }] }, { code: "1*foo", - output: "Number(foo)", + output: null, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "Number(foo)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "Number(foo)" }, + output: "Number(foo)" + }], type: "BinaryExpression" }] }, { code: "foo*1", - output: "Number(foo)", + output: null, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "Number(foo)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "Number(foo)" }, + output: "Number(foo)" + }], type: "BinaryExpression" }] }, { code: "1*foo.bar", - output: "Number(foo.bar)", + output: null, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "Number(foo.bar)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "Number(foo.bar)" }, + output: "Number(foo.bar)" + }], type: "BinaryExpression" }] }, { code: "foo.bar-0", - output: "Number(foo.bar)", + output: null, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "Number(foo.bar)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "Number(foo.bar)" }, + output: "Number(foo.bar)" + }], type: "BinaryExpression" }] }, { code: "\"\"+foo", - output: "String(foo)", + output: null, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "String(foo)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "String(foo)" }, + output: "String(foo)" + }], type: "BinaryExpression" }] }, { code: "``+foo", - output: "String(foo)", + output: null, languageOptions: { ecmaVersion: 6 }, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "String(foo)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "String(foo)" }, + output: "String(foo)" + }], type: "BinaryExpression" }] }, { code: "foo+\"\"", - output: "String(foo)", + output: null, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "String(foo)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "String(foo)" }, + output: "String(foo)" + }], type: "BinaryExpression" }] }, { code: "foo+``", - output: "String(foo)", + output: null, languageOptions: { ecmaVersion: 6 }, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "String(foo)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "String(foo)" }, + output: "String(foo)" + }], type: "BinaryExpression" }] }, { code: "\"\"+foo.bar", - output: "String(foo.bar)", + output: null, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "String(foo.bar)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "String(foo.bar)" }, + output: "String(foo.bar)" + }], type: "BinaryExpression" }] }, { code: "``+foo.bar", - output: "String(foo.bar)", + output: null, languageOptions: { ecmaVersion: 6 }, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "String(foo.bar)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "String(foo.bar)" }, + output: "String(foo.bar)" + }], type: "BinaryExpression" }] }, { code: "foo.bar+\"\"", - output: "String(foo.bar)", + output: null, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "String(foo.bar)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "String(foo.bar)" }, + output: "String(foo.bar)" + }], type: "BinaryExpression" }] }, { code: "foo.bar+``", - output: "String(foo.bar)", + output: null, languageOptions: { ecmaVersion: 6 }, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "String(foo.bar)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "String(foo.bar)" }, + output: "String(foo.bar)" + }], type: "BinaryExpression" }] }, { code: "`${foo}`", - output: "String(foo)", + output: null, options: [{ disallowTemplateShorthand: true }], languageOptions: { ecmaVersion: 6 }, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "String(foo)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "String(foo)" }, + output: "String(foo)" + }], type: "TemplateLiteral" }] }, { code: "`\\\n${foo}`", - output: "String(foo)", + output: null, options: [{ disallowTemplateShorthand: true }], languageOptions: { ecmaVersion: 6 }, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "String(foo)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "String(foo)" }, + output: "String(foo)" + }], type: "TemplateLiteral" }] }, { code: "`${foo}\\\n`", - output: "String(foo)", + output: null, options: [{ disallowTemplateShorthand: true }], languageOptions: { ecmaVersion: 6 }, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "String(foo)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "String(foo)" }, + output: "String(foo)" + }], type: "TemplateLiteral" }] }, { code: "foo += \"\"", - output: "foo = String(foo)", + output: null, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "foo = String(foo)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "foo = String(foo)" }, + output: "foo = String(foo)" + }], type: "AssignmentExpression" }] }, { code: "foo += ``", - output: "foo = String(foo)", + output: null, languageOptions: { ecmaVersion: 6 }, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "foo = String(foo)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "foo = String(foo)" }, + output: "foo = String(foo)" + }], type: "AssignmentExpression" }] }, @@ -354,7 +487,7 @@ ruleTester.run("no-implicit-coercion", rule, { output: "var a = Boolean(foo)", options: [{ boolean: true, allow: ["~"] }], errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "Boolean(foo)" }, type: "UnaryExpression" }] @@ -364,77 +497,112 @@ ruleTester.run("no-implicit-coercion", rule, { output: null, options: [{ boolean: true, allow: ["!!"] }], errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "foo.indexOf(1) !== -1" }, type: "UnaryExpression" }] }, { code: "var a = 1 * foo", - output: "var a = Number(foo)", + output: null, options: [{ boolean: true, allow: ["+"] }], errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "Number(foo)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "Number(foo)" }, + output: "var a = Number(foo)" + }], type: "BinaryExpression" }] }, { code: "var a = +foo", - output: "var a = Number(foo)", + output: null, options: [{ boolean: true, allow: ["*"] }], errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "Number(foo)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "Number(foo)" }, + output: "var a = Number(foo)" + }], type: "UnaryExpression" }] }, { code: "var a = \"\" + foo", - output: "var a = String(foo)", + output: null, options: [{ boolean: true, allow: ["*"] }], errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "String(foo)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "String(foo)" }, + output: "var a = String(foo)" + }], type: "BinaryExpression" }] }, { code: "var a = `` + foo", - output: "var a = String(foo)", + output: null, options: [{ boolean: true, allow: ["*"] }], languageOptions: { ecmaVersion: 6 }, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "String(foo)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "String(foo)" }, + output: "var a = String(foo)" + }], type: "BinaryExpression" }] }, { code: "typeof+foo", - output: "typeof Number(foo)", + output: null, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "Number(foo)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "Number(foo)" }, + output: "typeof Number(foo)" + }], type: "UnaryExpression" }] }, { code: "typeof +foo", - output: "typeof Number(foo)", + output: null, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "Number(foo)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "Number(foo)" }, + output: "typeof Number(foo)" + }], type: "UnaryExpression" }] }, { code: "let x ='' + 1n;", - output: "let x =String(1n);", + output: null, languageOptions: { ecmaVersion: 2020 }, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "String(1n)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "String(1n)" }, + output: "let x =String(1n);" + }], type: "BinaryExpression" }] }, @@ -445,7 +613,7 @@ ruleTester.run("no-implicit-coercion", rule, { output: null, languageOptions: { ecmaVersion: 2020 }, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "foo?.indexOf(1) >= 0" }, type: "UnaryExpression" }] @@ -455,7 +623,7 @@ ruleTester.run("no-implicit-coercion", rule, { output: null, languageOptions: { ecmaVersion: 2020 }, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "(foo?.indexOf)(1) !== -1" }, type: "UnaryExpression" }] @@ -464,37 +632,57 @@ ruleTester.run("no-implicit-coercion", rule, { // https://github.com/eslint/eslint/issues/16373 regression tests { code: "1 * a / 2", - output: "Number(a) / 2", + output: null, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "Number(a)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "Number(a)" }, + output: "Number(a) / 2" + }], type: "BinaryExpression" }] }, { code: "(a * 1) / 2", - output: "(Number(a)) / 2", + output: null, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "Number(a)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "Number(a)" }, + output: "(Number(a)) / 2" + }], type: "BinaryExpression" }] }, { code: "a * 1 / (b * 1)", - output: "a * 1 / (Number(b))", + output: null, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "Number(b)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "Number(b)" }, + output: "a * 1 / (Number(b))" + }], type: "BinaryExpression" }] }, { code: "a * 1 + 2", - output: "Number(a) + 2", + output: null, errors: [{ - messageId: "useRecommendation", + messageId: "implicitCoercion", data: { recommendation: "Number(a)" }, + suggestions: [{ + messageId: "useRecommendation", + data: { recommendation: "Number(a)" }, + output: "Number(a) + 2" + }], type: "BinaryExpression" }] }