feat(typescript-estree): strict class heritage clauses check (#6576)

typescript-eslint · Mar 8, 2023 · 530185b · 530185b
1 parent 67e05c8
commit 530185b
Show file tree

Hide file tree

Showing 12 changed files with 103 additions and 44 deletions.
diff --git a/...basics/fixtures/_error_/class-with-implements-and-extends/snapshots/1-TSESTree-Error.shot b/...basics/fixtures/_error_/class-with-implements-and-extends/snapshots/1-TSESTree-Error.shot
@@ -1,3 +1,10 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`AST Fixtures legacy-fixtures basics _error_ class-with-implements-and-extends TSESTree - Error 1`] = `"NO ERROR"`;
+exports[`AST Fixtures legacy-fixtures basics _error_ class-with-implements-and-extends TSESTree - Error 1`] = `
+"TSError
+  1 | // TODO: This fixture might be too large, and if so should be split up.
+  2 |
+> 3 | class ClassWithParentAndInterface implements MyInterface extends MyOtherClass {}
+    |                                                          ^^^^^^^^^^^^^^^^^^^^ 'extends' clause must precede 'implements' clause.
+  4 |"
+`;
diff --git a/...asics/fixtures/_error_/class-with-implements-and-extends/snapshots/3-Alignment-Error.shot b/...asics/fixtures/_error_/class-with-implements-and-extends/snapshots/3-Alignment-Error.shot
@@ -1,3 +1,3 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`AST Fixtures legacy-fixtures basics _error_ class-with-implements-and-extends Error Alignment 1`] = `"Babel errored but TSESTree didn't"`;
+exports[`AST Fixtures legacy-fixtures basics _error_ class-with-implements-and-extends Error Alignment 1`] = `"Both errored"`;
diff --git a/...rRecovery/fixtures/_error_/class-empty-extends-implements/snapshots/1-TSESTree-Error.shot b/...rRecovery/fixtures/_error_/class-empty-extends-implements/snapshots/1-TSESTree-Error.shot
@@ -1,3 +1,12 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`AST Fixtures legacy-fixtures errorRecovery _error_ class-empty-extends-implements TSESTree - Error 1`] = `"NO ERROR"`;
+exports[`AST Fixtures legacy-fixtures errorRecovery _error_ class-empty-extends-implements TSESTree - Error 1`] = `
+"TSError
+  1 | // TODO: This fixture might be too large, and if so should be split up.
+  2 |
+> 3 | class Foo extends implements Bar {
+    |           ^^^^^^^ 'extends' list cannot be empty.
+  4 |
+  5 | }
+  6 |"
+`;
diff --git a/...Recovery/fixtures/_error_/class-empty-extends-implements/snapshots/3-Alignment-Error.shot b/...Recovery/fixtures/_error_/class-empty-extends-implements/snapshots/3-Alignment-Error.shot
@@ -1,3 +1,3 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`AST Fixtures legacy-fixtures errorRecovery _error_ class-empty-extends-implements Error Alignment 1`] = `"Babel errored but TSESTree didn't"`;
+exports[`AST Fixtures legacy-fixtures errorRecovery _error_ class-empty-extends-implements Error Alignment 1`] = `"Both errored"`;
diff --git a/...xtures/errorRecovery/fixtures/_error_/class-empty-extends/snapshots/1-TSESTree-Error.shot b/...xtures/errorRecovery/fixtures/_error_/class-empty-extends/snapshots/1-TSESTree-Error.shot
@@ -1,3 +1,12 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`AST Fixtures legacy-fixtures errorRecovery _error_ class-empty-extends TSESTree - Error 1`] = `"NO ERROR"`;
+exports[`AST Fixtures legacy-fixtures errorRecovery _error_ class-empty-extends TSESTree - Error 1`] = `
+"TSError
+  1 | // TODO: This fixture might be too large, and if so should be split up.
+  2 |
+> 3 | class Foo extends {
+    |           ^^^^^^^ 'extends' list cannot be empty.
+  4 |
+  5 | }
+  6 |"
+`;
diff --git a/...tures/errorRecovery/fixtures/_error_/class-empty-extends/snapshots/3-Alignment-Error.shot b/...tures/errorRecovery/fixtures/_error_/class-empty-extends/snapshots/3-Alignment-Error.shot
@@ -1,3 +1,3 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`AST Fixtures legacy-fixtures errorRecovery _error_ class-empty-extends Error Alignment 1`] = `"Babel errored but TSESTree didn't"`;
+exports[`AST Fixtures legacy-fixtures errorRecovery _error_ class-empty-extends Error Alignment 1`] = `"Both errored"`;
diff --git a/...rRecovery/fixtures/_error_/class-extends-empty-implements/snapshots/1-TSESTree-Error.shot b/...rRecovery/fixtures/_error_/class-extends-empty-implements/snapshots/1-TSESTree-Error.shot
@@ -1,3 +1,12 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`AST Fixtures legacy-fixtures errorRecovery _error_ class-extends-empty-implements TSESTree - Error 1`] = `"NO ERROR"`;
+exports[`AST Fixtures legacy-fixtures errorRecovery _error_ class-extends-empty-implements TSESTree - Error 1`] = `
+"TSError
+  1 | // TODO: This fixture might be too large, and if so should be split up.
+  2 |
+> 3 | class Foo extends Bar implements {
+    |                       ^^^^^^^^^^ 'implements' list cannot be empty.
+  4 |
+  5 | }
+  6 |"
+`;
diff --git a/...Recovery/fixtures/_error_/class-extends-empty-implements/snapshots/3-Alignment-Error.shot b/...Recovery/fixtures/_error_/class-extends-empty-implements/snapshots/3-Alignment-Error.shot
@@ -1,3 +1,3 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`AST Fixtures legacy-fixtures errorRecovery _error_ class-extends-empty-implements Error Alignment 1`] = `"Babel errored but TSESTree didn't"`;
+exports[`AST Fixtures legacy-fixtures errorRecovery _error_ class-extends-empty-implements Error Alignment 1`] = `"Both errored"`;
diff --git a/.../errorRecovery/fixtures/_error_/class-multiple-implements/snapshots/1-TSESTree-Error.shot b/.../errorRecovery/fixtures/_error_/class-multiple-implements/snapshots/1-TSESTree-Error.shot
@@ -1,3 +1,10 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`AST Fixtures legacy-fixtures errorRecovery _error_ class-multiple-implements TSESTree - Error 1`] = `"NO ERROR"`;
+exports[`AST Fixtures legacy-fixtures errorRecovery _error_ class-multiple-implements TSESTree - Error 1`] = `
+"TSError
+  1 | // TODO: This fixture might be too large, and if so should be split up.
+  2 |
+> 3 | class a implements b implements c {}
+    |                      ^^^^^^^^^^^^ 'implements' clause already seen.
+  4 |"
+`;
diff --git a/...errorRecovery/fixtures/_error_/class-multiple-implements/snapshots/3-Alignment-Error.shot b/...errorRecovery/fixtures/_error_/class-multiple-implements/snapshots/3-Alignment-Error.shot
@@ -1,3 +1,3 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`AST Fixtures legacy-fixtures errorRecovery _error_ class-multiple-implements Error Alignment 1`] = `"Babel errored but TSESTree didn't"`;
+exports[`AST Fixtures legacy-fixtures errorRecovery _error_ class-multiple-implements Error Alignment 1`] = `"Both errored"`;
diff --git a/packages/ast-spec/tests/fixtures-with-differences-errors.shot b/packages/ast-spec/tests/fixtures-with-differences-errors.shot
@@ -26,7 +26,6 @@ Object {
     "legacy-fixtures/basics/fixtures/_error_/await-without-async-function/fixture.ts",
     "legacy-fixtures/basics/fixtures/_error_/class-private-identifier-field-with-accessibility-error/fixture.ts",
     "legacy-fixtures/basics/fixtures/_error_/class-with-constructor-and-type-parameters/fixture.ts",
-    "legacy-fixtures/basics/fixtures/_error_/class-with-implements-and-extends/fixture.ts",
     "legacy-fixtures/basics/fixtures/_error_/class-with-static-parameter-properties/fixture.ts",
     "legacy-fixtures/basics/fixtures/_error_/class-with-two-methods-computed-constructor/fixture.ts",
     "legacy-fixtures/basics/fixtures/_error_/const-assertions/fixture.ts",
@@ -39,10 +38,6 @@ Object {
     "legacy-fixtures/basics/fixtures/_error_/interface-with-construct-signature-with-parameter-accessibility/fixture.ts",
     "legacy-fixtures/basics/fixtures/_error_/new-target-in-arrow-function-body/fixture.ts",
     "legacy-fixtures/basics/fixtures/_error_/var-with-definite-assignment/fixture.ts",
-    "legacy-fixtures/errorRecovery/fixtures/_error_/class-empty-extends-implements/fixture.ts",
-    "legacy-fixtures/errorRecovery/fixtures/_error_/class-empty-extends/fixture.ts",
-    "legacy-fixtures/errorRecovery/fixtures/_error_/class-extends-empty-implements/fixture.ts",
-    "legacy-fixtures/errorRecovery/fixtures/_error_/class-multiple-implements/fixture.ts",
     "legacy-fixtures/errorRecovery/fixtures/_error_/decorator-on-function/fixture.ts",
     "legacy-fixtures/errorRecovery/fixtures/_error_/empty-type-arguments-in-call-expression/fixture.ts",
     "legacy-fixtures/errorRecovery/fixtures/_error_/empty-type-arguments-in-new-expression/fixture.ts",

diff --git a/packages/typescript-estree/src/convert.ts b/packages/typescript-estree/src/convert.ts
@@ -1677,20 +1677,52 @@ export class Converter {
             ? AST_NODE_TYPES.ClassDeclaration
             : AST_NODE_TYPES.ClassExpression;
 
-        const superClass = heritageClauses.find(
-          clause => clause.token === SyntaxKind.ExtendsKeyword,
-        );
+        let extendsClause: ts.HeritageClause | undefined;
+        let implementsClause: ts.HeritageClause | undefined;
+        for (const heritageClause of heritageClauses) {
+          const { token, types } = heritageClause;
 
-        if (superClass && superClass.types.length > 1) {
-          this.#throwUnlessAllowInvalidAST(
-            superClass.types[1],
-            'Classes can only extend a single class.',
-          );
-        }
+          if (types.length === 0) {
+            this.#throwUnlessAllowInvalidAST(
+              heritageClause,
+              `'${ts.tokenToString(token)}' list cannot be empty.`,
+            );
+          }
 
-        const implementsClause = heritageClauses.find(
-          clause => clause.token === SyntaxKind.ImplementsKeyword,
-        );
+          if (token === SyntaxKind.ExtendsKeyword) {
+            if (extendsClause) {
+              this.#throwUnlessAllowInvalidAST(
+                heritageClause,
+                "'extends' clause already seen.",
+              );
+            }
+
+            if (implementsClause) {
+              this.#throwUnlessAllowInvalidAST(
+                heritageClause,
+                "'extends' clause must precede 'implements' clause.",
+              );
+            }
+
+            if (types.length > 1) {
+              this.#throwUnlessAllowInvalidAST(
+                types[1],
+                'Classes can only extend a single class.',
+              );
+            }
+
+            extendsClause ??= heritageClause;
+          } else if (token === SyntaxKind.ImplementsKeyword) {
+            if (implementsClause) {
+              this.#throwUnlessAllowInvalidAST(
+                heritageClause,
+                "'implements' clause already seen.",
+              );
+            }
+
+            implementsClause ??= heritageClause;
+          }
+        }
 
         const result = this.createNode<
           TSESTree.ClassDeclaration | TSESTree.ClassExpression
