Skip to content

Commit

Permalink
Rollback "Teach goog.module rewriting pass to understand MODULE_BODY …
Browse files Browse the repository at this point in the history 
…nodes."

It broke lots of stuff.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=158269114
  • Loading branch information
@brad4d
tap-prod authored and brad4d committed Jun 7, 2017
1 parent f60fddd commit 0704b56
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 41 deletions.
70 changes: 33 additions & 37 deletions src/com/google/javascript/jscomp/ClosureRewriteModule.java
View file
Expand Up @@ -24,6 +24,7 @@
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.common.collect.Multimap; import com.google.common.collect.Multimap;
import com.google.javascript.jscomp.NodeTraversal.AbstractPreOrderCallback;
import com.google.javascript.jscomp.NodeTraversal.Callback; import com.google.javascript.jscomp.NodeTraversal.Callback;
import com.google.javascript.rhino.IR; import com.google.javascript.rhino.IR;
import com.google.javascript.rhino.JSDocInfo; import com.google.javascript.rhino.JSDocInfo;
Expand Down Expand Up @@ -290,7 +291,7 @@ private static final class ScriptDescription {
String legacyNamespace; // "a.b.c" String legacyNamespace; // "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<>(); final Deque<ScriptDescription> childScripts = new LinkedList<>(); // For goog.loadModule()
final Map<String, String> namesToInlineByAlias = new HashMap<>(); // For alias inlining. final Map<String, String> namesToInlineByAlias = new HashMap<>(); // For alias inlining.


/** /**
Expand All @@ -303,8 +304,9 @@ private static final class ScriptDescription {
Set<String> namedExports = new HashSet<>(); Set<String> namedExports = new HashSet<>();
Map<Var, ExportDefinition> exportsToInline = new HashMap<>(); Map<Var, ExportDefinition> exportsToInline = new HashMap<>();


// The root of the module. The MODULE_BODY node (or for goog.loadModule, the body of the // The root of the module. The SCRIPT node (or for goog.loadModule, the body of the
// function) that contains the module contents. For recognizing top level names. // function) that contains the module contents. For recognizing top level names. Changes when
// unwrapping a goog.loadModule() call.
Node rootNode; Node rootNode;


public void addChildScript(ScriptDescription childScript) { public void addChildScript(ScriptDescription childScript) {
Expand Down Expand Up @@ -332,17 +334,16 @@ String getExportedNamespace() {
} }
} }


private class ScriptRecorder implements Callback { private class ScriptRecorder extends AbstractPreOrderCallback {
@Override @Override
public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) { public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
if (NodeUtil.isGoogModuleFile(n)) { if (NodeUtil.isGoogModuleFile(n)) {
inlineModuleIntoGlobal(n);
t.reportCodeChange();
checkAndSetStrictModeDirective(t, n); checkAndSetStrictModeDirective(t, n);
} }


switch (n.getToken()) { switch (n.getToken()) {
case MODULE_BODY:
recordModuleBody(n);
break;
case CALL: case CALL:
Node method = n.getFirstChild(); Node method = n.getFirstChild();
if (!method.isGetProp()) { if (!method.isGetProp()) {
Expand Down Expand Up @@ -408,26 +409,15 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {


return true; return true;
} }

@Override
public void visit(NodeTraversal t, Node n, Node parent) {
if (n.isModuleBody()) {
popScript();
}
}
} }


private class ScriptUpdater implements Callback { private class ScriptUpdater implements Callback {
@Override @Override
public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) { public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
switch (n.getToken()) { switch (n.getToken()) {
case MODULE_BODY:
updateModuleBodyEarly(n);
break;

case EXPR_RESULT: case EXPR_RESULT:
if (isGoogLoadModuleStatement(n)) { if (isGoogLoadModuleStatement(n)) {
updateModuleBodyEarly(n.getFirstChild().getLastChild().getLastChild()); updateGoogLoadModuleEarly(n);
} }
break; break;


Expand Down Expand Up @@ -482,8 +472,8 @@ public void visit(NodeTraversal t, Node n, Node parent) {
} }
break; break;


case MODULE_BODY: case SCRIPT:
updateModuleBody(n); updateEndScript();
break; break;


case NAME: case NAME:
Expand Down Expand Up @@ -709,16 +699,9 @@ public void hotSwapScript(Node scriptRoot, Node originalRoot) {
} }


private void recordGoogLoadModule(Node call) { private void recordGoogLoadModule(Node call) {
Node moduleScopeRoot = call.getLastChild().getLastChild();
Preconditions.checkState(NodeUtil.isModuleScopeRoot(moduleScopeRoot),
"goog.loadModule called with non-module contents: %s", moduleScopeRoot);
recordModuleBody(moduleScopeRoot);
}

private void recordModuleBody(Node moduleRoot) {
pushScript(new ScriptDescription()); pushScript(new ScriptDescription());


currentScript.rootNode = moduleRoot; currentScript.rootNode = NodeUtil.getEnclosingStatement(call).getParent();
currentScript.isModule = true; currentScript.isModule = true;
} }


Expand All @@ -730,6 +713,8 @@ private void recordGoogModule(NodeTraversal t, Node call) {
} }
String legacyNamespace = legacyNamespaceNode.getString(); String legacyNamespace = legacyNamespaceNode.getString();


currentScript.isModule = true;
currentScript.rootNode = NodeUtil.getEnclosingStatement(call).getParent();
currentScript.legacyNamespace = legacyNamespace; currentScript.legacyNamespace = legacyNamespace;
currentScript.contentsPrefix = toModuleContentsPrefix(legacyNamespace); currentScript.contentsPrefix = toModuleContentsPrefix(legacyNamespace);


Expand Down Expand Up @@ -954,8 +939,18 @@ private void recordModuleReturn() {
popScript(); popScript();
} }


private void updateModuleBodyEarly(Node moduleScopeRoot) { static void inlineModuleIntoGlobal(Node scriptNode) {
Preconditions.checkArgument(NodeUtil.isGoogModuleFile(scriptNode));
Node moduleNode = scriptNode.getFirstChild();
scriptNode.removeChild(moduleNode);
scriptNode.addChildrenToBack(moduleNode.removeChildren());
}

private void updateGoogLoadModuleEarly(Node exprResultNode) {
pushScript(currentScript.removeFirstChildScript()); pushScript(currentScript.removeFirstChildScript());
Node moduleScopeRoot = exprResultNode.getFirstChild().getLastChild().getLastChild();
Preconditions.checkState(NodeUtil.isModuleScopeRoot(moduleScopeRoot),
"goog.loadModule called with non-module contents: %s", moduleScopeRoot);
currentScript.rootNode = moduleScopeRoot; currentScript.rootNode = moduleScopeRoot;
} }


Expand Down Expand Up @@ -1142,7 +1137,9 @@ private void recordExportsPropertyAssignment(NodeTraversal t, Node getpropNode)
Node exportsNameNode = getpropNode.getFirstChild(); Node exportsNameNode = getpropNode.getFirstChild();
Preconditions.checkState(exportsNameNode.getString().equals("exports")); Preconditions.checkState(exportsNameNode.getString().equals("exports"));


if (t.inModuleScope()) { // Would be just t.inModuleScope() if this ran before the inlineModuleIntoGlobal() call
// that happens at the beginning of module rewriting.
if (t.inModuleScope() || t.inGlobalScope()) {
String exportName = getpropNode.getLastChild().getString(); String exportName = getpropNode.getLastChild().getString();
currentScript.namedExports.add(exportName); currentScript.namedExports.add(exportName);
Node exportRhs = getpropNode.getNext(); Node exportRhs = getpropNode.getNext();
Expand Down Expand Up @@ -1221,7 +1218,7 @@ private void maybeUpdateTopLevelName(NodeTraversal t, Node nameNode) {
if (namespaceToInline.indexOf('.') != -1) { if (namespaceToInline.indexOf('.') != -1) {
String firstQualifiedName = namespaceToInline.substring(0, namespaceToInline.indexOf('.')); String firstQualifiedName = namespaceToInline.substring(0, namespaceToInline.indexOf('.'));
Var shadowedVar = t.getScope().getVar(firstQualifiedName); Var shadowedVar = t.getScope().getVar(firstQualifiedName);
if (shadowedVar != null && !shadowedVar.getScope().isModuleScope()) { if (shadowedVar != null && shadowedVar.isLocal()) {
t.report( t.report(
shadowedVar.getNode(), shadowedVar.getNode(),
IMPORT_INLINING_SHADOWS_VAR, IMPORT_INLINING_SHADOWS_VAR,
Expand Down Expand Up @@ -1377,13 +1374,12 @@ private void updateModuleReturn(Node returnNode) {
popScript(); popScript();
} }


void updateModuleBody(Node moduleBody) { private void updateEndScript() {
Preconditions.checkArgument(moduleBody.isModuleBody()); if (!currentScript.isModule) {
moduleBody.setToken(Token.BLOCK); return;
NodeUtil.tryMergeBlock(moduleBody); }


updateEndModule(); updateEndModule();
popScript();
} }


private void updateEndModule() { private void updateEndModule() {
Expand Down
5 changes: 1 addition & 4 deletions src/com/google/javascript/jscomp/WhitespaceWrapGoogModules.java
View file
Expand Up @@ -18,7 +18,6 @@
import com.google.common.base.Preconditions; import com.google.common.base.Preconditions;
import com.google.javascript.rhino.IR; import com.google.javascript.rhino.IR;
import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.Token;


/** /**
* Replicates the effect of {@literal ClosureBundler} in whitespace-only mode and wraps goog.modules * Replicates the effect of {@literal ClosureBundler} in whitespace-only mode and wraps goog.modules
Expand All @@ -45,9 +44,7 @@ public void hotSwapScript(Node scriptRoot, Node originalRoot) {
if (!NodeUtil.isGoogModuleFile(scriptRoot)) { if (!NodeUtil.isGoogModuleFile(scriptRoot)) {
return; return;
} }
Node moduleBody = scriptRoot.getFirstChild(); ClosureRewriteModule.inlineModuleIntoGlobal(scriptRoot);
moduleBody.setToken(Token.BLOCK);
NodeUtil.tryMergeBlock(moduleBody);
compiler.reportChangeToEnclosingScope(scriptRoot); compiler.reportChangeToEnclosingScope(scriptRoot);


// As per ClosureBundler: // As per ClosureBundler:
Expand Down

0 comments on commit 0704b56

Please sign in to comment.