Merge duplicate requires into a single require statement when possible.

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

src/com/google/javascript/refactoring/ErrorToFixMapper.java

@@ -36,6 +36,7 @@
 import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import javax.annotation.Nullable;
 
 /**
  * Maps a {@code JSError} to a list of {@code SuggestedFix}es, if possible.
@@ -323,20 +324,37 @@ private static SuggestedFix getFixForMissingRequire(JSError error, AbstractCompi
     return fix.build();
   }
 
+  @Nullable
   private static SuggestedFix getFixForDuplicateRequire(JSError error, AbstractCompiler compiler) {
-    if (!error.node.isExprResult()) {
-      return null;
-    }
     Matcher regexMatcher = DUPLICATE_REQUIRE.matcher(error.description);
     checkState(
         regexMatcher.matches(), "Unexpected error description: %s", error.description);
     String namespace = regexMatcher.group(1);
     NodeMetadata metadata = new NodeMetadata(compiler);
     Match match = new Match(error.node, metadata);
-    return new SuggestedFix.Builder()
-        .attachMatchedNodeInfo(error.node, compiler)
-        .removeGoogRequire(match, namespace)
-        .build();
+    if (error.node.isExprResult()) {
+      return new SuggestedFix.Builder()
+          .attachMatchedNodeInfo(error.node, compiler)
+          .removeGoogRequire(match, namespace)
+          .build();
+    } else {
+      checkState(NodeUtil.isNameDeclaration(error.node), error.node);
+      if (error.node.getFirstChild().isName()) {
+        return null;
+      }
+
+      checkState(error.node.getFirstChild().isDestructuringLhs(), error.node);
+
+      SuggestedFix fix =
+          new SuggestedFix.Builder()
+              .attachMatchedNodeInfo(error.node, compiler)
+              .mergeGoogRequire(error.node, match.getMetadata(), namespace, compiler)
+              .build();
+      if (!fix.isNoOp()) {
+        return fix;
+      }
+      return null;
+    }
   }
 
   private static SuggestedFix getFixForExtraRequire(JSError error, AbstractCompiler compiler) {

src/com/google/javascript/refactoring/SuggestedFix.java

@@ -109,8 +109,13 @@ public ImmutableList<SuggestedFix> getNonDefaultAlternatives() {
     return alternatives.subList(1, alternatives.size());
   }
 
-  @Override public String toString() {
-    if (replacements.isEmpty()) {
+  boolean isNoOp() {
+    return replacements.isEmpty();
+  }
+
+  @Override
+  public String toString() {
+    if (this.isNoOp()) {
       return "<no-op SuggestedFix>";
     }
     StringBuilder sb = new StringBuilder();
@@ -795,6 +800,35 @@ private boolean usesConstGoogRequires(final NodeMetadata metadata, Node script)
       return callback.getUsesConstRequires();
     }
 
+    /**
+     * Merge names from the lhs of |requireToMergeFrom|, which must be a destructuring require, if
+     * there is another destructuring require that its lhs can be merged into. If not, then no
+     * change is applied.
+     */
+    Builder mergeGoogRequire(
+        Node requireToMergeFrom, NodeMetadata m, String namespace, AbstractCompiler compiler) {
+      checkArgument(requireToMergeFrom.getFirstChild().isDestructuringLhs(), requireToMergeFrom);
+      checkArgument(requireToMergeFrom.getFirstFirstChild().isObjectPattern(), requireToMergeFrom);
+
+      Node googRequireNode = findGoogRequireNode(requireToMergeFrom, m, namespace);
+      if (googRequireNode == null) {
+        return this;
+      }
+      if (googRequireNode.isExprResult()) {
+        return this;
+      }
+      if (googRequireNode.getFirstChild().isDestructuringLhs()) {
+        Node objectPattern = googRequireNode.getFirstFirstChild();
+        Node newObjectPattern = objectPattern.cloneTree();
+        for (Node name : requireToMergeFrom.getFirstFirstChild().children()) {
+          newObjectPattern.addChildToBack(name.cloneTree());
+        }
+        this.replace(objectPattern, newObjectPattern, compiler);
+        this.deleteWithoutRemovingWhitespace(requireToMergeFrom);
+      }
+      return this;
+    }
+
     /**
      * Removes a goog.require for the given namespace to the file if it
      * already exists.
@@ -808,8 +842,14 @@ public Builder removeGoogRequire(Match m, String namespace) {
     }
 
     /**
-     * Find the goog.require node for the given namespace (or null if there isn't one).
-     * If there is more than one, this will return the first standalone goog.require statement.
+     * Find the goog.require node for the given namespace (or null if there isn't one). If there is
+     * more than one:
+     *
+     * <ul>
+     *   <li>If there is at least one standalone goog.require, this will return the first standalone
+     *       goog.require.
+     *   <li>If not, this will return the first goog.require.
+     * </ul>
      */
     @Nullable
     private static Node findGoogRequireNode(Node n, NodeMetadata metadata, String namespace) {

test/com/google/javascript/refactoring/ErrorToFixMapperTest.java

@@ -1600,27 +1600,80 @@ public void testDuplicateRequire_shorthand() {
 
   @Test
   public void testDuplicateRequire_destructuring1() {
-    // TODO(b/74166725): Can we merge the two requires to: "const {bar, foo} = goog.require(...)"?
-    assertNoChanges(
+    assertChanges(
         LINE_JOINER.join(
             "const {bar} = goog.require('goog.string');",
             "const {foo} = goog.require('goog.string');",
             "",
             "alert(bar('7'));",
+            "alert(foo('8'));"),
+        LINE_JOINER.join(
+            "const {bar, foo} = goog.require('goog.string');",
+            "",
+            "",
+            "alert(bar('7'));",
             "alert(foo('8'));"));
   }
 
   @Test
   public void testDuplicateRequire_destructuring2() {
+    assertChanges(
+        LINE_JOINER.join(
+            "const {bar} = goog.require('goog.string');",
+            "const {foo:x, qux:y} = goog.require('goog.string');",
+            "",
+            "alert(bar('7'));",
+            "alert(x(y));"),
+        LINE_JOINER.join(
+            "const {bar, foo:x, qux:y} = goog.require('goog.string');",
+            "",
+            "",
+            "alert(bar('7'));",
+            "alert(x(y));"));
+  }
+
+  @Test
+  public void testDuplicateRequire_destructuring3() {
     assertChanges(
         LINE_JOINER.join(
             "const {bar} = goog.require('goog.util');",
             "goog.require('goog.util');",
             "",
             "alert(bar('7'));"),
         LINE_JOINER.join(
+          "const {bar} = goog.require('goog.util');",
+          "alert(bar('7'));"));
+  }
+
+  // In this case, only the standalone require gets removed. Then a second run would merge
+  // the two remaining ones.
+  @Test
+  public void testDuplicateRequire_destructuring4() {
+    assertChanges(
+        LINE_JOINER.join(
+            "const {bar} = goog.require('goog.util');",
+            "const {foo} = goog.require('goog.util');",
+            "goog.require('goog.util');",
+            "",
+            "alert(bar('7'));",
+            "alert(foo('8'));"),
+        LINE_JOINER.join(
+            "const {bar} = goog.require('goog.util');",
+            "const {foo} = goog.require('goog.util');",
+            "alert(bar('7'));",
+            "alert(foo('8'));"));
+  }
+
+  @Test
+  public void testDuplicateRequire_destructuring5() {
+    // TODO(b/74166725): Here, we could remove the second require and add "const {bar} = util;".
+    assertNoChanges(
+        LINE_JOINER.join(
+            "const util = goog.require('goog.util');",
             "const {bar} = goog.require('goog.util');",
-            "alert(bar('7'));"));
+            "",
+            "alert(bar('7'));",
+            "alert(util.foo('8'));"));
   }
 
   private void assertChanges(String originalCode, String expectedCode) {
@@ -1633,8 +1686,9 @@ private void assertChanges(String originalCode, String expectedCode) {
     assertThat(warningsAndErrors).named("warnings/errors").isNotEmpty();
     Collection<SuggestedFix> fixes = errorManager.getAllFixes();
     assertThat(fixes).named("fixes").isNotEmpty();
-    String newCode = ApplySuggestedFixes.applySuggestedFixesToCode(
-        fixes, ImmutableMap.of("test", originalCode)).get("test");
+    String newCode =
+        ApplySuggestedFixes.applySuggestedFixesToCode(fixes, ImmutableMap.of("test", originalCode))
+            .get("test");
     assertThat(newCode).isEqualTo(expectedCode);
   }