Remove ordering restrictions in control flow analysis (#37134)

* Don't reset CFA type for x.y when x is narrowed * Add tests * Accept new baselines * Remove unnecessary type assertion
2026-05-24 11:43:18 -05:00 · 2020-02-29 12:03:09 -08:00
parent f914db0667
commit a5796cf3b2
6 changed files with 279 additions and 24 deletions
--- a/src/compiler/binder.ts
+++ b/src/compiler/binder.ts
@@ -3238,7 +3238,7 @@ namespace ts {
            // If this is a property-parameter, then also declare the property symbol into the
            // containing class.
            if (isParameterPropertyDeclaration(node, node.parent)) {
-                const classDeclaration = <ClassLikeDeclaration>node.parent.parent;
+                const classDeclaration = node.parent.parent;
                declareSymbol(classDeclaration.symbol.members!, classDeclaration.symbol, node, SymbolFlags.Property | (node.questionToken ? SymbolFlags.Optional : SymbolFlags.None), SymbolFlags.PropertyExcludes);
            }
        }
--- a/src/compiler/checker.ts
+++ b/src/compiler/checker.ts
@@ -18903,10 +18903,6 @@ namespace ts {
            return false;
        }

-        function isSyntheticThisPropertyAccess(expr: Node) {
-            return isAccessExpression(expr) && expr.expression.kind === SyntaxKind.ThisKeyword && !!(expr.expression.flags & NodeFlags.Synthesized);
-        }
-
        function findDiscriminantProperties(sourceProperties: Symbol[], target: Type): Symbol[] | undefined {
            let result: Symbol[] | undefined;
            for (const sourceProperty of sourceProperties) {
@@ -19634,7 +19630,7 @@ namespace ts {
            // on empty arrays are possible without implicit any errors and new element types can be inferred without
            // type mismatch errors.
            const resultType = getObjectFlags(evolvedType) & ObjectFlags.EvolvingArray && isEvolvingArrayOperationTarget(reference) ? autoArrayType : finalizeEvolvingArrayType(evolvedType);
-            if (resultType === unreachableNeverType|| reference.parent && reference.parent.kind === SyntaxKind.NonNullExpression && getTypeWithFacts(resultType, TypeFacts.NEUndefinedOrNull).flags & TypeFlags.Never) {
+            if (resultType === unreachableNeverType || reference.parent && reference.parent.kind === SyntaxKind.NonNullExpression && getTypeWithFacts(resultType, TypeFacts.NEUndefinedOrNull).flags & TypeFlags.Never) {
                return declaredType;
            }
            return resultType;
@@ -20241,11 +20237,6 @@ namespace ts {
                    if (strictNullChecks && optionalChainContainsReference(target, reference) && assumeTrue === (literal.text !== "undefined")) {
                        return getTypeWithFacts(type, TypeFacts.NEUndefinedOrNull);
                    }
-                    // For a reference of the form 'x.y', a 'typeof x === ...' type guard resets the
-                    // narrowed type of 'y' to its declared type.
-                    if (containsMatchingReference(reference, target)) {
-                        return declaredType;
-                    }
                    return type;
                }
                if (type.flags & TypeFlags.Any && literal.text === "function") {
@@ -20427,16 +20418,6 @@ namespace ts {
                    if (assumeTrue && strictNullChecks && optionalChainContainsReference(left, reference)) {
                        return getTypeWithFacts(type, TypeFacts.NEUndefinedOrNull);
                    }
-                    // For a reference of the form 'x.y', an 'x instanceof T' type guard resets the
-                    // narrowed type of 'y' to its declared type. We do this because preceding 'x.y'
-                    // references might reference a different 'y' property. However, we make an exception
-                    // for property accesses where x is a synthetic 'this' expression, indicating that we
-                    // were called from isPropertyInitializedInConstructor. Without this exception,
-                    // initializations of 'this' properties that occur before a 'this instanceof XXX'
-                    // check would not be considered.
-                    if (containsMatchingReference(reference, left) && !isSyntheticThisPropertyAccess(reference)) {
-                        return declaredType;
-                    }
                    return type;
                }

@@ -20517,9 +20498,6 @@ namespace ts {
                            !(getTypeFacts(predicate.type) & TypeFacts.EQUndefined)) {
                            return getTypeWithFacts(type, TypeFacts.NEUndefinedOrNull);
                        }
-                        if (containsMatchingReference(reference, predicateArgument)) {
-                            return declaredType;
-                        }
                    }
                }
                return type;
--- a/tests/baselines/reference/narrowingOrderIndependent.js
+++ b/tests/baselines/reference/narrowingOrderIndependent.js
@@ -0,0 +1,66 @@
+//// [narrowingOrderIndependent.ts]
+// Repro from #36709
+
+class A {
+    constructor(public stringOrUndefined: string | undefined) {}
+}
+
+class B {
+    constructor(public str: string) {}
+}
+
+const a = new A("123");
+
+if (a instanceof A && a.stringOrUndefined) {
+    new B(a.stringOrUndefined)
+}
+
+if (a.stringOrUndefined && a instanceof A) {
+    new B(a.stringOrUndefined)
+}
+
+if (a instanceof A) {
+    if (a.stringOrUndefined) {
+        new B(a.stringOrUndefined)
+    }
+}
+
+if (a.stringOrUndefined) {
+    if (a instanceof A) {
+        new B(a.stringOrUndefined)
+    }
+}
+
+
+//// [narrowingOrderIndependent.js]
+"use strict";
+// Repro from #36709
+var A = /** @class */ (function () {
+    function A(stringOrUndefined) {
+        this.stringOrUndefined = stringOrUndefined;
+    }
+    return A;
+}());
+var B = /** @class */ (function () {
+    function B(str) {
+        this.str = str;
+    }
+    return B;
+}());
+var a = new A("123");
+if (a instanceof A && a.stringOrUndefined) {
+    new B(a.stringOrUndefined);
+}
+if (a.stringOrUndefined && a instanceof A) {
+    new B(a.stringOrUndefined);
+}
+if (a instanceof A) {
+    if (a.stringOrUndefined) {
+        new B(a.stringOrUndefined);
+    }
+}
+if (a.stringOrUndefined) {
+    if (a instanceof A) {
+        new B(a.stringOrUndefined);
+    }
+}
--- a/tests/baselines/reference/narrowingOrderIndependent.symbols
+++ b/tests/baselines/reference/narrowingOrderIndependent.symbols
@@ -0,0 +1,83 @@
+=== tests/cases/compiler/narrowingOrderIndependent.ts ===
+// Repro from #36709
+
+class A {
+>A : Symbol(A, Decl(narrowingOrderIndependent.ts, 0, 0))
+
+    constructor(public stringOrUndefined: string | undefined) {}
+>stringOrUndefined : Symbol(A.stringOrUndefined, Decl(narrowingOrderIndependent.ts, 3, 16))
+}
+
+class B {
+>B : Symbol(B, Decl(narrowingOrderIndependent.ts, 4, 1))
+
+    constructor(public str: string) {}
+>str : Symbol(B.str, Decl(narrowingOrderIndependent.ts, 7, 16))
+}
+
+const a = new A("123");
+>a : Symbol(a, Decl(narrowingOrderIndependent.ts, 10, 5))
+>A : Symbol(A, Decl(narrowingOrderIndependent.ts, 0, 0))
+
+if (a instanceof A && a.stringOrUndefined) {
+>a : Symbol(a, Decl(narrowingOrderIndependent.ts, 10, 5))
+>A : Symbol(A, Decl(narrowingOrderIndependent.ts, 0, 0))
+>a.stringOrUndefined : Symbol(A.stringOrUndefined, Decl(narrowingOrderIndependent.ts, 3, 16))
+>a : Symbol(a, Decl(narrowingOrderIndependent.ts, 10, 5))
+>stringOrUndefined : Symbol(A.stringOrUndefined, Decl(narrowingOrderIndependent.ts, 3, 16))
+
+    new B(a.stringOrUndefined)
+>B : Symbol(B, Decl(narrowingOrderIndependent.ts, 4, 1))
+>a.stringOrUndefined : Symbol(A.stringOrUndefined, Decl(narrowingOrderIndependent.ts, 3, 16))
+>a : Symbol(a, Decl(narrowingOrderIndependent.ts, 10, 5))
+>stringOrUndefined : Symbol(A.stringOrUndefined, Decl(narrowingOrderIndependent.ts, 3, 16))
+}
+
+if (a.stringOrUndefined && a instanceof A) {
+>a.stringOrUndefined : Symbol(A.stringOrUndefined, Decl(narrowingOrderIndependent.ts, 3, 16))
+>a : Symbol(a, Decl(narrowingOrderIndependent.ts, 10, 5))
+>stringOrUndefined : Symbol(A.stringOrUndefined, Decl(narrowingOrderIndependent.ts, 3, 16))
+>a : Symbol(a, Decl(narrowingOrderIndependent.ts, 10, 5))
+>A : Symbol(A, Decl(narrowingOrderIndependent.ts, 0, 0))
+
+    new B(a.stringOrUndefined)
+>B : Symbol(B, Decl(narrowingOrderIndependent.ts, 4, 1))
+>a.stringOrUndefined : Symbol(A.stringOrUndefined, Decl(narrowingOrderIndependent.ts, 3, 16))
+>a : Symbol(a, Decl(narrowingOrderIndependent.ts, 10, 5))
+>stringOrUndefined : Symbol(A.stringOrUndefined, Decl(narrowingOrderIndependent.ts, 3, 16))
+}
+
+if (a instanceof A) {
+>a : Symbol(a, Decl(narrowingOrderIndependent.ts, 10, 5))
+>A : Symbol(A, Decl(narrowingOrderIndependent.ts, 0, 0))
+
+    if (a.stringOrUndefined) {
+>a.stringOrUndefined : Symbol(A.stringOrUndefined, Decl(narrowingOrderIndependent.ts, 3, 16))
+>a : Symbol(a, Decl(narrowingOrderIndependent.ts, 10, 5))
+>stringOrUndefined : Symbol(A.stringOrUndefined, Decl(narrowingOrderIndependent.ts, 3, 16))
+
+        new B(a.stringOrUndefined)
+>B : Symbol(B, Decl(narrowingOrderIndependent.ts, 4, 1))
+>a.stringOrUndefined : Symbol(A.stringOrUndefined, Decl(narrowingOrderIndependent.ts, 3, 16))
+>a : Symbol(a, Decl(narrowingOrderIndependent.ts, 10, 5))
+>stringOrUndefined : Symbol(A.stringOrUndefined, Decl(narrowingOrderIndependent.ts, 3, 16))
+    }
+}
+
+if (a.stringOrUndefined) {
+>a.stringOrUndefined : Symbol(A.stringOrUndefined, Decl(narrowingOrderIndependent.ts, 3, 16))
+>a : Symbol(a, Decl(narrowingOrderIndependent.ts, 10, 5))
+>stringOrUndefined : Symbol(A.stringOrUndefined, Decl(narrowingOrderIndependent.ts, 3, 16))
+
+    if (a instanceof A) {
+>a : Symbol(a, Decl(narrowingOrderIndependent.ts, 10, 5))
+>A : Symbol(A, Decl(narrowingOrderIndependent.ts, 0, 0))
+
+        new B(a.stringOrUndefined)
+>B : Symbol(B, Decl(narrowingOrderIndependent.ts, 4, 1))
+>a.stringOrUndefined : Symbol(A.stringOrUndefined, Decl(narrowingOrderIndependent.ts, 3, 16))
+>a : Symbol(a, Decl(narrowingOrderIndependent.ts, 10, 5))
+>stringOrUndefined : Symbol(A.stringOrUndefined, Decl(narrowingOrderIndependent.ts, 3, 16))
+    }
+}
+
--- a/tests/baselines/reference/narrowingOrderIndependent.types
+++ b/tests/baselines/reference/narrowingOrderIndependent.types
@@ -0,0 +1,95 @@
+=== tests/cases/compiler/narrowingOrderIndependent.ts ===
+// Repro from #36709
+
+class A {
+>A : A
+
+    constructor(public stringOrUndefined: string | undefined) {}
+>stringOrUndefined : string | undefined
+}
+
+class B {
+>B : B
+
+    constructor(public str: string) {}
+>str : string
+}
+
+const a = new A("123");
+>a : A
+>new A("123") : A
+>A : typeof A
+>"123" : "123"
+
+if (a instanceof A && a.stringOrUndefined) {
+>a instanceof A && a.stringOrUndefined : string | false | undefined
+>a instanceof A : boolean
+>a : A
+>A : typeof A
+>a.stringOrUndefined : string | undefined
+>a : A
+>stringOrUndefined : string | undefined
+
+    new B(a.stringOrUndefined)
+>new B(a.stringOrUndefined) : B
+>B : typeof B
+>a.stringOrUndefined : string
+>a : A
+>stringOrUndefined : string
+}
+
+if (a.stringOrUndefined && a instanceof A) {
+>a.stringOrUndefined && a instanceof A : boolean | "" | undefined
+>a.stringOrUndefined : string | undefined
+>a : A
+>stringOrUndefined : string | undefined
+>a instanceof A : boolean
+>a : A
+>A : typeof A
+
+    new B(a.stringOrUndefined)
+>new B(a.stringOrUndefined) : B
+>B : typeof B
+>a.stringOrUndefined : string
+>a : A
+>stringOrUndefined : string
+}
+
+if (a instanceof A) {
+>a instanceof A : boolean
+>a : A
+>A : typeof A
+
+    if (a.stringOrUndefined) {
+>a.stringOrUndefined : string | undefined
+>a : A
+>stringOrUndefined : string | undefined
+
+        new B(a.stringOrUndefined)
+>new B(a.stringOrUndefined) : B
+>B : typeof B
+>a.stringOrUndefined : string
+>a : A
+>stringOrUndefined : string
+    }
+}
+
+if (a.stringOrUndefined) {
+>a.stringOrUndefined : string | undefined
+>a : A
+>stringOrUndefined : string | undefined
+
+    if (a instanceof A) {
+>a instanceof A : boolean
+>a : A
+>A : typeof A
+
+        new B(a.stringOrUndefined)
+>new B(a.stringOrUndefined) : B
+>B : typeof B
+>a.stringOrUndefined : string
+>a : A
+>stringOrUndefined : string
+    }
+}
+
--- a/tests/cases/compiler/narrowingOrderIndependent.ts
+++ b/tests/cases/compiler/narrowingOrderIndependent.ts
@@ -0,0 +1,33 @@
+// @strict: true
+
+// Repro from #36709
+
+class A {
+    constructor(public stringOrUndefined: string | undefined) {}
+}
+
+class B {
+    constructor(public str: string) {}
+}
+
+const a = new A("123");
+
+if (a instanceof A && a.stringOrUndefined) {
+    new B(a.stringOrUndefined)
+}
+
+if (a.stringOrUndefined && a instanceof A) {
+    new B(a.stringOrUndefined)
+}
+
+if (a instanceof A) {
+    if (a.stringOrUndefined) {
+        new B(a.stringOrUndefined)
+    }
+}
+
+if (a.stringOrUndefined) {
+    if (a instanceof A) {
+        new B(a.stringOrUndefined)
+    }
+}