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.

Sign up for GitHub

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

Jump to bottom

Fixed inconsistent abstract constructor assignability and instantiation checks #60000

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fixed inconsistent abstract constructor assignability and instantiation checks #60000

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
123c274
Implement more consistent assignability for abstract constructors
bgenia Sep 17, 2024
a174a7e
Fix top signatures being checked before abstract assignability
bgenia Sep 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 25 additions & 23 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21374,6 +21374,20 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return Ternary.True;
}

const sourceIsAbstract = !!(source.flags & SignatureFlags.Abstract);
const targetIsAbstract = !!(target.flags & SignatureFlags.Abstract);

// An abstract constructor type is not assignable to a non-abstract constructor type
// as it would otherwise be possible to new an abstract class. Note that the assignability
// check we perform for an extends clause excludes construct signatures from the target,
// so this check never proceeds.
if (sourceIsAbstract && !targetIsAbstract) {
if (reportErrors) {
errorReporter!(Diagnostics.Cannot_assign_an_abstract_constructor_type_to_a_non_abstract_constructor_type);
}
return Ternary.False;
}

if (!(checkMode & SignatureCheckMode.StrictTopSignature && isTopSignature(source)) && isTopSignature(target)) {
return Ternary.True;
}
Expand Down Expand Up @@ -24044,18 +24058,6 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
);

