From 64a2db15b4396efedfbe5c9bd15a3527f33cea7f Mon Sep 17 00:00:00 2001
From: bangert <bangert@google.com>
Date: Mon, 16 Apr 2018 13:08:18 -0700
Subject: [PATCH] Suggest const aliases for suggested fixes in goog.requires in
 files that use the var foo = goog.require('bar.foo'); style.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=193087075
---
 .../javascript/refactoring/SuggestedFix.java  | 28 +++++++++----------
 .../refactoring/SuggestedFixTest.java         | 24 ++++++++++++++++
 2 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/src/com/google/javascript/refactoring/SuggestedFix.java b/src/com/google/javascript/refactoring/SuggestedFix.java
index 4ef8c2e5fb6..b61b7d77421 100644
--- a/src/com/google/javascript/refactoring/SuggestedFix.java
+++ b/src/com/google/javascript/refactoring/SuggestedFix.java
@@ -698,8 +698,8 @@ public Builder addGoogRequire(Match m, String namespace) {
           IR.string(namespace));
 
       String shortName = getShortNameForRequire(namespace);
-      boolean useConstRequire = usesConstGoogRequires(metadata, script);
-      if (useConstRequire) {
+      boolean useAliasedRequire = usesConstGoogRequires(metadata, script);
+      if (useAliasedRequire) {
         googRequireNode = IR.constNode(IR.name(shortName), googRequireNode);
       } else {
         googRequireNode = IR.exprResult(googRequireNode);
@@ -805,9 +805,9 @@ private boolean usesConstGoogRequires(final NodeMetadata metadata, Node script)
       if (script.isModuleBody()) {
         return true;
       }
-      HasConstRequireOrModuleCallback callback = new HasConstRequireOrModuleCallback(metadata);
+      HasAliasedRequireOrModuleCallback callback = new HasAliasedRequireOrModuleCallback(metadata);
       NodeTraversal.traverse(metadata.getCompiler(), script, callback);
-      return callback.getUsesConstRequires();
+      return callback.getUsesAliasedRequires();
     }
 
     /**
@@ -969,30 +969,30 @@ static MatchedNodeInfo create(Node node, boolean closurized) {
   }
 
   /** Traverse an AST and find {@code goog.module} or {@code const X = goog.require('...');}. */
-  private static class HasConstRequireOrModuleCallback extends AbstractPreOrderCallback {
-    private boolean usesConstRequires;
+  private static class HasAliasedRequireOrModuleCallback extends AbstractPreOrderCallback {
+    private boolean usesAliasedRequires;
     final NodeMetadata metadata;
 
-    public HasConstRequireOrModuleCallback(NodeMetadata metadata) {
-      this.usesConstRequires = false;
+    public HasAliasedRequireOrModuleCallback(NodeMetadata metadata) {
+      this.usesAliasedRequires = false;
       this.metadata = metadata;
     }
 
-    boolean getUsesConstRequires() {
-      return usesConstRequires;
+    boolean getUsesAliasedRequires() {
+      return usesAliasedRequires;
     }
 
     @Override
     public boolean shouldTraverse(NodeTraversal nodeTraversal, Node n, Node parent) {
-      if (Matchers.googModule().matches(n, metadata) || isConstRequire(n, metadata)) {
-        usesConstRequires = true;
+      if (Matchers.googModule().matches(n, metadata) || isAliasedRequire(n, metadata)) {
+        usesAliasedRequires = true;
         return false;
       }
       return true;
     }
 
-    private static boolean isConstRequire(Node node, NodeMetadata metadata) {
-      return node.isConst()
+    private static boolean isAliasedRequire(Node node, NodeMetadata metadata) {
+      return NodeUtil.isNameDeclaration(node)
           && node.getFirstFirstChild() != null
           && Matchers.googRequire().matches(node.getFirstFirstChild(), metadata);
     }
diff --git a/test/com/google/javascript/refactoring/SuggestedFixTest.java b/test/com/google/javascript/refactoring/SuggestedFixTest.java
index 46101b5e838..ec15fc62bcb 100644
--- a/test/com/google/javascript/refactoring/SuggestedFixTest.java
+++ b/test/com/google/javascript/refactoring/SuggestedFixTest.java
@@ -890,6 +890,30 @@ public void testAddRequireConst() {
     assertChanges(fix, "", input, expected);
   }
 
+  @Test
+  public void testAddRequireVar() {
+    String input =
+        Joiner.on('\n').join(
+            "var bar = goog.require('goog.bar');",
+            "",
+            "/** @private */",
+            "function foo_() {};");
+    String expected =
+        Joiner.on('\n').join(
+            "var bar = goog.require('goog.bar');",
+            // We add new imports as const per the Google Style Guide;
+            // TODO(bangert): we could add complexity to add new imports as var if we want to.
+            "const safe = goog.require('goog.safe');",
+            "",
+            "/** @private */",
+            "function foo_() {};");
+    Compiler compiler = getCompiler(input);
+    Node root = compileToScriptRoot(compiler);
+    Match match = new Match(root.getFirstChild(), new NodeMetadata(compiler));
+    SuggestedFix fix = new SuggestedFix.Builder().addGoogRequire(match, "goog.safe").build();
+    assertChanges(fix, "", input, expected);
+  }
+
   @Test
   public void testAddRequireModuleUnchanged() {
     String input =