From db88c8eac29aba048403e2a8a7ab868c79b99b00 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Wed, 1 Nov 2017 09:12:01 -0700 Subject: [PATCH 1/3] Check function type parameters for this references Previously, when checking for this references, the compiler did not check type parameters. This caused it to miss some instantiations that were necessary. Also some cleanup: 1. Rename isIndependent* to is*FreeOfThisReference. 'Independent' was a one-off concept that was unique to Typescript (and isn't referenced in the spec), so I think the new name is friendlier to new readers. 2. Use `Array.every` whenever possible, since Typescript added a dependency on ES5 since the code was first written. 3. Switch to JSDoc so that it shows up nicely in editors. --- src/compiler/checker.ts | 101 +++++++++++++++++++++------------------- 1 file changed, 52 insertions(+), 49 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 09354d957a3..ccb7c869137 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -5108,10 +5108,14 @@ namespace ts { } } - // Returns true if the interface given by the symbol is free of "this" references. Specifically, the result is - // true if the interface itself contains no references to "this" in its body, if all base types are interfaces, - // and if none of the base interfaces have a "this" type. - function isIndependentInterface(symbol: Symbol): boolean { + /** + * Returns true if the interface given by the symbol is free of "this" references. + * + * Specifically, the result is true if the interface itself contains no references + * to "this" in its body, if all base types are interfaces, + * and if none of the base interfaces have a "this" type. + */ + function isInterfaceFreeOfThisReference(symbol: Symbol): boolean { for (const declaration of symbol.declarations) { if (declaration.kind === SyntaxKind.InterfaceDeclaration) { if (declaration.flags & NodeFlags.ContainsThis) { @@ -5145,7 +5149,7 @@ namespace ts { // property types inferred from initializers and method return types inferred from return statements are very hard // to exhaustively analyze). We give interfaces a "this" type if we can't definitely determine that they are free of // "this" references. - if (outerTypeParameters || localTypeParameters || kind === ObjectFlags.Class || !isIndependentInterface(symbol)) { + if (outerTypeParameters || localTypeParameters || kind === ObjectFlags.Class || !isInterfaceFreeOfThisReference(symbol)) { type.objectFlags |= ObjectFlags.Reference; type.typeParameters = concatenate(outerTypeParameters, localTypeParameters); type.outerTypeParameters = outerTypeParameters; @@ -5327,22 +5331,17 @@ namespace ts { return undefined; } - // A type reference is considered independent if each type argument is considered independent. - function isIndependentTypeReference(node: TypeReferenceNode): boolean { - if (node.typeArguments) { - for (const typeNode of node.typeArguments) { - if (!isIndependentType(typeNode)) { - return false; - } - } - } - return true; + /** A type reference is free of this references if each type argument is free of this references. */ + function isTypeReferenceFreeOfThisReference(node: TypeReferenceNode): boolean { + return !node.typeArguments || node.typeArguments.every(isTypeFreeOfThisReference); } - // A type is considered independent if it the any, string, number, boolean, symbol, or void keyword, a string - // literal type, an array with an element type that is considered independent, or a type reference that is - // considered independent. - function isIndependentType(node: TypeNode): boolean { + /** + * A type is free of this references if it's the any, string, number, boolean, symbol, or void keyword, a string + * literal type, an array with an element type that is free of this references, or a type reference that is + * free of this references. + */ + function isTypeFreeOfThisReference(node: TypeNode): boolean { switch (node.kind) { case SyntaxKind.AnyKeyword: case SyntaxKind.StringKeyword: @@ -5357,54 +5356,58 @@ namespace ts { case SyntaxKind.LiteralType: return true; case SyntaxKind.ArrayType: - return isIndependentType((node).elementType); + return isTypeFreeOfThisReference((node).elementType); case SyntaxKind.TypeReference: - return isIndependentTypeReference(node); + return isTypeReferenceFreeOfThisReference(node); } return false; } - // A variable-like declaration is considered independent (free of this references) if it has a type annotation - // that specifies an independent type, or if it has no type annotation and no initializer (and thus of type any). - function isIndependentVariableLikeDeclaration(node: VariableLikeDeclaration): boolean { + /** A type parameter is this-free if its contraint is this-free, or if it has no constraint. */ + function isTypeParameterFreeOfThisReference(node: TypeParameterDeclaration) { + return !node.constraint || isTypeFreeOfThisReference(node.constraint); + } + + /** + * A variable-like declaration is free of this references if it has a type annotation + * that is this-free, or if it has no type annotation and no initializer (and is thus of type any). + */ + function isVariableLikeDeclarationFreeOfThisReference(node: VariableLikeDeclaration): boolean { const typeNode = getEffectiveTypeAnnotationNode(node); - return typeNode ? isIndependentType(typeNode) : !node.initializer; + return typeNode ? isTypeFreeOfThisReference(typeNode) : !node.initializer; } - // A function-like declaration is considered independent (free of this references) if it has a return type - // annotation that is considered independent and if each parameter is considered independent. - function isIndependentFunctionLikeDeclaration(node: FunctionLikeDeclaration): boolean { - if (node.kind !== SyntaxKind.Constructor) { - const typeNode = getEffectiveReturnTypeNode(node); - if (!typeNode || !isIndependentType(typeNode)) { - return false; - } - } - for (const parameter of node.parameters) { - if (!isIndependentVariableLikeDeclaration(parameter)) { - return false; - } - } - return true; + /** + * A function-like declaration is considered free of `this` references if it has a return type + * annotation that is free of this references and if each parameter is this-free and if + * each type parameter (if present) is this-free. + */ + function isFunctionLikeDeclarationFreeOfThisReference(node: FunctionLikeDeclaration): boolean { + const returnType = getEffectiveReturnTypeNode(node); + return (node.kind === SyntaxKind.Constructor || (returnType && isTypeFreeOfThisReference(returnType))) && + node.parameters.every(isVariableLikeDeclarationFreeOfThisReference) && + (!node.typeParameters || node.typeParameters.every(isTypeParameterFreeOfThisReference)); } - // Returns true if the class or interface member given by the symbol is free of "this" references. The - // function may return false for symbols that are actually free of "this" references because it is not - // feasible to perform a complete analysis in all cases. In particular, property members with types - // inferred from their initializers and function members with inferred return types are conservatively - // assumed not to be free of "this" references. - function isIndependentMember(symbol: Symbol): boolean { + /** + * Returns true if the class or interface member given by the symbol is free of "this" references. The + * function may return false for symbols that are actually free of "this" references because it is not + * feasible to perform a complete analysis in all cases. In particular, property members with types + * inferred from their initializers and function members with inferred return types are conservatively + * assumed not to be free of "this" references. + */ + function isFreeOfThisReference(symbol: Symbol): boolean { if (symbol.declarations && symbol.declarations.length === 1) { const declaration = symbol.declarations[0]; if (declaration) { switch (declaration.kind) { case SyntaxKind.PropertyDeclaration: case SyntaxKind.PropertySignature: - return isIndependentVariableLikeDeclaration(declaration); + return isVariableLikeDeclarationFreeOfThisReference(declaration); case SyntaxKind.MethodDeclaration: case SyntaxKind.MethodSignature: case SyntaxKind.Constructor: - return isIndependentFunctionLikeDeclaration(declaration); + return isFunctionLikeDeclarationFreeOfThisReference(declaration); } } } @@ -5416,7 +5419,7 @@ namespace ts { function createInstantiatedSymbolTable(symbols: Symbol[], mapper: TypeMapper, mappingThisOnly: boolean): SymbolTable { const result = createSymbolTable(); for (const symbol of symbols) { - result.set(symbol.escapedName, mappingThisOnly && isIndependentMember(symbol) ? symbol : instantiateSymbol(symbol, mapper)); + result.set(symbol.escapedName, mappingThisOnly && isFreeOfThisReference(symbol) ? symbol : instantiateSymbol(symbol, mapper)); } return result; } From e0e1a3b078723333ebdee629ebdea7ac9c9f4c1d Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Wed, 1 Nov 2017 09:17:52 -0700 Subject: [PATCH 2/3] Test:this instantiation in type parameters Make sure that `this` gets instantiated when it's used as a constraint of a type parameter, and nowhere else in a signature. --- .../reference/thisTypeInFunctions3.js | 33 +++++++++++++++++++ .../reference/thisTypeInFunctions3.symbols | 26 +++++++++++++++ .../reference/thisTypeInFunctions3.types | 27 +++++++++++++++ .../types/thisType/thisTypeInFunctions3.ts | 9 +++++ 4 files changed, 95 insertions(+) create mode 100644 tests/baselines/reference/thisTypeInFunctions3.js create mode 100644 tests/baselines/reference/thisTypeInFunctions3.symbols create mode 100644 tests/baselines/reference/thisTypeInFunctions3.types create mode 100644 tests/cases/conformance/types/thisType/thisTypeInFunctions3.ts diff --git a/tests/baselines/reference/thisTypeInFunctions3.js b/tests/baselines/reference/thisTypeInFunctions3.js new file mode 100644 index 00000000000..68af387c251 --- /dev/null +++ b/tests/baselines/reference/thisTypeInFunctions3.js @@ -0,0 +1,33 @@ +//// [thisTypeInFunctions3.ts] +declare class Base { + check(prop: TProp): boolean; +} + +class Test extends Base { + m() { + this.check(this); + } +} + + +//// [thisTypeInFunctions3.js] +var __extends = (this && this.__extends) || (function () { + var extendStatics = Object.setPrototypeOf || + ({ __proto__: [] } instanceof Array && function (d, b) { d.__proto__ = b; }) || + function (d, b) { for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p]; }; + return function (d, b) { + extendStatics(d, b); + function __() { this.constructor = d; } + d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __()); + }; +})(); +var Test = /** @class */ (function (_super) { + __extends(Test, _super); + function Test() { + return _super !== null && _super.apply(this, arguments) || this; + } + Test.prototype.m = function () { + this.check(this); + }; + return Test; +}(Base)); diff --git a/tests/baselines/reference/thisTypeInFunctions3.symbols b/tests/baselines/reference/thisTypeInFunctions3.symbols new file mode 100644 index 00000000000..061a33fd63d --- /dev/null +++ b/tests/baselines/reference/thisTypeInFunctions3.symbols @@ -0,0 +1,26 @@ +=== tests/cases/conformance/types/thisType/thisTypeInFunctions3.ts === +declare class Base { +>Base : Symbol(Base, Decl(thisTypeInFunctions3.ts, 0, 0)) + + check(prop: TProp): boolean; +>check : Symbol(Base.check, Decl(thisTypeInFunctions3.ts, 0, 20)) +>TProp : Symbol(TProp, Decl(thisTypeInFunctions3.ts, 1, 10)) +>prop : Symbol(prop, Decl(thisTypeInFunctions3.ts, 1, 30)) +>TProp : Symbol(TProp, Decl(thisTypeInFunctions3.ts, 1, 10)) +} + +class Test extends Base { +>Test : Symbol(Test, Decl(thisTypeInFunctions3.ts, 2, 1)) +>Base : Symbol(Base, Decl(thisTypeInFunctions3.ts, 0, 0)) + + m() { +>m : Symbol(Test.m, Decl(thisTypeInFunctions3.ts, 4, 25)) + + this.check(this); +>this.check : Symbol(Base.check, Decl(thisTypeInFunctions3.ts, 0, 20)) +>this : Symbol(Test, Decl(thisTypeInFunctions3.ts, 2, 1)) +>check : Symbol(Base.check, Decl(thisTypeInFunctions3.ts, 0, 20)) +>this : Symbol(Test, Decl(thisTypeInFunctions3.ts, 2, 1)) + } +} + diff --git a/tests/baselines/reference/thisTypeInFunctions3.types b/tests/baselines/reference/thisTypeInFunctions3.types new file mode 100644 index 00000000000..563d6488704 --- /dev/null +++ b/tests/baselines/reference/thisTypeInFunctions3.types @@ -0,0 +1,27 @@ +=== tests/cases/conformance/types/thisType/thisTypeInFunctions3.ts === +declare class Base { +>Base : Base + + check(prop: TProp): boolean; +>check : (prop: TProp) => boolean +>TProp : TProp +>prop : TProp +>TProp : TProp +} + +class Test extends Base { +>Test : Test +>Base : Base + + m() { +>m : () => void + + this.check(this); +>this.check(this) : boolean +>this.check : (prop: TProp) => boolean +>this : this +>check : (prop: TProp) => boolean +>this : this + } +} + diff --git a/tests/cases/conformance/types/thisType/thisTypeInFunctions3.ts b/tests/cases/conformance/types/thisType/thisTypeInFunctions3.ts new file mode 100644 index 00000000000..01d7fd0430b --- /dev/null +++ b/tests/cases/conformance/types/thisType/thisTypeInFunctions3.ts @@ -0,0 +1,9 @@ +declare class Base { + check(prop: TProp): boolean; +} + +class Test extends Base { + m() { + this.check(this); + } +} From 0c77b776ce23eac210f3ef094b055c210c15bf7c Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Tue, 14 Nov 2017 16:06:48 -0800 Subject: [PATCH 3/3] Rename and inline functions --- src/compiler/checker.ts | 47 ++++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 26 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 4f261fbff2c..be57daf6fea 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -5111,7 +5111,7 @@ namespace ts { * to "this" in its body, if all base types are interfaces, * and if none of the base interfaces have a "this" type. */ - function isInterfaceFreeOfThisReference(symbol: Symbol): boolean { + function isThislessInterface(symbol: Symbol): boolean { for (const declaration of symbol.declarations) { if (declaration.kind === SyntaxKind.InterfaceDeclaration) { if (declaration.flags & NodeFlags.ContainsThis) { @@ -5145,7 +5145,7 @@ namespace ts { // property types inferred from initializers and method return types inferred from return statements are very hard // to exhaustively analyze). We give interfaces a "this" type if we can't definitely determine that they are free of // "this" references. - if (outerTypeParameters || localTypeParameters || kind === ObjectFlags.Class || !isInterfaceFreeOfThisReference(symbol)) { + if (outerTypeParameters || localTypeParameters || kind === ObjectFlags.Class || !isThislessInterface(symbol)) { type.objectFlags |= ObjectFlags.Reference; type.typeParameters = concatenate(outerTypeParameters, localTypeParameters); type.outerTypeParameters = outerTypeParameters; @@ -5327,17 +5327,12 @@ namespace ts { return undefined; } - /** A type reference is free of this references if each type argument is free of this references. */ - function isTypeReferenceFreeOfThisReference(node: TypeReferenceNode): boolean { - return !node.typeArguments || node.typeArguments.every(isTypeFreeOfThisReference); - } - /** * A type is free of this references if it's the any, string, number, boolean, symbol, or void keyword, a string * literal type, an array with an element type that is free of this references, or a type reference that is * free of this references. */ - function isTypeFreeOfThisReference(node: TypeNode): boolean { + function isThislessType(node: TypeNode): boolean { switch (node.kind) { case SyntaxKind.AnyKeyword: case SyntaxKind.StringKeyword: @@ -5352,37 +5347,37 @@ namespace ts { case SyntaxKind.LiteralType: return true; case SyntaxKind.ArrayType: - return isTypeFreeOfThisReference((node).elementType); + return isThislessType((node).elementType); case SyntaxKind.TypeReference: - return isTypeReferenceFreeOfThisReference(node); + return !(node as TypeReferenceNode).typeArguments || (node as TypeReferenceNode).typeArguments.every(isThislessType); } return false; } - /** A type parameter is this-free if its contraint is this-free, or if it has no constraint. */ - function isTypeParameterFreeOfThisReference(node: TypeParameterDeclaration) { - return !node.constraint || isTypeFreeOfThisReference(node.constraint); + /** A type parameter is thisless if its contraint is thisless, or if it has no constraint. */ + function isThislessTypeParameter(node: TypeParameterDeclaration) { + return !node.constraint || isThislessType(node.constraint); } /** * A variable-like declaration is free of this references if it has a type annotation - * that is this-free, or if it has no type annotation and no initializer (and is thus of type any). + * that is thisless, or if it has no type annotation and no initializer (and is thus of type any). */ - function isVariableLikeDeclarationFreeOfThisReference(node: VariableLikeDeclaration): boolean { + function isThislessVariableLikeDeclaration(node: VariableLikeDeclaration): boolean { const typeNode = getEffectiveTypeAnnotationNode(node); - return typeNode ? isTypeFreeOfThisReference(typeNode) : !node.initializer; + return typeNode ? isThislessType(typeNode) : !node.initializer; } /** * A function-like declaration is considered free of `this` references if it has a return type - * annotation that is free of this references and if each parameter is this-free and if - * each type parameter (if present) is this-free. + * annotation that is free of this references and if each parameter is thisless and if + * each type parameter (if present) is thisless. */ - function isFunctionLikeDeclarationFreeOfThisReference(node: FunctionLikeDeclaration): boolean { + function isThislessFunctionLikeDeclaration(node: FunctionLikeDeclaration): boolean { const returnType = getEffectiveReturnTypeNode(node); - return (node.kind === SyntaxKind.Constructor || (returnType && isTypeFreeOfThisReference(returnType))) && - node.parameters.every(isVariableLikeDeclarationFreeOfThisReference) && - (!node.typeParameters || node.typeParameters.every(isTypeParameterFreeOfThisReference)); + return (node.kind === SyntaxKind.Constructor || (returnType && isThislessType(returnType))) && + node.parameters.every(isThislessVariableLikeDeclaration) && + (!node.typeParameters || node.typeParameters.every(isThislessTypeParameter)); } /** @@ -5392,18 +5387,18 @@ namespace ts { * inferred from their initializers and function members with inferred return types are conservatively * assumed not to be free of "this" references. */ - function isFreeOfThisReference(symbol: Symbol): boolean { + function isThisless(symbol: Symbol): boolean { if (symbol.declarations && symbol.declarations.length === 1) { const declaration = symbol.declarations[0]; if (declaration) { switch (declaration.kind) { case SyntaxKind.PropertyDeclaration: case SyntaxKind.PropertySignature: - return isVariableLikeDeclarationFreeOfThisReference(declaration); + return isThislessVariableLikeDeclaration(declaration); case SyntaxKind.MethodDeclaration: case SyntaxKind.MethodSignature: case SyntaxKind.Constructor: - return isFunctionLikeDeclarationFreeOfThisReference(declaration); + return isThislessFunctionLikeDeclaration(declaration); } } } @@ -5415,7 +5410,7 @@ namespace ts { function createInstantiatedSymbolTable(symbols: Symbol[], mapper: TypeMapper, mappingThisOnly: boolean): SymbolTable { const result = createSymbolTable(); for (const symbol of symbols) { - result.set(symbol.escapedName, mappingThisOnly && isFreeOfThisReference(symbol) ? symbol : instantiateSymbol(symbol, mapper)); + result.set(symbol.escapedName, mappingThisOnly && isThisless(symbol) ? symbol : instantiateSymbol(symbol, mapper)); } return result; }