Fix some spurious warnings on destructuring in CheckGlobalNames

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=209004728

src/com/google/javascript/jscomp/CheckGlobalNames.java

@@ -248,43 +248,33 @@ private boolean propertyMustBeInitializedByFullName(Name name) {
       return false;
     }
 
-    boolean parentIsAliased = false;
-    if (name.parent.aliasingGets > 0) {
-      for (Ref ref : name.parent.getRefs()) {
-        if (ref.type == Ref.Type.ALIASING_GET) {
-          Node aliaser = ref.getNode().getParent();
-
-          // We don't need to worry about known aliased, because
-          // they're already covered by the getIndirectlyDeclaredProperties
-          // call at the top.
-          boolean isKnownAlias =
-              aliaser.isCall() &&
-              (convention.getClassesDefinedByCall(aliaser) != null ||
-               convention.getSingletonGetterClassName(aliaser) != null);
-          if (!isKnownAlias) {
-            parentIsAliased = true;
-          }
-        }
-      }
-    }
-
-    if (parentIsAliased) {
+    if (isNameUnsafelyAliased(name.parent)) {
+      // e.g. if we have `const ns = {}; escape(ns); alert(ns.a.b);`
+      // we don't expect ns.a.b to be defined somewhere because `ns` has escaped
       return false;
     }
 
     if (objectPrototypeProps.contains(name.getBaseName())) {
+      // checks for things on Object.prototype, e.g. a call to
+      // something.hasOwnProperty('a');
       return false;
     }
 
     if (name.parent.type == Name.Type.OBJECTLIT) {
+      // if this is a property on an object literal, always expect an initialization somewhere
       return true;
     }
 
     if (name.parent.type == Name.Type.CLASS) {
+      // only warn on class properties if there is no superclass, because we don't handle
+      // class side inheritance here very well yet.
       return !hasSuperclass(name.parent);
     }
 
-    return name.parent.type == Name.Type.FUNCTION && name.parent.isDeclaredType()
+    // warn on remaining names if they are on a constructor and are not a Function.prototype
+    // property (e.g. f.call(1);)
+    return name.parent.type == Name.Type.FUNCTION
+        && name.parent.isDeclaredType()
         && !functionPrototypeProps.contains(name.getBaseName());
   }
 
@@ -297,4 +287,26 @@ private boolean hasSuperclass(Name es6Class) {
 
     return !superclass.isEmpty();
   }
+
+  private boolean isNameUnsafelyAliased(Name name) {
+    if (name.aliasingGets > 0) {
+      for (Ref ref : name.getRefs()) {
+        if (ref.type == Ref.Type.ALIASING_GET) {
+          Node aliaser = ref.getNode().getParent();
+
+          // We don't need to worry about known aliased, because
+          // they're already covered by the getIndirectlyDeclaredProperties
+          // call at the top.
+          boolean isKnownAlias =
+              aliaser.isCall()
+                  && (convention.getClassesDefinedByCall(aliaser) != null
+                      || convention.getSingletonGetterClassName(aliaser) != null);
+          if (!isKnownAlias) {
+            return true;
+          }
+        }
+      }
+    }
+    return name.parent != null && isNameUnsafelyAliased(name.parent);
+  }
 }

src/com/google/javascript/jscomp/GlobalNamespace.java

@@ -402,10 +402,16 @@ public void collect(JSModule module, Scope scope, Node n) {
                 type = Name.Type.CLASS;
               }
               break;
+            case STRING_KEY:
             case ARRAY_PATTERN:
-              // Specific case to handle inlining with array destructuring
-              isSet = true;
-              type = Name.Type.OTHER;
+            case DEFAULT_VALUE:
+            case COMPUTED_PROP:
+            case REST:
+              // This may be a set.
+              if (NodeUtil.isLhsByDestructuring(n)) {
+                isSet = true;
+                type = Name.Type.OTHER;
+              }
               break;
             default:
               if (NodeUtil.isAssignmentOp(parent) && parent.getFirstChild() == n) {

test/com/google/javascript/jscomp/CheckGlobalNamesTest.java

@@ -415,7 +415,8 @@ public void testCustomObjectPrototypeProperties() {
   }
 
   public void testFunctionPrototypeProperties() {
-    testSame("var x = {}; var y = x.hasOwnProperty('z');");
+    // don't warn for "Foo.call"
+    testSame("/** @constructor */ function Foo() {}; Foo.call({});");
   }
 
   public void testIndirectlyDeclaredProperties() {
@@ -489,4 +490,68 @@ public void testEs6NonSubclass_stillWarnsForMissingProperty() {
     // We still do warn for undefined properties on an ES6 class with no superclass
     testWarning("class Child {} Child.f();", UNDEFINED_NAME_WARNING);
   }
+
+  public void testObjectDestructuringAlias() {
+    testSame(
+        lines(
+            "var ns = {};",
+            "/** @enum */",
+            "ns.AdjustMode = {SELECT: 0, MOVE: 1};",
+            "const {AdjustMode} = ns;",
+            "alert(AdjustMode.SELECT);"));
+  }
+
+  public void testObjectDestructuringAlias_computedProperty() {
+    testSame(
+        lines(
+            "var ns = {};",
+            "/** @enum */",
+            "ns.AdjustMode = {SELECT: 0, MOVE: 1};",
+            "const {['AdjustMode']: AdjustMode} = ns;",
+            "alert(AdjustMode.SELECT);"));
+  }
+
+  public void testObjectDestructuringAlias_defaultValue() {
+    testSame(
+        lines(
+            "var ns = {};",
+            "/** @enum */",
+            "ns.AdjustMode = {SELECT: 0, MOVE: 1};",
+            "const {AdjustMode = {}} = ns;",
+            "alert(AdjustMode.SELECT);"));
+  }
+
+  public void testArrayDestructuring() {
+    // Currently, we never issue warnings for property access on an object created through
+    // destructuring.
+    testSame(lines("const [ns] = [{foo: 3}];", "alert(ns.foo);", "alert(ns.bar);")); // undefined
+  }
+
+  public void testArrayDestructuring_rest() {
+    testSame(
+        lines(
+            "const [...ns] = [{foo: 3}];", //
+            "alert(ns[0].foo);",
+            "alert(ns[0].bar);"));
+  }
+
+  public void testPropertyCreationOnDestructuringAlias() {
+    testSame(
+        lines(
+            "var ns = {};",
+            "ns.AdjustMode = {SELECT: 0, MOVE: 1};",
+            "const {AdjustMode} = ns;",
+            "AdjustMode.OTHER = 2;",
+            "alert(ns.AdjustMode.OTHER);"));
+  }
+
+  public void testNamespaceAliasPreventsWarning() {
+    // When a variable is aliased, we back off warning for potentially missing properties on that
+    // variable.
+    testSame(
+        lines(
+            "var a = {};",
+            "var alias = a;",
+            "alert(a.b.c.d);")); // This will cause a runtime error but not a compiler warning.
+  }
 }