Skip to content

Simplify abstract constructor type assignability checking #6111

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

Merged
merged 2 commits into from
Dec 19, 2015
Merged

Simplify abstract constructor type assignability checking #6111

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fd52e9a
Simplify abstract constructor type assignability checking
ahejlsberg Dec 15, 2015
c21193a
Removing unused function
ahejlsberg Dec 16, 2015
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
96 changes: 27 additions & 69 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5528,32 +5528,24 @@ namespace ts {
if (target === anyFunctionType || source === anyFunctionType) {
return Ternary.True;
}

const sourceSignatures = getSignaturesOfType(source, kind);
const targetSignatures = getSignaturesOfType(target, kind);
let result = Ternary.True;
const saveErrorInfo = errorInfo;



if (kind === SignatureKind.Construct) {
// Only want to compare the construct signatures for abstractness guarantees.

// Because the "abstractness" of a class is the same across all construct signatures
// (internally we are checking the corresponding declaration), it is enough to perform
// the check and report an error once over all pairs of source and target construct signatures.
//
// sourceSig and targetSig are (possibly) undefined.
//
// Note that in an extends-clause, targetSignatures is stripped, so the check never proceeds.
const sourceSig = sourceSignatures[0];
const targetSig = targetSignatures[0];

result &= abstractSignatureRelatedTo(source, sourceSig, target, targetSig);
if (result !== Ternary.True) {
return result;
if (kind === SignatureKind.Construct && sourceSignatures.length && targetSignatures.length &&
isAbstractConstructorType(source) && !isAbstractConstructorType(target)) {
// 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 assignablity
// 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;
}

let result = Ternary.True;
const saveErrorInfo = errorInfo;

outer: for (const t of targetSignatures) {
if (!t.hasStringLiterals || target.flags & TypeFlags.FromSignature) {
// Only elaborate errors from the first failure
Expand All @@ -5580,40 +5572,6 @@ namespace ts {
}
}
return result;

function abstractSignatureRelatedTo(source: Type, sourceSig: Signature, target: Type, targetSig: Signature) {
if (sourceSig && targetSig) {

const sourceDecl = source.symbol && getClassLikeDeclarationOfSymbol(source.symbol);
const targetDecl = target.symbol && getClassLikeDeclarationOfSymbol(target.symbol);

if (!sourceDecl) {
// If the source object isn't itself a class declaration, it can be freely assigned, regardless
// of whether the constructed object is abstract or not.
return Ternary.True;
}

const sourceErasedSignature = getErasedSignature(sourceSig);
const targetErasedSignature = getErasedSignature(targetSig);

const sourceReturnType = sourceErasedSignature && getReturnTypeOfSignature(sourceErasedSignature);
const targetReturnType = targetErasedSignature && getReturnTypeOfSignature(targetErasedSignature);

const sourceReturnDecl = sourceReturnType && sourceReturnType.symbol && getClassLikeDeclarationOfSymbol(sourceReturnType.symbol);
const targetReturnDecl = targetReturnType && targetReturnType.symbol && getClassLikeDeclarationOfSymbol(targetReturnType.symbol);
const sourceIsAbstract = sourceReturnDecl && sourceReturnDecl.flags & NodeFlags.Abstract;
const targetIsAbstract = targetReturnDecl && targetReturnDecl.flags & NodeFlags.Abstract;

if (sourceIsAbstract && !(targetIsAbstract && targetDecl)) {
// if target isn't a class-declaration type, then it can be new'd, so we forbid the assignment.
if (reportErrors) {
reportError(Diagnostics.Cannot_assign_an_abstract_constructor_type_to_a_non_abstract_constructor_type);
}
return Ternary.False;
}
}
return Ternary.True;
}
}

function signatureRelatedTo(source: Signature, target: Signature, reportErrors: boolean): Ternary {
Expand Down Expand Up @@ -5782,6 +5740,20 @@ namespace ts {
}
}

// Return true if the given type is the constructor type for an abstract class
function isAbstractConstructorType(type: Type) {
if (type.flags & TypeFlags.Anonymous) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate why this needs to be anonymous?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the static side of a class is an anonymous object type.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add that in as a quick comment then? It's not entirely obvious at first glance.

const symbol = type.symbol;
if (symbol && symbol.flags & SymbolFlags.Class) {
const declaration = getClassLikeDeclarationOfSymbol(symbol);
if (declaration && declaration.flags & NodeFlags.Abstract) {
return true;
}
}
}
return false;
}

// Return true if the given type is part of a deeply nested chain of generic instantiations. We consider this to be the case
// when structural type comparisons have been started for 10 or more instantiations of the same generic type. It is possible,
// though highly unlikely, for this test to be true in a situation where a chain of instantiations is not infinitely expanding.
Expand Down Expand Up @@ -15396,20 +15368,6 @@ namespace ts {
return symbol && getExportSymbolOfValueSymbolIfExported(symbol).valueDeclaration;
}

function instantiateSingleCallFunctionType(functionType: Type, typeArguments: Type[]): Type {
if (functionType === unknownType) {
return unknownType;
}

const signature = getSingleCallSignature(functionType);
if (!signature) {
return unknownType;
}

const instantiatedSignature = getSignatureInstantiation(signature, typeArguments);
return getOrCreateTypeFromSignature(instantiatedSignature);
}

function createResolver(): EmitResolver {
return {
getReferencedExportContainer,
Expand Down