Skip to content

Commit

Permalink
Fully support destructuring imports inside goog.module.
Browse files Browse the repository at this point in the history 
Previously, we were allowing it, but not fully typechecking it in JSDoc and
for checks based on qualified-names like goog.asserts.assert.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=123243993
  • Loading branch information
@blickly
blickly committed May 25, 2016
1 parent 5d4d7a6 commit 13f31d2
Show file tree
Hide file tree
Showing 2 changed files with 144 additions and 82 deletions.
117 changes: 69 additions & 48 deletions src/com/google/javascript/jscomp/ClosureRewriteModule.java
View file
Expand Up @@ -218,7 +218,7 @@ private final class ScriptDescription {
final Set<String> topLevelNames = new HashSet<>(); // For prefixed content renaming.
final Deque<ScriptDescription> childScripts = new LinkedList<>(); // For goog.loadModule()
final Set<String> googModuleGettedNamespaces = new HashSet<>(); // {"some.goog.module", ...}
final Map<String, String> legacyNamespacesByAlias = new HashMap<>(); // For alias inlining.
final Map<String, String> importedNamesByAlias = new HashMap<>(); // For alias inlining.

/**
* Transient state.
Expand Down Expand Up @@ -343,12 +343,6 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
}
break;

case Token.NAME:
maybeUpdateTopLevelName(t, n);
maybeUpdateExportDeclaration(t, n);
maybeUpdateExportNameRef(n);
break;

case Token.RETURN:
if (isTopLevel(t, n, ScopeType.EXEC_CONTEXT)) {
updateModuleReturn(n);
Expand All @@ -369,6 +363,12 @@ public void visit(NodeTraversal t, Node n, Node parent) {
case Token.SCRIPT:
updateEndScript();
break;

case Token.NAME:
maybeUpdateTopLevelName(t, n);
maybeUpdateExportDeclaration(t, n);
maybeUpdateExportNameRef(n);
break;
}
}
}
Expand Down Expand Up @@ -419,10 +419,9 @@ public void visit(Node typeRefNode) {
// "{module$exports$bar$Foo}" or
// "{bar.Foo}"
boolean nameIsAnAlias =
currentScript.legacyNamespacesByAlias.containsKey(prefixTypeName);
currentScript.importedNamesByAlias.containsKey(prefixTypeName);
if (nameIsAnAlias) {
String legacyNamespace = currentScript.legacyNamespacesByAlias.get(prefixTypeName);
String aliasedNamespace = rewriteState.getExportedNamespace(legacyNamespace);
String aliasedNamespace = currentScript.importedNamesByAlias.get(prefixTypeName);
safeSetString(typeRefNode, aliasedNamespace + suffix);
return;
}
Expand Down Expand Up @@ -491,9 +490,15 @@ boolean isLegacyModule(String legacyNamespace) {
return script == null ? null : script.getBinaryNamespace();
}

String getExportedNamespace(String legacyNamespace) {
String binaryNamespace = getBinaryNamespace(legacyNamespace);
return binaryNamespace != null ? binaryNamespace : legacyNamespace;
@Nullable String getExportedNamespace(String legacyNamespace) {
if (legacyScriptNamespaces.contains(legacyNamespace)) {
return legacyNamespace;
}
ScriptDescription script = scriptDescriptionsByGoogModuleNamespace.get(legacyNamespace);
if (script != null) {
return script.declareLegacyNamespace ? legacyNamespace : script.getBinaryNamespace();
}
return null;
}

void removeRoot(Node toRemove) {
Expand Down Expand Up @@ -637,27 +642,12 @@ private void recordGoogRequire(NodeTraversal t, Node call, boolean reportBadRequ
maybeSplitMultiVar(call);

Node legacyNamespaceNode = call.getLastChild();
Node statementNode = NodeUtil.getEnclosingStatement(call);
if (!legacyNamespaceNode.isString()) {
t.report(legacyNamespaceNode, INVALID_REQUIRE_NAMESPACE);
return;
}
String legacyNamespace = legacyNamespaceNode.getString();

// If the current script is a module or the require statement has a return value that is stored
// in an alias then the require is goog.module() style.
boolean currentScriptIsAModule = currentScript.isModule;
// "var Foo = goog.require("bar.Foo");" style.
boolean requireStoredInSimpleAlias =
call.getParent().isName() && NodeUtil.isNameDeclaration(call.getGrandparent());
if (currentScriptIsAModule
&& requireStoredInSimpleAlias
&& isTopLevel(t, statementNode, ScopeType.EXEC_CONTEXT)) {
// Record alias -> legacyNamespace associations for possible later inlining.
String aliasName = statementNode.getFirstChild().getString();
recordImportAlias(aliasName, legacyNamespace);
}

// Maybe report an error if there is an attempt to import something that is expected to be a
// goog.module() but no such goog.module() has been defined.
boolean targetIsAModule = rewriteState.containsModule(legacyNamespace);
Expand Down Expand Up @@ -826,27 +816,49 @@ private void updateGoogRequire(NodeTraversal t, Node call) {
&& !rewriteState.isLegacyModule(legacyNamespace);
boolean targetIsGoogModuleGetted =
currentScript.googModuleGettedNamespaces.contains(legacyNamespaceNode.getString());
boolean importHasAlias = currentScript.legacyNamespacesByAlias.containsValue(legacyNamespace);
boolean importHasAlias = NodeUtil.isNameDeclaration(statementNode);
boolean isDestructuring = statementNode.getFirstChild().isDestructuringLhs();
// Is "(.*)goog.require("bar.Foo").Foo;" style?.
boolean immediatePropertyAccess =
call.getParent().isGetProp()
&& call.getGrandparent().isName()
&& NodeUtil.isNameDeclaration(call.getGrandparent().getParent());

// If the current script is a module or the require statement has a return value that is stored
// in an alias then the require is goog.module() style.
boolean currentScriptIsAModule = currentScript.isModule;
// "var Foo = goog.require("bar.Foo");" or "const {Foo} = goog.require('bar');" style.
boolean requireDirectlyStoredInAlias = NodeUtil.isNameDeclaration(call.getGrandparent());
if (currentScriptIsAModule
&& requireDirectlyStoredInAlias
&& isTopLevel(t, statementNode, ScopeType.EXEC_CONTEXT)) {
// Record alias -> exportedNamespace associations for later inlining.
Node lhs = call.getParent();
String exportedNamespace = rewriteState.getExportedNamespace(legacyNamespace);
if (exportedNamespace == null) {
// There's nothing to inline. The missing provide/module will be reported elsewhere.
} else if (lhs.isName()) {
// `var Foo` case
String aliasName = statementNode.getFirstChild().getString();
recordImportAlias(aliasName, exportedNamespace);
} else if (lhs.isDestructuringLhs() && lhs.getFirstChild().isObjectPattern()) {
// `const {Foo}` case
for (Node importSpec : lhs.getFirstChild().children()) {
String importedProperty = importSpec.getString();
String aliasName =
importSpec.hasChildren() ? importSpec.getFirstChild().getString() : importedProperty;
String fullName = exportedNamespace + "." + importedProperty;
recordImportAlias(aliasName, fullName);
}
} else {
throw new RuntimeException("Illegal goog.module import: " + lhs);
}
}

if (currentScript.isModule || (targetIsNonLegacyGoogModule && targetIsGoogModuleGetted)) {
if (isDestructuring) {
// Rewrite
// "var {a, b} = goog.require('foo.Bar');" to
// "var {a, b} = module$exports$foo$Bar;" or
// "var {a, b} = foo.Bar;"
String exportedNamespace = rewriteState.getExportedNamespace(legacyNamespace);
Node replacementNamespaceName = NodeUtil.newQName(compiler, exportedNamespace);
replacementNamespaceName.putProp(Node.ORIGINALNAME_PROP, legacyNamespace);
replacementNamespaceName.putBooleanProp(Node.GOOG_MODULE_REQUIRE, true);
replacementNamespaceName.srcrefTree(call);
call.getParent().replaceChild(call, replacementNamespaceName);
markConst(statementNode);
// Delete the goog.require() because we're going to inline its alias later.
statementNode.detachFromParent();
} else if (targetIsNonLegacyGoogModule) {
if (immediatePropertyAccess || !isTopLevel(t, statementNode, ScopeType.EXEC_CONTEXT)) {
// Rewrite
Expand Down Expand Up @@ -917,9 +929,8 @@ private void updateGoogModuleGetCall(Node call) {
// 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.
// Replace "goog.module.get('pkg.Foo');" with "module$exports$pkg$Foo;".
Node exportedNamespaceName =
NodeUtil.newQName(compiler, rewriteState.getExportedNamespace(legacyNamespace))
.srcrefTree(call);
String exportedNamespace = rewriteState.getExportedNamespace(legacyNamespace);
Node exportedNamespaceName = NodeUtil.newQName(compiler, exportedNamespace).srcrefTree(call);
exportedNamespaceName.putProp(Node.ORIGINALNAME_PROP, legacyNamespace);
call.getParent().replaceChild(call, exportedNamespaceName);
}
Expand Down Expand Up @@ -970,12 +981,21 @@ private void maybeUpdateTopLevelName(NodeTraversal t, Node nameNode) {
return;
}

// If the name is part of a destructuring import, the import rewriting will take care of it
if (var.getNameNode() == nameNode
&& nameNode.getParent().isStringKey()
&& nameNode.getGrandparent().isObjectPattern()) {
Node destructuringLhsNode = nameNode.getGrandparent().getParent();
if (isCallTo(destructuringLhsNode.getLastChild(), "goog.require")) {
return;
}
}

// If the name is an alias for an imported namespace rewrite from
// "new Foo;" to "new module$exports$Foo;"
boolean nameIsAnAlias = currentScript.legacyNamespacesByAlias.containsKey(name);
boolean nameIsAnAlias = currentScript.importedNamesByAlias.containsKey(name);
if (nameIsAnAlias && var.getNode() != nameNode) {
String legacyNamespace = currentScript.legacyNamespacesByAlias.get(name);
String namespaceToInline = rewriteState.getExportedNamespace(legacyNamespace);
String namespaceToInline = currentScript.importedNamesByAlias.get(name);
safeSetMaybeQualifiedString(nameNode, namespaceToInline);

// Make sure this action won't shadow a local variable.
Expand All @@ -987,7 +1007,7 @@ private void maybeUpdateTopLevelName(NodeTraversal t, Node nameNode) {
shadowedVar.getNode(),
IMPORT_INLINING_SHADOWS_VAR,
shadowedVar.getName(),
legacyNamespace);
namespaceToInline);
}
}
return;
Expand Down Expand Up @@ -1260,7 +1280,8 @@ private void markConstAndCopyJsDoc(Node from, Node target, Node value) {
}

private void recordImportAlias(String aliasName, String legacyNamespace) {
currentScript.legacyNamespacesByAlias.put(aliasName, legacyNamespace);
Preconditions.checkNotNull(legacyNamespace);
currentScript.importedNamesByAlias.put(aliasName, legacyNamespace);
}

/**
Expand Down
109 changes: 75 additions & 34 deletions test/com/google/javascript/jscomp/ClosureRewriteModuleTest.java
View file
Expand Up @@ -123,48 +123,88 @@ public void testDestructuringInsideModule() {
}

public void testDestructuringImports() {
// TODO(blickly): Inline destructuring-based imports so that they can be used
// for importing type names like other imports.

// Var destructuring of both module and script goog.require() targets.
testEs6(
new String[] {
"goog.module('ns.b');",
"goog.provide('ns.c');",
LINE_JOINER.join(
"goog.module('ns.a');",
"var {foo, bar} = goog.require('ns.b');",
"var {baz, qux} = goog.require('ns.c');")},
"goog.module('ns.b'); /** @constructor */ exports.Foo = function() {};",
LINE_JOINER.join(
"goog.module('ns.a');",
"",
"var {Foo} = goog.require('ns.b');",
"",
"/** @type {Foo} */",
"var f = new Foo;")
},
new String[] {
LINE_JOINER.join(
"/** @const */ var module$exports$ns$b = {};",
"/** @constructor */ module$exports$ns$b.Foo = function() {};"),
LINE_JOINER.join(
"/** @const */ var module$exports$ns$a = {}",
"/** @type {module$exports$ns$b.Foo} */",
"var module$contents$ns$a_f = new module$exports$ns$b.Foo;")});

