[NTI] Fix bug in union of nominal types, and also keep more type info…

…rmation in the union. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=146848856
google · Feb 8, 2017 · 37cb005 · 37cb005
1 parent d31b7c9
commit 37cb005
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 7 deletions.
diff --git a/src/com/google/javascript/jscomp/newtypes/FunctionType.java b/src/com/google/javascript/jscomp/newtypes/FunctionType.java
@@ -583,7 +583,7 @@ private static JSType joinNominalTypes(JSType nt1, JSType nt2) {
     NominalType n1 = getNominalTypeIfSingletonObj(nt1);
     NominalType n2 = getNominalTypeIfSingletonObj(nt2);
     if (n1 != null && n2 != null) {
-      NominalType tmp = NominalType.pickSuperclass(n1, n2);
+      NominalType tmp = NominalType.join(n1, n2);
       if (tmp != null) {
         return tmp.getInstanceAsJSType();
       }

diff --git a/src/com/google/javascript/jscomp/newtypes/NominalType.java b/src/com/google/javascript/jscomp/newtypes/NominalType.java
@@ -509,22 +509,39 @@ private boolean instantiationIsUnknownOrIdentity() {
     return true;
   }
 
-  // A special-case of join
+  private static ImmutableMap<String, JSType> joinTypeMaps(
-  static NominalType pickSuperclass(NominalType c1, NominalType c2) {
+      Set<String> domain, ImmutableMap<String, JSType> m1, ImmutableMap<String, JSType> m2) {
+    ImmutableMap.Builder<String, JSType> builder = ImmutableMap.builder();
+    for (String typevar : domain) {
+      JSType t1 = m1.get(typevar);
+      JSType t2 = m2.get(typevar);
+      if (t1 == null) {
+        builder.put(typevar, Preconditions.checkNotNull(t2));
+      } else if (t2 == null) {
+        builder.put(typevar, t1);
+      } else {
+        builder.put(typevar, JSType.join(t1, t2));
+      }
+    }
+    return builder.build();
+  }
+
+  // A special-case of join. One of the raw types must be a subtype of the other
+  static NominalType join(NominalType c1, NominalType c2) {
     if (c1 == null || c2 == null) {
       return null;
     }
     if (c1.isNominalSubtypeOf(c2)) {
       return c2;
     }
     if (c1.isRawSubtypeOf(c2)) {
-      return c2.instantiateGenericsWithUnknown();
+      return new NominalType(joinTypeMaps(c2.typeMap.keySet(), c1.typeMap, c2.typeMap), c2.rawType);
     }
     if (c2.isNominalSubtypeOf(c1)) {
       return c1;
     }
     if (c2.isRawSubtypeOf(c1)) {
-      return c1.instantiateGenericsWithUnknown();
+      return new NominalType(joinTypeMaps(c1.typeMap.keySet(), c1.typeMap, c2.typeMap), c1.rawType);
     }
     return null;
   }

diff --git a/src/com/google/javascript/jscomp/newtypes/ObjectType.java b/src/com/google/javascript/jscomp/newtypes/ObjectType.java
@@ -959,7 +959,7 @@ static ObjectType join(ObjectType obj1, ObjectType obj2) {
     } else {
       props = joinProps(obj1.props, obj2.props, nt1, nt2);
     }
-    NominalType nominal = NominalType.pickSuperclass(nt1, nt2);
+    NominalType nominal = NominalType.join(nt1, nt2);
     if (nominal.isBuiltinObject() && fn != null) {
       if (isLoose) {
         nominal = obj1.commonTypes.getFunctionType();

diff --git a/test/com/google/javascript/jscomp/NewTypeInferenceTest.java b/test/com/google/javascript/jscomp/NewTypeInferenceTest.java
@@ -13301,6 +13301,21 @@ public void testJoinWithTopObject() {
         "}"));
   }
 
+  public void testJoinOfGenericNominalTypes() {
+    typeCheck(LINE_JOINER.join(
+        "/** @constructor */",
+        "function Foo() {}",
+        "/** @constructor */",
+        "function Bar() {}",
+        "/**",
+        " * @param {(!Array<number>| !Array<!Foo> | !Array<!Bar>)} x",
+        " * @param {!Array<number>} y",
+        " */",
+        "function f(x, y, z) {",
+        "  x = y;",
+        "}"));
+  }
+
   public void testUnificationWithSubtyping() {
     typeCheck(LINE_JOINER.join(
         "/** @constructor */ function Foo() {}",
@@ -18148,8 +18163,23 @@ public void testOnlyOneInstanceOfEachClassInUnion() {
         "}",
         "function f(x) {",
         "  var y = x ? new Foo(123) : new Foo('asdf');",
-        // prop is typed ?, no warning
         "  var /** null */ n = y.prop;",
+        "}"),
+        NewTypeInference.MISTYPED_ASSIGN_RHS);
+
+    typeCheck(LINE_JOINER.join(
+        "/**",
+        " * @constructor",
+        " * @template T",
+        " * @param {T} x",
+        " */",
+        "function Foo(x) {",
+        "  /** @type {T} */",
+        "  this.prop = x;",
+        "}",
+        "function f(x) {",
+        "  var y = x ? new Foo(123) : new Foo('asdf');",
+        "  var /** (number|string) */ n = y.prop;",
         "}"));
 
     typeCheck(LINE_JOINER.join(