Skip to content

Commit 2b0f351

Browse files
ahejlsbergahejlsberg
authored
Fix narrow-by-constructor logic (#37698)
* Fix narrow-by-constructor logic * Add regression test
1 parent 6ffbffb commit 2b0f351

7 files changed

Lines changed: 112 additions & 19 deletions

‎src/compiler/checker.ts‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20265,12 +20265,6 @@ namespace ts {
2026520265
if (right.kind === SyntaxKind.TypeOfExpression && isStringLiteralLike(left)) {
2026620266
return narrowTypeByTypeof(type, <TypeOfExpression>right, operator, left, assumeTrue);
2026720267
}
20268-
if (isConstructorAccessExpression(left)) {
20269-
return narrowTypeByConstructor(type, left, operator, right, assumeTrue);
20270-
}
20271-
if (isConstructorAccessExpression(right)) {
20272-
return narrowTypeByConstructor(type, right, operator, left, assumeTrue);
20273-
}
2027420268
if (isMatchingReference(reference, left)) {
2027520269
return narrowTypeByEquality(type, operator, right, assumeTrue);
2027620270
}
@@ -20291,6 +20285,12 @@ namespace ts {
2029120285
if (isMatchingReferenceDiscriminant(right, declaredType)) {
2029220286
return narrowTypeByDiscriminant(type, <AccessExpression>right, t => narrowTypeByEquality(t, operator, left, assumeTrue));
2029320287
}
20288+
if (isMatchingConstructorReference(left)) {
20289+
return narrowTypeByConstructor(type, operator, right, assumeTrue);
20290+
}
20291+
if (isMatchingConstructorReference(right)) {
20292+
return narrowTypeByConstructor(type, operator, left, assumeTrue);
20293+
}
2029420294
break;
2029520295
case SyntaxKind.InstanceOfKeyword:
2029620296
return narrowTypeByInstanceof(type, expr, assumeTrue);
@@ -20564,17 +20564,18 @@ namespace ts {
2056420564
return getTypeWithFacts(mapType(type, narrowTypeForTypeofSwitch(impliedType)), switchFacts);
2056520565
}
2056620566

20567-
function narrowTypeByConstructor(type: Type, constructorAccessExpr: AccessExpression, operator: SyntaxKind, identifier: Expression, assumeTrue: boolean): Type {
20567+
function isMatchingConstructorReference(expr: Expression) {
20568+
return (isPropertyAccessExpression(expr) && idText(expr.name) === "constructor" ||
20569+
isElementAccessExpression(expr) && isStringLiteralLike(expr.argumentExpression) && expr.argumentExpression.text === "constructor") &&
20570+
isMatchingReference(reference, expr.expression);
20571+
}
20572+
20573+
function narrowTypeByConstructor(type: Type, operator: SyntaxKind, identifier: Expression, assumeTrue: boolean): Type {
2056820574
// Do not narrow when checking inequality.
2056920575
if (assumeTrue ? (operator !== SyntaxKind.EqualsEqualsToken && operator !== SyntaxKind.EqualsEqualsEqualsToken) : (operator !== SyntaxKind.ExclamationEqualsToken && operator !== SyntaxKind.ExclamationEqualsEqualsToken)) {
2057020576
return type;
2057120577
}
2057220578

20573-
// In the case of `x.y`, a `x.constructor === T` type guard resets the narrowed type of `y` to its declared type.
20574-
if (!isMatchingReference(reference, constructorAccessExpr.expression)) {
20575-
return declaredType;
20576-
}
20577-
2057820579
// Get the type of the constructor identifier expression, if it is not a function then do not narrow.
2057920580
const identifierType = getTypeOfExpression(identifier);
2058020581
if (!isFunctionType(identifierType) && !isConstructorType(identifierType)) {

‎src/compiler/utilities.ts‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4432,13 +4432,6 @@ namespace ts {
44324432
return isPropertyAccessExpression(node) && isEntityNameExpression(node.expression);
44334433
}
44344434

4435-
export function isConstructorAccessExpression(expr: Expression): expr is AccessExpression {
4436-
return (
4437-
isPropertyAccessExpression(expr) && idText(expr.name) === "constructor" ||
4438-
isElementAccessExpression(expr) && isStringLiteralLike(expr.argumentExpression) && expr.argumentExpression.text === "constructor"
4439-
);
4440-
}
4441-
44424435
export function tryGetPropertyAccessOrIdentifierToString(expr: Expression): string | undefined {
44434436
if (isPropertyAccessExpression(expr)) {
44444437
const baseStr = tryGetPropertyAccessOrIdentifierToString(expr.expression);

‎tests/baselines/reference/typeGuardConstructorClassAndNumber.errors.txt‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,4 +160,14 @@ tests/cases/compiler/typeGuardConstructorClassAndNumber.ts(115,10): error TS2339
160160
else {
161161
var1; // C1
162162
}
163+
164+
// Repro from #37660
165+
166+
function foo(instance: Function | object) {
167+
if (typeof instance === 'function') {
168+
if (instance.prototype == null || instance.prototype.constructor == null) {
169+
return instance.length;
170+
}
171+
}
172+
}
163173

‎tests/baselines/reference/typeGuardConstructorClassAndNumber.js‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,16 @@ if (C1 !== var1["constructor"]) {
118118
else {
119119
var1; // C1
120120
}
121+
122+
// Repro from #37660
123+
124+
function foo(instance: Function | object) {
125+
if (typeof instance === 'function') {
126+
if (instance.prototype == null || instance.prototype.constructor == null) {
127+
return instance.length;
128+
}
129+
}
130+
}
121131

122132

123133
//// [typeGuardConstructorClassAndNumber.js]
@@ -240,3 +250,11 @@ if (C1 !== var1["constructor"]) {
240250
else {
241251
var1; // C1
242252
}
253+
// Repro from #37660
254+
function foo(instance) {
255+
if (typeof instance === 'function') {
256+
if (instance.prototype == null || instance.prototype.constructor == null) {
257+
return instance.length;
258+
}
259+
}
260+
}

‎tests/baselines/reference/typeGuardConstructorClassAndNumber.symbols‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,3 +277,29 @@ else {
277277
>var1 : Symbol(var1, Decl(typeGuardConstructorClassAndNumber.ts, 5, 3))
278278
}
279279

280+
// Repro from #37660
281+
282+
function foo(instance: Function | object) {
283+
>foo : Symbol(foo, Decl(typeGuardConstructorClassAndNumber.ts, 118, 1))
284+
>instance : Symbol(instance, Decl(typeGuardConstructorClassAndNumber.ts, 122, 13))
285+
>Function : Symbol(Function, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
286+
287+
if (typeof instance === 'function') {
288+
>instance : Symbol(instance, Decl(typeGuardConstructorClassAndNumber.ts, 122, 13))
289+
290+
if (instance.prototype == null || instance.prototype.constructor == null) {
291+
>instance.prototype : Symbol(Function.prototype, Decl(lib.es5.d.ts, --, --))
292+
>instance : Symbol(instance, Decl(typeGuardConstructorClassAndNumber.ts, 122, 13))
293+
>prototype : Symbol(Function.prototype, Decl(lib.es5.d.ts, --, --))
294+
>instance.prototype : Symbol(Function.prototype, Decl(lib.es5.d.ts, --, --))
295+
>instance : Symbol(instance, Decl(typeGuardConstructorClassAndNumber.ts, 122, 13))
296+
>prototype : Symbol(Function.prototype, Decl(lib.es5.d.ts, --, --))
297+
298+
return instance.length;
299+
>instance.length : Symbol(Function.length, Decl(lib.es5.d.ts, --, --))
300+
>instance : Symbol(instance, Decl(typeGuardConstructorClassAndNumber.ts, 122, 13))
301+
>length : Symbol(Function.length, Decl(lib.es5.d.ts, --, --))
302+
}
303+
}
304+
}
305+

‎tests/baselines/reference/typeGuardConstructorClassAndNumber.types‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,3 +316,38 @@ else {
316316
>var1 : C1
317317
}
318318

319+
// Repro from #37660
320+
321+
function foo(instance: Function | object) {
322+
>foo : (instance: object | Function) => number
323+
>instance : object | Function
324+
325+
if (typeof instance === 'function') {
326+
>typeof instance === 'function' : boolean
327+
>typeof instance : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function"
328+
>instance : object | Function
329+
>'function' : "function"
330+
331+
if (instance.prototype == null || instance.prototype.constructor == null) {
332+
>instance.prototype == null || instance.prototype.constructor == null : boolean
333+
>instance.prototype == null : boolean
334+
>instance.prototype : any
335+
>instance : Function
336+
>prototype : any
337+
>null : null
338+
>instance.prototype.constructor == null : boolean
339+
>instance.prototype.constructor : any
340+
>instance.prototype : any
341+
>instance : Function
342+
>prototype : any
343+
>constructor : any
344+
>null : null
345+
346+
return instance.length;
347+
>instance.length : number
348+
>instance : Function
349+
>length : number
350+
}
351+
}
352+
}
353+

‎tests/cases/compiler/typeGuardConstructorClassAndNumber.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,3 +117,13 @@ if (C1 !== var1["constructor"]) {
117117
else {
118118
var1; // C1
119119
}
120+
121+
// Repro from #37660
122+
123+
function foo(instance: Function | object) {
124+
if (typeof instance === 'function') {
125+
if (instance.prototype == null || instance.prototype.constructor == null) {
126+
return instance.length;
127+
}
128+
}
129+
}

0 commit comments

Comments
 (0)