From 25a54dec67fce3749813e09c03daba1c5b0c5a98 Mon Sep 17 00:00:00 2001 From: "Bradford C. Smith" Date: Wed, 25 Jul 2018 08:33:04 -0700 Subject: [PATCH] Rollfoward "Inline require'd symbols when rewriting ES6 modules." This is needed as some later stages of the compiler specifically check for well-known Closure properties, such as goog.asserts.assert. So `const {assert} = goog.require('goog.asserts'); assert(0);` must be rewritten to `goog.asserts.assert(0);` to be recognized correctly later. NEW: Don't detach the variable statement if there is one so that it can be re-exported. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=205889853 --- .../javascript/jscomp/Es6RewriteModules.java | 74 ++++++++++++++++++- .../Es6RewriteModulesWithGoogInteropTest.java | 66 ++++++++++++----- 2 files changed, 119 insertions(+), 21 deletions(-) diff --git a/src/com/google/javascript/jscomp/Es6RewriteModules.java b/src/com/google/javascript/jscomp/Es6RewriteModules.java index e34067ad6ab..26e448c6534 100644 --- a/src/com/google/javascript/jscomp/Es6RewriteModules.java +++ b/src/com/google/javascript/jscomp/Es6RewriteModules.java @@ -103,6 +103,36 @@ 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. + * + *

We use this to rewrite something like: + * + * 

