Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from Mar 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -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 |"
`;
@@ -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"`;
@@ -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 |"
`;
@@ -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"`;
@@ -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 |"
`;
@@ -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"`;
@@ -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 |"
`;
@@ -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"`;
@@ -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 |"
`;
@@ -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"`;
5 changes: 0 additions & 5 deletions packages/ast-spec/tests/fixtures-with-differences-errors.shot
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
81 changes: 52 additions & 29 deletions packages/typescript-estree/src/convert.ts
Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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);
Expand Down