From eb521a4e04be619c890742c71f316fca7b01cfe9 Mon Sep 17 00:00:00 2001 From: tjgq Date: Tue, 2 Oct 2018 11:11:54 -0700 Subject: [PATCH] Handle goog.requireType correctly in Es6RewriteModules. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=215422628 --- .../jscomp/ClosurePrimitiveErrors.java | 5 + .../jscomp/ClosureRewriteModule.java | 20 +- .../jscomp/Es6RenameReferences.java | 10 +- .../javascript/jscomp/Es6RewriteModules.java | 120 ++++--- .../jscomp/ClosureRewriteModuleTest.java | 3 +- .../Es6RewriteModulesWithGoogInteropTest.java | 332 +++++++++++++++++- 6 files changed, 436 insertions(+), 54 deletions(-) diff --git a/src/com/google/javascript/jscomp/ClosurePrimitiveErrors.java b/src/com/google/javascript/jscomp/ClosurePrimitiveErrors.java index 1a3c9b70c4b..b9871b7cf19 100644 --- a/src/com/google/javascript/jscomp/ClosurePrimitiveErrors.java +++ b/src/com/google/javascript/jscomp/ClosurePrimitiveErrors.java @@ -46,6 +46,11 @@ private ClosurePrimitiveErrors() {} "JSC_GOOG_MODULE_INVALID_REQUIRE_NAMESPACE", "goog.require parameter must be a string literal."); + static final DiagnosticType INVALID_REQUIRE_TYPE_NAMESPACE = + DiagnosticType.error( + "JSC_GOOG_MODULE_INVALID_REQUIRE_TYPE_NAMESPACE", + "goog.requireType parameter must be a string literal."); + static final DiagnosticType MISSING_MODULE_OR_PROVIDE = DiagnosticType.error( "JSC_MISSING_MODULE_OR_PROVIDE", "Required namespace \"{0}\" never defined."); diff --git a/src/com/google/javascript/jscomp/ClosureRewriteModule.java b/src/com/google/javascript/jscomp/ClosureRewriteModule.java index 3a30f3d36f9..ed98ba8f783 100644 --- a/src/com/google/javascript/jscomp/ClosureRewriteModule.java +++ b/src/com/google/javascript/jscomp/ClosureRewriteModule.java @@ -22,6 +22,7 @@ import static com.google.javascript.jscomp.ClosurePrimitiveErrors.INVALID_GET_CALL_SCOPE; import static com.google.javascript.jscomp.ClosurePrimitiveErrors.INVALID_GET_NAMESPACE; import static com.google.javascript.jscomp.ClosurePrimitiveErrors.INVALID_REQUIRE_NAMESPACE; +import static com.google.javascript.jscomp.ClosurePrimitiveErrors.INVALID_REQUIRE_TYPE_NAMESPACE; import static com.google.javascript.jscomp.ClosurePrimitiveErrors.MISSING_MODULE_OR_PROVIDE; import com.google.common.base.Joiner; @@ -376,7 +377,7 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) { } else if (method.matchesQualifiedName(GOOG_REQUIRE)) { recordGoogRequire(t, n, /* mustBeOrdered= */ true); } else if (method.matchesQualifiedName(GOOG_REQUIRETYPE)) { - recordGoogRequire(t, n, /* mustBeOrdered= */ false); + recordGoogRequireType(t, n); } else if (method.matchesQualifiedName(GOOG_FORWARDDECLARE) && !parent.isExprResult()) { recordGoogForwardDeclare(t, n); } else if (method.matchesQualifiedName(GOOG_MODULE_GET)) { @@ -864,6 +865,21 @@ private void recordGoogRequire(NodeTraversal t, Node call, boolean mustBeOrdered } } + private void recordGoogRequireType(NodeTraversal t, Node call) { + Node legacyNamespaceNode = call.getLastChild(); + if (!legacyNamespaceNode.isString()) { + t.report(legacyNamespaceNode, INVALID_REQUIRE_TYPE_NAMESPACE); + return; + } + + // A goog.requireType call is not required to appear after the corresponding namespace + // definition. + boolean mustBeOrdered = false; + + // For purposes of import collection, goog.requireType is the same as goog.require. + recordGoogRequire(t, call, mustBeOrdered); + } + private void recordGoogForwardDeclare(NodeTraversal t, Node call) { Node namespaceNode = call.getLastChild(); if (!call.hasTwoChildren() || !namespaceNode.isString()) { @@ -876,7 +892,7 @@ private void recordGoogForwardDeclare(NodeTraversal t, Node call) { // goog.module.get(). To avoid reporting the error twice suppress it here. boolean mustBeOrdered = false; - // For purposes of import collection goog.forwardDeclare is the same as goog.require; + // For purposes of import collection, goog.forwardDeclare is the same as goog.require. recordGoogRequire(t, call, mustBeOrdered); } diff --git a/src/com/google/javascript/jscomp/Es6RenameReferences.java b/src/com/google/javascript/jscomp/Es6RenameReferences.java index f379e2465ea..a6fff88eb92 100644 --- a/src/com/google/javascript/jscomp/Es6RenameReferences.java +++ b/src/com/google/javascript/jscomp/Es6RenameReferences.java @@ -47,7 +47,7 @@ final class Es6RenameReferences extends AbstractPostOrderCallback { @Override public void visit(NodeTraversal t, Node n, Node parent) { if (!typesOnly && NodeUtil.isReferenceName(n)) { - renameReference(t, n); + renameReference(t, n, false); } JSDocInfo info = n.getJSDocInfo(); @@ -59,13 +59,13 @@ public void visit(NodeTraversal t, Node n, Node parent) { private void renameTypeNode(NodeTraversal t, Iterable typeNodes) { for (Node type : typeNodes) { if (type.isString()) { - renameReference(t, type); + renameReference(t, type, true); } renameTypeNode(t, type.children()); } } - private void renameReference(NodeTraversal t, Node n) { + private void renameReference(NodeTraversal t, Node n, boolean isType) { String fullName = n.getString(); List split = SPLIT_ON_DOT.splitToList(fullName); String oldName = split.get(0); @@ -75,7 +75,9 @@ private void renameReference(NodeTraversal t, Node n) { if (newName != null) { String rest = split.size() == 2 ? "." + split.get(1) : ""; n.setString(newName + rest); - t.reportCodeChange(); + if (!isType) { + t.reportCodeChange(); + } return; } else if (current.hasOwnSlot(oldName)) { return; diff --git a/src/com/google/javascript/jscomp/Es6RewriteModules.java b/src/com/google/javascript/jscomp/Es6RewriteModules.java index 82b9a2c1c46..8793ff226e7 100644 --- a/src/com/google/javascript/jscomp/Es6RewriteModules.java +++ b/src/com/google/javascript/jscomp/Es6RewriteModules.java @@ -24,6 +24,7 @@ import static com.google.javascript.jscomp.ClosurePrimitiveErrors.INVALID_GET_CALL_SCOPE; import static com.google.javascript.jscomp.ClosurePrimitiveErrors.INVALID_GET_NAMESPACE; import static com.google.javascript.jscomp.ClosurePrimitiveErrors.INVALID_REQUIRE_NAMESPACE; +import static com.google.javascript.jscomp.ClosurePrimitiveErrors.INVALID_REQUIRE_TYPE_NAMESPACE; import static com.google.javascript.jscomp.ClosurePrimitiveErrors.MISSING_MODULE_OR_PROVIDE; import static com.google.javascript.jscomp.ClosurePrimitiveErrors.MODULE_USES_GOOG_MODULE_GET; import static com.google.javascript.jscomp.parsing.parser.FeatureSet.Feature.MODULES; @@ -66,7 +67,8 @@ public final class Es6RewriteModules extends AbstractPostOrderCallback static final DiagnosticType LHS_OF_GOOG_REQUIRE_MUST_BE_CONST = DiagnosticType.error( "JSC_LHS_OF_GOOG_REQUIRE_MUST_BE_CONST", - "The left side of a goog.require() must use ''const'' (not ''let'' or ''var'')"); + "The left side of a goog.require() or goog.requireType() " + + "must use ''const'' (not ''let'' or ''var'')"); static final DiagnosticType NAMESPACE_IMPORT_CANNOT_USE_STAR = DiagnosticType.error( @@ -77,6 +79,11 @@ public final class Es6RewriteModules extends AbstractPostOrderCallback static final DiagnosticType DUPLICATE_EXPORT = DiagnosticType.error("JSC_DUPLICATE_EXPORT", "Duplicate export ''{0}''."); + static final DiagnosticType REQUIRE_TYPE_FOR_ES6_SHOULD_BE_CONST = + DiagnosticType.error( + "JSC_REQUIRE_TYPE_FOR_ES6_SHOULD_BE_CONST", + "goog.requireType alias for ES6 module should be const."); + static final DiagnosticType FORWARD_DECLARE_FOR_ES6_SHOULD_BE_CONST = DiagnosticType.error( "JSC_FORWARD_DECLARE_FOR_ES6_SHOULD_BE_CONST", @@ -108,9 +115,10 @@ public final class Es6RewriteModules extends AbstractPostOrderCallback private Map importMap; /** - * Local variable names that were goog.require'd to qualified name we need to line. We need to - * inline all required names since there are certain well-known Closure symbols (like - * goog.asserts) that later stages of the compiler check for and cannot handle aliases. + * Local variable names that were goog.require'd to qualified name we need to line. + * + *

We need to inline all required names since there are certain well-known Closure symbols + * (like goog.asserts) that later stages of the compiler check for and cannot handle aliases. * *

We use this to rewrite something like: * @@ -208,27 +216,28 @@ public void clearState() { } /** - * Checks for goog.require + goog.module.get + goog.forwardDeclare calls in non-ES6 modules that + * Checks for goog.require, goog.requireType, goog.module.get and goog.forwardDeclare calls that * are meant to import ES6 modules and rewrites them. */ private class RewriteRequiresForEs6Modules extends AbstractPostOrderCallback { private boolean transpiled = false; - private Table forwardDeclareRenameTable; + // An (s, old, new) entry indicates that occurrences of `old` in scope `s` should be rewritten + // as `new`. This is used to rewrite namespaces that appear in calls to goog.requireType and + // goog.forwardDeclare. + private Table renameTable; void rewrite(Node scriptNode) { transpiled = false; - forwardDeclareRenameTable = HashBasedTable.create(); + renameTable = HashBasedTable.create(); NodeTraversal.traverse(compiler, scriptNode, this); if (transpiled) { scriptNode.putBooleanProp(Node.TRANSPILED, true); } - if (!forwardDeclareRenameTable.isEmpty()) { + if (!renameTable.isEmpty()) { NodeTraversal.traverse( - compiler, - scriptNode, - new Es6RenameReferences(forwardDeclareRenameTable, /* typesOnly= */ true)); + compiler, scriptNode, new Es6RenameReferences(renameTable, /* typesOnly= */ true)); } } @@ -239,16 +248,19 @@ public void visit(NodeTraversal t, Node n, Node parent) { } boolean isRequire = n.getFirstChild().matchesQualifiedName("goog.require"); + boolean isRequireType = n.getFirstChild().matchesQualifiedName("goog.requireType"); boolean isGet = n.getFirstChild().matchesQualifiedName("goog.module.get"); boolean isForwardDeclare = n.getFirstChild().matchesQualifiedName("goog.forwardDeclare"); - if (!isRequire && !isGet && !isForwardDeclare) { + if (!isRequire && !isRequireType && !isGet && !isForwardDeclare) { return; } if (!n.hasTwoChildren() || !n.getLastChild().isString()) { if (isRequire) { t.report(n, INVALID_REQUIRE_NAMESPACE); + } else if (isRequireType) { + t.report(n, INVALID_REQUIRE_TYPE_NAMESPACE); } else if (isGet) { t.report(n, INVALID_GET_NAMESPACE); } else { @@ -282,41 +294,62 @@ public void visit(NodeTraversal t, Node n, Node parent) { Node statementNode = NodeUtil.getEnclosingStatement(n); boolean importHasAlias = NodeUtil.isNameDeclaration(statementNode); - if (importHasAlias) { if (statementNode.getFirstChild().isDestructuringLhs()) { if (isForwardDeclare) { + // const {a, c:b} = goog.forwardDeclare('an.es6.namespace'); t.report(n, INVALID_DESTRUCTURING_FORWARD_DECLARE); return; } - // Work around a bug in the type checker where destructing can create - // too many layers of aliases and confuse the type checker. b/112061124. - - // const {a, c:b} = goog.require('an.es6.namespace'); - // const a = module$es6.a; - // const b = module$es6.c; - for (Node child : statementNode.getFirstFirstChild().children()) { - checkState(child.isStringKey()); - checkState(child.getFirstChild().isName()); - Node constNode = - IR.constNode( - IR.name(child.getFirstChild().getString()), - IR.getprop( - IR.name(ModuleRenaming.getGlobalName(moduleMetadata, name)), - IR.string(child.getString()))); - constNode.useSourceInfoFromForTree(child); - statementNode.getParent().addChildBefore(constNode, statementNode); + if (isRequireType) { + if (!statementNode.isConst()) { + t.report(statementNode, REQUIRE_TYPE_FOR_ES6_SHOULD_BE_CONST); + return; + } + // const {a, c:b} = goog.requireType('an.es6.namespace'); + for (Node child : statementNode.getFirstFirstChild().children()) { + checkState(child.isStringKey()); + checkState(child.getFirstChild().isName()); + renameTable.put( + t.getScopeRoot(), + child.getFirstChild().getString(), + ModuleRenaming.getGlobalName(moduleMetadata, name) + "." + child.getString()); + } + } else { + // Work around a bug in the type checker where destructing can create + // too many layers of aliases and confuse the type checker. b/112061124. + + // const {a, c:b} = goog.require('an.es6.namespace'); + // const a = module$es6.a; + // const b = module$es6.c; + for (Node child : statementNode.getFirstFirstChild().children()) { + checkState(child.isStringKey()); + checkState(child.getFirstChild().isName()); + Node constNode = + IR.constNode( + IR.name(child.getFirstChild().getString()), + IR.getprop( + IR.name(ModuleRenaming.getGlobalName(moduleMetadata, name)), + IR.string(child.getString()))); + constNode.useSourceInfoFromForTree(child); + statementNode.getParent().addChildBefore(constNode, statementNode); + } } statementNode.detach(); t.reportCodeChange(); } else { - if (isForwardDeclare) { + if (isForwardDeclare || isRequireType) { if (!statementNode.isConst()) { - t.report(statementNode, FORWARD_DECLARE_FOR_ES6_SHOULD_BE_CONST); + DiagnosticType diagnostic = + isForwardDeclare + ? FORWARD_DECLARE_FOR_ES6_SHOULD_BE_CONST + : REQUIRE_TYPE_FOR_ES6_SHOULD_BE_CONST; + t.report(statementNode, diagnostic); return; } // const namespace = goog.forwardDeclare('an.es6.namespace'); - forwardDeclareRenameTable.put( + // const namespace = goog.requireType('an.es6.namespace'); + renameTable.put( t.getScopeRoot(), statementNode.getFirstChild().getString(), ModuleRenaming.getGlobalName(moduleMetadata, name)); @@ -332,9 +365,10 @@ public void visit(NodeTraversal t, Node n, Node parent) { } } } else { - if (isForwardDeclare) { + if (isForwardDeclare || isRequireType) { // goog.forwardDeclare('an.es6.namespace') - forwardDeclareRenameTable.put( + // goog.requireType('an.es6.namespace') + renameTable.put( t.getScopeRoot(), name, ModuleRenaming.getGlobalName(moduleMetadata, name)); statementNode.detach(); } else { @@ -714,9 +748,15 @@ private void rewriteRequires(Node script) { script, (NodeTraversal t, Node n, Node parent) -> { if (n.isCall()) { - if (n.getFirstChild().matchesQualifiedName("goog.require")) { + Node fn = n.getFirstChild(); + if (fn.matchesQualifiedName("goog.require") + || fn.matchesQualifiedName("goog.requireType")) { + // TODO(tjgq): This will rewrite both type references and code references. For + // goog.requireType, the latter are potentially broken because the symbols aren't + // guaranteed to be available at run time. A separate pass needs to be added to + // detect these incorrect uses of goog.requireType. visitRequireOrGet(t, n, parent, /* isRequire= */ true); - } else if (n.getFirstChild().matchesQualifiedName("goog.module.get")) { + } else if (fn.matchesQualifiedName("goog.module.get")) { visitGoogModuleGet(t, n, parent); } } @@ -728,7 +768,7 @@ private void rewriteRequires(Node script) { JSDocInfo info = n.getJSDocInfo(); if (info != null) { for (Node typeNode : info.getTypeNodes()) { - inlineRequiredType(t, typeNode); + inlineAliasedTypes(t, typeNode); } } @@ -744,7 +784,7 @@ private void rewriteRequires(Node script) { }); } - private void inlineRequiredType(NodeTraversal t, Node typeNode) { + private void inlineAliasedTypes(NodeTraversal t, Node typeNode) { if (typeNode.isString()) { String name = typeNode.getString(); List split = Splitter.on('.').limit(2).splitToList(name); @@ -764,7 +804,7 @@ private void inlineRequiredType(NodeTraversal t, Node typeNode) { } } for (Node child : typeNode.children()) { - inlineRequiredType(t, child); + inlineAliasedTypes(t, child); } } diff --git a/test/com/google/javascript/jscomp/ClosureRewriteModuleTest.java b/test/com/google/javascript/jscomp/ClosureRewriteModuleTest.java index b46cc9ca95d..8b84a4eed2a 100644 --- a/test/com/google/javascript/jscomp/ClosureRewriteModuleTest.java +++ b/test/com/google/javascript/jscomp/ClosureRewriteModuleTest.java @@ -19,6 +19,7 @@ import static com.google.javascript.jscomp.ClosurePrimitiveErrors.INVALID_GET_CALL_SCOPE; import static com.google.javascript.jscomp.ClosurePrimitiveErrors.INVALID_GET_NAMESPACE; import static com.google.javascript.jscomp.ClosurePrimitiveErrors.INVALID_REQUIRE_NAMESPACE; +import static com.google.javascript.jscomp.ClosurePrimitiveErrors.INVALID_REQUIRE_TYPE_NAMESPACE; import static com.google.javascript.jscomp.ClosurePrimitiveErrors.MISSING_MODULE_OR_PROVIDE; import static com.google.javascript.jscomp.ClosureRewriteModule.DUPLICATE_MODULE; import static com.google.javascript.jscomp.ClosureRewriteModule.DUPLICATE_NAMESPACE; @@ -1043,7 +1044,7 @@ public void testInvalidRequire() { @Test public void testInvalidRequireType() { - testError("goog.module('ns.a');" + "goog.requireType(a);", INVALID_REQUIRE_NAMESPACE); + testError("goog.module('ns.a');" + "goog.requireType(a);", INVALID_REQUIRE_TYPE_NAMESPACE); } @Test diff --git a/test/com/google/javascript/jscomp/Es6RewriteModulesWithGoogInteropTest.java b/test/com/google/javascript/jscomp/Es6RewriteModulesWithGoogInteropTest.java index 693c48bff4a..cdca7654570 100644 --- a/test/com/google/javascript/jscomp/Es6RewriteModulesWithGoogInteropTest.java +++ b/test/com/google/javascript/jscomp/Es6RewriteModulesWithGoogInteropTest.java @@ -23,6 +23,7 @@ import static com.google.javascript.jscomp.Es6RewriteModules.FORWARD_DECLARE_FOR_ES6_SHOULD_BE_CONST; import static com.google.javascript.jscomp.Es6RewriteModules.LHS_OF_GOOG_REQUIRE_MUST_BE_CONST; import static com.google.javascript.jscomp.Es6RewriteModules.NAMESPACE_IMPORT_CANNOT_USE_STAR; +import static com.google.javascript.jscomp.Es6RewriteModules.REQUIRE_TYPE_FOR_ES6_SHOULD_BE_CONST; import com.google.common.collect.ImmutableList; import com.google.javascript.jscomp.CompilerOptions.LanguageMode; @@ -118,6 +119,12 @@ public void testGoogRequireInNonEs6ModuleUnchanged() { testSame("var bar = goog.require('foo.bar');"); } + @Test + public void testGoogRequireTypeInNonEs6ModuleUnchanged() { + testSame("goog.requireType('foo.bar');"); + testSame("var bar = goog.requireType('foo.bar');"); + } + @Test public void testForwardDeclareInNonEs6ModuleUnchanged() { testSame("goog.forwardDeclare('foo.bar');"); @@ -129,7 +136,12 @@ public void testGoogRequireInEs6ModuleDoesNotExistIsError() { testError("export var x; goog.require('foo.bar');", MISSING_MODULE_OR_PROVIDE); } - // TODO(tjgq): Add tests for require and forwardDeclare with an invalid namespace. + @Test + public void testGoogRequireTypeInEs6ModuleDoesNotExistIsError() { + testError("export var x; goog.requireType('foo.bar');", MISSING_MODULE_OR_PROVIDE); + } + + // TODO(tjgq): Add tests for require, requireType and forwardDeclare with an invalid namespace. // They currently produce a DEPS_PARSE_ERROR in JsFileLineParser. @Test @@ -146,6 +158,26 @@ public void testGoogRequireForProvide() { "use(closure.provide, closure.provide.x);", "/** @const */ var module$testcode = {};")); } + @Test + public void testGoogRequireTypeForProvide() { + testModules( + lines( + "const y = goog.requireType('closure.provide');", + "/**", + " * @param {y} a", + " * @param {y.x} b", + "*/", + "function f(a, b) {}", + "export {};"), + lines( + "/**", + " * @param {closure.provide} a", + " * @param {closure.provide.x} b", + " */", + "function f$$module$testcode(a, b) {}", + "/** @const */ var module$testcode = {};")); + } + @Test public void testGoogRequireForProvideWithDestructure() { testModules( @@ -155,6 +187,28 @@ public void testGoogRequireForProvideWithDestructure() { "/** @const */ var module$testcode = {};")); } + @Test + public void testGoogRequireTypeForProvideWithDestructure() { + testModules( + lines( + "const {a, b:c} = goog.requireType('closure.provide');", + "/**", + " * @param {a} x", + " * @param {a.d} y", + " * @param {c} z", + "*/", + "function f(x, y, z) {}", + "export {};"), + lines( + "/**", + " * @param {closure.provide.a} x", + " * @param {closure.provide.a.d} y", + " * @param {closure.provide.b} z", + " */", + "function f$$module$testcode(x, y, z) {}", + "/** @const */ var module$testcode = {};")); + } + @Test public void testGoogRequireForGoogModule() { testModules( @@ -164,6 +218,26 @@ public void testGoogRequireForGoogModule() { "/** @const */ var module$testcode = {};")); } + @Test + public void testGoogRequireTypeForGoogModule() { + testModules( + lines( + "const y = goog.requireType('closure.module');", + "/**", + " * @param {y} a", + " * @param {y.x} b", + "*/", + "function f(a, b) {}", + "export {};"), + lines( + "/**", + " * @param {module$exports$closure$module} a", + " * @param {module$exports$closure$module.x} b", + " */", + "function f$$module$testcode(a, b) {}", + "/** @const */ var module$testcode = {};")); + } + @Test public void testGoogRequireForGoogModuleWithDestructure() { testModules( @@ -174,6 +248,28 @@ public void testGoogRequireForGoogModuleWithDestructure() { "/** @const */ var module$testcode = {};")); } + @Test + public void testGoogRequireTypeForGoogModuleWithDestructure() { + testModules( + lines( + "const {a, b:c} = goog.requireType('closure.module');", + "/**", + " * @param {a} x", + " * @param {a.d} y", + " * @param {c} z", + "*/", + "function f(x, y, z) {}", + "export {};"), + lines( + "/**", + " * @param {module$exports$closure$module.a} x", + " * @param {module$exports$closure$module.a.d} y", + " * @param {module$exports$closure$module.b} z", + " */", + "function f$$module$testcode(x, y, z) {}", + "/** @const */ var module$testcode = {};")); + } + @Test public void testGoogRequireForLegacyGoogModule() { testModules( @@ -183,6 +279,26 @@ public void testGoogRequireForLegacyGoogModule() { "/** @const */ var module$testcode = {};")); } + @Test + public void testGoogRequireTypeForLegacyGoogModule() { + testModules( + lines( + "const y = goog.requireType('closure.legacy.module');", + "/**", + " * @param {y} a", + " * @param {y.x} b", + "*/", + "function f(a, b) {}", + "export {};"), + lines( + "/**", + " * @param {closure.legacy.module} a", + " * @param {closure.legacy.module.x} b", + " */", + "function f$$module$testcode(a, b) {}", + "/** @const */ var module$testcode = {};")); + } + @Test public void testGoogRequireForLegacyGoogModuleWithDestructure() { testModules( @@ -195,6 +311,28 @@ public void testGoogRequireForLegacyGoogModuleWithDestructure() { "/** @const */ var module$testcode = {};")); } + @Test + public void testGoogRequireTypeForLegacyGoogModuleWithDestructure() { + testModules( + lines( + "const {a, b:c} = goog.requireType('closure.legacy.module');", + "/**", + " * @param {a} x", + " * @param {a.d} y", + " * @param {c} z", + "*/", + "function f(x, y, z) {}", + "export {};"), + lines( + "/**", + " * @param {closure.legacy.module.a} x", + " * @param {closure.legacy.module.a.d} y", + " * @param {closure.legacy.module.b} z", + " */", + "function f$$module$testcode(x, y, z) {}", + "/** @const */ var module$testcode = {};")); + } + @Test public void testGoogModuleGetForGoogModule() { testModules( @@ -273,7 +411,7 @@ public void testDeclareModuleId() { } @Test - public void testDeclareNamespace() { + public void testGoogRequireForDeclareNamespace() { test( srcs( SourceFile.fromCode("es6.js", "export var x; goog.module.declareNamespace('my.es6');"), @@ -288,10 +426,43 @@ public void testDeclareNamespace() { } @Test - public void testDestructureDeclareModuleId() { + public void testGoogRequireTypeForDeclareNamespace() { + test( + srcs( + SourceFile.fromCode("es6.js", "export var x; goog.module.declareNamespace('my.es6');"), + SourceFile.fromCode( + "goog.js", + lines( + "goog.module('bar')", + "const es6 = goog.requireType('my.es6');", + "/**", + " * @param {es6} a", + " * @param {es6.x} b", + " */", + "function f(a, b) {}"))), + expected( + SourceFile.fromCode( + "es6.js", + lines( + "var x$$module$es6;/** @const */ var module$es6={};", + "/** @const */ module$es6.x=x$$module$es6;")), + SourceFile.fromCode( + "goog.js", + lines( + "goog.module('bar')", + "/**", + " * @param {module$es6} a", + " * @param {module$es6.x} b", + " */", + "function f(a, b) {}")))); + } + + @Test + public void testGoogRequireForDeclareNamespaceWithDestructure() { test( srcs( - SourceFile.fromCode("es6.js", "export var x, z; goog.declareModuleId('my.es6');"), + SourceFile.fromCode( + "es6.js", "export var x, z; goog.module.declareNamespace('my.es6');"), SourceFile.fromCode("goog.js", "const {x, z: y} = goog.require('my.es6'); use(x, y);")), expected( SourceFile.fromCode( @@ -307,12 +478,20 @@ public void testDestructureDeclareModuleId() { } @Test - public void testDestructureDeclareNamespace() { + public void testGoogRequireTypeForDeclareNamespaceWithDestructure() { test( srcs( SourceFile.fromCode( "es6.js", "export var x, z; goog.module.declareNamespace('my.es6');"), - SourceFile.fromCode("goog.js", "const {x, z: y} = goog.require('my.es6'); use(x, y);")), + SourceFile.fromCode( + "goog.js", + lines( + "const {x, z: y} = goog.requireType('my.es6');", + "/**", + " * @param {x} a", + " * @param {y} b", + " */", + "function f(a, b) {}"))), expected( SourceFile.fromCode( "es6.js", @@ -323,7 +502,12 @@ public void testDestructureDeclareNamespace() { "/** @const */ module$es6.z=z$$module$es6;")), SourceFile.fromCode( "goog.js", - lines("const x = module$es6.x;", "const y = module$es6.z;", "use(x, y);")))); + lines( + "/**", + " * @param {module$es6.x} a", + " * @param {module$es6.z} b", + " */", + "function f(a, b) {}")))); } @Test @@ -345,6 +529,58 @@ public void testGoogRequireLhsNonConstIsError() { LHS_OF_GOOG_REQUIRE_MUST_BE_CONST); } + @Test + public void testGoogRequireTypeLhsNonConstIsError() { + testModulesError( + lines("var bar = goog.requireType('closure.provide');", "export var x;"), + LHS_OF_GOOG_REQUIRE_MUST_BE_CONST); + + testModulesError( + lines("export var x;", "var bar = goog.requireType('closure.provide');"), + LHS_OF_GOOG_REQUIRE_MUST_BE_CONST); + + testModulesError( + lines( + "export {};", + "var {foo, bar} = goog.requireType('closure.provide');", + "/**", + " * @param {foo} x", + " * @param {bar} y", + " */", + "function f(x, y) {}"), + LHS_OF_GOOG_REQUIRE_MUST_BE_CONST); + + testModulesError( + lines( + "export {};", + "let {foo, bar} = goog.requireType('closure.provide');", + "/**", + " * @param {foo} x", + " * @param {bar} y", + " */", + "function f(x, y) {}"), + LHS_OF_GOOG_REQUIRE_MUST_BE_CONST); + } + + @Test + public void testGoogRequireTypeForNonEs6LhsNonConst() { + testError( + ImmutableList.of( + SourceFile.fromCode("es6.js", "export var x; goog.declareModuleId('es6');"), + SourceFile.fromCode( + "closure.js", + lines("goog.module('my.module');", "var es6 = goog.requireType('es6');"))), + REQUIRE_TYPE_FOR_ES6_SHOULD_BE_CONST); + + testError( + ImmutableList.of( + SourceFile.fromCode("es6.js", "export var x; goog.declareModuleId('es6');"), + SourceFile.fromCode( + "closure.js", + lines("goog.module('my.module');", "let { x } = goog.requireType('es6');"))), + REQUIRE_TYPE_FOR_ES6_SHOULD_BE_CONST); + } + @Test public void testNamespaceImports() { testModules( @@ -389,6 +625,12 @@ public void testGoogRequireMustBeModuleScope() { testModulesError("{ goog.require('closure.provide'); } export {}", INVALID_CLOSURE_CALL_ERROR); } + @Test + public void testGoogRequireTypeMustBeModuleScope() { + testModulesError( + "{ goog.requireType('closure.provide'); } export {}", INVALID_CLOSURE_CALL_ERROR); + } + @Test public void testGoogModuleGetCannotBeModuleHoistScope() { testModulesError("goog.module.get('closure.module'); export {}", MODULE_USES_GOOG_MODULE_GET); @@ -435,6 +677,26 @@ public void testGoogRequireAnnotationIsRenamed() { "/** @const */ module$testcode.value = value$$module$testcode;")); } + @Test + public void testGoogRequireTypeAnnotationIsRenamed() { + testModules( + lines( + "const {Type} = goog.requireType('closure.provide');", + "export let /** !Type */ value;"), + lines( + "let /** !closure.provide.Type */ value$$module$testcode;", + "/** @const */ var module$testcode={};", + "/** @const */ module$testcode.value = value$$module$testcode;")); + + testModules( + lines( + "const Type = goog.requireType('closure.provide');", "export let /** !Type */ value;"), + lines( + "let /** !closure.provide */ value$$module$testcode;", + "/** @const */ var module$testcode={};", + "/** @const */ module$testcode.value = value$$module$testcode;")); + } + @Test public void testGoogRequireCorrectAnnotationIsRenamed() { testModules( @@ -455,6 +717,26 @@ public void testGoogRequireCorrectAnnotationIsRenamed() { "/** @const */ module$testcode.value = value$$module$testcode;")); } + @Test + public void testGoogRequireTypeCorrectAnnotationIsRenamed() { + testModules( + lines( + "const {Type} = goog.requireType('closure.provide');", + "export let /** !Type */ value;", + "function foo() {", + " class Type {}", + " let /** !Type */ value;", + "}"), + lines( + "let /** !closure.provide.Type */ value$$module$testcode;", + "function foo$$module$testcode() {", + " class Type {}", + " let /** !Type */ value;", + "}", + "/** @const */ var module$testcode={};", + "/** @const */ module$testcode.value = value$$module$testcode;")); + } + @Test public void testForwardDeclareEs6Module() { test( @@ -576,6 +858,42 @@ public void testWarnAboutRequireEs6FromEs6() { warning(Es6RewriteModules.SHOULD_IMPORT_ES6_MODULE)); } + @Test + public void testGoogRequireTypeEs6ModuleInEs6Module() { + // TODO(tjgq): Make these warn once there's a requireType equivalent for ES6 modules. + + test( + srcs( + SourceFile.fromCode("es6.js", "export {}; goog.module.declareNamespace('es6');"), + SourceFile.fromCode( + "requiretype.js", + lines( + "export {}", + "const alias = goog.requireType('es6');", + "let /** !alias.Type */ x;"))), + expected( + SourceFile.fromCode("es6.js", "/** @const */ var module$es6 = {};"), + SourceFile.fromCode( + "requiretype.js", + lines( + "let /** !module$es6.Type */ x$$module$requiretype;", + "/** @const */ var module$requiretype = {};")))); + + test( + srcs( + SourceFile.fromCode("es6.js", "export {}; goog.module.declareNamespace('es6');"), + SourceFile.fromCode( + "requiretype.js", + lines("export {};", "goog.requireType('es6');", "let /** !es6.Type */ x;"))), + expected( + SourceFile.fromCode("es6.js", "/** @const */ var module$es6 = {};"), + SourceFile.fromCode( + "closure.js", + lines( + "let /** !module$es6.Type */ x$$module$requiretype;", + "/** @const */ var module$requiretype = {};")))); + } + @Test public void testGoogModuleGetEs6ModuleInEs6Module() { test(