Automated g4 rollback of changelist 196569438.

*** Reason for rollback *** Breaks continuous builds *** Original change description *** Improve handling of symbols for types with known computed property access values types (IObject subclasses such as Array). Specifically: * when an index type is known well known Symbol. * allow well known symbols for index'd types. This correct the issue of Symbol.iterator being typed incorrectly by typing them as unknown. *** ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=196579719
google · May 15, 2018 · 765ff73 · 765ff73
1 parent 3cfa2f7
commit 765ff73
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 47 deletions.
diff --git a/src/com/google/javascript/jscomp/TypeInference.java b/src/com/google/javascript/jscomp/TypeInference.java
@@ -1587,22 +1587,12 @@ private FlowScope traverseChildren(Node n, FlowScope scope) {
 
   private FlowScope traverseGetElem(Node n, FlowScope scope) {
     scope = traverseChildren(n, scope);
-    Node indexKey = n.getLastChild();
-    JSType indexType = getJSType(indexKey);
-    if (indexType.isSymbolValueType()) {
-      // For now, allow symbols definitions/access on any type. In the future only allow them
-      // on the subtypes for which they are defined.
-
-      // TODO(b/77474174): Type well known symbol accesses.
-      n.setJSType(unknownType);
-    } else {
-      JSType type = getJSType(n.getFirstChild()).restrictByNotNullOrUndefined();
-      TemplateTypeMap typeMap = type.getTemplateTypeMap();
-      if (typeMap.hasTemplateType(registry.getObjectElementKey())) {
-        n.setJSType(typeMap.getResolvedTemplateType(registry.getObjectElementKey()));
-      }
+    JSType type = getJSType(n.getFirstChild()).restrictByNotNullOrUndefined();
+    TemplateTypeMap typeMap = type.getTemplateTypeMap();
+    if (typeMap.hasTemplateType(registry.getObjectElementKey())) {
+      n.setJSType(typeMap.getResolvedTemplateType(registry.getObjectElementKey()));
     }
-    return tightenTypeAfterDereference(n.getFirstChild(), scope);
+    return dereferencePointer(n.getFirstChild(), scope);
   }
 
   private FlowScope traverseGetProp(Node n, FlowScope scope) {
@@ -1613,7 +1603,7 @@ private FlowScope traverseGetProp(Node n, FlowScope scope) {
     n.setJSType(
         getPropertyType(
             objNode.getJSType(), property.getString(), n, scope));
-    return tightenTypeAfterDereference(n.getFirstChild(), scope);
+    return dereferencePointer(n.getFirstChild(), scope);
   }
 
   /**
@@ -1643,7 +1633,7 @@ private static void inferPropertyTypesToMatchConstraint(
    * If we access a property of a symbol, then that symbol is not
    * null or undefined.
    */
-  private FlowScope tightenTypeAfterDereference(Node n, FlowScope scope) {
+  private FlowScope dereferencePointer(Node n, FlowScope scope) {
     if (n.isQualifiedName()) {
       JSType type = getJSType(n);
       JSType narrowed = type.restrictByNotNullOrUndefined();

diff --git a/src/com/google/javascript/jscomp/TypeValidator.java b/src/com/google/javascript/jscomp/TypeValidator.java
@@ -504,12 +504,7 @@ void expectSwitchMatchesCase(NodeTraversal t, Node n, JSType switchType, JSType
   void expectIndexMatch(NodeTraversal t, Node n, JSType objType, JSType indexType) {
     checkState(n.isGetElem() || n.isComputedProp(), n);
     Node indexNode = n.isGetElem() ? n.getLastChild() : n.getFirstChild();
-    if (indexType.isSymbolValueType()) {
-      // For now, allow symbols definitions/access on any type. In the future only allow them
-      // on the subtypes for which they are defined.
-      return;
-    }
-    if (objType.isStruct()) {
+    if (objType.isStruct() && !isWellKnownSymbol(indexNode)) {
       report(JSError.make(indexNode,
                           ILLEGAL_PROPERTY_ACCESS, "'[]'", "struct"));
     }
@@ -535,6 +530,14 @@ void expectIndexMatch(NodeTraversal t, Node n, JSType objType, JSType indexType)
     }
   }
 
+  // TODO(sdh): Replace isWellKnownSymbol with a real type-based
+  // check once the type system understands the symbol primitive.
+  // Any @const symbol reference should be allowed for a @struct.
+  private static boolean isWellKnownSymbol(Node n) {
+    return n.isGetProp() && n.getFirstChild().isName()
+        && n.getFirstChild().getString().equals("Symbol");
+  }
+
   /**
    * Expect that the first type can be assigned to a symbol of the second
    * type.

diff --git a/test/com/google/javascript/jscomp/TypeCheckTest.java b/test/com/google/javascript/jscomp/TypeCheckTest.java
@@ -668,30 +668,6 @@ public void testBooleanPreservation4() {
         "required: boolean");
   }
 
-  public void testWellKnownSymbolAccess1() {
-    testTypesWithCommonExterns(
-        lines(
-            "/**",
-            " * @param {Array<string>} x",
-            " */",
-            "function f(x) {",
-            "  const iter = x[Symbol.iterator]();",
-            "}"
-        ));
-  }
-
-  public void testWellKnownSymbolAccess2() {
-    testTypesWithCommonExterns(
-        lines(
-            "/**",
-            " * @param {IObject<string, number>} x",
-            " */",
-            "function f(x) {",
-            "  const iter = x[Symbol.iterator]();",
-            "}"
-        ));
-  }
-
   public void testSymbolComparison1() {
     testTypes(
         lines(