Fix optimizer bug inlining usages of non-constant properties

PiperOrigin-RevId: 322194564

Author: lauraharker

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();

test/com/google/javascript/jscomp/GlobalNamespaceTest.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("");

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

@@ -652,4 +652,28 @@ public void testForwardedExportNested() {
             "  const proto = 0;",
             "}"));
   }
+
+  @Test
+  public void testQualifiedNameSetViaUnaryDecrementNotInlined() {
+    testSame(
+        lines(
+            "const a = {b: 0, c: 0};",
+            "const v1 = a.b;",
+            "a.b--;",
+            "const v2 = a.b;",
+            "a.b--;",
+            "use(v1 + v2);"));
+  }
+
+  @Test
+  public void testQualifiedNameSetViaUnaryIncrementNotInlined() {
+    testSame(
+        lines(
+            "const a = {b: 0};",
+            "const v1 = a.b;",
+            "a.b++;",
+            "const v2 = a.b;",
+            "a.b++;",
+            "use(v1 + v2);"));
+  }
 }