testEs6(
new String[] {
"/** @const */ var module$exports$ns$b = {}",
"goog.provide('ns.c');",
LINE_JOINER.join(
"/** @const */ var module$exports$ns$a = {}",
"/** @const */ var {foo: module$contents$ns$a_foo,",
" bar: module$contents$ns$a_bar} = module$exports$ns$b;",
"/** @const */ var {baz: module$contents$ns$a_baz,",
" qux: module$contents$ns$a_qux} = ns.c;")});
"goog.provide('ns.b'); /** @constructor */ ns.b.Foo = function() {};",
LINE_JOINER.join(
"goog.module('ns.a');",
"",
"var {Foo} = goog.require('ns.b');",
"",
"/** @type {Foo} */",
"var f = new Foo;")
},
new String[] {
"goog.provide('ns.b'); /** @constructor */ ns.b.Foo = function() {};",
LINE_JOINER.join(
"/** @const */ var module$exports$ns$a = {}",
"/** @type {ns.b.Foo} */",
"var module$contents$ns$a_f = new ns.b.Foo;")});

// Const destructuring of both module and script goog.require() targets.
testEs6(
new String[] {
"goog.module('ns.b');",
"goog.provide('ns.c');",
LINE_JOINER.join(
"goog.module('ns.a');",
"const {foo, bar} = goog.require('ns.b');",
"const {baz, qux} = goog.require('ns.c');")},
LINE_JOINER.join(
"goog.module('ns.b');",
"goog.module.declareLegacyNamespace();",
"",
"/** @constructor */ exports.Foo = function() {};"),
LINE_JOINER.join(
"goog.module('ns.a');",
"",
"var {Foo} = goog.require('ns.b');",
"",
"/** @type {Foo} */",
"var f = new Foo;")
},
new String[] {
LINE_JOINER.join(
"goog.provide('ns.b');",
"/** @constructor */ ns.b.Foo = function() {};"),
LINE_JOINER.join(
"/** @const */ var module$exports$ns$a = {}",
"/** @type {ns.b.Foo} */",
"var module$contents$ns$a_f = new ns.b.Foo;")});

