Skip to content

Commit

Permalink
Switch to the shorthand name when adding a goog.require in a goog.module
Browse files Browse the repository at this point in the history 
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=136646544
  • Loading branch information
@blickly
tbreisacher authored and blickly committed Oct 19, 2016
1 parent 30c350b commit 322e145
Show file tree
Hide file tree
Showing 3 changed files with 116 additions and 9 deletions.
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/NodeUtil.java
View file
Expand Up @@ -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<Node>() {
Expand Down
22 changes: 19 additions & 3 deletions src/com/google/javascript/refactoring/ErrorToFixMapper.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
101 changes: 96 additions & 5 deletions test/com/google/javascript/refactoring/ErrorToFixMapperTest.java
View file
Expand Up @@ -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(
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 322e145

Please sign in to comment.