Skip to content

Commit

Permalink
Extend ProcessClosurePrimitives to handle goog.requireType calls.
Browse files Browse the repository at this point in the history 
From the perspective of ProcessClosurePrimitives, goog.requireType and goog.require are identical, except that a goog.requireType is allowed to appear before the matching goog.provide.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=213056346
  • Loading branch information
@tjgq
tjgq committed Sep 18, 2018
1 parent 0ffed17 commit 32086bf
Show file tree
Hide file tree
Showing 2 changed files with 129 additions and 30 deletions.
54 changes: 33 additions & 21 deletions src/com/google/javascript/jscomp/ProcessClosurePrimitives.java
View file
Expand Up @@ -43,14 +43,13 @@
import javax.annotation.Nullable;

/**
* Replaces goog.provide calls, removes goog.require calls, verifies that
* goog.require has a corresponding goog.provide and some closure specific
* Replaces goog.provide calls, removes goog.{require,requireType} calls, verifies that each
* goog.{require,requireType} has a corresponding goog.provide, and performs some Closure-pecific
* simplifications.
*
* @author chrisn@google.com (Chris Nokleberg)
*/
class ProcessClosurePrimitives extends AbstractPostOrderCallback
implements HotSwapCompilerPass {
class ProcessClosurePrimitives extends AbstractPostOrderCallback implements HotSwapCompilerPass {

static final DiagnosticType NULL_ARGUMENT_ERROR = DiagnosticType.error(
"JSC_NULL_ARGUMENT_ERROR",
Expand Down Expand Up @@ -208,17 +207,7 @@ public void process(Node externs, Node root) {

if (requiresLevel.isOn()) {
for (UnrecognizedRequire r : unrecognizedRequires) {
DiagnosticType error;
ProvidedName expectedName = providedNames.get(r.namespace);
if (expectedName != null && expectedName.firstNode != null) {
// The namespace ended up getting provided after it was required.
error = LATE_PROVIDE_ERROR;
} else {
error = MISSING_PROVIDE_ERROR;
}

compiler.report(JSError.make(
r.requireNode, requiresLevel, error, r.namespace));
checkForLateOrMissingProvide(r);
}
}

Expand All @@ -231,6 +220,23 @@ public void process(Node externs, Node root) {
}
}

private void checkForLateOrMissingProvide(UnrecognizedRequire r) {
// Both goog.require and goog.requireType must have a matching goog.provide.
// However, goog.require must match an earlier goog.provide, while goog.requireType is allowed
// to match a later goog.provide.
DiagnosticType error;
ProvidedName expectedName = providedNames.get(r.namespace);
if (expectedName != null && expectedName.firstNode != null) {
if (r.isRequireType) {
return;
}
error = LATE_PROVIDE_ERROR;
} else {
error = MISSING_PROVIDE_ERROR;
}
compiler.report(JSError.make(r.requireNode, requiresLevel, error, r.namespace));
}

private Node getAnyValueOfType(JSDocInfo jsdoc) {
checkArgument(jsdoc.hasType());
Node typeAst = jsdoc.getType().getRoot();
Expand Down Expand Up @@ -296,6 +302,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
}
break;
case "require":
case "requireType":
if (validPrimitiveCall(t, n)) {
processRequireCall(t, n, parent);
}
Expand Down Expand Up @@ -440,17 +447,16 @@ static boolean isValidDefineValue(Node val) {
}
}

/**
* Handles a goog.require call.
*/
/** Handles a goog.require or goog.requireType call. */
private void processRequireCall(NodeTraversal t, Node n, Node parent) {
Node left = n.getFirstChild();
Node arg = left.getNext();
String method = left.getFirstChild().getNext().getString();
if (verifyLastArgumentIsString(t, left, arg)) {
String ns = arg.getString();
ProvidedName provided = providedNames.get(ns);
if (provided == null || !provided.isExplicitlyProvided()) {
unrecognizedRequires.add(new UnrecognizedRequire(n, ns));
unrecognizedRequires.add(new UnrecognizedRequire(n, ns, method.equals("requireType")));
} else {
JSModule providedModule = provided.explicitModule;

Expand All @@ -459,7 +465,11 @@ private void processRequireCall(NodeTraversal t, Node n, Node parent) {
checkNotNull(providedModule, n);

JSModule module = t.getModule();
if (module != providedModule && !moduleGraph.dependsOn(module, providedModule)) {
// A cross-chunk goog.require must match a goog.provide in an earlier chunk. However, a
// cross-chunk goog.requireType is allowed to match a goog.provide in a later chunk.
if (module != providedModule
&& !moduleGraph.dependsOn(module, providedModule)
&& !method.equals("requireType")) {
compiler.report(
t.makeError(n, XMODULE_REQUIRE_ERROR, ns,
providedModule.getName(),
Expand Down Expand Up @@ -1686,10 +1696,12 @@ private void maybeAddToSymbolTable(Node n) {
private static class UnrecognizedRequire {
final Node requireNode;
final String namespace;
final boolean isRequireType;

UnrecognizedRequire(Node requireNode, String namespace) {
UnrecognizedRequire(Node requireNode, String namespace, boolean isRequireType) {
this.requireNode = requireNode;
this.namespace = namespace;
this.isRequireType = isRequireType;
}
}
}
105 changes: 96 additions & 9 deletions test/com/google/javascript/jscomp/ProcessClosurePrimitivesTest.java
View file
Expand Up @@ -446,7 +446,7 @@ public void testProvideValidObjectType() {
"/** @type {Object<string>} */ var foo = {};");
}

public void testRemovalOfRequires() {
public void testRemovalOfRequire() {
test("goog.provide('foo'); goog.require('foo');", "/** @const */ var foo={};");
test(
"goog.provide('foo.bar'); goog.require('foo.bar');",
Expand All @@ -458,7 +458,21 @@ public void testRemovalOfRequires() {
testSame("foo.require('foo.bar');");
}

public void testPreserveGoogRequires() {
public void testRemovalOfRequireType() {
test("goog.provide('foo'); goog.requireType('foo');", "/** @const */ var foo={};");
test(
"goog.provide('foo.bar'); goog.requireType('foo.bar');",
"/** @const */ var foo={}; /** @const */ foo.bar={};");
test(
"goog.provide('foo.bar.baz'); goog.requireType('foo.bar.baz');",
"/** @const */ var foo={}; /** @const */ foo.bar={}; /** @const */ foo.bar.baz={};");
test(
"goog.provide('foo'); var x = 3; goog.requireType('foo'); something();",
"/** @const */ var foo={}; var x = 3; something();");
testSame("foo.requireType('foo.bar');");
}

public void testPreserveGoogRequire() {
preserveGoogProvidesAndRequires = true;
test(
"goog.provide('foo'); goog.require('foo');",
Expand All @@ -468,25 +482,51 @@ public void testPreserveGoogRequires() {
"/** @const */ var foo = {}; goog.provide('foo'); goog.require('foo'); var a = {};");
}

public void testRequireErrorCases() {
public void testPreserveGoogRequireType() {
preserveGoogProvidesAndRequires = true;

test(
"goog.provide('foo'); goog.requireType('foo');",
"/** @const */ var foo={}; goog.provide('foo'); goog.requireType('foo');");
test(
"goog.provide('foo'); goog.requireType('foo'); var a = {};",
"/** @const */ var foo = {}; goog.provide('foo'); goog.requireType('foo'); var a = {};");
}

public void testRequireBadArguments() {
testError("goog.require();", NULL_ARGUMENT_ERROR);
testError("goog.require(5);", INVALID_ARGUMENT_ERROR);
testError("goog.require([]);", INVALID_ARGUMENT_ERROR);
testError("goog.require({});", INVALID_ARGUMENT_ERROR);

testError("goog.require(`template`);", INVALID_ARGUMENT_ERROR);
testError("goog.require(tagged`template`);", INVALID_ARGUMENT_ERROR);
testError("goog.require(`${template}Sub`);", INVALID_ARGUMENT_ERROR);
}

public void testLateProvides() {
public void testRequireTypeBadArguments() {
testError("goog.requireType();", NULL_ARGUMENT_ERROR);
testError("goog.requireType(5);", INVALID_ARGUMENT_ERROR);
testError("goog.requireType([]);", INVALID_ARGUMENT_ERROR);
testError("goog.requireType({});", INVALID_ARGUMENT_ERROR);
testError("goog.requireType(`template`);", INVALID_ARGUMENT_ERROR);
testError("goog.requireType(tagged`template`);", INVALID_ARGUMENT_ERROR);
testError("goog.requireType(`${template}Sub`);", INVALID_ARGUMENT_ERROR);
}

public void testLateProvideForRequire() {
testError("goog.require('foo'); goog.provide('foo');", LATE_PROVIDE_ERROR);
testError("goog.require('foo.bar'); goog.provide('foo.bar');", LATE_PROVIDE_ERROR);
testError("goog.provide('foo.bar'); goog.require('foo'); goog.provide('foo');",
LATE_PROVIDE_ERROR);
}

public void testMissingProvides() {
public void testLateProvideForRequireType() {
testNoWarning("goog.requireType('foo'); goog.provide('foo');");
testNoWarning("goog.requireType('foo.bar'); goog.provide('foo.bar');");
testNoWarning("goog.provide('foo.bar'); goog.requireType('foo'); goog.provide('foo');");
}

public void testMissingProvideForRequire() {
testError("goog.require('foo');", MISSING_PROVIDE_ERROR);
testError("goog.provide('foo'); goog.require('Foo');", MISSING_PROVIDE_ERROR);
testError("goog.provide('foo'); goog.require('foo.bar');", MISSING_PROVIDE_ERROR);
Expand All @@ -495,6 +535,16 @@ public void testMissingProvides() {
MISSING_PROVIDE_ERROR);
}

public void testMissingProvideForRequireType() {
testError("goog.requireType('foo');", MISSING_PROVIDE_ERROR);
testError("goog.provide('foo'); goog.requireType('Foo');", MISSING_PROVIDE_ERROR);
testError("goog.provide('foo'); goog.requireType('foo.bar');", MISSING_PROVIDE_ERROR);
testError(
"goog.provide('foo'); var EXPERIMENT_FOO = true; "
+ "if (EXPERIMENT_FOO) {goog.requireType('foo.bar');}",
MISSING_PROVIDE_ERROR);
}

public void testProvideInExterns() {
allowExternsChanges();

Expand Down Expand Up @@ -628,6 +678,15 @@ public void testGoodCrossModuleRequire2() {
});
}

public void testCrossModuleRequireType() {
test(
createModuleStar("goog.requireType('goog.ui');", "", "goog.provide('goog.ui')"),
new String[] {"", "", "/** @const */ goog.ui = {};"});
test(
createModuleStar("", "goog.provide('goog.ui');", "goog.requireType('goog.ui');"),
new String[] {"", "/** @const */ goog.ui = {};", ""});
}

// Tests providing additional code with non-overlapping var namespace.
public void testSimpleAdditionalProvide() {
additionalCode = "goog.provide('b.B'); b.B = {};";
Expand Down Expand Up @@ -693,19 +752,39 @@ public void testRequireOfAdditionalProvide() {
"/** @const */ var b={};b.B={}; /** @const */ var a={};a.A={};");
}

// Tests that a requireType of additional code generates no error.
public void testRequireTypeOfAdditionalProvide() {
additionalCode = "goog.provide('b.B'); b.B = {};";
test(
"goog.requireType('b.B'); goog.provide('a.A'); a.A = {};",
"/** @const */ var b={};b.B={}; /** @const */ var a={};a.A={};");
}

// Tests that a require not in additional code generates (only) one error.
public void testMissingRequireWithAdditionalProvide() {
additionalCode = "goog.provide('b.B'); b.B = {};";
testError("goog.require('b.C'); goog.provide('a.A'); a.A = {};",
MISSING_PROVIDE_ERROR);
}

// Tests that a requireType not in additional code generates (only) one error.
public void testMissingRequireTypeWithAdditionalProvide() {
additionalCode = "goog.provide('b.B'); b.B = {};";
testError("goog.requireType('b.C'); goog.provide('a.A'); a.A = {};", MISSING_PROVIDE_ERROR);
}

// Tests that a require in additional code generates no error.
public void testLateRequire() {
additionalEndCode = "goog.require('a.A');";
test("goog.provide('a.A'); a.A = {};", "/** @const */ var a={}; a.A={};");
}

// Tests that a requireType in additional code generates no error.
public void testLateRequireType() {
additionalEndCode = "goog.requireType('a.A');";
test("goog.provide('a.A'); a.A = {};", "/** @const */ var a={}; a.A={};");
}

// Tests a case where code is reordered after processing provides and then
// provides are processed again.
public void testReorderedProvides() {
Expand Down Expand Up @@ -868,9 +947,6 @@ public void testInvalidProvide() {
}

public void testInvalidRequire() {
test(
"goog.provide('a.b'); goog.require('a.b');",
"/** @const */ var a = {}; /** @const */ a.b = {};");
testError("goog.provide('a.b'); var x = x || goog.require('a.b');",
INVALID_CLOSURE_CALL_ERROR);
testError("goog.provide('a.b'); x = goog.require('a.b');",
Expand All @@ -879,6 +955,15 @@ public void testInvalidRequire() {
INVALID_CLOSURE_CALL_ERROR);
}

public void testInvalidRequireType() {
testError(
"goog.provide('a.b'); var x = x || goog.requireType('a.b');", INVALID_CLOSURE_CALL_ERROR);
testError("goog.provide('a.b'); x = goog.requireType('a.b');", INVALID_CLOSURE_CALL_ERROR);
testError(
"goog.provide('a.b'); function f() { goog.requireType('a.b'); }",
INVALID_CLOSURE_CALL_ERROR);
}

public void testValidGoogMethod() {
testSame("function f() { goog.isDef('a.b'); }");
testSame("function f() { goog.inherits(a, b); }");
Expand Down Expand Up @@ -1261,6 +1346,7 @@ public void testProvideInIndependentModules4() {

public void testRequireOfBaseGoog() {
testError("goog.require('goog');", MISSING_PROVIDE_ERROR);
testError("goog.requireType('goog');", MISSING_PROVIDE_ERROR);
}

public void testSourcePositionPreservation() {
Expand Down Expand Up @@ -1303,6 +1389,7 @@ public void testNoStubForProvidedTypedef4() {

public void testProvideRequireSameFile() {
test("goog.provide('x');\ngoog.require('x');", "/** @const */ var x = {};");
test("goog.provide('x');\ngoog.requireType('x');", "/** @const */ var x = {};");
}

public void testDefineCases() {
Expand Down

0 comments on commit 32086bf

Please sign in to comment.