+   *   import {x} from '';
+   *   const {assert} = goog.require('goog.asserts');
+   *   assert(x);
+   *
+ * + * To: + * + * 
+   *   import {x} from '';
+   *   goog.asserts.assert(x);
+   *
+ * + * Because if we used an alias like below the assertion would not be recognized: + * + * 
+   *   import {x} from '';
+   *   const {assert} = goog.asserts;
+   *   assert(x);
+   *
+ */ + private Map namesToInlineByAlias; + private final Set importedGoogNames; private Set typedefs; @@ -179,6 +209,7 @@ public void clearState() { this.importMap = new HashMap<>(); this.importedGoogNames.clear(); this.typedefs = new HashSet<>(); + this.namesToInlineByAlias = new HashMap<>(); } /** @@ -586,12 +617,26 @@ private void rewriteRequires(Node script) { (NodeTraversal t, Node n, Node parent) -> { if (n.isCall()) { if (n.getFirstChild().matchesQualifiedName("goog.require")) { - visitRequire(t, n, parent, true /* checkScope */); + visitRequireOrGet(t, n, parent, /* isRequire= */ true); } else if (n.getFirstChild().matchesQualifiedName("goog.module.get")) { visitGoogModuleGet(t, n, parent); } } }); + NodeTraversal.traversePostOrder( + compiler, + script, + (NodeTraversal t, Node n, Node parent) -> { + if (n.isName() && namesToInlineByAlias.containsKey(n.getString())) { + Var v = t.getScope().getVar(n.getString()); + if (v == null || v.getNameNode() != n) { + Node replacement = + NodeUtil.newQName(compiler, namesToInlineByAlias.get(n.getString())); + replacement.useSourceInfoFromForTree(n); + n.replaceWith(replacement); + } + } + }); } private void visitGoogModuleGet(NodeTraversal t, Node getCall, Node parent) { @@ -606,17 +651,18 @@ private void visitGoogModuleGet(NodeTraversal t, Node getCall, Node parent) { return; } - visitRequire(t, getCall, parent, false /* checkScope */); + visitRequireOrGet(t, getCall, parent, /* isRequire= */ false); } - private void visitRequire(NodeTraversal t, Node requireCall, Node parent, boolean checkScope) { + private void visitRequireOrGet( + NodeTraversal t, Node requireCall, Node parent, boolean isRequire) { if (!requireCall.hasTwoChildren() || !requireCall.getLastChild().isString()) { t.report(requireCall, INVALID_REQUIRE_NAMESPACE); return; } // Module has already been turned into a script at this point. - if (checkScope && !t.getScope().isGlobal()) { + if (isRequire && !t.getScope().isGlobal()) { t.report(requireCall, INVALID_CLOSURE_CALL_ERROR); return; } @@ -637,6 +683,26 @@ private void visitRequire(NodeTraversal t, Node requireCall, Node parent, boolea } if (isStoredInDeclaration) { + if (isRequire) { + if (parent.isDestructuringLhs()) { + checkState(parent.getFirstChild().isObjectPattern()); + for (Node child : parent.getFirstChild().children()) { + if (child.isStringKey()) { + checkState(child.getFirstChild().isName()); + namesToInlineByAlias.put( + child.getFirstChild().getString(), + m.getGlobalName(namespace) + "." + child.getString()); + } else { + checkState(child.isName()); + namesToInlineByAlias.put( + child.getString(), m.getGlobalName(namespace) + "." + child.getString()); + } + } + } else { + checkState(parent.isName()); + namesToInlineByAlias.put(parent.getString(), m.getGlobalName(namespace)); + } + } Node replacement = NodeUtil.newQName(compiler, m.getGlobalName(namespace)).srcrefTree(requireCall); parent.replaceChild(requireCall, replacement); diff --git a/test/com/google/javascript/jscomp/Es6RewriteModulesWithGoogInteropTest.java b/test/com/google/javascript/jscomp/Es6RewriteModulesWithGoogInteropTest.java index 45292e1baee..e1199c989e4 100644 --- a/test/com/google/javascript/jscomp/Es6RewriteModulesWithGoogInteropTest.java +++ b/test/com/google/javascript/jscomp/Es6RewriteModulesWithGoogInteropTest.java @@ -117,17 +117,38 @@ public void testGoogModuleGetNonStringIsError() { public void testGoogRequireForProvide() { testModules( - "const y = goog.require('closure.provide');\nexport {};", + lines("const y = goog.require('closure.provide');", "use(y, y.x)", "export {};"), lines( "const y$$module$testcode = closure.provide;", + "use(closure.provide, closure.provide.x);", + "/** @const */ var module$testcode = {};")); + } + + public void testGoogRequireForProvideWithDestructure() { + testModules( + lines("const {a, b:c} = goog.require('closure.provide');", "use(a, a.z, c)", "export {};"), + lines( + "const {a:a$$module$testcode, b:c$$module$testcode} = closure.provide;", + "use(closure.provide.a, closure.provide.a.z, closure.provide.b);", "/** @const */ var module$testcode = {};")); } public void testGoogRequireForGoogModule() { testModules( - "const y = goog.require('closure.module');\nexport {};", + lines("const y = goog.require('closure.module');", "use(y, y.x)", "export {};"), lines( "const y$$module$testcode = module$exports$closure$module;", + "use(module$exports$closure$module, module$exports$closure$module.x);", + "/** @const */ var module$testcode = {};")); + } + + public void testGoogRequireForGoogModuleWithDestructure() { + testModules( + lines("const {a, b:c} = goog.require('closure.module');", "use(a, a.z, c)", "export {};"), + lines( + "const {a:a$$module$testcode, b:c$$module$testcode} = module$exports$closure$module;", + "use(module$exports$closure$module.a, module$exports$closure$module.a.z,", + " module$exports$closure$module.b);", "/** @const */ var module$testcode = {};")); } @@ -141,9 +162,22 @@ public void testGoogModuleGetForGoogModule() { public void testGoogRequireForLegacyGoogModule() { testModules( - "const y = goog.require('closure.legacy.module');\nexport {};", + lines("const y = goog.require('closure.legacy.module');", "use(y, y.x)", "export {};"), lines( - "const y$$module$testcode = closure.legacy.module;", + "const y$$module$testcode=closure.legacy.module;", + "use(closure.legacy.module, closure.legacy.module.x);", + "/** @const */ var module$testcode = {};")); + } + + public void testGoogRequireForLegacyGoogModuleWithDestructure() { + testModules( + lines( + "const {a, b:c} = goog.require('closure.legacy.module');", + "use(a, a.z, c)", + "export {};"), + lines( + "const {a:a$$module$testcode,b:c$$module$testcode} = closure.legacy.module;", + "use(closure.legacy.module.a, closure.legacy.module.a.z, closure.legacy.module.b);", "/** @const */ var module$testcode = {};")); } @@ -195,19 +229,6 @@ public void testGoogRequireLhsNonConstIsError() { testModulesError( "export var x;\nvar bar = goog.require('closure.provide');", LHS_OF_GOOG_REQUIRE_MUST_BE_CONST); - } - - public void testGoogRequiresDestructuring_rewrite() { - testModules( - lines( - "export {};", "const {foo, bar} = goog.require('closure.provide');", "use(foo, bar);"), - lines( - "const {", - " foo: foo$$module$testcode,", - " bar: bar$$module$testcode,", - "} = closure.provide;", - "use(foo$$module$testcode, bar$$module$testcode);", - "/** @const */ var module$testcode = {};")); testModulesError( lines("export {};", "var {foo, bar} = goog.require('closure.provide');", "use(foo, bar);"), @@ -265,4 +286,15 @@ public void testGoogModuleGetCannotBeModuleHoistScope() { testModulesError( "{ goog.module.get('closure.module'); } export {}", MODULE_USES_GOOG_MODULE_GET); } + + public void testExportSpecRequiredSymbol() { + testModules( + lines( + "const {a} = goog.require('closure.provide');", + "export {a};"), + lines( + "const {a:a$$module$testcode} = closure.provide", + "/** @const */ var module$testcode={};", + "/** @const */ module$testcode.a = a$$module$testcode;")); + } }