Fix findGoogRequire so that it can correctly find destructuring requi…

…res. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=187911325
google · Mar 6, 2018 · 45d6df9 · 45d6df9
1 parent 3d4f525
commit 45d6df9
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 5 deletions.
diff --git a/src/com/google/javascript/refactoring/SuggestedFix.java b/src/com/google/javascript/refactoring/SuggestedFix.java
@@ -662,8 +662,7 @@ public Builder addLhsToGoogRequire(Match m, String namespace) {
     }
 
     /**
-     * Adds a goog.require for the given namespace to the file if it does not
+     * Adds a goog.require for the given namespace to the file if it does not already exist.
-     * already exist.
      */
     public Builder addGoogRequire(Match m, String namespace) {
       Node node = m.getNode();
@@ -831,8 +830,9 @@ private static Node findGoogRequireNode(Node n, NodeMetadata metadata, String na
 
       for (Node child : script.children()) {
         if (NodeUtil.isNameDeclaration(child)
-            && child.getFirstFirstChild() != null
+            && child.getFirstChild().getLastChild() != null
-            && Matchers.googRequire(namespace).matches(child.getFirstFirstChild(), metadata)) {
+            && Matchers.googRequire(namespace).matches(
+                child.getFirstChild().getLastChild(), metadata)) {
           return child;
         }
       }

diff --git a/test/com/google/javascript/refactoring/ErrorToFixMapperTest.java b/test/com/google/javascript/refactoring/ErrorToFixMapperTest.java
@@ -1599,7 +1599,7 @@ public void testDuplicateRequire_shorthand() {
   }
 
   @Test
-  public void testDuplicateRequire_destructuring() {
+  public void testDuplicateRequire_destructuring1() {
     // TODO(b/74166725): Can we merge the two requires to: "const {bar, foo} = goog.require(...)"?
     assertNoChanges(
         LINE_JOINER.join(
@@ -1610,6 +1610,19 @@ public void testDuplicateRequire_destructuring() {
             "alert(foo('8'));"));
   }
 
+  @Test
+  public void testDuplicateRequire_destructuring2() {
+    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'));"));
+  }
+
   private void assertChanges(String originalCode, String expectedCode) {
     compiler.compile(
         ImmutableList.<SourceFile>of(), // Externs