testEs6(
new String[] {
"/** @const */ var module$exports$ns$b = {}",
"goog.provide('ns.c');",
LINE_JOINER.join(
"/** @const */ var module$exports$ns$a = {}",
"/** @const */ const {foo: module$contents$ns$a_foo,",
" bar: module$contents$ns$a_bar} = module$exports$ns$b;",
"/** @const */ const {baz: module$contents$ns$a_baz,",
" qux: module$contents$ns$a_qux} = ns.c;")});
"goog.module('ns.b'); /** @constructor */ exports.Foo = function() {};",
LINE_JOINER.join(
"goog.module('ns.a');",
"",
"var {Foo: Bar} = goog.require('ns.b');",
"",
"/** @type {Bar} */",
"var f = new Bar;")
},
new String[] {
LINE_JOINER.join(
"/** @const */ var module$exports$ns$b = {};",
"/** @constructor */ module$exports$ns$b.Foo = function() {};"),
LINE_JOINER.join(
"/** @const */ var module$exports$ns$a = {}",
"/** @type {module$exports$ns$b.Foo} */",
"var module$contents$ns$a_f = new module$exports$ns$b.Foo;")});

}

public void testDeclareLegacyNamespace() {
Expand Down Expand Up @@ -1285,7 +1325,8 @@ public void testGoogModuleReferencedWithGlobalName() {
public void testGoogModuleValidReferences() {
test(
new String[] {
"goog.module('a.b.c');", "goog.module('x.y.z'); var c = goog.require('a.b.c'); use(c);"
"goog.module('a.b.c');",
"goog.module('x.y.z'); var c = goog.require('a.b.c'); use(c);"
},
new String[] {
"/** @const */ var module$exports$a$b$c={};",
Expand Down

0 comments on commit 13f31d2

Please sign in to comment.