diff --git a/src/com/google/javascript/jscomp/ClosureRewriteModule.java b/src/com/google/javascript/jscomp/ClosureRewriteModule.java index 8590ccf0d5d..b0619973d7c 100644 --- a/src/com/google/javascript/jscomp/ClosureRewriteModule.java +++ b/src/com/google/javascript/jscomp/ClosureRewriteModule.java @@ -215,6 +215,8 @@ private static final class UnrecognizedRequire { } private static final class ExportDefinition { + // Null if the export is a default export (exports = expr) + @Nullable String exportName; // Null if the export is of a @typedef @Nullable Node rhs; // Null if the export is of anything other than a name @@ -223,8 +225,13 @@ private static final class ExportDefinition { private static final Set INLINABLE_NAME_PARENTS = ImmutableSet.of(Token.VAR, Token.CONST, Token.LET, Token.FUNCTION, Token.CLASS); - static ExportDefinition fromRhs(NodeTraversal t, Node rhs) { + static ExportDefinition newDefaultExport(NodeTraversal t, Node rhs) { + return newNamedExport(t, null, rhs); + } + + static ExportDefinition newNamedExport(NodeTraversal t, String name, Node rhs) { ExportDefinition newExport = new ExportDefinition(); + newExport.exportName = name; newExport.rhs = rhs; if (rhs != null && (rhs.isName() || rhs.isStringKey())) { newExport.nameDecl = t.getScope().getVar(rhs.getString()); @@ -232,6 +239,13 @@ static ExportDefinition fromRhs(NodeTraversal t, Node rhs) { return newExport; } + String getExportPostfix() { + if (exportName == null) { + return ""; + } + return "." + exportName; + } + boolean hasInlinableName() { if (nameDecl == null || !INLINABLE_NAME_PARENTS.contains(nameDecl.getParentNode().getToken())) { @@ -271,6 +285,7 @@ private static final class ScriptDescription { Node defaultExportRhs; String defaultExportLocalName; Set namedExports = new HashSet<>(); + Map exportsToInline = new HashMap<>(); // 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. Changes when @@ -844,22 +859,39 @@ private void maybeRecordExportDeclaration(NodeTraversal t, Node n) { } Preconditions.checkState(currentScript.defaultExportRhs == null); - currentScript.willCreateExportsObject = true; Node exportRhs = n.getNext(); if (isNamedExportsLiteral(exportRhs)) { + boolean areAllExportsInlinable = true; + List inlinableExports = new ArrayList<>(); for (Node key = exportRhs.getFirstChild(); key != null; key = key.getNext()) { String exportName = key.getString(); + Node rhs = key.hasChildren() ? key.getFirstChild() : key; + ExportDefinition namedExport = ExportDefinition.newNamedExport(t, exportName, rhs); currentScript.namedExports.add(exportName); + if (currentScript.declareLegacyNamespace || !namedExport.hasInlinableName()) { + areAllExportsInlinable = false; + } else { + inlinableExports.add(namedExport); + } + } + if (areAllExportsInlinable) { + for (ExportDefinition export : inlinableExports) { + recordExportToInline(export); + } + NodeUtil.removeChild(n.getParent().getParent(), n.getParent()); + } else { + currentScript.willCreateExportsObject = true; } return; } currentScript.defaultExportRhs = exportRhs; - ExportDefinition defaultExport = ExportDefinition.fromRhs(t, exportRhs); + currentScript.willCreateExportsObject = true; + ExportDefinition defaultExport = ExportDefinition.newDefaultExport(t, exportRhs); if (!currentScript.declareLegacyNamespace && defaultExport.hasInlinableName()) { String localName = defaultExport.getLocalName(); currentScript.defaultExportLocalName = localName; - recordNameToInline(localName, currentScript.getBinaryNamespace()); + recordExportToInline(defaultExport); } return; @@ -1091,6 +1123,14 @@ private void recordExportsPropertyAssignment(NodeTraversal t, Node getpropNode) if (t.inModuleScope() || t.inGlobalScope()) { String exportName = getpropNode.getLastChild().getString(); currentScript.namedExports.add(exportName); + Node exportRhs = getpropNode.getNext(); + ExportDefinition namedExport = ExportDefinition.newNamedExport(t, exportName, exportRhs); + if (!currentScript.declareLegacyNamespace + && currentScript.defaultExportRhs == null + && namedExport.hasInlinableName()) { + recordExportToInline(namedExport); + parent.getParent().detach(); + } } } @@ -1145,18 +1185,14 @@ private void maybeUpdateTopLevelName(NodeTraversal t, Node nameNode) { } } - boolean nameIsExported = name.equals(currentScript.defaultExportLocalName); - if (nameIsExported) { - safeSetString(nameNode, currentScript.getBinaryNamespace()); - currentScript.hasCreatedExportObject = true; - return; - } - // If the name is an alias for an imported namespace rewrite from // "new Foo;" to "new module$exports$Foo;" boolean nameIsAnAlias = currentScript.namesToInlineByAlias.containsKey(name); if (nameIsAnAlias && var.getNode() != nameNode) { String namespaceToInline = currentScript.namesToInlineByAlias.get(name); + if (namespaceToInline.equals(currentScript.getBinaryNamespace())) { + currentScript.hasCreatedExportObject = true; + } safeSetMaybeQualifiedString(nameNode, namespaceToInline); // Make sure this action won't shadow a local variable. @@ -1314,6 +1350,7 @@ private void updateModuleReturn(Node returnNode) { } returnStatementNode.detach(); + updateEndModule(); popScript(); } @@ -1326,6 +1363,11 @@ private void updateEndScript() { } private void updateEndModule() { + for (ExportDefinition export : currentScript.exportsToInline.values()) { + Node nameNode = export.nameDecl.getNameNode(); + safeSetMaybeQualifiedString( + nameNode, currentScript.getBinaryNamespace() + export.getExportPostfix()); + } Preconditions.checkState(currentScript.isModule); Preconditions.checkState( currentScript.declareLegacyNamespace || currentScript.hasCreatedExportObject); @@ -1418,6 +1460,14 @@ private void markConstAndCopyJsDoc(Node from, Node target, Node value) { target.setJSDocInfo(builder.build()); } + private void recordExportToInline(ExportDefinition exportDefinition) { + currentScript.exportsToInline.put(exportDefinition.nameDecl, exportDefinition); + String localName = exportDefinition.getLocalName(); + String fullExportedName = + currentScript.getBinaryNamespace() + exportDefinition.getExportPostfix(); + recordNameToInline(localName, fullExportedName); + } + private void recordNameToInline(String aliasName, String legacyNamespace) { Preconditions.checkNotNull(aliasName); Preconditions.checkNotNull(legacyNamespace); @@ -1471,57 +1521,57 @@ private void reportUnrecognizedRequires() { private void safeSetString(Node n, String newString) { String originalName = n.getString(); n.setString(newString); - n.putProp(Node.ORIGINALNAME_PROP, originalName); + if (n.getOriginalName() == null) { + n.setOriginalName(originalName); + } compiler.reportCodeChange(); } private void safeSetMaybeQualifiedString(Node nameNode, String newString) { - if (newString.contains(".")) { - // When replacing with a dotted fully qualified name it's already better than an original - // name. - Node nameParent = nameNode.getParent(); - JSDocInfo jsdoc = nameParent.getJSDocInfo(); - switch (nameParent.getToken()) { - case FUNCTION: - case CLASS: - if (NodeUtil.isStatement(nameParent) && nameParent.getFirstChild() == nameNode) { - Node statementParent = nameParent.getParent(); - Node placeholder = IR.empty(); - statementParent.replaceChild(nameParent, placeholder); - Node newStatement = - NodeUtil.newQNameDeclaration(compiler, newString, nameParent, jsdoc); - nameParent.setJSDocInfo(null); - newStatement.useSourceInfoIfMissingFromForTree(nameParent); - statementParent.replaceChild(placeholder, newStatement); - NodeUtil.removeName(nameParent); - return; - } - break; - case VAR: - case LET: - case CONST: - { - Node rhs = nameNode.hasChildren() ? nameNode.getLastChild().detach() : null; - Node newStatement = NodeUtil.newQNameDeclaration(compiler, newString, rhs, jsdoc); - newStatement.useSourceInfoIfMissingFromForTree(nameParent); - NodeUtil.replaceDeclarationChild(nameNode, newStatement); - return; - } - case OBJECT_PATTERN: - case ARRAY_PATTERN: - case PARAM_LIST: - throw new RuntimeException("Not supported"); - default: - break; - } - Node newQualifiedNameNode = NodeUtil.newQName(compiler, newString); - newQualifiedNameNode.srcrefTree(nameNode); - nameParent.replaceChild(nameNode, newQualifiedNameNode); - } else { - String originalName = nameNode.getString(); - nameNode.setString(newString); - nameNode.putProp(Node.ORIGINALNAME_PROP, originalName); + if (!newString.contains(".")) { + safeSetString(nameNode, newString); + return; } + // When replacing with a dotted fully qualified name it's already better than an original + // name. + Node nameParent = nameNode.getParent(); + JSDocInfo jsdoc = nameParent.getJSDocInfo(); + switch (nameParent.getToken()) { + case FUNCTION: + case CLASS: + if (NodeUtil.isStatement(nameParent) && nameParent.getFirstChild() == nameNode) { + Node statementParent = nameParent.getParent(); + Node placeholder = IR.empty(); + statementParent.replaceChild(nameParent, placeholder); + Node newStatement = + NodeUtil.newQNameDeclaration(compiler, newString, nameParent, jsdoc); + nameParent.setJSDocInfo(null); + newStatement.useSourceInfoIfMissingFromForTree(nameParent); + statementParent.replaceChild(placeholder, newStatement); + NodeUtil.removeName(nameParent); + return; + } + break; + case VAR: + case LET: + case CONST: + { + Node rhs = nameNode.hasChildren() ? nameNode.getLastChild().detach() : null; + Node newStatement = NodeUtil.newQNameDeclaration(compiler, newString, rhs, jsdoc); + newStatement.useSourceInfoIfMissingFromForTree(nameParent); + NodeUtil.replaceDeclarationChild(nameNode, newStatement); + return; + } + case OBJECT_PATTERN: + case ARRAY_PATTERN: + case PARAM_LIST: + throw new RuntimeException("Not supported"); + default: + break; + } + Node newQualifiedNameNode = NodeUtil.newQName(compiler, newString); + newQualifiedNameNode.srcrefTree(nameNode); + nameParent.replaceChild(nameNode, newQualifiedNameNode); compiler.reportCodeChange(); } diff --git a/test/com/google/javascript/jscomp/ClosureRewriteModuleTest.java b/test/com/google/javascript/jscomp/ClosureRewriteModuleTest.java index 9a3fe02e048..de99e28b74a 100644 --- a/test/com/google/javascript/jscomp/ClosureRewriteModuleTest.java +++ b/test/com/google/javascript/jscomp/ClosureRewriteModuleTest.java @@ -156,8 +156,8 @@ public void testShortObjectLiteralsInsideModule() { "var x = foo();", "exports = {x};"), LINE_JOINER.join( - "var module$contents$a_x = foo();", - "/** @const */ var module$exports$a = {/** @const */ x: module$contents$a_x};")); + "/** @const */ var module$exports$a = {};", + "module$exports$a.x = foo();")); } public void testDestructuringImports() { @@ -264,7 +264,46 @@ public void testDestructuringImports() { "/** @type {module$exports$ns$b.Foo} */", "var module$contents$ns$a_f = new module$exports$ns$b.Foo;")}); - // TODO(blickly): We don't want any module$contents variables for this definition of Foo + testEs6( + new String[] { + "goog.module('modA'); class Foo {} exports.Foo = Foo;", + LINE_JOINER.join( + "goog.module('modB');", + "", + "var {Foo:importedFoo} = goog.require('modA');", + "", + "/** @type {importedFoo} */", + "var f = new importedFoo;") + }, + new String[] { + LINE_JOINER.join( + "/** @const */ var module$exports$modA = {};", + "module$exports$modA.Foo = class {}"), + LINE_JOINER.join( + "/** @const */ var module$exports$modB = {}", + "/** @type {module$exports$modA.Foo} */", + "var module$contents$modB_f = new module$exports$modA.Foo;")}); + + testEs6( + new String[] { + "goog.module('modA'); class Foo {} exports.Foo = Foo;", + LINE_JOINER.join( + "goog.module('modB');", + "", + "var {Foo} = goog.require('modA');", + "", + "/** @type {Foo} */", + "var f = new Foo;") + }, + new String[] { + LINE_JOINER.join( + "/** @const */ var module$exports$modA = {};", + "module$exports$modA.Foo = class {}"), + LINE_JOINER.join( + "/** @const */ var module$exports$modB = {}", + "/** @type {module$exports$modA.Foo} */", + "var module$contents$modB_f = new module$exports$modA.Foo;")}); + testEs6( new String[] { "goog.module('modA'); class Foo {} exports = {Foo};", @@ -278,15 +317,34 @@ public void testDestructuringImports() { }, new String[] { LINE_JOINER.join( - "class module$contents$modA_Foo {}", - "/** @const */ var module$exports$modA = {", - " /** @const */ Foo: module$contents$modA_Foo", - "};"), + "/** @const */ var module$exports$modA = {};", + "module$exports$modA.Foo = class {};"), LINE_JOINER.join( "/** @const */ var module$exports$modB = {}", "/** @type {module$exports$modA.Foo} */", "var module$contents$modB_f = new module$exports$modA.Foo;")}); + testEs6( + new String[] { + "goog.module('modA'); class Bar {} exports = class Foo {}; exports.Bar = Bar;", + LINE_JOINER.join( + "goog.module('modB');", + "", + "var Foo = goog.require('modA');", + "", + "/** @type {Foo} */", + "var f = new Foo;") + }, + new String[] { + LINE_JOINER.join( + "class module$contents$modA_Bar {}", + "/** @const */ var module$exports$modA = class Foo {};", + "/** @const */ module$exports$modA.Bar = module$contents$modA_Bar;"), + LINE_JOINER.join( + "/** @const */ var module$exports$modB = {}", + "/** @type {module$exports$modA} */", + "var module$contents$modB_f = new module$exports$modA;")}); + testEs6( new String[] { LINE_JOINER.join( @@ -316,6 +374,28 @@ public void testDestructuringImports() { "var module$contents$modB_f = new modA.Foo;")}); } + public void testUninlinableExports() { + testEs6( + new String[] { + "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 = {};", + "/** @const @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 testObjectLiteralDefaultExport() { testErrorEs6( new String[] { @@ -330,6 +410,64 @@ public void testObjectLiteralDefaultExport() { ILLEGAL_DESTRUCTURING_DEFAULT_EXPORT); } + public void testUninlinableNamedExports() { + testEs6( + new String[] { + "goog.module('modA'); \n exports = class {};", + LINE_JOINER.join( + "goog.module('modB');", + "", + "var Foo = goog.require('modA');", + "", + "exports.Foo = Foo;"), + }, + new String[] { + "/** @const */ var module$exports$modA = class {};", + LINE_JOINER.join( + "/** @const */ var module$exports$modB = {};", + "/** @const */ module$exports$modB.Foo = module$exports$modA;"), + }); + + testEs6( + new String[] { + "goog.module('modA'); \n exports = class {};", + LINE_JOINER.join( + "goog.module('modB');", + "", + "var Foo = goog.require('modA');", + "", + "exports = {Foo};"), + }, + new String[] { + "/** @const */ var module$exports$modA = class {};", + LINE_JOINER.join( + "/** @const */ var module$exports$modB = {", + " /** @const */ Foo: module$exports$modA,", + "};"), + }); + + testEs6( + new String[] { + "goog.module('modA'); \n exports = class {};", + LINE_JOINER.join( + "goog.module('modB');", + "", + "var Foo = goog.require('modA');", + "class Bar {}", + "", + "exports = {Foo, Bar};"), + }, + new String[] { + "/** @const */ var module$exports$modA = class {};", + LINE_JOINER.join( + "class module$contents$modB_Bar {}", + "/** @const */ var module$exports$modB = {", + " /** @const */ Foo: module$exports$modA,", + " /** @const */ Bar: module$contents$modB_Bar,", + "};"), + }); + } + public void testIllegalDestructuringImports() { testErrorEs6( new String[] { @@ -647,8 +785,7 @@ public void testBundleWithDestructuringImport() { "});"), LINE_JOINER.join( "/** @const */ var module$exports$mod_B = {};", - "/** @interface */ function module$contents$mod_B_B(){}", - "/** @const */ module$exports$mod_B.B = module$contents$mod_B_B;", + "/** @interface */ module$exports$mod_B.B = function(){};", "", "/** @const */ var module$exports$mod_A = {};", "/** @constructor @implements {module$exports$mod_B.B} */", @@ -1136,8 +1273,7 @@ public void testExport5() { LINE_JOINER.join( "/** @const */ var module$exports$ns$a = {};", - "/** @typedef {string} */ var module$contents$ns$a_x;", - "/** @const */ module$exports$ns$a.x = module$contents$ns$a_x;")); + "/** @typedef {string} */ module$exports$ns$a.x;")); } public void testExport6() { @@ -1148,10 +1284,20 @@ public void testExport6() { "exports = { something: x };"), LINE_JOINER.join( - "/** @typedef {string} */ var module$contents$ns$a_x;", - "/** @const */ var module$exports$ns$a = {", - " /** @typedef {string} */ something : module$contents$ns$a_x", - "};")); + "/** @const */ var module$exports$ns$a = {};", + "/** @typedef {string} */ module$exports$ns$a.something;")); + } + + public void testExport6_1() { + test( + LINE_JOINER.join( + "goog.module('ns.a');", + "/** @typedef {string} */ var x;", + "exports.something = x;"), + + LINE_JOINER.join( + "/** @const */ var module$exports$ns$a = {};", + "/** @typedef {string} */ module$exports$ns$a.something;")); } public void testExport7() { @@ -1238,11 +1384,8 @@ public void testExportEnhancedObjectLiteral() { "exports = { Something };"), LINE_JOINER.join( - "class module$contents$ns$a_Something {}", - "/** @const */ var module$exports$ns$a = {", - " /** @const */", - " Something: module$contents$ns$a_Something", - "};")); + "/** @const */ var module$exports$ns$a = {};", + "module$exports$ns$a.Something = class {};")); testErrorEs6( LINE_JOINER.join( @@ -1462,6 +1605,29 @@ public void testImportInliningShadowsVar() { IMPORT_INLINING_SHADOWS_VAR); } + public void testExportRewritingShadows() { + test( + LINE_JOINER.join( + "goog.module('a.b.c');", + "function test() {}", + "function f(test) { return test; }", + "exports = test;"), + LINE_JOINER.join( + "function module$exports$a$b$c() {}", + "function module$contents$a$b$c_f(test) { return test; }")); + + test( + LINE_JOINER.join( + "goog.module('a.b.c');", + "function test() {}", + "function f(test) { return test; }", + "exports.test = test;"), + LINE_JOINER.join( + "/** @const */ var module$exports$a$b$c = {};", + "module$exports$a$b$c.test = function() {};", + "function module$contents$a$b$c_f(test) { return test; }")); + } + public void testRequireTooEarly1() { // Module to Module require. testError( @@ -1661,7 +1827,7 @@ public void testLegacyGoogModuleValidReferences() { "goog.module('a.b.Foo');", "goog.module.declareLegacyNamespace();", "", - "/** @constructor */ function Foo() {};", + "/** @constructor */ function Foo() {}", "", "exports = Foo;"), "/** @param {a.b.Foo} x */ function f(x) {}" @@ -1669,7 +1835,7 @@ public void testLegacyGoogModuleValidReferences() { new String[] { LINE_JOINER.join( "goog.provide('a.b.Foo');", - "/** @constructor */ function module$contents$a$b$Foo_Foo() {};", + "/** @constructor */ function module$contents$a$b$Foo_Foo() {}", "/** @const */ a.b.Foo = module$contents$a$b$Foo_Foo;"), "/** @param {a.b.Foo} x */ function f(x) {}" }); diff --git a/test/com/google/javascript/jscomp/CodePrinterTest.java b/test/com/google/javascript/jscomp/CodePrinterTest.java index 3eb6a91ea6a..ba7395b5b18 100644 --- a/test/com/google/javascript/jscomp/CodePrinterTest.java +++ b/test/com/google/javascript/jscomp/CodePrinterTest.java @@ -2395,10 +2395,9 @@ public void testEs6GoogModule() { String expectedCode = "" + "var module$exports$foo$bar = {};\n" + "const STR = '3';\n" - + "function fn() {\n" + + "module$exports$foo$bar.fn = function fn() {\n" + " alert(STR);\n" - + "}\n" - + "exports.fn = fn;\n"; + + "};\n"; CompilerOptions compilerOptions = new CompilerOptions(); compilerOptions.setClosurePass(true);