@@ -1710,8 +1742,8 @@ export class Converter {
           id: this.convertChild(node.name),
           implements:
             implementsClause?.types.map(el => this.convertChild(el)) ?? [],
-          superClass: superClass?.types[0]
-            ? this.convertChild(superClass.types[0].expression)
+          superClass: extendsClause?.types[0]
+            ? this.convertChild(extendsClause.types[0].expression)
             : null,
           superTypeArguments: undefined,
           superTypeParameters: undefined,
@@ -1722,22 +1754,13 @@ export class Converter {
             ),
         });
 
-        if (superClass) {
-          if (superClass.types.length > 1) {
-            this.#throwUnlessAllowInvalidAST(
-              superClass.types[1],
-              'Classes can only extend a single class.',
+        if (extendsClause?.types[0]?.typeArguments) {
+          // eslint-disable-next-line deprecation/deprecation
+          result.superTypeArguments = result.superTypeParameters =
+            this.convertTypeArgumentsToTypeParameterInstantiation(
+              extendsClause.types[0].typeArguments,
+              extendsClause.types[0],
             );
-          }
-
-          if (superClass.types[0]?.typeArguments) {
-            // eslint-disable-next-line deprecation/deprecation
-            result.superTypeArguments = result.superTypeParameters =
-              this.convertTypeArgumentsToTypeParameterInstantiation(
-                superClass.types[0].typeArguments,
-                superClass.types[0],
-              );
-          }
         }
 
         return this.fixExports(node, result);