Skip to content

Commit

Permalink
Don't create a binary namespace for legacy modules.
Browse files Browse the repository at this point in the history 
Update module rewriting to not create a binary module name for those modules
declared with goog.module.declareLegacyNamespace. This also fixes issues
where the wrong goog.exportSymbol name would be generated in some legacy
goog.module cases (see the new integration tests).
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=122312797
  • Loading branch information
@blickly
blickly committed May 16, 2016
1 parent 775979f commit b227132
Show file tree
Hide file tree
Showing 3 changed files with 244 additions and 119 deletions.
165 changes: 88 additions & 77 deletions src/com/google/javascript/jscomp/ClosureRewriteModule.java
View file
Expand Up @@ -39,6 +39,8 @@
import java.util.Map; import java.util.Map;
import java.util.Set; import java.util.Set;


import javax.annotation.Nullable;

/** /**
* Process aliases in goog.modules. * Process aliases in goog.modules.
* <pre> * <pre>
Expand Down Expand Up @@ -210,7 +212,6 @@ private final class ScriptDescription {
boolean isModule; boolean isModule;
boolean declareLegacyNamespace; boolean declareLegacyNamespace;
String legacyNamespace; // "a.b.c" String legacyNamespace; // "a.b.c"
String binaryNamespace; // "module$exports$a$b$c
String contentsPrefix; // "module$contents$a$b$c_ String contentsPrefix; // "module$contents$a$b$c_
final Set<String> topLevelNames = new HashSet<>(); // For prefixed content renaming. final Set<String> topLevelNames = new HashSet<>(); // For prefixed content renaming.
final Deque<ScriptDescription> childScripts = new LinkedList<>(); // For goog.loadModule() final Deque<ScriptDescription> childScripts = new LinkedList<>(); // For goog.loadModule()
Expand All @@ -235,6 +236,14 @@ public void addChildScript(ScriptDescription childScript) {
public ScriptDescription removeFirstChildScript() { public ScriptDescription removeFirstChildScript() {
return childScripts.removeFirst(); return childScripts.removeFirst();
} }

// "module$exports$a$b$c" for non-legacy modules
@Nullable String getBinaryNamespace() {
if (this.declareLegacyNamespace) {
return null;
}
return MODULE_EXPORTS_PREFIX + this.legacyNamespace.replace('.', '$');
}
} }


private class ScriptRecorder extends AbstractPreOrderCallback { private class ScriptRecorder extends AbstractPreOrderCallback {
Expand Down Expand Up @@ -411,11 +420,7 @@ public void visit(Node typeRefNode) {
currentScript.legacyNamespacesByAlias.containsKey(prefixTypeName); currentScript.legacyNamespacesByAlias.containsKey(prefixTypeName);
if (nameIsAnAlias) { if (nameIsAnAlias) {
String legacyNamespace = currentScript.legacyNamespacesByAlias.get(prefixTypeName); String legacyNamespace = currentScript.legacyNamespacesByAlias.get(prefixTypeName);
boolean targetNamespaceIsAGoogModule = rewriteState.containsModule(legacyNamespace); String aliasedNamespace = rewriteState.getExportedNamespace(legacyNamespace);
String aliasedNamespace =
targetNamespaceIsAGoogModule
? toBinaryNamespace(legacyNamespace)
: legacyNamespace;
safeSetString(typeRefNode, aliasedNamespace + suffix); safeSetString(typeRefNode, aliasedNamespace + suffix);
return; return;
} }
Expand All @@ -428,9 +433,10 @@ public void visit(Node typeRefNode) {
return; return;
} }


boolean targetIsAGoogModule = rewriteState.containsModule(prefixTypeName);
String binaryNamespaceIfModule = rewriteState.getBinaryNamespace(prefixTypeName);
if (legacyScriptNamespacesAndPrefixes.contains(prefixTypeName) if (legacyScriptNamespacesAndPrefixes.contains(prefixTypeName)
&& !targetIsAGoogModule) { && binaryNamespaceIfModule == null) {
// This thing is definitely coming from a legacy script and so the fully qualified // This thing is definitely coming from a legacy script and so the fully qualified
// type name will always resolve as is. // type name will always resolve as is.
return; return;
Expand All @@ -439,8 +445,8 @@ public void visit(Node typeRefNode) {
// If the typeName is a reference to a fully qualified legacy namespace like // If the typeName is a reference to a fully qualified legacy namespace like
// "foo.bar.Baz" of something that is actually a module then rewrite the JsDoc reference // "foo.bar.Baz" of something that is actually a module then rewrite the JsDoc reference
// to "module$exports$Bar". // to "module$exports$Bar".
if (targetIsAGoogModule) { if (binaryNamespaceIfModule != null) {
safeSetString(typeRefNode, toBinaryNamespace(prefixTypeName) + suffix); safeSetString(typeRefNode, binaryNamespaceIfModule + suffix);
return; return;
} }


Expand Down Expand Up @@ -477,6 +483,16 @@ boolean isLegacyModule(String legacyNamespace) {
return scriptDescriptionsByGoogModuleNamespace.get(legacyNamespace).declareLegacyNamespace; return scriptDescriptionsByGoogModuleNamespace.get(legacyNamespace).declareLegacyNamespace;
} }


@Nullable String getBinaryNamespace(String legacyNamespace) {
ScriptDescription script = scriptDescriptionsByGoogModuleNamespace.get(legacyNamespace);
return script == null ? null : script.getBinaryNamespace();
}

String getExportedNamespace(String legacyNamespace) {
String binaryNamespace = getBinaryNamespace(legacyNamespace);
return binaryNamespace != null ? binaryNamespace : legacyNamespace;
}

void trackModule(String legacyNamespace, ScriptDescription description) { void trackModule(String legacyNamespace, ScriptDescription description) {
scriptDescriptionsByGoogModuleNamespace.put(legacyNamespace, description); scriptDescriptionsByGoogModuleNamespace.put(legacyNamespace, description);
moduleNamesByRootNode.put(description.rootNode, legacyNamespace); moduleNamesByRootNode.put(description.rootNode, legacyNamespace);
Expand Down Expand Up @@ -576,7 +592,6 @@ private void recordGoogModule(NodeTraversal t, Node call) {
currentScript.isModule = true; currentScript.isModule = true;
currentScript.rootNode = NodeUtil.getEnclosingStatement(call).getParent(); currentScript.rootNode = NodeUtil.getEnclosingStatement(call).getParent();
currentScript.legacyNamespace = legacyNamespace; currentScript.legacyNamespace = legacyNamespace;
currentScript.binaryNamespace = toBinaryNamespace(legacyNamespace);
currentScript.contentsPrefix = toModuleContentsPrefix(legacyNamespace); currentScript.contentsPrefix = toModuleContentsPrefix(legacyNamespace);


// If some other script is advertising itself as a goog.module() with this same namespace. // If some other script is advertising itself as a goog.module() with this same namespace.
Expand Down Expand Up @@ -726,7 +741,7 @@ private void recordGoogModuleGet(NodeTraversal t, Node call) {


if (aliasName != null if (aliasName != null
&& isTopLevel(t, NodeUtil.getEnclosingStatement(call), ScopeType.EXEC_CONTEXT)) { && isTopLevel(t, NodeUtil.getEnclosingStatement(call), ScopeType.EXEC_CONTEXT)) {
// Record alias -> binaryNamespace associations for later inlining. // Record alias -> legacyNamespace associations for later inlining.
recordImportAlias(aliasName, legacyNamespace); recordImportAlias(aliasName, legacyNamespace);
} }
} }
Expand All @@ -751,7 +766,10 @@ private void recordTopLevelVarNames(Node varNode) {
} }


private void maybeRecordExportDeclaration(Node n) { private void maybeRecordExportDeclaration(Node n) {
if (!currentScript.isModule || !n.getString().equals("exports") || !isAssignTarget(n)) { if (!currentScript.isModule
|| currentScript.declareLegacyNamespace
|| !n.getString().equals("exports")
|| !isAssignTarget(n)) {
return; return;
} }


Expand Down Expand Up @@ -810,8 +828,10 @@ private void updateGoogRequire(NodeTraversal t, Node call) {
Node statementNode = NodeUtil.getEnclosingStatement(call); Node statementNode = NodeUtil.getEnclosingStatement(call);
String legacyNamespace = legacyNamespaceNode.getString(); String legacyNamespace = legacyNamespaceNode.getString();


boolean targetIsGoogModule = rewriteState.containsModule(legacyNamespace); boolean targetIsNonLegacyGoogModule =
boolean targetIsAGoogModuleGetted = rewriteState.containsModule(legacyNamespace)
&& !rewriteState.isLegacyModule(legacyNamespace);
boolean targetIsGoogModuleGetted =
currentScript.googModuleGettedNamespaces.contains(legacyNamespaceNode.getString()); currentScript.googModuleGettedNamespaces.contains(legacyNamespaceNode.getString());
boolean importHasAlias = currentScript.legacyNamespacesByAlias.containsValue(legacyNamespace); boolean importHasAlias = currentScript.legacyNamespacesByAlias.containsValue(legacyNamespace);
boolean isDestructuring = statementNode.getFirstChild().isDestructuringLhs(); boolean isDestructuring = statementNode.getFirstChild().isDestructuringLhs();
Expand All @@ -821,30 +841,28 @@ private void updateGoogRequire(NodeTraversal t, Node call) {
&& call.getGrandparent().isName() && call.getGrandparent().isName()
&& NodeUtil.isNameDeclaration(call.getGrandparent().getParent()); && NodeUtil.isNameDeclaration(call.getGrandparent().getParent());


if (currentScript.isModule || (targetIsGoogModule && targetIsAGoogModuleGetted)) { if (currentScript.isModule || (targetIsNonLegacyGoogModule && targetIsGoogModuleGetted)) {
if (isDestructuring) { if (isDestructuring) {
// Rewrite // Rewrite
// "var {a, b} = goog.require('foo.Bar');" to // "var {a, b} = goog.require('foo.Bar');" to
// "var {a, b} = module$exports$foo$Bar;" or // "var {a, b} = module$exports$foo$Bar;" or
// "var {a, b} = foo.Bar;" // "var {a, b} = foo.Bar;"
Node replacementNamespaceName = String exportedNamespace = rewriteState.getExportedNamespace(legacyNamespace);
NodeUtil.newQName( Node replacementNamespaceName = NodeUtil.newQName(compiler, exportedNamespace);
compiler,
targetIsGoogModule ? toBinaryNamespace(legacyNamespace) : legacyNamespace);
replacementNamespaceName.putProp(Node.ORIGINALNAME_PROP, legacyNamespace); replacementNamespaceName.putProp(Node.ORIGINALNAME_PROP, legacyNamespace);
replacementNamespaceName.putBooleanProp(Node.GOOG_MODULE_REQUIRE, true); replacementNamespaceName.putBooleanProp(Node.GOOG_MODULE_REQUIRE, true);
replacementNamespaceName.srcrefTree(call); replacementNamespaceName.srcrefTree(call);
call.getParent().replaceChild(call, replacementNamespaceName); call.getParent().replaceChild(call, replacementNamespaceName);
markConst(statementNode); markConst(statementNode);
} else if (targetIsGoogModule) { } else if (targetIsNonLegacyGoogModule) {
if (immediatePropertyAccess || !isTopLevel(t, statementNode, ScopeType.EXEC_CONTEXT)) { if (immediatePropertyAccess || !isTopLevel(t, statementNode, ScopeType.EXEC_CONTEXT)) {
// Rewrite // Rewrite
// "var Foo = goog.require("bar.Foo").Foo;" to // "var Foo = goog.require("bar.Foo").Foo;" to
// "var Foo = module$exports$bar$Foo.Foo;" // "var Foo = module$exports$bar$Foo.Foo;"
// or // or
// "function() {var Foo = goog.require("bar.Foo");}" to // "function() {var Foo = goog.require("bar.Foo");}" to
// "function() {var Foo = module$exports$bar$Foo;}" // "function() {var Foo = module$exports$bar$Foo;}"
Node binaryNamespaceName = IR.name(toBinaryNamespace(legacyNamespace)); Node binaryNamespaceName = IR.name(rewriteState.getBinaryNamespace(legacyNamespace));
binaryNamespaceName.putProp(Node.ORIGINALNAME_PROP, legacyNamespace); binaryNamespaceName.putProp(Node.ORIGINALNAME_PROP, legacyNamespace);
call.getParent().replaceChild(call, binaryNamespaceName); call.getParent().replaceChild(call, binaryNamespaceName);
} else if (importHasAlias || !rewriteState.isLegacyModule(legacyNamespace)) { } else if (importHasAlias || !rewriteState.isLegacyModule(legacyNamespace)) {
Expand Down Expand Up @@ -874,7 +892,7 @@ private void updateGoogRequire(NodeTraversal t, Node call) {
statementNode.getParent().replaceChild(statementNode, IR.exprResult(call)); statementNode.getParent().replaceChild(statementNode, IR.exprResult(call));
} }
} }
if (targetIsGoogModule) { if (targetIsNonLegacyGoogModule) {
// Add goog.require() and namespace name to preprocessor table because they're removed // Add goog.require() and namespace name to preprocessor table because they're removed
// by current pass. If target is not a module then goog.require() is retained for // by current pass. If target is not a module then goog.require() is retained for
// ProcessClosurePrimitives pass and symbols will be added there instead. // ProcessClosurePrimitives pass and symbols will be added there instead.
Expand Down Expand Up @@ -906,9 +924,11 @@ private void updateGoogModuleGetCall(Node call) {
// In a non-module script goog.module.get() is used inside of a goog.scope() and it's // In a non-module script goog.module.get() is used inside of a goog.scope() and it's
// imported alias need only be made available within that scope. // imported alias need only be made available within that scope.
// Replace "goog.module.get('pkg.Foo');" with "module$exports$pkg$Foo;". // Replace "goog.module.get('pkg.Foo');" with "module$exports$pkg$Foo;".
Node binaryNamespaceName = IR.name(toBinaryNamespace(legacyNamespace)); Node exportedNamespaceName =
binaryNamespaceName.putProp(Node.ORIGINALNAME_PROP, legacyNamespace); NodeUtil.newQName(compiler, rewriteState.getExportedNamespace(legacyNamespace))
call.getParent().replaceChild(call, binaryNamespaceName); .srcrefTree(call);
exportedNamespaceName.putProp(Node.ORIGINALNAME_PROP, legacyNamespace);
call.getParent().replaceChild(call, exportedNamespaceName);
} }
compiler.reportCodeChange(); compiler.reportCodeChange();
} }
Expand All @@ -924,7 +944,11 @@ private void updateExportsPropertyAssignment(Node getpropNode) {
// Update "exports.foo = Foo" to "module$exports$pkg$Foo.foo = Foo"; // Update "exports.foo = Foo" to "module$exports$pkg$Foo.foo = Foo";
Node exportsNameNode = getpropNode.getFirstChild(); Node exportsNameNode = getpropNode.getFirstChild();
Preconditions.checkState(exportsNameNode.getString().equals("exports")); Preconditions.checkState(exportsNameNode.getString().equals("exports"));
safeSetString(exportsNameNode, currentScript.binaryNamespace); String exportedNamespace =
currentScript.declareLegacyNamespace
? currentScript.legacyNamespace
: currentScript.getBinaryNamespace();
safeSetMaybeQualifiedString(exportsNameNode, exportedNamespace);


Node jsdocNode = parent.isAssign() ? parent : getpropNode; Node jsdocNode = parent.isAssign() ? parent : getpropNode;
markConstAndCopyJsDoc(jsdocNode, jsdocNode, parent.getLastChild()); markConstAndCopyJsDoc(jsdocNode, jsdocNode, parent.getLastChild());
Expand Down Expand Up @@ -958,9 +982,7 @@ private void maybeUpdateTopLevelName(NodeTraversal t, Node nameNode) {
boolean nameIsAnAlias = currentScript.legacyNamespacesByAlias.containsKey(name); boolean nameIsAnAlias = currentScript.legacyNamespacesByAlias.containsKey(name);
if (nameIsAnAlias && var.getNode() != nameNode) { if (nameIsAnAlias && var.getNode() != nameNode) {
String legacyNamespace = currentScript.legacyNamespacesByAlias.get(name); String legacyNamespace = currentScript.legacyNamespacesByAlias.get(name);
boolean targetNamespaceIsAGoogModule = rewriteState.containsModule(legacyNamespace); String namespaceToInline = rewriteState.getExportedNamespace(legacyNamespace);
String namespaceToInline =
targetNamespaceIsAGoogModule ? toBinaryNamespace(legacyNamespace) : legacyNamespace;
safeSetMaybeQualifiedString(nameNode, namespaceToInline); safeSetMaybeQualifiedString(nameNode, namespaceToInline);


// Make sure this action won't shadow a local variable. // Make sure this action won't shadow a local variable.
Expand Down Expand Up @@ -1044,39 +1066,54 @@ private void maybeUpdateExportDeclToNode(NodeTraversal t, Node target, Node valu
* In module "foo.Bar", rewrite "exports = Bar" to "var module$exports$foo$Bar = Bar". * In module "foo.Bar", rewrite "exports = Bar" to "var module$exports$foo$Bar = Bar".
*/ */
private void maybeUpdateExportDeclaration(NodeTraversal t, Node n) { private void maybeUpdateExportDeclaration(NodeTraversal t, Node n) {
if (!currentScript.isModule || !n.getString().equals("exports") || !isAssignTarget(n)) { if (!currentScript.isModule
|| !n.getString().equals("exports")
|| !isAssignTarget(n)) {
return; return;
} }


// Rewrite "exports = ..." as "var module$exports$foo$Bar = ..." // Rewrite "exports = ..." as "var module$exports$foo$Bar = ..."
Node assignNode = n.getParent(); Node assignNode = n.getParent();
Node exprResultNode = assignNode.getParent();
Node rhs = assignNode.getLastChild(); Node rhs = assignNode.getLastChild();
rhs.detachFromParent(); Node jsdocNode;
Node binaryNamespaceName = IR.name(currentScript.binaryNamespace); if (currentScript.declareLegacyNamespace) {
binaryNamespaceName.putProp(Node.ORIGINALNAME_PROP, currentScript.legacyNamespace); Node legacyQname = NodeUtil.newQName(compiler, currentScript.legacyNamespace).srcrefTree(n);
Node exportsObjectCreationNode = IR.var(binaryNamespaceName, rhs); assignNode.replaceChild(n, legacyQname);
exportsObjectCreationNode.srcrefTree(exprResultNode); jsdocNode = assignNode;
exportsObjectCreationNode.putBooleanProp(Node.IS_NAMESPACE, true); } else {
exprResultNode.getParent().replaceChild(exprResultNode, exportsObjectCreationNode); rhs.detachFromParent();
Node exprResultNode = assignNode.getParent();
Node binaryNamespaceName = IR.name(currentScript.getBinaryNamespace());
binaryNamespaceName.putProp(Node.ORIGINALNAME_PROP, currentScript.legacyNamespace);
Node exportsObjectCreationNode = IR.var(binaryNamespaceName, rhs);
exportsObjectCreationNode.srcrefTree(exprResultNode);
exportsObjectCreationNode.putBooleanProp(Node.IS_NAMESPACE, true);
exprResultNode.getParent().replaceChild(exprResultNode, exportsObjectCreationNode);
jsdocNode = exportsObjectCreationNode;
currentScript.hasCreatedExportObject = true;
}
markConstAndCopyJsDoc(assignNode, jsdocNode, rhs);
compiler.reportCodeChange(); compiler.reportCodeChange();
currentScript.hasCreatedExportObject = true;


markConstAndCopyJsDoc(assignNode, exportsObjectCreationNode, rhs);
maybeExportLegacyNamespaceAfter(exportsObjectCreationNode, assignNode);
maybeUpdateExportObjectLiteral(t, rhs); maybeUpdateExportObjectLiteral(t, rhs);
return; return;
} }


