Merge remote-tracking branch 'upstream/master'

Author: ctjlewis

externs/browser/html5.js

@@ -3766,7 +3766,8 @@ XMLHttpRequest.prototype.response;
  *     refreshPolicy: string,
  *     signRequestData: string,
  *     includeTimestampHeader: boolean,
- *     additionalSignedHeaders: !Array<string>
+ *     additionalSignedHeaders: (!Array<string>|undefined),
+ *     additionalSigningData: (string|undefined)
  *   }}
  * @see https://docs.google.com/document/d/1qUjtKgA7nMv9YGMhi0xWKEojkSITKzGLdIcZgoz6ZkI
  */

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

@@ -444,6 +444,7 @@ private void collect(JSModule module, Scope scope, Node n) {
           type = getValueType(n.getFirstChild());
           break;
         case NAME:
+        case GETPROP:
           // TODO(b/127505242): CAST parents may indicate a set.
           // This may be a variable get or set.
           switch (parent.getToken()) {
@@ -492,7 +493,9 @@ private void collect(JSModule module, Scope scope, Node n) {
             case ITER_REST:
             case OBJECT_REST:
               // This may be a set.
-              if (NodeUtil.isLhsByDestructuring(n)) {
+              // TODO(b/120303257): this should extend to qnames too, but doing
+              // so causes invalid output. Covered in CollapsePropertiesTest
+              if (n.isName() && NodeUtil.isLhsByDestructuring(n)) {
                 isSet = true;
                 type = NameType.OTHER;
               }
@@ -506,42 +509,12 @@ private void collect(JSModule module, Scope scope, Node n) {
                 type = NameType.OTHER;
               }
           }
-          name = n.getString();
-          break;
-        case GETPROP:
-          // TODO(b/117673791): Merge this case with NAME case to fix.
-          // TODO(b/120303257): Merging this case with the NAME case makes this a breaking bug.
-          // TODO(b/127505242): CAST parents may indicate a set.
-          // This may be a namespaced name get or set.
-          if (parent != null) {
-            switch (parent.getToken()) {
-              case ASSIGN:
-                if (parent.getFirstChild() == n) {
-                  isSet = true;
-                  type = getValueType(n.getNext());
-                }
-                break;
-              case GETPROP:
-                // This is nested in another getprop. Return and only create a Ref for the outermost
-                // getprop in the chain.
-                return;
-              case INC:
-              case DEC:
-              case ITER_SPREAD:
-              case OBJECT_SPREAD:
-                break; // isSet = false, type = OTHER.
-              default:
-                if (NodeUtil.isAssignmentOp(parent) && parent.getFirstChild() == n) {
-                  isSet = true;
-                  type = NameType.OTHER;
-                }
-            }
-          }
           if (!n.isQualifiedName()) {
             return;
           }
           name = n.getQualifiedName();
           break;
+
         case CALL:
           if (isObjectHasOwnPropertyCall(n)) {
             String qname = n.getFirstFirstChild().getQualifiedName();

src/com/google/javascript/jscomp/colors/Color.java

@@ -25,5 +25,7 @@ public interface Color {
 
   boolean isUnion();
 
+  boolean isObject();
+
   ImmutableCollection<Color> getAlternates();
 }

src/com/google/javascript/jscomp/colors/ObjectColor.java

@@ -0,0 +1,55 @@
+/*
+ * Copyright 2020 The Closure Compiler Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.javascript.jscomp.colors;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableCollection;
+
+/**
+ * A user-defined object type. For now each type is defined by a unique class name and file source.
+ */
+@AutoValue
+public abstract class ObjectColor implements Color {
+
+  public static ObjectColor create(String className, String fileName) {
+    return new AutoValue_ObjectColor(className, fileName);
+  }
+
+  @Override
+  public boolean isPrimitive() {
+    return false;
+  }
+
+  @Override
+  public boolean isUnion() {
+    return false;
+  }
+
+  @Override
+  public boolean isObject() {
+    return true;
+  }
+
+  @Override
+  public ImmutableCollection<Color> getAlternates() {
+    throw new UnsupportedOperationException();
+  }
+
+  public abstract String getClassName();
+
+  public abstract String getFilename();
+}

src/com/google/javascript/jscomp/colors/PrimitiveColor.java

@@ -40,6 +40,11 @@ public boolean isUnion() {
     return false;
   }
 
+  @Override
+  public boolean isObject() {
+    return false;
+  }
+
   @Override
   public ImmutableCollection<Color> getAlternates() {
     // In theory we could consider a primitive a union of a single value. It's not clear whether

src/com/google/javascript/jscomp/colors/UnionColor.java

@@ -53,6 +53,11 @@ public boolean isUnion() {
     return true;
   }
 
+  @Override
+  public boolean isObject() {
+    return false;
+  }
+
   @Override
   public abstract ImmutableCollection<Color> getAlternates();
 }

src/com/google/javascript/jscomp/resources/resources.json

@@ -56,6 +56,10 @@ public void isUnion() {
     check("isUnion").that(actualNonNull().isUnion()).isTrue();
   }
 
+  public void isObject() {
+    check("isObject").that(actualNonNull().isObject()).isTrue();
+  }
+
   public void hasAlternates(Color... alternates) {
     isUnion();
     check("getAlternates().containsExactly()")

src/com/google/javascript/jscomp/testing/ColorSubject.java

@@ -1249,6 +1249,48 @@ public void testGenericPrototypeObject() {
     test(js, output);
   }
 
+  @Test
+  public void testRelatedInstanceAndPrototypePropNotAmbiguated() {
+    test(
+        lines(
+            "class Foo {", //
+            "  constructor() {",
+            "    this.y = 0;",
+            "  }",
+            "  x() {}",
+            "}"),
+        lines(
+            "class Foo {", //
+            "  constructor() {",
+            "    this.b = 0;",
+            "  }",
+            "  a() {}",
+            "}"));
+  }
+
+  @Test
+  public void testRelatedInstanceAndSuperclassPrototypePropNotAmbiguated() {
+    test(
+        lines(
+            "class Foo {", //
+            "  x() {}",
+            "}",
+            "class Bar extends Foo {",
+            "  constructor() {",
+            "    this.y = 0;",
+            "  }",
+            "}"),
+        lines(
+            "class Foo {", //
+            "  a() {}",
+            "}",
+            "class Bar extends Foo {",
+            "  constructor() {",
+            "    this.b = 0;",
+            "  }",
+            "}"));
+  }
+
   @Test
   public void testPropertiesWithTypesThatHaveBeenNarrowed() {
     String js = lines(

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

@@ -45,16 +45,34 @@
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
-/**
- * Tests for {@link GlobalNamespace}.
- *
- * @author [email protected] (Nick Santos)
- */
+/** Tests for {@link GlobalNamespace}. */
 @RunWith(JUnit4.class)
 public final class GlobalNamespaceTest {
 
   @Nullable private Compiler lastCompiler = null;
 
+  @Test
+  public void detectsPropertySetsInAssignmentOperators() {
+    GlobalNamespace namespace = parse("const a = {b: 0}; a.b += 1; a.b = 2;");
+
+    assertThat(namespace.getSlot("a.b").getGlobalSets()).isEqualTo(3);
+  }
+
+  @Test
+  public void detectsPropertySetsInDestructuring() {
+    GlobalNamespace namespace = parse("const a = {b: 0}; [a.b] = [1]; ({b: a.b} = {b: 2});");
+
+    // TODO(b/120303257): this should be 3
+    assertThat(namespace.getSlot("a.b").getGlobalSets()).isEqualTo(1);
+  }
+
+  @Test
+  public void detectsPropertySetsInIncDecOperators() {
+    GlobalNamespace namespace = parse("const a = {b: 0}; a.b++; a.b--;");
+
+    assertThat(namespace.getSlot("a.b").getGlobalSets()).isEqualTo(3);
+  }
+
   @Test
   public void firstGlobalAssignmentIsConsideredDeclaration() {
     GlobalNamespace namespace = parse("");