fix(eslint-plugin-template): [eqeqeq] change fix to suggest (#465)

rafaelss95 · web-flow · commit a497fde0ddab · 2021-05-18T16:36:37.000+04:00
diff --git a/packages/eslint-plugin-template/src/rules/eqeqeq.ts b/packages/eslint-plugin-template/src/rules/eqeqeq.ts
@@ -1,13 +1,14 @@
 import type { AST, Binary } from '@angular/compiler';
 import { ASTWithSource, LiteralPrimitive } from '@angular/compiler';
+import type { TSESLint } from '@typescript-eslint/experimental-utils';
 import {
   createESLintRule,
   ensureTemplateParser,
 } from '../utils/create-eslint-rule';
 import { getNearestNodeFrom } from '../utils/get-nearest-node-from';
 
 type Options = [{ readonly allowNullOrUndefined?: boolean }];
-export type MessageIds = 'eqeqeq';
+export type MessageIds = 'eqeqeq' | 'suggestStrictEquality';
 export const RULE_NAME = 'eqeqeq';
 const DEFAULT_OPTIONS: Options[0] = { allowNullOrUndefined: false };
 
@@ -19,6 +20,7 @@ export default createESLintRule<Options, MessageIds>({
       description: 'Requires `===` and `!==` in place of `==` and `!=`',
       category: 'Best Practices',
       recommended: 'error',
+      suggestion: true,
     },
     fixable: 'code',
     schema: [
@@ -36,6 +38,8 @@ export default createESLintRule<Options, MessageIds>({
     messages: {
       eqeqeq:
         'Expected `{{expectedOperation}}` but received `{{actualOperation}}`',
+      suggestStrictEquality:
+        'Replace `{{expectedOperation}}` with `{{actualOperation}}`',
     },
   },
   defaultOptions: [DEFAULT_OPTIONS],
@@ -51,30 +55,37 @@ export default createESLintRule<Options, MessageIds>({
           right,
           sourceSpan: { start, end },
         } = node;
-        const isNilComparison = [left, right].some(isNilValue);
 
-        if (allowNullOrUndefined && isNilComparison) return;
+        if (allowNullOrUndefined && (isNilValue(left) || isNilValue(right))) {
+          return;
+        }
 
+        const data = {
+          actualOperation: operation,
+          expectedOperation: `${operation}=`,
+        } as const;
         context.report({
           loc: {
             start: sourceCode.getLocFromIndex(start),
             end: sourceCode.getLocFromIndex(end),
           },
           messageId: 'eqeqeq',
-          data: {
-            actualOperation: operation,
-            expectedOperation: `${operation}=`,
-          },
-          fix: (fixer) => {
-            const { source } = getNearestNodeFrom(node, isASTWithSource) ?? {};
-
-            if (!source) return [];
-
-            return fixer.insertTextAfterRange(
-              [start + getSpanLength(left) + 1, end - getSpanLength(right) - 1],
-              '=',
-            );
-          },
+          data,
+          ...(isStringNonNumericValue(left) || isStringNonNumericValue(right)
+            ? {
+                fix: (fixer) =>
+                  getFix({ node, left, right, start, end, fixer }),
+              }
+            : {
+                suggest: [
+                  {
+                    messageId: 'suggestStrictEquality',
+                    fix: (fixer) =>
+                      getFix({ node, left, right, start, end, fixer }),
+                    data,
+                  },
+                ],
+              }),
         });
       },
     };
@@ -85,13 +96,62 @@ function getSpanLength({ span: { start, end } }: AST): number {
   return end - start;
 }
 
+const getFix = ({
+  node,
+  left,
+  right,
+  start,
+  end,
+  fixer,
+}: {
+  node: Binary;
+  left: AST;
+  right: AST;
+  start: number;
+  end: number;
+  fixer: TSESLint.RuleFixer;
+}): TSESLint.RuleFix | TSESLint.RuleFix[] => {
+  const { source } = getNearestNodeFrom(node, isASTWithSource) ?? {};
+
+  if (!source) return [];
+
+  return fixer.insertTextAfterRange(
+    [start + getSpanLength(left) + 1, end - getSpanLength(right) - 1],
+    '=',
+  );
+};
+
 function isASTWithSource(node: unknown): node is ASTWithSource {
   return node instanceof ASTWithSource;
 }
 
-function isNilValue(ast: AST): ast is LiteralPrimitive {
+function isLiteralPrimitive(node: unknown): node is LiteralPrimitive {
+  return node instanceof LiteralPrimitive;
+}
+
+function isNumeric(value: unknown): value is number | string {
+  return (
+    !Number.isNaN(Number.parseFloat(String(value))) &&
+    Number.isFinite(Number(value))
+  );
+}
+
+function isString(value: unknown): value is string {
+  return typeof value === 'string';
+}
+
+function isStringNonNumericValue(
+  ast: AST,
+): ast is LiteralPrimitive & { value: string } {
+  return (
+    isLiteralPrimitive(ast) && isString(ast.value) && !isNumeric(ast.value)
+  );
+}
+
+function isNilValue(
+  ast: AST,
+): ast is LiteralPrimitive & { value: null | undefined } {
   return (
-    ast instanceof LiteralPrimitive &&
-    (ast.value === null || ast.value === undefined)
+    isLiteralPrimitive(ast) && (ast.value === null || ast.value === undefined)
   );
 }
diff --git a/packages/eslint-plugin-template/tests/rules/eqeqeq.test.ts b/packages/eslint-plugin-template/tests/rules/eqeqeq.test.ts
@@ -13,6 +13,7 @@ const ruleTester = new RuleTester({
   parser: '@angular-eslint/template-parser',
 });
 const messageId: MessageIds = 'eqeqeq';
+const suggestStrictEquality: MessageIds = 'suggestStrictEquality';
 
 ruleTester.run(RULE_NAME, rule, {
   valid: [
@@ -31,7 +32,7 @@ ruleTester.run(RULE_NAME, rule, {
       description:
         'it should fail if the operation is not strict within interpolation',
       annotatedSource: `
-        {{ test == 'null' }}
+        {{ 'null' == test }}
            ~~~~~~~~~~~~~~
       `,
       messageId,
@@ -40,15 +41,15 @@ ruleTester.run(RULE_NAME, rule, {
         expectedOperation: '===',
       },
       annotatedOutput: `
-        {{ test === 'null' }}
+        {{ 'null' === test }}
            ~~~~~~~~~~~~~~~
       `,
     }),
     convertAnnotatedSourceToFailureCase({
       description:
         'it should fail if the operation is not strict within attribute directive',
       annotatedSource: `
-        <div [attr.disabled]="test != 'undefined' && null == 3"></div>
+        <div [attr.disabled]="test != 'undefined' && null == '3'"></div>
                               ~~~~~~~~~~~~~~~~~~~
       `,
       messageId,
@@ -58,7 +59,7 @@ ruleTester.run(RULE_NAME, rule, {
       },
       options: [{ allowNullOrUndefined: true }],
       annotatedOutput: `
-        <div [attr.disabled]="test !== 'undefined' && null == 3"></div>
+        <div [attr.disabled]="test !== 'undefined' && null == '3'"></div>
                               ~~~~~~~~~~~~~~~~~~~~
       `,
     }),
@@ -74,27 +75,45 @@ ruleTester.run(RULE_NAME, rule, {
         actualOperation: '==',
         expectedOperation: '===',
       },
-      annotatedOutput: `
+      suggestions: [
+        {
+          messageId: suggestStrictEquality,
+          output: `
         <div *ngIf="test === true || test1 !== undefined"></div>
-                    ~~~~~~~~~~~~~
-      `,
+                    
+      `,
+          data: {
+            actualOperation: '==',
+            expectedOperation: '===',
+          },
+        },
+      ],
     }),
     convertAnnotatedSourceToFailureCase({
       description:
         'it should fail if the operation is not strict within conditional',
       annotatedSource: `
-        {{ one != two ? c > d : 'hey!' }}
-           ~~~~~~~~~~
+        {{ one != '02' ? c > d : 'hey!' }}
+           ~~~~~~~~~~~
       `,
       messageId,
       data: {
         actualOperation: '!=',
         expectedOperation: '!==',
       },
-      annotatedOutput: `
-        {{ one !== two ? c > d : 'hey!' }}
-           ~~~~~~~~~~~
-      `,
+      suggestions: [
+        {
+          messageId: suggestStrictEquality,
+          output: `
+        {{ one !== '02' ? c > d : 'hey!' }}
+           
+      `,
+          data: {
+            actualOperation: '!=',
+            expectedOperation: '!==',
+          },
+        },
+      ],
     }),
     convertAnnotatedSourceToFailureCase({
       description:
@@ -108,10 +127,19 @@ ruleTester.run(RULE_NAME, rule, {
         actualOperation: '==',
         expectedOperation: '===',
       },
-      annotatedOutput: `
+      suggestions: [
+        {
+          messageId: suggestStrictEquality,
+          output: `
         {{ a === b && 1 === b ? c > d : 'hey!' }}
-                      ~~~~~~~
-      `,
+                      
+      `,
+          data: {
+            actualOperation: '==',
+            expectedOperation: '===',
+          },
+        },
+      ],
     }),
     convertAnnotatedSourceToFailureCase({
       description:
@@ -125,45 +153,72 @@ ruleTester.run(RULE_NAME, rule, {
         actualOperation: '!=',
         expectedOperation: '!==',
       },
-      annotatedOutput: `
+      suggestions: [
+        {
+          messageId: suggestStrictEquality,
+          output: `
         {{ c > d ? a !== b : 'hey!' }}
-                   ~~~~~~~
-      `,
+                   
+      `,
+          data: {
+            actualOperation: '!=',
+            expectedOperation: '!==',
+          },
+        },
+      ],
     }),
     convertAnnotatedSourceToFailureCase({
       description:
         'it should fail if the operation is not strict within conditional (falseExp)',
       annotatedSource: `
-        {{ c > d ? 'hey!' : a == b }}
-                            ~~~~~~
+        {{ c > d ? 'hey!' : a == false }}
+                            ~~~~~~~~~~
       `,
       messageId,
       data: {
         actualOperation: '==',
         expectedOperation: '===',
       },
-      annotatedOutput: `
-        {{ c > d ? 'hey!' : a === b }}
-                            ~~~~~~~
-      `,
+      suggestions: [
+        {
+          messageId: suggestStrictEquality,
+          output: `
+        {{ c > d ? 'hey!' : a === false }}
+                            
+      `,
+          data: {
+            actualOperation: '==',
+            expectedOperation: '===',
+          },
+        },
+      ],
     }),
     convertAnnotatedSourceToFailureCase({
       description:
         'it should fail if the operation is not strict within recursive conditional',
       annotatedSource: `
-        {{ undefined == test1 && a === b ? (c > d ? d != 3 : v === 4) : 'hey!' }}
-                                                    ~~~~~~
+        {{ undefined == test1 && a === b ? (c > d ? d != '0' : v === 4) : 'hey!' }}
+                                                    ~~~~~~~~
       `,
       messageId,
       options: [{ allowNullOrUndefined: true }],
       data: {
         actualOperation: '!=',
         expectedOperation: '!==',
       },
-      annotatedOutput: `
-        {{ undefined == test1 && a === b ? (c > d ? d !== 3 : v === 4) : 'hey!' }}
-                                                    ~~~~~~~
-      `,
+      suggestions: [
+        {
+          messageId: suggestStrictEquality,
+          output: `
+        {{ undefined == test1 && a === b ? (c > d ? d !== '0' : v === 4) : 'hey!' }}
+                                                    
+      `,
+          data: {
+            actualOperation: '!=',
+            expectedOperation: '!==',
+          },
+        },
+      ],
     }),
     convertAnnotatedSourceToFailureCase({
       description:
@@ -177,10 +232,19 @@ ruleTester.run(RULE_NAME, rule, {
         actualOperation: '!=',
         expectedOperation: '!==',
       },
-      annotatedOutput: `
+      suggestions: [
+        {
+          messageId: suggestStrictEquality,
+          output: `
         {{ undefined !== test1 }}
-           ~~~~~~~~~~~~~~~~~~~
-      `,
+           
+      `,
+          data: {
+            actualOperation: '!=',
+            expectedOperation: '!==',
+          },
+        },
+      ],
     }),
   ],
 });
