Skip to content

Commit

Permalink
Rollfoward "Inline require'd symbols when rewriting ES6 modules."
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@brad4d
brad4d committed Jul 25, 2018
1 parent c6853d0 commit 25a54de
Show file tree
Hide file tree
Showing 2 changed files with 119 additions and 21 deletions.
74 changes: 70 additions & 4 deletions src/com/google/javascript/jscomp/Es6RewriteModules.java
View file
Expand Up @@ -103,6 +103,36 @@ public final class Es6RewriteModules extends AbstractPostOrderCallback
*/
private Map<String, ModuleOriginalNamePair> 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.
*
* <p>We use this to rewrite something like:
*
* <pre>
* import {x} from '';
* const {assert} = goog.require('goog.asserts');
* assert(x);
* </pre>
*
* To:
*
* <pre>
* import {x} from '';
* goog.asserts.assert(x);
* </pre>
*
* Because if we used an alias like below the assertion would not be recognized:
*
* <pre>
* import {x} from '';
* const {assert} = goog.asserts;
* assert(x);
* </pre>
*/
private Map<String, String> namesToInlineByAlias;

private final Set<Node> importedGoogNames;

private Set<String> typedefs;
Expand Down Expand Up @@ -179,6 +209,7 @@ public void clearState() {
this.importMap = new HashMap<>();
this.importedGoogNames.clear();
this.typedefs = new HashSet<>();
this.namesToInlineByAlias = new HashMap<>();
}

/**
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
}
Expand All @@ -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);
Expand Down
66 changes: 49 additions & 17 deletions test/com/google/javascript/jscomp/Es6RewriteModulesWithGoogInteropTest.java
View file
Expand Up @@ -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 = {};"));
}

Expand All @@ -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 = {};"));
}

Expand Down Expand Up @@ -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);"),
Expand Down Expand Up @@ -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;"));
}
}

0 comments on commit 25a54de

Please sign in to comment.