diff --git a/src/com/google/javascript/jscomp/NodeUtil.java b/src/com/google/javascript/jscomp/NodeUtil.java index 15cd695528c..cc761b4dd7a 100644 --- a/src/com/google/javascript/jscomp/NodeUtil.java +++ b/src/com/google/javascript/jscomp/NodeUtil.java @@ -2125,7 +2125,7 @@ static boolean containsFunction(Node n) { /** * Gets the closest ancestor to the given node of the provided type. */ - static Node getEnclosingType(Node n, final Token type) { + public static Node getEnclosingType(Node n, final Token type) { return getEnclosingNode( n, new Predicate() { diff --git a/src/com/google/javascript/refactoring/ErrorToFixMapper.java b/src/com/google/javascript/refactoring/ErrorToFixMapper.java index 019cb6d32f1..2cd6c9dcee9 100644 --- a/src/com/google/javascript/refactoring/ErrorToFixMapper.java +++ b/src/com/google/javascript/refactoring/ErrorToFixMapper.java @@ -26,6 +26,7 @@ import com.google.javascript.rhino.IR; import com.google.javascript.rhino.JSDocInfo; import com.google.javascript.rhino.Node; +import com.google.javascript.rhino.Token; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; @@ -267,10 +268,25 @@ private static SuggestedFix getFixForMissingRequire(JSError error, AbstractCompi String namespaceToRequire = regexMatcher.group(1); NodeMetadata metadata = new NodeMetadata(compiler); Match match = new Match(error.node, metadata); - return new SuggestedFix.Builder() + SuggestedFix.Builder fix = new SuggestedFix.Builder() .attachMatchedNodeInfo(error.node, compiler) - .addGoogRequire(match, namespaceToRequire) - .build(); + .addGoogRequire(match, namespaceToRequire); + if (NodeUtil.getEnclosingType(error.node, Token.MODULE_BODY) != null) { + Node nodeToReplace = null; + if (error.node.isNew()) { + nodeToReplace = error.node.getFirstChild(); + } else if (error.node.isCall()) { + nodeToReplace = error.node.getFirstFirstChild(); + } else if (error.node.isQualifiedName()) { + nodeToReplace = error.node; + } + + if (nodeToReplace != null) { + String shortName = namespaceToRequire.substring(namespaceToRequire.lastIndexOf('.') + 1); + fix.replace(nodeToReplace, IR.name(shortName), compiler); + } + } + return fix.build(); } private static SuggestedFix getFixForDuplicateRequire(JSError error, AbstractCompiler compiler) { diff --git a/test/com/google/javascript/refactoring/ErrorToFixMapperTest.java b/test/com/google/javascript/refactoring/ErrorToFixMapperTest.java index be0c2195309..e282e95b739 100644 --- a/test/com/google/javascript/refactoring/ErrorToFixMapperTest.java +++ b/test/com/google/javascript/refactoring/ErrorToFixMapperTest.java @@ -449,6 +449,20 @@ public void testSortRequiresInGoogModule_allThreeStyles() { "goog.require('standalone.two');")); } + @Test + public void testMissingRequireInGoogProvideFile() { + assertChanges( + LINE_JOINER.join( + "goog.provide('p');", + "", + "alert(new a.b.C());"), + LINE_JOINER.join( + "goog.provide('p');", + "goog.require('a.b.C');", + "", + "alert(new a.b.C());")); + } + @Test public void testMissingRequireInGoogModule() { assertChanges( @@ -460,8 +474,86 @@ public void testMissingRequireInGoogModule() { "goog.module('m');", "var C = goog.require('a.b.C');", "", - // TODO(tbreisacher): Change this to use the shorthand: alert(new C()); - "alert(new a.b.C());")); + "alert(new C());")); + } + + @Test + public void testMissingRequireInGoogModuleTwice() { + assertChanges( + LINE_JOINER.join( + "goog.module('m');", + "", + "alert(new a.b.C());", + "alert(new a.b.C());"), + LINE_JOINER.join( + "goog.module('m');", + "var C = goog.require('a.b.C');", + "", + // TODO(tbreisacher): Can we make automatically switch both lines to use 'new C()'? + "alert(new a.b.C());", + "alert(new C());")); + } + + @Test + public void testMissingRequireInGoogModule_call() { + assertChanges( + LINE_JOINER.join( + "goog.module('m');", + "", + "alert(a.b.c());"), + LINE_JOINER.join( + "goog.module('m');", + "var b = goog.require('a.b');", + "", + "alert(b.c());")); + } + + @Test + public void testMissingRequireInGoogModule_extends() { + assertChanges( + LINE_JOINER.join( + "goog.module('m');", + "", + "class Cat extends world.util.Animal {}"), + LINE_JOINER.join( + "goog.module('m');", + "var Animal = goog.require('world.util.Animal');", + "", + "class Cat extends Animal {}")); + } + + @Test + public void testMissingRequireInGoogModule_atExtends() { + assertChanges( + LINE_JOINER.join( + "goog.module('m');", + "", + "/** @constructor @extends {world.util.Animal} */", + "function Cat() {}"), + LINE_JOINER.join( + "goog.module('m');", + "var Animal = goog.require('world.util.Animal');", + "", + // TODO(tbreisacher): Change this to "@extends {Animal}" + "/** @constructor @extends {world.util.Animal} */", + "function Cat() {}")); + } + + @Test + public void testMissingRequireInGoogModule_atExtends_qname() { + assertChanges( + LINE_JOINER.join( + "goog.module('m');", + "", + "/** @constructor @extends {world.util.Animal} */", + "world.util.Cat = function() {};"), + LINE_JOINER.join( + "goog.module('m');", + "var Animal = goog.require('world.util.Animal');", + "", + // TODO(tbreisacher): Change this to "@extends {Animal}" + "/** @constructor @extends {world.util.Animal} */", + "world.util.Cat = function() {};")); } @Test @@ -482,8 +574,7 @@ public void testMissingRequireInGoogModule_insertedInCorrectOrder() { "var B = goog.require('x.B');", "var C = goog.require('c.C');", "", - // TODO(tbreisacher): Change this to use B instead of x.B. - "alert(new A(new x.B(new C())));")); + "alert(new A(new B(new C())));")); } @Test @@ -504,7 +595,7 @@ public void testMissingRequireInGoogModule_alwaysInsertsVar() { "var B = goog.require('x.B');", "const C = goog.require('c.C');", "", - "alert(new A(new x.B(new C())));")); + "alert(new A(new B(new C())));")); } @Test