From 8268f2adecba780c9589355c7117bf6652745068 Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Thu, 15 Jul 2021 17:06:56 -0700 Subject: [PATCH] Avoid bogus circularity error on context sensitive constructor property assignments (#44601) * Avoid bogus circularity error on context sensitive constructor property assignments * Add JS case and ensure its fixed --- src/compiler/checker.ts | 36 ++++++++++++++++++- .../classAttributeInferenceTemplate.js | 26 ++++++++++++++ .../classAttributeInferenceTemplate.symbols | 30 ++++++++++++++++ .../classAttributeInferenceTemplate.types | 36 +++++++++++++++++++ .../classAttributeInferenceTemplateJS.symbols | 30 ++++++++++++++++ .../classAttributeInferenceTemplateJS.types | 36 +++++++++++++++++++ .../classAttributeInferenceTemplate.ts | 14 ++++++++ .../classAttributeInferenceTemplateJS.ts | 17 +++++++++ 8 files changed, 224 insertions(+), 1 deletion(-) create mode 100644 tests/baselines/reference/classAttributeInferenceTemplate.js create mode 100644 tests/baselines/reference/classAttributeInferenceTemplate.symbols create mode 100644 tests/baselines/reference/classAttributeInferenceTemplate.types create mode 100644 tests/baselines/reference/classAttributeInferenceTemplateJS.symbols create mode 100644 tests/baselines/reference/classAttributeInferenceTemplateJS.types create mode 100644 tests/cases/compiler/classAttributeInferenceTemplate.ts create mode 100644 tests/cases/compiler/classAttributeInferenceTemplateJS.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 87f3b5a70b6..86d2c8989a4 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -25368,14 +25368,48 @@ namespace ts { } } + /** + * Try to find a resolved symbol for an expression without also resolving its type, as + * getSymbolAtLocation would (as that could be reentrant into contextual typing) + */ + function getSymbolForExpression(e: Expression) { + if (e.symbol) { + return e.symbol; + } + if (isIdentifier(e)) { + return getResolvedSymbol(e); + } + if (isPropertyAccessExpression(e)) { + const lhsType = getTypeOfExpression(e.expression); + return isPrivateIdentifier(e.name) ? tryGetPrivateIdentifierPropertyOfType(lhsType, e.name) : getPropertyOfType(lhsType, e.name.escapedText); + } + return undefined; + + function tryGetPrivateIdentifierPropertyOfType(type: Type, id: PrivateIdentifier) { + const lexicallyScopedSymbol = lookupSymbolForPrivateIdentifierDeclaration(id.escapedText, id); + return lexicallyScopedSymbol && getPrivateIdentifierPropertyOfType(type, lexicallyScopedSymbol); + } + } + // In an assignment expression, the right operand is contextually typed by the type of the left operand. // Don't do this for assignment declarations unless there is a type tag on the assignment, to avoid circularity from checking the right operand. function getContextualTypeForAssignmentDeclaration(binaryExpression: BinaryExpression): Type | undefined { const kind = getAssignmentDeclarationKind(binaryExpression); switch (kind) { case AssignmentDeclarationKind.None: - return getTypeOfExpression(binaryExpression.left); case AssignmentDeclarationKind.ThisProperty: + const lhsSymbol = getSymbolForExpression(binaryExpression.left); + const decl = lhsSymbol && lhsSymbol.valueDeclaration; + // Unannotated, uninitialized property declarations have a type implied by their usage in the constructor. + // We avoid calling back into `getTypeOfExpression` and reentering contextual typing to avoid a bogus circularity error in that case. + if (decl && (isPropertyDeclaration(decl) || isPropertySignature(decl))) { + const overallAnnotation = getEffectiveTypeAnnotationNode(decl); + return (overallAnnotation && getTypeFromTypeNode(overallAnnotation)) || + (decl.initializer && getTypeOfExpression(binaryExpression.left)); + } + if (kind === AssignmentDeclarationKind.None) { + return getTypeOfExpression(binaryExpression.left); + } return getContextualTypeForThisPropertyAssignment(binaryExpression); case AssignmentDeclarationKind.Property: if (isPossiblyAliasedThisProperty(binaryExpression, kind)) { diff --git a/tests/baselines/reference/classAttributeInferenceTemplate.js b/tests/baselines/reference/classAttributeInferenceTemplate.js new file mode 100644 index 00000000000..24a90d0301a --- /dev/null +++ b/tests/baselines/reference/classAttributeInferenceTemplate.js @@ -0,0 +1,26 @@ +//// [classAttributeInferenceTemplate.ts] +class MyClass { + property; + property2; + + constructor() { + const variable = 'something' + + this.property = `foo`; // Correctly inferred as `string` + this.property2 = `foo-${variable}`; // Causes an error + + const localProperty = `foo-${variable}`; // Correctly inferred as `string` + } +} + +//// [classAttributeInferenceTemplate.js] +"use strict"; +var MyClass = /** @class */ (function () { + function MyClass() { + var variable = 'something'; + this.property = "foo"; // Correctly inferred as `string` + this.property2 = "foo-" + variable; // Causes an error + var localProperty = "foo-" + variable; // Correctly inferred as `string` + } + return MyClass; +}()); diff --git a/tests/baselines/reference/classAttributeInferenceTemplate.symbols b/tests/baselines/reference/classAttributeInferenceTemplate.symbols new file mode 100644 index 00000000000..f4594f7531c --- /dev/null +++ b/tests/baselines/reference/classAttributeInferenceTemplate.symbols @@ -0,0 +1,30 @@ +=== tests/cases/compiler/classAttributeInferenceTemplate.ts === +class MyClass { +>MyClass : Symbol(MyClass, Decl(classAttributeInferenceTemplate.ts, 0, 0)) + + property; +>property : Symbol(MyClass.property, Decl(classAttributeInferenceTemplate.ts, 0, 15)) + + property2; +>property2 : Symbol(MyClass.property2, Decl(classAttributeInferenceTemplate.ts, 1, 13)) + + constructor() { + const variable = 'something' +>variable : Symbol(variable, Decl(classAttributeInferenceTemplate.ts, 5, 13)) + + this.property = `foo`; // Correctly inferred as `string` +>this.property : Symbol(MyClass.property, Decl(classAttributeInferenceTemplate.ts, 0, 15)) +>this : Symbol(MyClass, Decl(classAttributeInferenceTemplate.ts, 0, 0)) +>property : Symbol(MyClass.property, Decl(classAttributeInferenceTemplate.ts, 0, 15)) + + this.property2 = `foo-${variable}`; // Causes an error +>this.property2 : Symbol(MyClass.property2, Decl(classAttributeInferenceTemplate.ts, 1, 13)) +>this : Symbol(MyClass, Decl(classAttributeInferenceTemplate.ts, 0, 0)) +>property2 : Symbol(MyClass.property2, Decl(classAttributeInferenceTemplate.ts, 1, 13)) +>variable : Symbol(variable, Decl(classAttributeInferenceTemplate.ts, 5, 13)) + + const localProperty = `foo-${variable}`; // Correctly inferred as `string` +>localProperty : Symbol(localProperty, Decl(classAttributeInferenceTemplate.ts, 10, 13)) +>variable : Symbol(variable, Decl(classAttributeInferenceTemplate.ts, 5, 13)) + } +} diff --git a/tests/baselines/reference/classAttributeInferenceTemplate.types b/tests/baselines/reference/classAttributeInferenceTemplate.types new file mode 100644 index 00000000000..90f1f381900 --- /dev/null +++ b/tests/baselines/reference/classAttributeInferenceTemplate.types @@ -0,0 +1,36 @@ +=== tests/cases/compiler/classAttributeInferenceTemplate.ts === +class MyClass { +>MyClass : MyClass + + property; +>property : string + + property2; +>property2 : string + + constructor() { + const variable = 'something' +>variable : "something" +>'something' : "something" + + this.property = `foo`; // Correctly inferred as `string` +>this.property = `foo` : "foo" +>this.property : string +>this : this +>property : string +>`foo` : "foo" + + this.property2 = `foo-${variable}`; // Causes an error +>this.property2 = `foo-${variable}` : string +>this.property2 : string +>this : this +>property2 : string +>`foo-${variable}` : string +>variable : "something" + + const localProperty = `foo-${variable}`; // Correctly inferred as `string` +>localProperty : string +>`foo-${variable}` : string +>variable : "something" + } +} diff --git a/tests/baselines/reference/classAttributeInferenceTemplateJS.symbols b/tests/baselines/reference/classAttributeInferenceTemplateJS.symbols new file mode 100644 index 00000000000..4fff0993257 --- /dev/null +++ b/tests/baselines/reference/classAttributeInferenceTemplateJS.symbols @@ -0,0 +1,30 @@ +=== tests/cases/compiler/index.js === +class MyClass { +>MyClass : Symbol(MyClass, Decl(index.js, 0, 0)) + + property; +>property : Symbol(MyClass.property, Decl(index.js, 0, 15)) + + property2; +>property2 : Symbol(MyClass.property2, Decl(index.js, 1, 13)) + + constructor() { + const variable = 'something' +>variable : Symbol(variable, Decl(index.js, 5, 13)) + + this.property = `foo`; // Correctly inferred as `string` +>this.property : Symbol(MyClass.property, Decl(index.js, 0, 15)) +>this : Symbol(MyClass, Decl(index.js, 0, 0)) +>property : Symbol(MyClass.property, Decl(index.js, 0, 15)) + + this.property2 = `foo-${variable}`; // Causes an error +>this.property2 : Symbol(MyClass.property2, Decl(index.js, 1, 13)) +>this : Symbol(MyClass, Decl(index.js, 0, 0)) +>property2 : Symbol(MyClass.property2, Decl(index.js, 1, 13)) +>variable : Symbol(variable, Decl(index.js, 5, 13)) + + const localProperty = `foo-${variable}`; // Correctly inferred as `string` +>localProperty : Symbol(localProperty, Decl(index.js, 10, 13)) +>variable : Symbol(variable, Decl(index.js, 5, 13)) + } +} diff --git a/tests/baselines/reference/classAttributeInferenceTemplateJS.types b/tests/baselines/reference/classAttributeInferenceTemplateJS.types new file mode 100644 index 00000000000..29cf5086be3 --- /dev/null +++ b/tests/baselines/reference/classAttributeInferenceTemplateJS.types @@ -0,0 +1,36 @@ +=== tests/cases/compiler/index.js === +class MyClass { +>MyClass : MyClass + + property; +>property : string + + property2; +>property2 : string + + constructor() { + const variable = 'something' +>variable : "something" +>'something' : "something" + + this.property = `foo`; // Correctly inferred as `string` +>this.property = `foo` : "foo" +>this.property : string +>this : this +>property : string +>`foo` : "foo" + + this.property2 = `foo-${variable}`; // Causes an error +>this.property2 = `foo-${variable}` : string +>this.property2 : string +>this : this +>property2 : string +>`foo-${variable}` : string +>variable : "something" + + const localProperty = `foo-${variable}`; // Correctly inferred as `string` +>localProperty : string +>`foo-${variable}` : string +>variable : "something" + } +} diff --git a/tests/cases/compiler/classAttributeInferenceTemplate.ts b/tests/cases/compiler/classAttributeInferenceTemplate.ts new file mode 100644 index 00000000000..d7a7be3bcb6 --- /dev/null +++ b/tests/cases/compiler/classAttributeInferenceTemplate.ts @@ -0,0 +1,14 @@ +// @strict: true +class MyClass { + property; + property2; + + constructor() { + const variable = 'something' + + this.property = `foo`; // Correctly inferred as `string` + this.property2 = `foo-${variable}`; // Causes an error + + const localProperty = `foo-${variable}`; // Correctly inferred as `string` + } +} \ No newline at end of file diff --git a/tests/cases/compiler/classAttributeInferenceTemplateJS.ts b/tests/cases/compiler/classAttributeInferenceTemplateJS.ts new file mode 100644 index 00000000000..4a1ea9f3cba --- /dev/null +++ b/tests/cases/compiler/classAttributeInferenceTemplateJS.ts @@ -0,0 +1,17 @@ +// @noEmit: true +// @checkJs: true +// @strict: true +// @filename: index.js +class MyClass { + property; + property2; + + constructor() { + const variable = 'something' + + this.property = `foo`; // Correctly inferred as `string` + this.property2 = `foo-${variable}`; // Causes an error + + const localProperty = `foo-${variable}`; // Correctly inferred as `string` + } +} \ No newline at end of file