Merge pull request #29110 from Microsoft/fixDiscriminantCheck

ahejlsberg · web-flow · commit beebda3574be · 2018-12-30T17:46:01.000-10:00
Fix discriminant property check
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -7699,38 +7699,37 @@ namespace ts {
                 return props[0];
             }
             let declarations: Declaration[] | undefined;
-            let commonType: Type | undefined;
+            let firstType: Type | undefined;
             let nameType: Type | undefined;
             const propTypes: Type[] = [];
-            let first = true;
-            let commonValueDeclaration: Declaration | undefined;
+            let firstValueDeclaration: Declaration | undefined;
             let hasNonUniformValueDeclaration = false;
             for (const prop of props) {
-                if (!commonValueDeclaration) {
-                    commonValueDeclaration = prop.valueDeclaration;
+                if (!firstValueDeclaration) {
+                    firstValueDeclaration = prop.valueDeclaration;
                 }
-                else if (prop.valueDeclaration !== commonValueDeclaration) {
+                else if (prop.valueDeclaration !== firstValueDeclaration) {
                     hasNonUniformValueDeclaration = true;
                 }
                 declarations = addRange(declarations, prop.declarations);
                 const type = getTypeOfSymbol(prop);
-                if (first) {
-                    commonType = type;
+                if (!firstType) {
+                    firstType = type;
                     nameType = prop.nameType;
-                    first = false;
                 }
-                else {
-                    if (type !== commonType) {
-                        checkFlags |= CheckFlags.HasNonUniformType;
-                    }
+                else if (type !== firstType) {
+                    checkFlags |= CheckFlags.HasNonUniformType;
+                }
+                if (isLiteralType(type)) {
+                    checkFlags |= CheckFlags.HasLiteralType;
                 }
                 propTypes.push(type);
             }
             addRange(propTypes, indexTypes);
             const result = createSymbol(SymbolFlags.Property | commonFlags, name, syntheticFlag | checkFlags);
             result.containingType = containingType;
-            if (!hasNonUniformValueDeclaration && commonValueDeclaration) {
-                result.valueDeclaration = commonValueDeclaration;
+            if (!hasNonUniformValueDeclaration && firstValueDeclaration) {
+                result.valueDeclaration = firstValueDeclaration;
             }
             result.declarations = declarations!;
             result.nameType = nameType;
@@ -14839,25 +14838,18 @@ namespace ts {
         }
 
         function isDiscriminantType(type: Type): boolean {
-            if (type.flags & TypeFlags.Union) {
-                if (type.flags & (TypeFlags.Boolean | TypeFlags.EnumLiteral)) {
-                    return true;
-                }
-                let combined = 0;
-                for (const t of (<UnionType>type).types) combined |= t.flags;
-                if (combined & TypeFlags.Unit && !(combined & TypeFlags.Instantiable)) {
-                    return true;
-                }
-            }
-            return false;
+            return !!(type.flags & TypeFlags.Union &&
+                (type.flags & (TypeFlags.Boolean | TypeFlags.EnumLiteral) || !isGenericIndexType(type)));
         }
 
         function isDiscriminantProperty(type: Type | undefined, name: __String) {
             if (type && type.flags & TypeFlags.Union) {
                 const prop = getUnionOrIntersectionProperty(<UnionType>type, name);
                 if (prop && getCheckFlags(prop) & CheckFlags.SyntheticProperty) {
                     if ((<TransientSymbol>prop).isDiscriminantProperty === undefined) {
-                        (<TransientSymbol>prop).isDiscriminantProperty = !!((<TransientSymbol>prop).checkFlags & CheckFlags.HasNonUniformType) && isDiscriminantType(getTypeOfSymbol(prop));
+                        (<TransientSymbol>prop).isDiscriminantProperty =
+                            ((<TransientSymbol>prop).checkFlags & CheckFlags.Discriminant) === CheckFlags.Discriminant &&
+                            isDiscriminantType(getTypeOfSymbol(prop));
                     }
                     return !!(<TransientSymbol>prop).isDiscriminantProperty;
                 }
diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -3677,15 +3677,17 @@ namespace ts {
         Readonly          = 1 << 3,         // Readonly transient symbol
         Partial           = 1 << 4,         // Synthetic property present in some but not all constituents
         HasNonUniformType = 1 << 5,         // Synthetic property with non-uniform type in constituents
-        ContainsPublic    = 1 << 6,         // Synthetic property with public constituent(s)
-        ContainsProtected = 1 << 7,         // Synthetic property with protected constituent(s)
-        ContainsPrivate   = 1 << 8,         // Synthetic property with private constituent(s)
-        ContainsStatic    = 1 << 9,         // Synthetic property with static constituent(s)
-        Late              = 1 << 10,        // Late-bound symbol for a computed property with a dynamic name
-        ReverseMapped     = 1 << 11,        // Property of reverse-inferred homomorphic mapped type
-        OptionalParameter = 1 << 12,        // Optional parameter
-        RestParameter     = 1 << 13,        // Rest parameter
-        Synthetic = SyntheticProperty | SyntheticMethod
+        HasLiteralType    = 1 << 6,         // Synthetic property with at least one literal type in constituents
+        ContainsPublic    = 1 << 7,         // Synthetic property with public constituent(s)
+        ContainsProtected = 1 << 8,         // Synthetic property with protected constituent(s)
+        ContainsPrivate   = 1 << 9,         // Synthetic property with private constituent(s)
+        ContainsStatic    = 1 << 10,        // Synthetic property with static constituent(s)
+        Late              = 1 << 11,        // Late-bound symbol for a computed property with a dynamic name
+        ReverseMapped     = 1 << 12,        // Property of reverse-inferred homomorphic mapped type
+        OptionalParameter = 1 << 13,        // Optional parameter
+        RestParameter     = 1 << 14,        // Rest parameter
+        Synthetic = SyntheticProperty | SyntheticMethod,
+        Discriminant = HasNonUniformType | HasLiteralType
     }
 
     /* @internal */
diff --git a/tests/baselines/reference/discriminantPropertyCheck.errors.txt b/tests/baselines/reference/discriminantPropertyCheck.errors.txt
@@ -106,4 +106,26 @@ tests/cases/compiler/discriminantPropertyCheck.ts(65,9): error TS2532: Object is
             }
         }
     }
+    
+    // Repro from #29106
+    
+    const f = (_a: string, _b: string): void => {};
+    
+    interface A {
+      a?: string;
+      b?: string;
+    }
+    
+    interface B {
+      a: string;
+      b: string;
+    }
+    
+    type U = A | B;
+    
+    const u: U = {} as any;
+    
+    u.a && u.b && f(u.a, u.b);
+    
+    u.b && u.a && f(u.a, u.b);
     
diff --git a/tests/baselines/reference/discriminantPropertyCheck.js b/tests/baselines/reference/discriminantPropertyCheck.js
@@ -98,6 +98,28 @@ function func2(inst: Instance) {
         }
     }
 }
+
+// Repro from #29106
+
+const f = (_a: string, _b: string): void => {};
+
+interface A {
+  a?: string;
+  b?: string;
+}
+
+interface B {
+  a: string;
+  b: string;
+}
+
+type U = A | B;
+
+const u: U = {} as any;
+
+u.a && u.b && f(u.a, u.b);
+
+u.b && u.a && f(u.a, u.b);
 
 
 //// [discriminantPropertyCheck.js]
@@ -161,3 +183,8 @@ function func2(inst) {
         }
     }
 }
