feat: convert unsafe autofixes to suggestions in `no-implicit-coercio…

…n` (#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
eslint · Jan 20, 2024 · 5471e43 · 5471e43
1 parent 2e1d549
commit 5471e43
Show file tree

Hide file tree

Showing 5 changed files with 310 additions and 99 deletions.
diff --git 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,23 +316,23 @@ 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
                 operatorAllowed = options.allow.includes("+");
                 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)
                 operatorAllowed = options.allow.includes("- -");
                 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,23 +348,23 @@ module.exports = {
                 if (nonNumericOperand) {
                     const recommendation = `Number(${sourceCode.getText(nonNumericOperand)})`;
 
-                    report(node, recommendation, true);
+                    report(node, recommendation, true, false);
                 }
 
                 // foo - 0
                 operatorAllowed = options.allow.includes("-");
                 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
                 operatorAllowed = options.allow.includes("+");
                 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
@@ -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
@@ -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
@@ -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 */