From c3efb695f8c3add97924885d921f5093a7c37c56 Mon Sep 17 00:00:00 2001
From: bradfordcsmith <bradfordcsmith@google.com>
Date: Wed, 20 Dec 2017 10:57:13 -0800
Subject: [PATCH] RemoveUnusedCode: remove compound assignment to unused this
 properties

This feature is only enabled for tests for now.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=179706963
---
 .../javascript/jscomp/RemoveUnusedCode.java   | 34 +++++++++++++++++++
 .../RemoveUnusedCodeClassPropertiesTest.java  | 25 +++++++-------
 2 files changed, 46 insertions(+), 13 deletions(-)

diff --git a/src/com/google/javascript/jscomp/RemoveUnusedCode.java b/src/com/google/javascript/jscomp/RemoveUnusedCode.java
index 44b3519842b..1534ab3ff97 100644
--- a/src/com/google/javascript/jscomp/RemoveUnusedCode.java
+++ b/src/com/google/javascript/jscomp/RemoveUnusedCode.java
@@ -292,6 +292,21 @@ private void traverseNode(Node n, Scope scope) {
         traverseAssign(n, scope);
         break;
 
+      case ASSIGN_BITOR:
+      case ASSIGN_BITXOR:
+      case ASSIGN_BITAND:
+      case ASSIGN_LSH:
+      case ASSIGN_RSH:
+      case ASSIGN_URSH:
+      case ASSIGN_ADD:
+      case ASSIGN_SUB:
+      case ASSIGN_MUL:
+      case ASSIGN_EXPONENT:
+      case ASSIGN_DIV:
+      case ASSIGN_MOD:
+        traverseCompoundAssign(n, scope);
+        break;
+
       case CALL:
         traverseCall(n, scope);
         break;
@@ -385,6 +400,25 @@ private void traverseNode(Node n, Scope scope) {
     }
   }
 
+  private void traverseCompoundAssign(Node compoundAssignNode, Scope scope) {
+    // We'll allow removal of compound assignment to a this property as long as the result of the
+    // assignment is unused.
+    // e.g. `this.x += 3;`
+    Node targetNode = compoundAssignNode.getFirstChild();
+    Node valueNode = compoundAssignNode.getLastChild();
+    if (targetNode.isGetProp()
+        && targetNode.getFirstChild().isThis()
+        && !NodeUtil.isExpressionResultUsed(compoundAssignNode)) {
+      RemovableBuilder builder = new RemovableBuilder().setIsThisNamedPropertyAssignment(true);
+      traverseRemovableAssignValue(valueNode, builder, scope);
+      considerForIndependentRemoval(
+          builder.buildNamedPropertyAssign(compoundAssignNode, targetNode.getLastChild()));
+    } else {
+      traverseNode(targetNode, scope);
+      traverseNode(valueNode, scope);
+    }
+  }
+
   private VarInfo traverseNameNode(Node n, Scope scope) {
     return traverseVar(getVarForNameNode(n, scope));
   }
diff --git a/test/com/google/javascript/jscomp/RemoveUnusedCodeClassPropertiesTest.java b/test/com/google/javascript/jscomp/RemoveUnusedCodeClassPropertiesTest.java
index be05233e7ab..596a71d31e8 100644
--- a/test/com/google/javascript/jscomp/RemoveUnusedCodeClassPropertiesTest.java
+++ b/test/com/google/javascript/jscomp/RemoveUnusedCodeClassPropertiesTest.java
@@ -141,25 +141,22 @@ public void testExport() {
     testSame("function f() { this.ext = 2; } window['export'] = this.ext;");
   }
 
-
-  // TODO(b/66971163): make this pass
-  public void disabledTestAssignOp1() {
+  public void testAssignOp1() {
     // Properties defined using a compound assignment can be removed if the
     // result of the assignment expression is not immediately used.
-    test("this.x += 2", "2");
+    test("this.x += 2", "");
     testSame("const x = (this.x += 2)");
     testSame("this.x += 2; const x = this.x;");
     // But, of course, a later use prevents its removal.
     testSame("this.x += 2; let x = {}; x.x;");
   }
 
-  // TODO(b/66971163): make this pass
-  public void disabledTestAssignOp2() {
+  public void testAssignOp2() {
     // Properties defined using a compound assignment can be removed if the
     // result of the assignment expression is not immediately used.
-    test("this.a += 2, f()", "2, f()");
-    test("x = (this.a += 2, f())", "x = (2, f())");
-    testSame("x = (f(), this.a += 2)");
+    test("this.a += 2, alert(1)", "0, alert(1)");
+    test("const x = (this.a += 2, alert(1))", "const x = (0, alert(1))");
+    testSame("const x = (alert(1), this.a += 2)");
   }
 
   // TODO(b/66971163): make this pass
@@ -214,15 +211,17 @@ public void testJSCompiler_renameProperty() {
   public void testForIn() {
     // This is the basic assumption that this pass makes:
     // it can remove properties even when the object is used in a FOR-IN loop
-    test("let x = {}; this.y = 1;for (var a in x) { alert(x[a]) }",
-         "let x = {};            for (var a in x) { alert(x[a]) }");
+    test(
+        "let x = {}; this.y = 1;for (var a in x) { alert(x[a]) }",
+        "let x = {};            for (var a in x) { alert(x[a]) }");
   }
 
   public void testObjectKeys() {
     // This is the basic assumption that this pass makes:
     // it can remove properties even when the object are referenced
-    test("this.y = 1;alert(Object.keys(this))",
-         "           alert(Object.keys(this))");
+    test(
+        "this.y = 1;alert(Object.keys(this))", // preserve format
+        "           alert(Object.keys(this))");
   }
 
   public void testObjectReflection1() {