fix: correct decorator traversal for AssignmentPattern

typescript-eslint · Aug 8, 2020 · 2b75c5b · 2b75c5b
1 parent 3529589
commit 2b75c5b
Show file tree

Hide file tree

Showing 11 changed files with 46 additions and 169 deletions.
diff --git a/packages/eslint-plugin/tests/rules/no-unused-vars.test.ts b/packages/eslint-plugin/tests/rules/no-unused-vars.test.ts
@@ -749,6 +749,16 @@ export interface Event<T> {
         },
       ],
     },
+    // https://github.com/typescript-eslint/typescript-eslint/issues/2369
+    `
+export default function (@Optional() value = []) {
+  return value;
+}
+
+function Optional() {
+  return () => {};
+}
+    `,
   ],
 
   invalid: [

diff --git a/packages/types/package.json b/packages/types/package.json
@@ -30,6 +30,7 @@
   "scripts": {
     "build": "tsc -b tsconfig.build.json",
     "clean": "tsc -b tsconfig.build.json --clean",
+    "postclean": "rimraf dist",
     "format": "prettier --write \"./**/*.{ts,js,json,md}\" --ignore-path ../../.prettierignore",
     "generate:lib": "../../node_modules/.bin/ts-node --files --transpile-only ../scope-manager/tools/generate-lib.ts",
     "lint": "eslint . --ext .js,.ts --ignore-path='../../.eslintignore'",

diff --git a/packages/types/src/ast-node-types.ts b/packages/types/src/ast-node-types.ts
@@ -33,7 +33,6 @@ enum AST_NODE_TYPES {
   FunctionExpression = 'FunctionExpression',
   Identifier = 'Identifier',
   IfStatement = 'IfStatement',
-  Import = 'Import',
   ImportDeclaration = 'ImportDeclaration',
   ImportDefaultSpecifier = 'ImportDefaultSpecifier',
   ImportExpression = 'ImportExpression',
@@ -163,3 +162,18 @@ enum AST_NODE_TYPES {
 }
 
 export { AST_NODE_TYPES };
+
+// Below is a special type-only test which ensures that we don't accidentally leave unused keys in this enum
+// eslint-disable-next-line import/first -- purposely down here to colocate it with this hack of a test
+import type { Node } from './ts-estree';
+
+type GetKeys<T extends AST_NODE_TYPES> = keyof Extract<Node, { type: T }>;
+type AllKeys = {
+  readonly [T in AST_NODE_TYPES]: GetKeys<T>;
+};
+type TakesString<T extends Record<string, string>> = T;
+// @ts-expect-error: purposely unused
+type _Test =
+  // forcing the test onto a new line so it isn't covered by the expect error
+  // If there are any enum members that don't have a corresponding TSESTree.Node, then this line will error with "Type 'string | number | symbol' is not assignable to type 'string'."
+  void | TakesString<AllKeys>;
diff --git a/packages/types/src/ts-estree.ts b/packages/types/src/ts-estree.ts
@@ -946,7 +946,6 @@ export interface ForStatement extends BaseNode {
 export interface FunctionDeclaration extends FunctionDeclarationBase {
   type: AST_NODE_TYPES.FunctionDeclaration;
   body: BlockStatement;
-  decorators?: Decorator[];
 }
 
 export interface FunctionExpression extends FunctionDeclarationBase {
@@ -1341,7 +1340,6 @@ export interface TSEnumDeclaration extends BaseNode {
   const?: boolean;
   declare?: boolean;
   modifiers?: Modifier[];
-  decorators?: Decorator[];
 }
 
 /**
@@ -1426,7 +1424,6 @@ export interface TSInterfaceDeclaration extends BaseNode {
   typeParameters?: TSTypeParameterDeclaration;
   extends?: TSInterfaceHeritage[];
   implements?: TSInterfaceHeritage[];
-  decorators?: Decorator[];
   abstract?: boolean;
   declare?: boolean;
 }

diff --git a/packages/typescript-estree/src/convert.ts b/packages/typescript-estree/src/convert.ts
@@ -839,17 +839,6 @@ export class Converter {
           );
         }
 
-        /**
-         * Semantically, decorators are not allowed on function declarations,
-         * but the TypeScript compiler will parse them and produce a valid AST,
-         * so we handle them here too.
-         */
-        if (node.decorators) {
-          (result as any).decorators = node.decorators.map(el =>
-            this.convertChild(el),
-          );
-        }
-
         // check for exports
         return this.fixExports(node, result);
       }
@@ -2514,14 +2503,6 @@ export class Converter {
           }
         }
 
-        /**
-         * Semantically, decorators are not allowed on interface declarations,
-         * but the TypeScript compiler will parse them and produce a valid AST,
-         * so we handle them here too.
-         */
-        if (node.decorators) {
-          result.decorators = node.decorators.map(el => this.convertChild(el));
-        }
         if (hasModifier(SyntaxKind.AbstractKeyword, node)) {
           result.abstract = true;
         }
@@ -2573,14 +2554,6 @@ export class Converter {
         });
         // apply modifiers first...
         this.applyModifiersToResult(result, node.modifiers);
-        /**
-         * Semantically, decorators are not allowed on enum declarations,
-         * but the TypeScript compiler will parse them and produce a valid AST,
-         * so we handle them here too.
-         */
-        if (node.decorators) {
-          result.decorators = node.decorators.map(el => this.convertChild(el));
-        }
         // ...then check for exports
         return this.fixExports(node, result);
       }

diff --git a/packages/typescript-estree/src/ts-estree/estree-to-ts-node-types.ts b/packages/typescript-estree/src/ts-estree/estree-to-ts-node-types.ts
@@ -72,7 +72,6 @@ export interface EstreeToTsNodeTypes {
     | ts.ConstructorDeclaration
     | ts.Token<ts.SyntaxKind.NewKeyword | ts.SyntaxKind.ImportKeyword>;
   [AST_NODE_TYPES.IfStatement]: ts.IfStatement;
-  [AST_NODE_TYPES.Import]: ts.ImportExpression;
   [AST_NODE_TYPES.ImportDeclaration]: ts.ImportDeclaration;
   [AST_NODE_TYPES.ImportDefaultSpecifier]: ts.ImportClause;
   [AST_NODE_TYPES.ImportExpression]: ts.CallExpression;

diff --git a/...estree/tests/snapshots/typescript/errorRecovery/decorator-on-enum-declaration.src.ts.shot b/...estree/tests/snapshots/typescript/errorRecovery/decorator-on-enum-declaration.src.ts.shot
@@ -4,43 +4,6 @@ exports[`typescript errorRecovery decorator-on-enum-declaration.src 1`] = `
 Object {
   "body": Array [
     Object {
-      "decorators": Array [
-        Object {
-          "expression": Object {
-            "loc": Object {
-              "end": Object {
-                "column": 4,
-                "line": 1,
-              },
-              "start": Object {
-                "column": 1,
-                "line": 1,
-              },
-            },
-            "name": "dec",
-            "range": Array [
-              1,
-              4,
-            ],
-            "type": "Identifier",
-          },
-          "loc": Object {
-            "end": Object {
-              "column": 4,
-              "line": 1,
-            },
-            "start": Object {
-              "column": 0,
-              "line": 1,
-            },
-          },
-          "range": Array [
-            0,
-            4,
-          ],
-          "type": "Decorator",
-        },
-      ],
       "id": Object {
         "loc": Object {
           "end": Object {

diff --git a/...escript-estree/tests/snapshots/typescript/errorRecovery/decorator-on-function.src.ts.shot b/...escript-estree/tests/snapshots/typescript/errorRecovery/decorator-on-function.src.ts.shot
@@ -23,43 +23,6 @@ Object {
         ],
         "type": "BlockStatement",
       },
-      "decorators": Array [
-        Object {
-          "expression": Object {
-            "loc": Object {
-              "end": Object {
-                "column": 4,
-                "line": 1,
-              },
-              "start": Object {
-                "column": 1,
-                "line": 1,
-              },
-            },
-            "name": "dec",
-            "range": Array [
-              1,
-              4,
-            ],
-            "type": "Identifier",
-          },
-          "loc": Object {
-            "end": Object {
-              "column": 4,
-              "line": 1,
-            },
-            "start": Object {
-              "column": 0,
-              "line": 1,
-            },
-          },
-          "range": Array [
-            0,
-            4,
-          ],
-          "type": "Decorator",
-        },
-      ],
       "expression": false,
       "generator": false,
       "id": Object {

diff --git a/...e/tests/snapshots/typescript/errorRecovery/decorator-on-interface-declaration.src.ts.shot b/...e/tests/snapshots/typescript/errorRecovery/decorator-on-interface-declaration.src.ts.shot
@@ -22,62 +22,6 @@ Object {
         ],
         "type": "TSInterfaceBody",
       },
-      "decorators": Array [
-        Object {
-          "expression": Object {
-            "arguments": Array [],
-            "callee": Object {
-              "loc": Object {
-                "end": Object {
-                  "column": 5,
-                  "line": 1,
-                },
-                "start": Object {
-                  "column": 1,
-                  "line": 1,
-                },
-              },
-              "name": "deco",
-              "range": Array [
-                1,
-                5,
-              ],
-              "type": "Identifier",
-            },
-            "loc": Object {
-              "end": Object {
-                "column": 7,
-                "line": 1,
-              },
-              "start": Object {
-                "column": 1,
-                "line": 1,
-              },
-            },
-            "optional": false,
-            "range": Array [
-              1,
-              7,
-            ],
-            "type": "CallExpression",
-          },
-          "loc": Object {
-            "end": Object {
-              "column": 7,
-              "line": 1,
-            },
-            "start": Object {
-              "column": 0,
-              "line": 1,
-            },
-          },
-          "range": Array [
-            0,
-            7,
-          ],
-          "type": "Decorator",
-        },
-      ],
       "id": Object {
         "loc": Object {
           "end": Object {

diff --git a/packages/visitor-keys/package.json b/packages/visitor-keys/package.json
@@ -30,6 +30,7 @@
   "scripts": {
     "build": "tsc -b tsconfig.build.json",
     "clean": "tsc -b tsconfig.build.json --clean",
+    "postclean": "rimraf dist",
     "format": "prettier --write \"./**/*.{ts,js,json,md}\" --ignore-path ../../.prettierignore",
     "lint": "eslint . --ext .js,.ts --ignore-path='../../.eslintignore'",
     "test": "jest --coverage",

diff --git a/packages/visitor-keys/src/visitor-keys.ts b/packages/visitor-keys/src/visitor-keys.ts
@@ -1,17 +1,28 @@
+import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/types';
 import * as eslintVisitorKeys from 'eslint-visitor-keys';
 
 interface VisitorKeys {
   readonly [type: string]: readonly string[] | undefined;
 }
 
-const visitorKeys: VisitorKeys = eslintVisitorKeys.unionWith({
-  // Additional estree nodes.
-  Import: [],
+type GetNodeTypeKeys<T extends AST_NODE_TYPES> = Exclude<
+  keyof Extract<TSESTree.Node, { type: T }>,
+  'type' | 'loc' | 'range' | 'parent'
+>;
+
+// strictly type the arrays of keys provided to make sure we keep this config in sync with the type defs
+type AdditionalKeys = {
+  readonly [T in AST_NODE_TYPES]?: readonly GetNodeTypeKeys<T>[];
+};
+
+const additionalKeys: AdditionalKeys = {
   // ES2020
   ImportExpression: ['source'],
   // Additional Properties.
   ArrayPattern: ['decorators', 'elements', 'typeAnnotation'],
   ArrowFunctionExpression: ['typeParameters', 'params', 'returnType', 'body'],
+  AssignmentPattern: ['decorators', 'left', 'right', 'typeAnnotation'],
+  CallExpression: ['callee', 'typeParameters', 'arguments'],
   ClassDeclaration: [
     'decorators',
     'id',
@@ -30,15 +41,14 @@ const visitorKeys: VisitorKeys = eslintVisitorKeys.unionWith({
     'implements',
     'body',
   ],
-  TaggedTemplateExpression: ['tag', 'typeParameters', 'quasi'],
   FunctionDeclaration: ['id', 'typeParameters', 'params', 'returnType', 'body'],
   FunctionExpression: ['id', 'typeParameters', 'params', 'returnType', 'body'],
   Identifier: ['decorators', 'typeAnnotation'],
   MethodDefinition: ['decorators', 'key', 'value'],
+  NewExpression: ['callee', 'typeParameters', 'arguments'],
   ObjectPattern: ['decorators', 'properties', 'typeAnnotation'],
   RestElement: ['decorators', 'argument', 'typeAnnotation'],
-  NewExpression: ['callee', 'typeParameters', 'arguments'],
-  CallExpression: ['callee', 'typeParameters', 'arguments'],
+  TaggedTemplateExpression: ['tag', 'typeParameters', 'quasi'],
   // JSX
   JSXOpeningElement: ['name', 'typeParameters', 'attributes'],
   JSXClosingFragment: [],
@@ -126,6 +136,8 @@ const visitorKeys: VisitorKeys = eslintVisitorKeys.unionWith({
   TSUndefinedKeyword: [],
   TSUnknownKeyword: [],
   TSVoidKeyword: [],
-});
+} as const;
+
+const visitorKeys: VisitorKeys = eslintVisitorKeys.unionWith(additionalKeys);
 
 export { visitorKeys, VisitorKeys };