Skip to content

Commit

Permalink
Allow multiple exports of the typescript namespace pattern
Browse files Browse the repository at this point in the history
Typescript currently emits this pattern for exports of multiply defined
namespaces. So long as the RHS is an object literal, this doesn't violate any
of our assumptions for module rewriting.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=219175763
  • Loading branch information
blickly authored and EatingW committed Oct 30, 2018
1 parent 0b969c8 commit 118f582
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 37 deletions.
62 changes: 47 additions & 15 deletions src/com/google/javascript/jscomp/ClosureCheckModule.java
Expand Up @@ -101,10 +101,12 @@ public final class ClosureCheckModule extends AbstractModuleCallback
"JSC_INCORRECT_SHORTNAME_CAPITALIZATION",
"The capitalization of short name {0} is incorrect; it should be {1}.");

static final DiagnosticType EXPORT_NOT_A_MODULE_LEVEL_STATEMENT =
static final DiagnosticType EXPORT_NOT_AT_MODULE_SCOPE =
DiagnosticType.error(
"JSC_EXPORT_NOT_A_MODULE_LEVEL_STATEMENT",
"Exports must be a statement at the top-level of a module");
"JSC_EXPORT_NOT_AT_MODULE_SCOPE", "Exports must be at the top-level of a module");

static final DiagnosticType EXPORT_NOT_A_STATEMENT =
DiagnosticType.error("JSC_EXPORT_NOT_A_STATEMENT", "Exports should be a statement.");