if (kind === SignatureKind.Construct && sourceSignatures.length && targetSignatures.length) {
const sourceIsAbstract = !!(sourceSignatures[0].flags & SignatureFlags.Abstract);
const targetIsAbstract = !!(targetSignatures[0].flags & SignatureFlags.Abstract);
if (sourceIsAbstract && !targetIsAbstract) {
// An abstract constructor type is not assignable to a non-abstract constructor type
// as it would otherwise be possible to new an abstract class. Note that the assignability
// check we perform for an extends clause excludes construct signatures from the target,
// so this check never proceeds.
if (reportErrors) {
reportError(Diagnostics.Cannot_assign_an_abstract_constructor_type_to_a_non_abstract_constructor_type);
}
return Ternary.False;
}
if (!constructorVisibilitiesAreCompatible(sourceSignatures[0], targetSignatures[0], reportErrors)) {
return Ternary.False;
}
Expand Down Expand Up @@ -36429,17 +36431,24 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
// then it cannot be instantiated.
// In the case of a merged class-module or class-interface declaration,
// only the class declaration node will have the Abstract flag set.
if (someSignature(constructSignatures, signature => !!(signature.flags & SignatureFlags.Abstract))) {
const valueDecl = expressionType.symbol && getClassLikeDeclarationOfSymbol(expressionType.symbol);
if (valueDecl && hasSyntacticModifier(valueDecl, ModifierFlags.Abstract)) {
error(node, Diagnostics.Cannot_create_an_instance_of_an_abstract_class);
return resolveErrorCall(node);
}
const valueDecl = expressionType.symbol && getClassLikeDeclarationOfSymbol(expressionType.symbol);
if (valueDecl && hasSyntacticModifier(valueDecl, ModifierFlags.Abstract)) {

const resolvedSignature = resolveCall(node, constructSignatures, candidatesOutArray, checkMode, SignatureFlags.None);

// Composite signature check is done to prevent instantiating unions of mixed concrete/asbtract signatures.
if (
(resolvedSignature.flags & SignatureFlags.Abstract)
|| (resolvedSignature.compositeSignatures?.some(signature => signature.flags & SignatureFlags.Abstract))
) {
error(node, Diagnostics.Cannot_create_an_instance_of_an_abstract_class);
return resolveErrorCall(node);
}

return resolveCall(node, constructSignatures, candidatesOutArray, checkMode, SignatureFlags.None);
return resolvedSignature;
}

// If expressionType's apparent type is an object type with no construct signatures but
Expand All @@ -36464,13 +36473,6 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return resolveErrorCall(node);
}

function someSignature(signatures: Signature | readonly Signature[], f: (s: Signature) => boolean): boolean {
if (isArray(signatures)) {
return some(signatures, signature => someSignature(signature, f));
}
return signatures.compositeKind === TypeFlags.Union ? some(signatures.compositeSignatures, f) : f(signatures);
}

function typeHasProtectedAccessibleBase(target: Symbol, type: InterfaceType): boolean {
const baseTypes = getBaseTypes(type);
if (!length(baseTypes)) {
Expand Down
31 changes: 31 additions & 0 deletions tests/baselines/reference/abstractConstructorAssignability.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
abstractConstructorAssignability.ts(7,1): error TS2322: Type 'AbstractConstructor' is not assignable to type 'ConcreteConstructor'.
Cannot assign an abstract constructor type to a non-abstract constructor type.
abstractConstructorAssignability.ts(16,1): error TS2322: Type 'typeof AbstractClass' is not assignable to type 'typeof ConcreteClass'.
Cannot assign an abstract constructor type to a non-abstract constructor type.


==== abstractConstructorAssignability.ts (2 errors) ====
type ConcreteConstructor = new () => void;
type AbstractConstructor = abstract new () => void;

declare let concreteConstructor: ConcreteConstructor;
declare let abstractConstructor: AbstractConstructor;

concreteConstructor = abstractConstructor; // should error
~~~~~~~~~~~~~~~~~~~
!!! error TS2322: Type 'AbstractConstructor' is not assignable to type 'ConcreteConstructor'.
!!! error TS2322: Cannot assign an abstract constructor type to a non-abstract constructor type.
abstractConstructor = concreteConstructor; // should work

class ConcreteClass {}
abstract class AbstractClass {}

declare let concreteClass: typeof ConcreteClass
declare let abstractClass: typeof AbstractClass

concreteClass = abstractClass; // should error
~~~~~~~~~~~~~
!!! error TS2322: Type 'typeof AbstractClass' is not assignable to type 'typeof ConcreteClass'.
!!! error TS2322: Cannot assign an abstract constructor type to a non-abstract constructor type.
abstractClass = concreteClass; // should work

37 changes: 37 additions & 0 deletions tests/baselines/reference/abstractConstructorAssignability.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//// [tests/cases/compiler/abstractConstructorAssignability.ts] ////

//// [abstractConstructorAssignability.ts]
type ConcreteConstructor = new () => void;
type AbstractConstructor = abstract new () => void;

declare let concreteConstructor: ConcreteConstructor;
declare let abstractConstructor: AbstractConstructor;

concreteConstructor = abstractConstructor; // should error
abstractConstructor = concreteConstructor; // should work

class ConcreteClass {}
abstract class AbstractClass {}

declare let concreteClass: typeof ConcreteClass
declare let abstractClass: typeof AbstractClass

concreteClass = abstractClass; // should error
abstractClass = concreteClass; // should work


//// [abstractConstructorAssignability.js]
concreteConstructor = abstractConstructor; // should error
abstractConstructor = concreteConstructor; // should work
var ConcreteClass = /** @class */ (function () {
function ConcreteClass() {
}
return ConcreteClass;
}());
var AbstractClass = /** @class */ (function () {
function AbstractClass() {
}
return AbstractClass;
}());
concreteClass = abstractClass; // should error
abstractClass = concreteClass; // should work
47 changes: 47 additions & 0 deletions tests/baselines/reference/abstractConstructorAssignability.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
//// [tests/cases/compiler/abstractConstructorAssignability.ts] ////

=== abstractConstructorAssignability.ts ===
type ConcreteConstructor = new () => void;
>ConcreteConstructor : Symbol(ConcreteConstructor, Decl(abstractConstructorAssignability.ts, 0, 0))

type AbstractConstructor = abstract new () => void;
>AbstractConstructor : Symbol(AbstractConstructor, Decl(abstractConstructorAssignability.ts, 0, 42))

declare let concreteConstructor: ConcreteConstructor;
>concreteConstructor : Symbol(concreteConstructor, Decl(abstractConstructorAssignability.ts, 3, 11))
>ConcreteConstructor : Symbol(ConcreteConstructor, Decl(abstractConstructorAssignability.ts, 0, 0))

declare let abstractConstructor: AbstractConstructor;
>abstractConstructor : Symbol(abstractConstructor, Decl(abstractConstructorAssignability.ts, 4, 11))
>AbstractConstructor : Symbol(AbstractConstructor, Decl(abstractConstructorAssignability.ts, 0, 42))

concreteConstructor = abstractConstructor; // should error
>concreteConstructor : Symbol(concreteConstructor, Decl(abstractConstructorAssignability.ts, 3, 11))
>abstractConstructor : Symbol(abstractConstructor, Decl(abstractConstructorAssignability.ts, 4, 11))

abstractConstructor = concreteConstructor; // should work
>abstractConstructor : Symbol(abstractConstructor, Decl(abstractConstructorAssignability.ts, 4, 11))
>concreteConstructor : Symbol(concreteConstructor, Decl(abstractConstructorAssignability.ts, 3, 11))

class ConcreteClass {}
>ConcreteClass : Symbol(ConcreteClass, Decl(abstractConstructorAssignability.ts, 7, 42))

abstract class AbstractClass {}
>AbstractClass : Symbol(AbstractClass, Decl(abstractConstructorAssignability.ts, 9, 22))

declare let concreteClass: typeof ConcreteClass
>concreteClass : Symbol(concreteClass, Decl(abstractConstructorAssignability.ts, 12, 11))
>ConcreteClass : Symbol(ConcreteClass, Decl(abstractConstructorAssignability.ts, 7, 42))

declare let abstractClass: typeof AbstractClass
>abstractClass : Symbol(abstractClass, Decl(abstractConstructorAssignability.ts, 13, 11))
>AbstractClass : Symbol(AbstractClass, Decl(abstractConstructorAssignability.ts, 9, 22))

concreteClass = abstractClass; // should error
>concreteClass : Symbol(concreteClass, Decl(abstractConstructorAssignability.ts, 12, 11))
>abstractClass : Symbol(abstractClass, Decl(abstractConstructorAssignability.ts, 13, 11))

abstractClass = concreteClass; // should work
>abstractClass : Symbol(abstractClass, Decl(abstractConstructorAssignability.ts, 13, 11))
>concreteClass : Symbol(concreteClass, Decl(abstractConstructorAssignability.ts, 12, 11))

71 changes: 71 additions & 0 deletions tests/baselines/reference/abstractConstructorAssignability.types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
//// [tests/cases/compiler/abstractConstructorAssignability.ts] ////

=== abstractConstructorAssignability.ts ===
type ConcreteConstructor = new () => void;
>ConcreteConstructor : ConcreteConstructor
> : ^^^^^^^^^^^^^^^^^^^

type AbstractConstructor = abstract new () => void;
>AbstractConstructor : AbstractConstructor
> : ^^^^^^^^^^^^^^^^^^^

declare let concreteConstructor: ConcreteConstructor;
>concreteConstructor : ConcreteConstructor
> : ^^^^^^^^^^^^^^^^^^^

declare let abstractConstructor: AbstractConstructor;
>abstractConstructor : AbstractConstructor
> : ^^^^^^^^^^^^^^^^^^^

concreteConstructor = abstractConstructor; // should error
>concreteConstructor = abstractConstructor : AbstractConstructor
> : ^^^^^^^^^^^^^^^^^^^
>concreteConstructor : ConcreteConstructor
> : ^^^^^^^^^^^^^^^^^^^
>abstractConstructor : AbstractConstructor
> : ^^^^^^^^^^^^^^^^^^^

abstractConstructor = concreteConstructor; // should work
>abstractConstructor = concreteConstructor : ConcreteConstructor
> : ^^^^^^^^^^^^^^^^^^^
>abstractConstructor : AbstractConstructor
> : ^^^^^^^^^^^^^^^^^^^
>concreteConstructor : ConcreteConstructor
> : ^^^^^^^^^^^^^^^^^^^

class ConcreteClass {}
>ConcreteClass : ConcreteClass
> : ^^^^^^^^^^^^^

abstract class AbstractClass {}
>AbstractClass : AbstractClass
> : ^^^^^^^^^^^^^

declare let concreteClass: typeof ConcreteClass
>concreteClass : typeof ConcreteClass
> : ^^^^^^^^^^^^^^^^^^^^
>ConcreteClass : typeof ConcreteClass
> : ^^^^^^^^^^^^^^^^^^^^

declare let abstractClass: typeof AbstractClass
>abstractClass : typeof AbstractClass
> : ^^^^^^^^^^^^^^^^^^^^
>AbstractClass : typeof AbstractClass
> : ^^^^^^^^^^^^^^^^^^^^

concreteClass = abstractClass; // should error
>concreteClass = abstractClass : typeof AbstractClass
> : ^^^^^^^^^^^^^^^^^^^^
>concreteClass : typeof ConcreteClass
> : ^^^^^^^^^^^^^^^^^^^^
>abstractClass : typeof AbstractClass
> : ^^^^^^^^^^^^^^^^^^^^

abstractClass = concreteClass; // should work
>abstractClass = concreteClass : typeof ConcreteClass
> : ^^^^^^^^^^^^^^^^^^^^
>abstractClass : typeof AbstractClass
> : ^^^^^^^^^^^^^^^^^^^^
>concreteClass : typeof ConcreteClass
> : ^^^^^^^^^^^^^^^^^^^^

60 changes: 60 additions & 0 deletions tests/baselines/reference/abstractConstructorOverloadAssignability.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
//// [tests/cases/compiler/abstractConstructorOverloadAssignability.ts] ////

//// [abstractConstructorOverloadAssignability.ts]
type AbstractConstructor = abstract new (arg: "abstract") => "abstract";
type ConcreteConstructor = new (arg: "concrete") => "concrete";

type MixedConstructorAbstractFirst =
& AbstractConstructor
& ConcreteConstructor;

type MixedConstructorAbstractLast =
& ConcreteConstructor
& AbstractConstructor;

declare let mixedConstructorAbstractFirst: MixedConstructorAbstractFirst;
declare let mixedConstructorAbstractLast: MixedConstructorAbstractLast;

mixedConstructorAbstractFirst = mixedConstructorAbstractLast; // should work
mixedConstructorAbstractLast = mixedConstructorAbstractFirst; // should work

interface MixedConstructorInterface1 extends AbstractConstructor { // should work
new (arg: "concrete"): "concrete";
}

interface MixedConstructorInterface2 extends AbstractConstructor, ConcreteConstructor { // should work

}

declare let mixedConstructorInterface1: MixedConstructorInterface1;
declare let mixedConstructorInterface2: MixedConstructorInterface2;

mixedConstructorInterface2 = mixedConstructorInterface1; // should work
mixedConstructorInterface1 = mixedConstructorInterface2; // should work

mixedConstructorAbstractFirst = mixedConstructorInterface1; // should work
mixedConstructorInterface1 = mixedConstructorAbstractFirst; // should work

mixedConstructorAbstractLast = mixedConstructorInterface1; // should work
mixedConstructorInterface1 = mixedConstructorAbstractLast; // should work

mixedConstructorAbstractFirst = mixedConstructorInterface2; // should work
mixedConstructorInterface2 = mixedConstructorAbstractFirst; // should work

mixedConstructorAbstractLast = mixedConstructorInterface2; // should work
mixedConstructorInterface2 = mixedConstructorAbstractLast; // should work


//// [abstractConstructorOverloadAssignability.js]
mixedConstructorAbstractFirst = mixedConstructorAbstractLast; // should work
mixedConstructorAbstractLast = mixedConstructorAbstractFirst; // should work
mixedConstructorInterface2 = mixedConstructorInterface1; // should work
mixedConstructorInterface1 = mixedConstructorInterface2; // should work
mixedConstructorAbstractFirst = mixedConstructorInterface1; // should work
mixedConstructorInterface1 = mixedConstructorAbstractFirst; // should work
mixedConstructorAbstractLast = mixedConstructorInterface1; // should work
mixedConstructorInterface1 = mixedConstructorAbstractLast; // should work
mixedConstructorAbstractFirst = mixedConstructorInterface2; // should work
mixedConstructorInterface2 = mixedConstructorAbstractFirst; // should work
mixedConstructorAbstractLast = mixedConstructorInterface2; // should work
mixedConstructorInterface2 = mixedConstructorAbstractLast; // should work
Loading