+// Repro from #29106
+var f = function (_a, _b) { };
+var u = {};
+u.a && u.b && f(u.a, u.b);
+u.b && u.a && f(u.a, u.b);
diff --git a/tests/baselines/reference/discriminantPropertyCheck.symbols b/tests/baselines/reference/discriminantPropertyCheck.symbols
@@ -311,3 +311,69 @@ function func2(inst: Instance) {
     }
 }
 
+// Repro from #29106
+
+const f = (_a: string, _b: string): void => {};
+>f : Symbol(f, Decl(discriminantPropertyCheck.ts, 102, 5))
+>_a : Symbol(_a, Decl(discriminantPropertyCheck.ts, 102, 11))
+>_b : Symbol(_b, Decl(discriminantPropertyCheck.ts, 102, 22))
+
+interface A {
+>A : Symbol(A, Decl(discriminantPropertyCheck.ts, 102, 47))
+
+  a?: string;
+>a : Symbol(A.a, Decl(discriminantPropertyCheck.ts, 104, 13))
+
+  b?: string;
+>b : Symbol(A.b, Decl(discriminantPropertyCheck.ts, 105, 13))
+}
+
+interface B {
+>B : Symbol(B, Decl(discriminantPropertyCheck.ts, 107, 1))
+
+  a: string;
+>a : Symbol(B.a, Decl(discriminantPropertyCheck.ts, 109, 13))
+
+  b: string;
+>b : Symbol(B.b, Decl(discriminantPropertyCheck.ts, 110, 12))
+}
+
+type U = A | B;
+>U : Symbol(U, Decl(discriminantPropertyCheck.ts, 112, 1))
+>A : Symbol(A, Decl(discriminantPropertyCheck.ts, 102, 47))
+>B : Symbol(B, Decl(discriminantPropertyCheck.ts, 107, 1))
+
+const u: U = {} as any;
+>u : Symbol(u, Decl(discriminantPropertyCheck.ts, 116, 5))
+>U : Symbol(U, Decl(discriminantPropertyCheck.ts, 112, 1))
+
+u.a && u.b && f(u.a, u.b);
+>u.a : Symbol(a, Decl(discriminantPropertyCheck.ts, 104, 13), Decl(discriminantPropertyCheck.ts, 109, 13))
+>u : Symbol(u, Decl(discriminantPropertyCheck.ts, 116, 5))
+>a : Symbol(a, Decl(discriminantPropertyCheck.ts, 104, 13), Decl(discriminantPropertyCheck.ts, 109, 13))
+>u.b : Symbol(b, Decl(discriminantPropertyCheck.ts, 105, 13), Decl(discriminantPropertyCheck.ts, 110, 12))
+>u : Symbol(u, Decl(discriminantPropertyCheck.ts, 116, 5))
+>b : Symbol(b, Decl(discriminantPropertyCheck.ts, 105, 13), Decl(discriminantPropertyCheck.ts, 110, 12))
+>f : Symbol(f, Decl(discriminantPropertyCheck.ts, 102, 5))
+>u.a : Symbol(a, Decl(discriminantPropertyCheck.ts, 104, 13), Decl(discriminantPropertyCheck.ts, 109, 13))
+>u : Symbol(u, Decl(discriminantPropertyCheck.ts, 116, 5))
+>a : Symbol(a, Decl(discriminantPropertyCheck.ts, 104, 13), Decl(discriminantPropertyCheck.ts, 109, 13))
+>u.b : Symbol(b, Decl(discriminantPropertyCheck.ts, 105, 13), Decl(discriminantPropertyCheck.ts, 110, 12))
+>u : Symbol(u, Decl(discriminantPropertyCheck.ts, 116, 5))
+>b : Symbol(b, Decl(discriminantPropertyCheck.ts, 105, 13), Decl(discriminantPropertyCheck.ts, 110, 12))
+
+u.b && u.a && f(u.a, u.b);
+>u.b : Symbol(b, Decl(discriminantPropertyCheck.ts, 105, 13), Decl(discriminantPropertyCheck.ts, 110, 12))
+>u : Symbol(u, Decl(discriminantPropertyCheck.ts, 116, 5))
+>b : Symbol(b, Decl(discriminantPropertyCheck.ts, 105, 13), Decl(discriminantPropertyCheck.ts, 110, 12))
+>u.a : Symbol(a, Decl(discriminantPropertyCheck.ts, 104, 13), Decl(discriminantPropertyCheck.ts, 109, 13))
+>u : Symbol(u, Decl(discriminantPropertyCheck.ts, 116, 5))
+>a : Symbol(a, Decl(discriminantPropertyCheck.ts, 104, 13), Decl(discriminantPropertyCheck.ts, 109, 13))
+>f : Symbol(f, Decl(discriminantPropertyCheck.ts, 102, 5))
+>u.a : Symbol(a, Decl(discriminantPropertyCheck.ts, 104, 13), Decl(discriminantPropertyCheck.ts, 109, 13))
+>u : Symbol(u, Decl(discriminantPropertyCheck.ts, 116, 5))
+>a : Symbol(a, Decl(discriminantPropertyCheck.ts, 104, 13), Decl(discriminantPropertyCheck.ts, 109, 13))
+>u.b : Symbol(b, Decl(discriminantPropertyCheck.ts, 105, 13), Decl(discriminantPropertyCheck.ts, 110, 12))
+>u : Symbol(u, Decl(discriminantPropertyCheck.ts, 116, 5))
+>b : Symbol(b, Decl(discriminantPropertyCheck.ts, 105, 13), Decl(discriminantPropertyCheck.ts, 110, 12))
+
diff --git a/tests/baselines/reference/discriminantPropertyCheck.types b/tests/baselines/reference/discriminantPropertyCheck.types
@@ -310,3 +310,71 @@ function func2(inst: Instance) {
     }
 }
 
