fix(eslint-plugin): [no-unnecessary-type-assertion] fix false negativ…

…e for const variable declarations (#8558)

* false negative for consts

* add more testcases

* remove unused fn

* add NNA

* split into two ifs

* restore comment

* finish

* change order for diffs

* fix error triggered by new lint logic

* WIP clean up tests

* WIP

* WIP

* remove duplicate tests

* fix docs

* make test cases more self-contained

Co-authored-by: auvred <61150013+auvred@users.noreply.github.com>

* suppress lint error

* remove unnecessary type assertions

---------

Co-authored-by: auvred <61150013+auvred@users.noreply.github.com>
Co-authored-by: Josh Goldberg <git@joshuakgoldberg.com>

packages/eslint-plugin/docs/rules/no-unnecessary-type-assertion.mdx

@@ -37,6 +37,10 @@ type Foo = 3;
 const foo = 3 as Foo;
 ```
 
+```ts
+const foo = 'foo' as const;
+```
+
 ```ts
 function foo(x: number): number {
   return x!; // unnecessary non-null
@@ -55,7 +59,7 @@ const foo = 3 as number;
 ```
 
 ```ts
-const foo = 'foo' as const;
+let foo = 'foo' as const;
 ```
 
 ```ts

packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts

@@ -61,37 +61,6 @@ export default createRule<Options, MessageIds>({
     const checker = services.program.getTypeChecker();
     const compilerOptions = services.program.getCompilerOptions();
 
-    /**
-     * Sometimes tuple types don't have ObjectFlags.Tuple set, like when they're being matched against an inferred type.
-     * So, in addition, check if there are integer properties 0..n and no other numeric keys
-     */
-    function couldBeTupleType(type: ts.ObjectType): boolean {
-      const properties = type.getProperties();
-
-      if (properties.length === 0) {
-        return false;
-      }
-      let i = 0;
-
-      for (; i < properties.length; ++i) {
-        const name = properties[i].name;
-
-        if (String(i) !== name) {
-          if (i === 0) {
-            // if there are no integer properties, this is not a tuple
-            return false;
-          }
-          break;
-        }
-      }
-      for (; i < properties.length; ++i) {
-        if (String(+properties[i].name) === properties[i].name) {
-          return false; // if there are any other numeric properties, this is not a tuple
-        }
-      }
-      return true;
-    }
-
     /**
      * Returns true if there's a chance the variable has been used before a value has been assigned to it
      */
@@ -139,6 +108,13 @@ export default createRule<Options, MessageIds>({
       );
     }
 
+    function isConstVariableDeclaration(node: TSESTree.Node): boolean {
+      return (
+        node.type === AST_NODE_TYPES.VariableDeclaration &&
+        node.kind === 'const'
+      );
+    }
+
     return {
       TSNonNullExpression(node): void {
         if (
@@ -236,28 +212,21 @@ export default createRule<Options, MessageIds>({
         if (
           options.typesToIgnore?.includes(
             context.sourceCode.getText(node.typeAnnotation),
-          ) ||
-          isConstAssertion(node.typeAnnotation)
+          )
         ) {
           return;
         }
 
         const castType = services.getTypeAtLocation(node);
-
-        if (
-          isTypeFlagSet(castType, ts.TypeFlags.Literal) ||
-          (tsutils.isObjectType(castType) &&
-            (tsutils.isObjectFlagSet(castType, ts.ObjectFlags.Tuple) ||
-              couldBeTupleType(castType)))
-        ) {
-          // It's not always safe to remove a cast to a literal type or tuple
-          // type, as those types are sometimes widened without the cast.
-          return;
-        }
-
         const uncastType = services.getTypeAtLocation(node.expression);
+        const typeIsUnchanged = uncastType === castType;
+
+        const wouldSameTypeBeInferred = castType.isLiteral()
+          ? // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+            isConstVariableDeclaration(node.parent.parent!)
+          : !isConstAssertion(node.typeAnnotation);
 
-        if (uncastType === castType) {
+        if (typeIsUnchanged && wouldSameTypeBeInferred) {
           context.report({
             node,
             messageId: 'unnecessaryAssertion',

packages/eslint-plugin/tests/rules/no-array-constructor.test.ts

@@ -7,7 +7,7 @@ const ruleTester = new RuleTester({
   parser: '@typescript-eslint/parser',
 });
 
-const messageId = 'useLiteral' as const;
+const messageId = 'useLiteral';
 
 ruleTester.run('no-array-constructor', rule, {
   valid: [

packages/eslint-plugin/tests/rules/no-unnecessary-type-assertion.test.ts

@@ -31,10 +31,34 @@ if (
   const name = member.id as TSESTree.StringLiteral;
 }
     `,
+    `
+      const c = 1;
+      let z = c as number;
+    `,
+    `
+      const c = 1;
+      let z = c as const;
+    `,
+    `
+      const c = 1;
+      let z = c as 1;
+    `,
+    `
+      type Bar = 'bar';
+      const data = {
+        x: 'foo' as 'foo',
+        y: 'bar' as Bar,
+      };
+    `,
+    "[1, 2, 3, 4, 5].map(x => [x, 'A' + x] as [number, string]);",
+    `
+      let x: Array<[number, string]> = [1, 2, 3, 4, 5].map(
+        x => [x, 'A' + x] as [number, string],
+      );
+    `,
+    'let y = 1 as 1;',
     'const foo = 3 as number;',
     'const foo = <number>3;',
-    'const foo = <3>3;',
-    'const foo = 3 as 3;',
     `
 type Tuple = [3, 'hi', 'bye'];
 const foo = [3, 'hi', 'bye'] as Tuple;
@@ -180,9 +204,6 @@ const c = [...a, ...b] as const;
     {
       code: 'const a = [1, 2] as const;',
     },
-    {
-      code: "const a = 'a' as const;",
-    },
     {
       code: "const a = { foo: 'foo' } as const;",
     },
@@ -196,9 +217,6 @@ const c = <const>[...a, ...b];
     {
       code: 'const a = <const>[1, 2];',
     },
-    {
-      code: "const a = <const>'a';",
-    },
     {
       code: "const a = <const>{ foo: 'foo' };",
     },
@@ -262,6 +280,48 @@ function bar(items: string[]) {
   ],
 
   invalid: [
+    {
+      code: "const a = 'a' as const;",
+      output: "const a = 'a';",
+      errors: [{ messageId: 'unnecessaryAssertion', line: 1 }],
+    },
+    {
+      code: "const a = <const>'a';",
+      output: "const a = 'a';",
+      errors: [{ messageId: 'unnecessaryAssertion', line: 1 }],
+    },
+    {
+      code: 'const foo = <3>3;',
+      output: 'const foo = 3;',
+      errors: [{ messageId: 'unnecessaryAssertion', line: 1, column: 13 }],
+    },
+    {
+      code: 'const foo = 3 as 3;',
+      output: 'const foo = 3;',
+      errors: [{ messageId: 'unnecessaryAssertion', line: 1, column: 13 }],
+    },
+    {
+      code: `
+        type Foo = 3;
+        const foo = <Foo>3;
+      `,
+      output: `
+        type Foo = 3;
+        const foo = 3;
+      `,
+      errors: [{ messageId: 'unnecessaryAssertion', line: 3, column: 21 }],
+    },
+    {
+      code: `
+        type Foo = 3;
+        const foo = 3 as Foo;
+      `,
+      output: `
+        type Foo = 3;
+        const foo = 3;
+      `,
+      errors: [{ messageId: 'unnecessaryAssertion', line: 3, column: 21 }],
+    },
     {
       code: `
 const foo = 3;

packages/typescript-estree/tests/lib/persistentParse.test.ts

@@ -124,7 +124,7 @@ function baseTests(
 
   it('allows parsing of deeply nested new files', () => {
     const PROJECT_DIR = setup(tsConfigIncludeAll, false);
-    const bazSlashBar = 'baz/bar' as const;
+    const bazSlashBar = 'baz/bar';
 
     // parse once to: assert the config as correct, and to make sure the program is setup
     expect(() => parseFile('foo', PROJECT_DIR)).not.toThrow();
@@ -149,7 +149,7 @@ function baseTests(
     fs.mkdirSync(path.join(PROJECT_DIR, 'src', 'bat'));
     fs.mkdirSync(path.join(PROJECT_DIR, 'src', 'bat', 'baz'));
 
-    const bazSlashBar = 'bat/baz/bar' as const;
+    const bazSlashBar = 'bat/baz/bar';
 
     // write a new file and attempt to parse it
     writeFile(PROJECT_DIR, bazSlashBar);
@@ -159,7 +159,7 @@ function baseTests(
 
   it('allows renaming of files', () => {
     const PROJECT_DIR = setup(tsConfigIncludeAll, true);
-    const bazSlashBar = 'baz/bar' as const;
+    const bazSlashBar = 'baz/bar';
 
     // parse once to: assert the config as correct, and to make sure the program is setup
     expect(() => parseFile('foo', PROJECT_DIR)).not.toThrow();
@@ -291,7 +291,7 @@ describe('persistent parse', () => {
 
       it('handles tsconfigs with no includes/excludes (nested)', () => {
         const PROJECT_DIR = setup({}, false);
-        const bazSlashBar = 'baz/bar' as const;
+        const bazSlashBar = 'baz/bar';
 
         // parse once to: assert the config as correct, and to make sure the program is setup
         expect(() => parseFile('foo', PROJECT_DIR)).not.toThrow();