private void maybeUpdateExportNameRef(Node n) { private void maybeUpdateExportNameRef(Node n) {
if (!currentScript.isModule || !"exports".equals(n.getString())) { if (!currentScript.isModule || !"exports".equals(n.getString()) || n.getParent() == null) {
return; return;
} }
if (n.getParent().isParamList()) { if (n.getParent().isParamList()) {
return; return;
} }


safeSetString(n, currentScript.binaryNamespace); if (currentScript.declareLegacyNamespace) {
Node legacyQname = NodeUtil.newQName(compiler, currentScript.legacyNamespace).srcrefTree(n);
n.getParent().replaceChild(n, legacyQname);
return;
}

safeSetString(n, currentScript.getBinaryNamespace());


// Either this module is going to create it's own exports object at some point or else if it's // Either this module is going to create it's own exports object at some point or else if it's
// going to be defensively created automatically then that should have occurred at the top of // going to be defensively created automatically then that should have occurred at the top of
Expand Down Expand Up @@ -1110,7 +1147,8 @@ private void updateEndScript() {


private void updateEndModule() { private void updateEndModule() {
Preconditions.checkState(currentScript.isModule); Preconditions.checkState(currentScript.isModule);
Preconditions.checkState(currentScript.hasCreatedExportObject); Preconditions.checkState(
currentScript.declareLegacyNamespace || currentScript.hasCreatedExportObject);
} }


/** /**
Expand Down Expand Up @@ -1138,7 +1176,11 @@ private void popScript() {
* Add the missing "var module$exports$pkg$Foo = {};" line. * Add the missing "var module$exports$pkg$Foo = {};" line.
*/ */
private void exportTheEmptyBinaryNamespaceAt(Node atNode, AddAt addAt) { private void exportTheEmptyBinaryNamespaceAt(Node atNode, AddAt addAt) {
Node binaryNamespaceName = IR.name(currentScript.binaryNamespace); if (currentScript.declareLegacyNamespace) {
return;
}

Node binaryNamespaceName = IR.name(currentScript.getBinaryNamespace());
binaryNamespaceName.putProp(Node.ORIGINALNAME_PROP, currentScript.legacyNamespace); binaryNamespaceName.putProp(Node.ORIGINALNAME_PROP, currentScript.legacyNamespace);
Node binaryNamespaceExportNode = IR.var(binaryNamespaceName, IR.objectlit()); Node binaryNamespaceExportNode = IR.var(binaryNamespaceName, IR.objectlit());
if (addAt == AddAt.BEFORE) { if (addAt == AddAt.BEFORE) {
Expand All @@ -1151,33 +1193,6 @@ private void exportTheEmptyBinaryNamespaceAt(Node atNode, AddAt addAt) {
markConst(binaryNamespaceExportNode); markConst(binaryNamespaceExportNode);
compiler.reportCodeChange(); compiler.reportCodeChange();
currentScript.hasCreatedExportObject = true; currentScript.hasCreatedExportObject = true;

maybeExportLegacyNamespaceAfter(binaryNamespaceExportNode, null);
}

/**
* Maybe add a "foo.Bar = module$exports$foo$Bar;" statement to satisfy legacy namespace refs.
*/
private void maybeExportLegacyNamespaceAfter(Node afterNode, Node jsDocSourceNode) {
if (!currentScript.declareLegacyNamespace) {
return;
}

Node binaryNamespaceName = IR.name(currentScript.binaryNamespace);
binaryNamespaceName.putProp(Node.ORIGINALNAME_PROP, currentScript.legacyNamespace);
Node assignmentNode =
IR.assign(NodeUtil.newQName(compiler, currentScript.legacyNamespace), binaryNamespaceName);
Node binaryToLegacyBridgeStatement = IR.exprResult(assignmentNode);
binaryToLegacyBridgeStatement.srcrefTree(afterNode);
if (jsDocSourceNode != null) {
markConstAndCopyJsDoc(jsDocSourceNode, assignmentNode, binaryNamespaceName);
} else {
markConst(assignmentNode);
}
// Do NOT mark this statement 'IS_NAMESPACE' because that would prevent
// ProcessClosurePrimitives from properly desugaring goog.provide().
afterNode.getParent().addChildAfter(binaryToLegacyBridgeStatement, afterNode);
compiler.reportCodeChange();
} }


/** /**
Expand Down Expand Up @@ -1337,10 +1352,6 @@ private boolean isTopLevel(NodeTraversal t, Node n, ScopeType scopeType) {
} }
} }


private static String toBinaryNamespace(String legacyNamespace) {
return MODULE_EXPORTS_PREFIX + legacyNamespace.replace('.', '$');
}

private static String toModuleContentsPrefix(String legacyNamespace) { private static String toModuleContentsPrefix(String legacyNamespace) {
return MODULE_CONTENTS_PREFIX + legacyNamespace.replace('.', '$') + "_"; return MODULE_CONTENTS_PREFIX + legacyNamespace.replace('.', '$') + "_";
} }
Expand Down

0 comments on commit b227132

Please sign in to comment.