From 1421c39d8ccf85b2fb7319834053705d1c18d6a9 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Thu, 6 Aug 2015 17:27:43 -0700 Subject: [PATCH 1/5] Fixed assignability bug --- src/compiler/checker.ts | 73 +++++++++++++++++++++++++++-------------- src/compiler/types.ts | 4 +-- 2 files changed, 51 insertions(+), 26 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 6a65743b6482f..25e763a727746 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -5090,30 +5090,21 @@ namespace ts { let result = Ternary.True; let saveErrorInfo = errorInfo; - // 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. - let sourceSig = sourceSignatures[0]; - // Note that in an extends-clause, targetSignatures is stripped, so the check never proceeds. - let targetSig = targetSignatures[0]; - - if (sourceSig && targetSig) { - let sourceErasedSignature = getErasedSignature(sourceSig); - let targetErasedSignature = getErasedSignature(targetSig); - - let sourceReturnType = sourceErasedSignature && getReturnTypeOfSignature(sourceErasedSignature); - let targetReturnType = targetErasedSignature && getReturnTypeOfSignature(targetErasedSignature); - - let sourceReturnDecl = sourceReturnType && sourceReturnType.symbol && getDeclarationOfKind(sourceReturnType.symbol, SyntaxKind.ClassDeclaration); - let targetReturnDecl = targetReturnType && targetReturnType.symbol && getDeclarationOfKind(targetReturnType.symbol, SyntaxKind.ClassDeclaration); - let sourceIsAbstract = sourceReturnDecl && sourceReturnDecl.flags & NodeFlags.Abstract; - let targetIsAbstract = targetReturnDecl && targetReturnDecl.flags & NodeFlags.Abstract; - - if (sourceIsAbstract && !targetIsAbstract) { - if (reportErrors) { - reportError(Diagnostics.Cannot_assign_an_abstract_constructor_type_to_a_non_abstract_constructor_type); - } - return Ternary.False; + + + 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. + let sourceSig = sourceSignatures[0]; + // Note that in an extends-clause, targetSignatures is stripped, so the check never proceeds. + let targetSig = targetSignatures[0]; + + result &= abstractSignatureRelatedTo(source, sourceSig, target, targetSig); + if (result !== Ternary.True) { + return result; } } @@ -5137,6 +5128,40 @@ namespace ts { } } return result; + + function abstractSignatureRelatedTo(source: Type, sourceSig: Signature, target: Type, targetSig: Signature) { + if (sourceSig && targetSig) { + + let sourceDecl = getDeclarationOfKind(source.symbol, SyntaxKind.ClassDeclaration); + let targetDecl = getDeclarationOfKind(target.symbol, SyntaxKind.ClassDeclaration); + + 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; + } + + let sourceErasedSignature = getErasedSignature(sourceSig); + let targetErasedSignature = getErasedSignature(targetSig); + + let sourceReturnType = sourceErasedSignature && getReturnTypeOfSignature(sourceErasedSignature); + let targetReturnType = targetErasedSignature && getReturnTypeOfSignature(targetErasedSignature); + + let sourceReturnDecl = sourceReturnType && sourceReturnType.symbol && getDeclarationOfKind(sourceReturnType.symbol, SyntaxKind.ClassDeclaration); + let targetReturnDecl = targetReturnType && targetReturnType.symbol && getDeclarationOfKind(targetReturnType.symbol, SyntaxKind.ClassDeclaration); + let sourceIsAbstract = sourceReturnDecl && sourceReturnDecl.flags & NodeFlags.Abstract; + let targetIsAbstract = targetReturnDecl && targetReturnDecl.flags & NodeFlags.Abstract; + + // abstract constructor functions are only + if (sourceIsAbstract && !(targetIsAbstract && targetDecl)) { + 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 { diff --git a/src/compiler/types.ts b/src/compiler/types.ts index da43984ee1377..d19ed62332ec4 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -1786,10 +1786,10 @@ namespace ts { ObjectType = Class | Interface | Reference | Tuple | Anonymous, UnionOrIntersection = Union | Intersection, StructuredType = ObjectType | Union | Intersection, - /* @internal */ + /* @internal */ RequiresWidening = ContainsUndefinedOrNull | ContainsObjectLiteral, /* @internal */ - PropagatingFlags = ContainsUndefinedOrNull | ContainsObjectLiteral | ContainsAnyFunctionType + PropagatingFlags = ContainsUndefinedOrNull | ContainsObjectLiteral | ContainsAnyFunctionType } // Properties common to all types From a693e82e27c729c37f58d90a862abccdeda7451b Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Thu, 6 Aug 2015 17:27:52 -0700 Subject: [PATCH 2/5] Added test --- .../classAbstractAssignabilityConstructorFunction.ts | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractAssignabilityConstructorFunction.ts diff --git a/tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractAssignabilityConstructorFunction.ts b/tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractAssignabilityConstructorFunction.ts new file mode 100644 index 0000000000000..4a6f6cc3d610a --- /dev/null +++ b/tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractAssignabilityConstructorFunction.ts @@ -0,0 +1,8 @@ +abstract class A { } + +// var AA: typeof A; +var AAA: new() => A; + +// AA = A; // okay +AAA = A; // error. +AAA = "asdf"; \ No newline at end of file From e8497d3d8a78a37c883c3df826648656962abbdd Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Thu, 6 Aug 2015 17:28:16 -0700 Subject: [PATCH 3/5] Updated baselines --- ...ssignabilityConstructorFunction.errors.txt | 19 +++++++++++++++++ ...bstractAssignabilityConstructorFunction.js | 21 +++++++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 tests/baselines/reference/classAbstractAssignabilityConstructorFunction.errors.txt create mode 100644 tests/baselines/reference/classAbstractAssignabilityConstructorFunction.js diff --git a/tests/baselines/reference/classAbstractAssignabilityConstructorFunction.errors.txt b/tests/baselines/reference/classAbstractAssignabilityConstructorFunction.errors.txt new file mode 100644 index 0000000000000..c6c242396d8d8 --- /dev/null +++ b/tests/baselines/reference/classAbstractAssignabilityConstructorFunction.errors.txt @@ -0,0 +1,19 @@ +tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractAssignabilityConstructorFunction.ts(7,1): error TS2322: Type 'typeof A' is not assignable to type 'new () => A'. + Cannot assign an abstract constructor type to a non-abstract constructor type. +tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractAssignabilityConstructorFunction.ts(8,1): error TS2322: Type 'string' is not assignable to type 'new () => A'. + + +==== tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractAssignabilityConstructorFunction.ts (2 errors) ==== + abstract class A { } + + // var AA: typeof A; + var AAA: new() => A; + + // AA = A; // okay + AAA = A; // error. + ~~~ +!!! error TS2322: Type 'typeof A' is not assignable to type 'new () => A'. +!!! error TS2322: Cannot assign an abstract constructor type to a non-abstract constructor type. + AAA = "asdf"; + ~~~ +!!! error TS2322: Type 'string' is not assignable to type 'new () => A'. \ No newline at end of file diff --git a/tests/baselines/reference/classAbstractAssignabilityConstructorFunction.js b/tests/baselines/reference/classAbstractAssignabilityConstructorFunction.js new file mode 100644 index 0000000000000..3d8fd404afc38 --- /dev/null +++ b/tests/baselines/reference/classAbstractAssignabilityConstructorFunction.js @@ -0,0 +1,21 @@ +//// [classAbstractAssignabilityConstructorFunction.ts] +abstract class A { } + +// var AA: typeof A; +var AAA: new() => A; + +// AA = A; // okay +AAA = A; // error. +AAA = "asdf"; + +//// [classAbstractAssignabilityConstructorFunction.js] +var A = (function () { + function A() { + } + return A; +})(); +// var AA: typeof A; +var AAA; +// AA = A; // okay +AAA = A; // error. +AAA = "asdf"; From cbb159770f1a60a718424df86f891559511f1904 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Thu, 6 Aug 2015 17:43:06 -0700 Subject: [PATCH 4/5] fixed null de-reference --- src/compiler/checker.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 25e763a727746..77aebe9118a93 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -5132,8 +5132,8 @@ namespace ts { function abstractSignatureRelatedTo(source: Type, sourceSig: Signature, target: Type, targetSig: Signature) { if (sourceSig && targetSig) { - let sourceDecl = getDeclarationOfKind(source.symbol, SyntaxKind.ClassDeclaration); - let targetDecl = getDeclarationOfKind(target.symbol, SyntaxKind.ClassDeclaration); + let sourceDecl = source.symbol && getDeclarationOfKind(source.symbol, SyntaxKind.ClassDeclaration); + let targetDecl = target.symbol && getDeclarationOfKind(target.symbol, SyntaxKind.ClassDeclaration); if (!sourceDecl) { // If the source object isn't itself a class declaration, it can be freely assigned, regardless @@ -5152,8 +5152,8 @@ namespace ts { let sourceIsAbstract = sourceReturnDecl && sourceReturnDecl.flags & NodeFlags.Abstract; let targetIsAbstract = targetReturnDecl && targetReturnDecl.flags & NodeFlags.Abstract; - // abstract constructor functions are only 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); } From 410e4e4df7b5698312e323a63477c429cf4919d6 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Fri, 7 Aug 2015 11:07:06 -0700 Subject: [PATCH 5/5] added a clarifying comment --- src/compiler/checker.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 77aebe9118a93..7f09917ea1ecc 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -3474,8 +3474,10 @@ namespace ts { return emptyArray; } - // Return the signatures of the given kind in the given type. Creates synthetic union signatures when necessary and - // maps primitive types and type parameters are to their apparent types. + /** + * Return the signatures of the given kind in the given type. Creates synthetic union signatures when necessary and + * maps primitive types and type parameters are to their apparent types. + */ function getSignaturesOfType(type: Type, kind: SignatureKind): Signature[] { return getSignaturesOfStructuredType(getApparentType(type), kind); } @@ -5098,8 +5100,11 @@ namespace ts { // 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. - let sourceSig = sourceSignatures[0]; + // + // sourceSig and targetSig are (possibly) undefined. + // // Note that in an extends-clause, targetSignatures is stripped, so the check never proceeds. + let sourceSig = sourceSignatures[0]; let targetSig = targetSignatures[0]; result &= abstractSignatureRelatedTo(source, sourceSig, target, targetSig);