Warn for quoted struct access and unquoted dict access in an object p…

…attern e.g /** @struct */ const obj = {a: 1}; const {'a': a} = obj; ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=222440708
google · Nov 26, 2018 · f72f181 · f72f181
1 parent 8107b34
commit f72f181
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 22 deletions.
diff --git a/src/com/google/javascript/jscomp/TypeCheck.java b/src/com/google/javascript/jscomp/TypeCheck.java
@@ -891,27 +891,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
         break;
 
       case OBJECT_PATTERN:
-        // We only check that COMPUTED_PROP keys are valid here
+        visitObjectPattern(t, n);
-        // Other checks for object patterns are done when visiting DEFAULT_VALUEs or assignments/
-        // declarations
-        JSType patternType = getJSType(n);
-        for (Node child : n.children()) {
-          DestructuredTarget target =
-              DestructuredTarget.createTarget(typeRegistry, patternType, child);
-
-          if (target.hasComputedProperty()) {
-            Node computedProperty = target.getComputedProperty();
-            validator.expectIndexMatch(
-                t, computedProperty, patternType, getJSType(computedProperty.getFirstChild()));
-          }
-
-          if (target.hasStringKey() && !target.getStringKey().isQuotedString()) {
-            // check `const {a} = obj;` but not `const {'a': a} = obj;`
-            checkPropertyAccessForDestructuring(
-                t, n, getJSType(n), target.getStringKey(), getJSType(target.getNode()));
-          }
-        }
-        ensureTyped(n);
         break;
 
       case DEFAULT_VALUE:
@@ -1301,6 +1281,39 @@ private void checkPropertyInheritance(
     }
   }
 
+  /**
+   * Validates all keys in an object pattern
+   *
+   * <p>Validating the types assigned to any lhs nodes in the pattern is done at the ASSIGN/VAR/
+   * PARAM_LIST/etc. node
+   */
+  private void visitObjectPattern(NodeTraversal t, Node pattern) {
+    JSType patternType = getJSType(pattern);
+    for (Node child : pattern.children()) {
+      DestructuredTarget target = DestructuredTarget.createTarget(typeRegistry, patternType, child);
+
+      if (target.hasComputedProperty()) {
+        Node computedProperty = target.getComputedProperty();
+        validator.expectIndexMatch(
+            t, computedProperty, patternType, getJSType(computedProperty.getFirstChild()));
+      } else if (target.hasStringKey()) {
+        Node stringKey = target.getStringKey();
+        if (!stringKey.isQuotedString()) {
+          if (patternType.isDict()) {
+            report(t, stringKey, TypeValidator.ILLEGAL_PROPERTY_ACCESS, "unquoted", "dict");
+          }
+          // check for missing properties given `const {a} = obj;` but not `const {'a': a} = obj;`
+          checkPropertyAccessForDestructuring(
+              t, pattern, patternType, stringKey, getJSType(target.getNode()));
+        } else if (patternType.isStruct()) {
+          // check that we are not accessing a struct with a quoted string
+          report(t, stringKey, TypeValidator.ILLEGAL_PROPERTY_ACCESS, "quoted", "struct");
+        }
+      }
+    }
+    ensureTyped(pattern);
+  }
+
   /**
    * Visits an object literal field definition <code>key : value</code>, or a class member
    * definition <code>key() { ... }</code> If the <code>lvalue</code> is a prototype modification,

diff --git a/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java b/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java
@@ -4898,7 +4898,7 @@ public void testArrayPatternAssign_badPropertyAssignment() {
   }
 
   @Test
-  public void testBadComputedPropertyKeyInObjectPattern() {
+  public void testBadComputedPropertyKeyTypeInObjectPattern() {
     testTypes(
         "const {[{}]: x} = {};",
         lines(
@@ -4907,6 +4907,27 @@ public void testBadComputedPropertyKeyInObjectPattern() {
             "required: (string|symbol)"));
   }
 
+  @Test
+  public void testComputedPropertyAccessOnStructInObjectPattern() {
+    testTypes(
+        "/** @struct */ const myStruct = {a: 1}; const {['a']: a} = myStruct;",
+        "Cannot do '[]' access on a struct");
+  }
+
+  @Test
+  public void testQuotedPropertyAccessOnStructInObjectPattern() {
+    testTypes(
+        "/** @struct */ const myStruct = {a: 1}; const {'a': a} = myStruct;",
+        "Cannot do quoted access on a struct");
+  }
+
+  @Test
+  public void testNonQuotedPropertyAccessOnDictInObjectPattern() {
+    testTypes(
+        "/** @dict*/ const myDict = {'a': 1}; const {a} = myDict;",
+        "Cannot do unquoted access on a dict");
+  }
+
   @Test
   public void testRestrictedIndexTypeInComputedPropertyKeyInObjectPattern() {
     testTypes(