+// Repro from #29106
+
+const f = (_a: string, _b: string): void => {};
+>f : (_a: string, _b: string) => void
+>(_a: string, _b: string): void => {} : (_a: string, _b: string) => void
+>_a : string
+>_b : string
+
+interface A {
+  a?: string;
+>a : string | undefined
+
+  b?: string;
+>b : string | undefined
+}
+
+interface B {
+  a: string;
+>a : string
+
+  b: string;
+>b : string
+}
+
+type U = A | B;
+>U : U
+
+const u: U = {} as any;
+>u : U
+>{} as any : any
+>{} : {}
+
+u.a && u.b && f(u.a, u.b);
+>u.a && u.b && f(u.a, u.b) : void | "" | undefined
+>u.a && u.b : string | undefined
+>u.a : string | undefined
+>u : U
+>a : string | undefined
+>u.b : string | undefined
+>u : U
+>b : string | undefined
+>f(u.a, u.b) : void
+>f : (_a: string, _b: string) => void
+>u.a : string
+>u : U
+>a : string
+>u.b : string
+>u : U
+>b : string
+
+u.b && u.a && f(u.a, u.b);
+>u.b && u.a && f(u.a, u.b) : void | "" | undefined
+>u.b && u.a : string | undefined
+>u.b : string | undefined
+>u : U
+>b : string | undefined
+>u.a : string | undefined
+>u : U
+>a : string | undefined
+>f(u.a, u.b) : void
+>f : (_a: string, _b: string) => void
+>u.a : string
+>u : U
+>a : string
+>u.b : string
+>u : U
+>b : string
+
diff --git a/tests/cases/compiler/discriminantPropertyCheck.ts b/tests/cases/compiler/discriminantPropertyCheck.ts
@@ -99,3 +99,25 @@ function func2(inst: Instance) {
         }
     }
 }
+
+// Repro from #29106
+
+const f = (_a: string, _b: string): void => {};
+
+interface A {
+  a?: string;
+  b?: string;
+}
+
+interface B {
+  a: string;
+  b: string;
+}
+
+type U = A | B;
+
+const u: U = {} as any;
+
+u.a && u.b && f(u.a, u.b);
+
+u.b && u.a && f(u.a, u.b);
