From 32086bf9dc5d91ed195179d50e264272da98e368 Mon Sep 17 00:00:00 2001 From: tjgq Date: Fri, 14 Sep 2018 15:41:45 -0700 Subject: [PATCH] Extend ProcessClosurePrimitives to handle goog.requireType calls. 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 --- .../jscomp/ProcessClosurePrimitives.java | 54 +++++---- .../jscomp/ProcessClosurePrimitivesTest.java | 105 ++++++++++++++++-- 2 files changed, 129 insertions(+), 30 deletions(-) diff --git a/src/com/google/javascript/jscomp/ProcessClosurePrimitives.java b/src/com/google/javascript/jscomp/ProcessClosurePrimitives.java index b6c4134ca0a..37eb327ecf8 100644 --- a/src/com/google/javascript/jscomp/ProcessClosurePrimitives.java +++ b/src/com/google/javascript/jscomp/ProcessClosurePrimitives.java @@ -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", @@ -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); } } @@ -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(); @@ -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); } @@ -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; @@ -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(), @@ -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; } } } diff --git a/test/com/google/javascript/jscomp/ProcessClosurePrimitivesTest.java b/test/com/google/javascript/jscomp/ProcessClosurePrimitivesTest.java index 8be18d76e46..261d655f341 100644 --- a/test/com/google/javascript/jscomp/ProcessClosurePrimitivesTest.java +++ b/test/com/google/javascript/jscomp/ProcessClosurePrimitivesTest.java @@ -446,7 +446,7 @@ public void testProvideValidObjectType() { "/** @type {Object} */ 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');", @@ -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');", @@ -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); @@ -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(); @@ -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 = {};"; @@ -693,6 +752,14 @@ 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 = {};"; @@ -700,12 +767,24 @@ public void testMissingRequireWithAdditionalProvide() { 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() { @@ -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');", @@ -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); }"); @@ -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() { @@ -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() {