Skip to content

Commit 8268f2a

Browse files
weswighamweswigham
authored
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
1 parent 87cff4e commit 8268f2a

8 files changed

+224
-1
lines changed

‎src/compiler/checker.ts

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25368,14 +25368,48 @@ namespace ts {
2536825368
}
2536925369
}
2537025370

25371+
/**
25372+
* Try to find a resolved symbol for an expression without also resolving its type, as
25373+
* getSymbolAtLocation would (as that could be reentrant into contextual typing)
25374+
*/
25375+
function getSymbolForExpression(e: Expression) {
25376+
if (e.symbol) {
25377+
return e.symbol;
25378+
}
25379+
if (isIdentifier(e)) {
25380+
return getResolvedSymbol(e);
25381+
}
25382+
if (isPropertyAccessExpression(e)) {
25383+
const lhsType = getTypeOfExpression(e.expression);
25384+
return isPrivateIdentifier(e.name) ? tryGetPrivateIdentifierPropertyOfType(lhsType, e.name) : getPropertyOfType(lhsType, e.name.escapedText);
25385+
}
25386+
return undefined;
25387+
25388+
function tryGetPrivateIdentifierPropertyOfType(type: Type, id: PrivateIdentifier) {
25389+
const lexicallyScopedSymbol = lookupSymbolForPrivateIdentifierDeclaration(id.escapedText, id);
25390+
return lexicallyScopedSymbol && getPrivateIdentifierPropertyOfType(type, lexicallyScopedSymbol);
25391+
}
25392+
}
25393+
2537125394
// In an assignment expression, the right operand is contextually typed by the type of the left operand.
2537225395
// Don't do this for assignment declarations unless there is a type tag on the assignment, to avoid circularity from checking the right operand.
2537325396
function getContextualTypeForAssignmentDeclaration(binaryExpression: BinaryExpression): Type | undefined {
2537425397
const kind = getAssignmentDeclarationKind(binaryExpression);
2537525398
switch (kind) {
2537625399
case AssignmentDeclarationKind.None:
25377-
return getTypeOfExpression(binaryExpression.left);
2537825400
case AssignmentDeclarationKind.ThisProperty:
25401+
const lhsSymbol = getSymbolForExpression(binaryExpression.left);
25402+
const decl = lhsSymbol && lhsSymbol.valueDeclaration;
25403+
// Unannotated, uninitialized property declarations have a type implied by their usage in the constructor.
25404+
// We avoid calling back into `getTypeOfExpression` and reentering contextual typing to avoid a bogus circularity error in that case.
25405+
if (decl && (isPropertyDeclaration(decl) || isPropertySignature(decl))) {
25406+
const overallAnnotation = getEffectiveTypeAnnotationNode(decl);
25407+
return (overallAnnotation && getTypeFromTypeNode(overallAnnotation)) ||
25408+
(decl.initializer && getTypeOfExpression(binaryExpression.left));
25409+
}
25410+
if (kind === AssignmentDeclarationKind.None) {
25411+
return getTypeOfExpression(binaryExpression.left);
25412+
}
2537925413
return getContextualTypeForThisPropertyAssignment(binaryExpression);
2538025414
case AssignmentDeclarationKind.Property:
2538125415
if (isPossiblyAliasedThisProperty(binaryExpression, kind)) {

‎tests/baselines/reference/classAttributeInferenceTemplate.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
//// [classAttributeInferenceTemplate.ts]
2+
class MyClass {
3+
property;
4+
property2;
5+
6+
constructor() {
7+
const variable = 'something'
8+
9+
this.property = `foo`; // Correctly inferred as `string`
10+
this.property2 = `foo-${variable}`; // Causes an error
11+
12+
const localProperty = `foo-${variable}`; // Correctly inferred as `string`
13+
}
14+
}
15+
16+
//// [classAttributeInferenceTemplate.js]
17+
"use strict";
18+
var MyClass = /** @class */ (function () {
19+
function MyClass() {
20+
var variable = 'something';
21+
this.property = "foo"; // Correctly inferred as `string`
22+
this.property2 = "foo-" + variable; // Causes an error
23+
var localProperty = "foo-" + variable; // Correctly inferred as `string`
24+
}
25+
return MyClass;
26+
}());

‎tests/baselines/reference/classAttributeInferenceTemplate.symbols

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
=== tests/cases/compiler/classAttributeInferenceTemplate.ts ===
2+
class MyClass {
3+
>MyClass : Symbol(MyClass, Decl(classAttributeInferenceTemplate.ts, 0, 0))
4+
5+
property;
6+
>property : Symbol(MyClass.property, Decl(classAttributeInferenceTemplate.ts, 0, 15))
7+
8+
property2;
9+
>property2 : Symbol(MyClass.property2, Decl(classAttributeInferenceTemplate.ts, 1, 13))
10+
11+
constructor() {
12+
const variable = 'something'
13+
>variable : Symbol(variable, Decl(classAttributeInferenceTemplate.ts, 5, 13))
14+
15+
this.property = `foo`; // Correctly inferred as `string`
16+
>this.property : Symbol(MyClass.property, Decl(classAttributeInferenceTemplate.ts, 0, 15))
17+
>this : Symbol(MyClass, Decl(classAttributeInferenceTemplate.ts, 0, 0))
18+
>property : Symbol(MyClass.property, Decl(classAttributeInferenceTemplate.ts, 0, 15))
19+
20+
this.property2 = `foo-${variable}`; // Causes an error
21+
>this.property2 : Symbol(MyClass.property2, Decl(classAttributeInferenceTemplate.ts, 1, 13))
22+
>this : Symbol(MyClass, Decl(classAttributeInferenceTemplate.ts, 0, 0))
23+
>property2 : Symbol(MyClass.property2, Decl(classAttributeInferenceTemplate.ts, 1, 13))
24+
>variable : Symbol(variable, Decl(classAttributeInferenceTemplate.ts, 5, 13))
25+
26+
const localProperty = `foo-${variable}`; // Correctly inferred as `string`
27+
>localProperty : Symbol(localProperty, Decl(classAttributeInferenceTemplate.ts, 10, 13))
28+
>variable : Symbol(variable, Decl(classAttributeInferenceTemplate.ts, 5, 13))
29+
}
30+
}

‎tests/baselines/reference/classAttributeInferenceTemplate.types

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
=== tests/cases/compiler/classAttributeInferenceTemplate.ts ===
2+
class MyClass {
3+
>MyClass : MyClass
4+
5+
property;
6+
>property : string
7+
8+
property2;
9+
>property2 : string
10+
11+
constructor() {
12+
const variable = 'something'
13+
>variable : "something"
14+
>'something' : "something"
15+
16+
this.property = `foo`; // Correctly inferred as `string`
17+
>this.property = `foo` : "foo"
18+
>this.property : string
19+
>this : this
20+
>property : string
21+
>`foo` : "foo"
22+
23+
this.property2 = `foo-${variable}`; // Causes an error
24+
>this.property2 = `foo-${variable}` : string
25+
>this.property2 : string
26+
>this : this
27+
>property2 : string
28+
>`foo-${variable}` : string
29+
>variable : "something"
30+
31+
const localProperty = `foo-${variable}`; // Correctly inferred as `string`
32+
>localProperty : string
33+
>`foo-${variable}` : string
34+
>variable : "something"
35+
}
36+
}

‎tests/baselines/reference/classAttributeInferenceTemplateJS.symbols

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
=== tests/cases/compiler/index.js ===
2+
class MyClass {
3+
>MyClass : Symbol(MyClass, Decl(index.js, 0, 0))
4+
5+
property;
6+
>property : Symbol(MyClass.property, Decl(index.js, 0, 15))
7+
8+
property2;
9+
>property2 : Symbol(MyClass.property2, Decl(index.js, 1, 13))
10+
11+
constructor() {
12+
const variable = 'something'
13+
>variable : Symbol(variable, Decl(index.js, 5, 13))
14+
15+
this.property = `foo`; // Correctly inferred as `string`
16+
>this.property : Symbol(MyClass.property, Decl(index.js, 0, 15))
17+
>this : Symbol(MyClass, Decl(index.js, 0, 0))
18+
>property : Symbol(MyClass.property, Decl(index.js, 0, 15))
19+
20+
this.property2 = `foo-${variable}`; // Causes an error
21+
>this.property2 : Symbol(MyClass.property2, Decl(index.js, 1, 13))
22+
>this : Symbol(MyClass, Decl(index.js, 0, 0))
23+
>property2 : Symbol(MyClass.property2, Decl(index.js, 1, 13))
24+
>variable : Symbol(variable, Decl(index.js, 5, 13))
25+
26+
const localProperty = `foo-${variable}`; // Correctly inferred as `string`
27+
>localProperty : Symbol(localProperty, Decl(index.js, 10, 13))
28+
>variable : Symbol(variable, Decl(index.js, 5, 13))
29+
}
30+
}

‎tests/baselines/reference/classAttributeInferenceTemplateJS.types

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
=== tests/cases/compiler/index.js ===
2+
class MyClass {
3+
>MyClass : MyClass
4+
5+
property;
6+
>property : string
7+
8+
property2;
9+
>property2 : string
10+
11+
constructor() {
12+
const variable = 'something'
13+
>variable : "something"
14+
>'something' : "something"
15+
16+
this.property = `foo`; // Correctly inferred as `string`
17+
>this.property = `foo` : "foo"
18+
>this.property : string
19+
>this : this
20+
>property : string
21+
>`foo` : "foo"
22+
23+
this.property2 = `foo-${variable}`; // Causes an error
24+
>this.property2 = `foo-${variable}` : string
25+
>this.property2 : string
26+
>this : this
27+
>property2 : string
28+
>`foo-${variable}` : string
29+
>variable : "something"
30+
31+
const localProperty = `foo-${variable}`; // Correctly inferred as `string`
32+
>localProperty : string
33+
>`foo-${variable}` : string
34+
>variable : "something"
35+
}
36+
}

‎tests/cases/compiler/classAttributeInferenceTemplate.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// @strict: true
2+
class MyClass {
3+
property;
4+
property2;
5+
6+
constructor() {
7+
const variable = 'something'
8+
9+
this.property = `foo`; // Correctly inferred as `string`
10+
this.property2 = `foo-${variable}`; // Causes an error
11+
12+
const localProperty = `foo-${variable}`; // Correctly inferred as `string`
13+
}
14+
}

‎tests/cases/compiler/classAttributeInferenceTemplateJS.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// @noEmit: true
2+
// @checkJs: true
3+
// @strict: true
4+
// @filename: index.js
5+
class MyClass {
6+
property;
7+
property2;
8+
9+
constructor() {
10+
const variable = 'something'
11+
12+
this.property = `foo`; // Correctly inferred as `string`
13+
this.property2 = `foo-${variable}`; // Causes an error
14+
15+
const localProperty = `foo-${variable}`; // Correctly inferred as `string`
16+
}
17+
}

0 commit comments

Comments
 (0)