static final DiagnosticType EXPORT_REPEATED_ERROR =
DiagnosticType.error(
Expand Down Expand Up @@ -157,8 +159,9 @@ private static class ModuleInfo {
private final Map<String, Node> importsByLongRequiredName = new HashMap<>();
// Module-local short names for goog.required symbols.
private final Set<String> shortImportNames = new HashSet<>();
// The node of the default (non-named) export of this module, if it exists.
private Node defaultExportNode = null;
// A map from the export names "exports.name" to the nodes of those named exports.
// The default export is keyed with just "exports"
private final Map<String, Node> exportNodesByName = new HashMap<>();

ModuleInfo(String moduleName) {
this.name = moduleName;
Expand Down Expand Up @@ -372,23 +375,52 @@ private boolean isExportLhs(Node lhs) {
|| (lhs.isGetProp() && lhs.getFirstChild().matchesQualifiedName("exports"));
}

/**
* Returns if the given export RHS nodes are safe to export multiple times. previousLhs is the LHS
* of the first assignment to the given export, and newLhs is a later assignment to the same
* export.
*
* <p>This is specifically to allow the TypeScript like the following: var foo; ((object) => { ...
* })(foo = exports.foo || (exports.name = {})); ((object) => { ... })(foo = exports.foo ||
* (exports.name = {})); which doesn't abide by the goog.module restriction that each export be
* exported only once. Since these exports do have a constant value at the end of loading the
* goog.module, and we only rewrite named exports whose RHS is a name node in
* ClosureRewriteModule, this pattern with an object literal on the RHS is safe to process.
*/
private boolean isPermittedTypeScriptMultipleExportPattern(Node previousLhs, Node newLhs) {
Node previousRhs = NodeUtil.getRValueOfLValue(previousLhs);
Node newRhs = NodeUtil.getRValueOfLValue(newLhs);
return previousRhs != null
&& previousRhs.isObjectLit()
&& newRhs != null
&& newRhs.isObjectLit();
}

private void checkModuleExport(NodeTraversal t, Node n, Node parent) {
checkArgument(n.isAssign());
Node lhs = n.getFirstChild();
checkState(isExportLhs(lhs));
if (currentModule.defaultExportNode == null && (!t.inModuleScope() || !parent.isExprResult())) {
// Invalid export location.
t.report(n, EXPORT_NOT_A_MODULE_LEVEL_STATEMENT);
// Check multiple exports of the same name
Node previousDefinition = currentModule.exportNodesByName.get(lhs.getQualifiedName());
if (previousDefinition != null
&& !isPermittedTypeScriptMultipleExportPattern(previousDefinition, lhs)) {
int previousLine = previousDefinition.getLineno();
t.report(n, EXPORT_REPEATED_ERROR, String.valueOf(previousLine));
}
if (lhs.isName()) {
if (currentModule.defaultExportNode != null) {
// Multiple exports
int previousLine = currentModule.defaultExportNode.getLineno();
t.report(n, EXPORT_REPEATED_ERROR, String.valueOf(previousLine));
// Check exports in invalid program position
Node defaultExportNode = currentModule.exportNodesByName.get("exports");
// If we have never seen an `exports =` default export assignment, or this is the
// default export, then treat this assignment as an export and do the checks it is well formed.
if (defaultExportNode == null || lhs.matchesQualifiedName("exports")) {
currentModule.exportNodesByName.put(lhs.getQualifiedName(), lhs);
if (!t.inModuleScope()) {
t.report(n, EXPORT_NOT_AT_MODULE_SCOPE);
} else if (!parent.isExprResult()) {
t.report(n, EXPORT_NOT_A_STATEMENT);
}
currentModule.defaultExportNode = lhs;
}
if ((lhs.isName() || !NodeUtil.isPrototypeProperty(lhs))
// Check @export on a module local name
if (!NodeUtil.isPrototypeProperty(lhs)
&& !NodeUtil.isLegacyGoogModuleFile(NodeUtil.getEnclosingScript(n))) {
JSDocInfo jsDoc = n.getJSDocInfo();
if (jsDoc != null && jsDoc.isExport()) {
Expand Down
4 changes: 4 additions & 0 deletions src/com/google/javascript/jscomp/DiagnosticGroups.java
Expand Up @@ -687,6 +687,10 @@ public DiagnosticGroup forName(String name) {
DiagnosticGroups.registerGroup("es6Typed", RhinoErrorReporter.MISPLACED_TYPE_SYNTAX);

DiagnosticGroups.registerDeprecatedGroup("duplicateZipContents");

// Only exposed for tsickle-generated code.
DiagnosticGroups.registerGroup(
"googModuleExportNotAStatement", ClosureCheckModule.EXPORT_NOT_A_STATEMENT);
}

/** Adds warning levels by name. */
Expand Down
Expand Up @@ -173,6 +173,7 @@ jsdoc.suppressions =\
extraRequire,\
fileoverviewTags,\
globalThis,\
googModuleExportNotAStatement,\
invalidCasts,\
legacyGoogScopeRequire,\
lateProvide,\
Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/resources.json

Large diffs are not rendered by default.

55 changes: 34 additions & 21 deletions test/com/google/javascript/jscomp/ClosureCheckModuleTest.java
Expand Up @@ -17,7 +17,8 @@

import static com.google.javascript.jscomp.ClosureCheckModule.DECLARE_LEGACY_NAMESPACE_IN_NON_MODULE;
import static com.google.javascript.jscomp.ClosureCheckModule.DUPLICATE_NAME_SHORT_REQUIRE;
import static com.google.javascript.jscomp.ClosureCheckModule.EXPORT_NOT_A_MODULE_LEVEL_STATEMENT;
import static com.google.javascript.jscomp.ClosureCheckModule.EXPORT_NOT_AT_MODULE_SCOPE;
import static com.google.javascript.jscomp.ClosureCheckModule.EXPORT_NOT_A_STATEMENT;
import static com.google.javascript.jscomp.ClosureCheckModule.EXPORT_REPEATED_ERROR;
import static com.google.javascript.jscomp.ClosureCheckModule.GOOG_MODULE_REFERENCES_THIS;
import static com.google.javascript.jscomp.ClosureCheckModule.GOOG_MODULE_USES_THROW;
Expand Down Expand Up @@ -664,43 +665,55 @@ public void testShorthandNameConvention() {
}

@Test
public void testIllegalExports() {
public void testNonModuleLevelExports() {
testError(
lines(
"goog.module('xyz');",
"",
"if (window.exportMe) { exports = 5; }"),
EXPORT_NOT_A_MODULE_LEVEL_STATEMENT);
lines("goog.module('xyz');", "", "if (window.exportMe) { exports = 5; }"),
EXPORT_NOT_AT_MODULE_SCOPE);

testError(
lines(
"goog.module('xyz');",
"",
"window.exportMe && (exports = 5);"),
EXPORT_NOT_A_MODULE_LEVEL_STATEMENT);
lines("goog.module('xyz');", "", "window.exportMe && (exports = 5);"),
EXPORT_NOT_A_STATEMENT);

testError(
lines("goog.module('xyz');", "", "exports.f = () => { exports.me = 5; }"),
EXPORT_NOT_AT_MODULE_SCOPE);

testSame(
lines(
"goog.module('xyz');",
"",
"if (window.exportMe) { exports.me = 5; }"),
EXPORT_NOT_A_MODULE_LEVEL_STATEMENT);
"exports = class {};",
"exports.staticMethod = () => { exports.me = 5; }"));
}

@Test
public void testRepeatedExports() {
testError(
lines("goog.module('xyz');", "", "exports = 5;", "exports = 'str';"),
EXPORT_REPEATED_ERROR);

testError(
lines("goog.module('xyz');", "", "exports.y = 5;", "exports.y = 'str';"),
EXPORT_REPEATED_ERROR);

testSame(
lines(
"goog.module('xyz');",
"",
"exports = {};",
"if (window.exportMe) { exports.me = 5; }"));
"exports = class {};",
"exports.y = 'str';",
"exports.y = decorate(exports.y);"));

testError(
// This pattern is used by typescript in a way that won't violate our goog.module assumptions.
testSame(
lines(
"/** @fileoverview @suppress {googModuleExportNotAStatement} */",
"goog.module('xyz');",
"",
"exports = 5;",
"exports = 'str';"),
EXPORT_REPEATED_ERROR);

"((x) => {})(exports.y || (exports.y = {}));",
"",
"((x) => {})(exports.y || (exports.y = {}));",
""));
}

@Test
Expand Down

0 comments on commit 118f582

Please sign in to comment.