Skip to content

Commit

Permalink
Have Es6RewriteModules and RewriteGoogJsImports still do their best t…
Browse files Browse the repository at this point in the history
…o rewrite in the event of missing files / namespaces. This is required for some tools that just want information of ES modules after type checking, like clutz.

Once modules are type checked this should no longer be a concern.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=225434089
  • Loading branch information
johnplaisted authored and blickly committed Dec 15, 2018
1 parent 4981400 commit e6fbcfd
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 15 deletions.
27 changes: 23 additions & 4 deletions src/com/google/javascript/jscomp/Es6RewriteModules.java
Expand Up @@ -431,10 +431,11 @@ private void visitImport(NodeTraversal t, Node importDecl, Node parent) {


if (m == null) { if (m == null) {
t.report(importDecl, MISSING_MODULE_OR_PROVIDE, namespace); t.report(importDecl, MISSING_MODULE_OR_PROVIDE, namespace);
} else { m = getFallbackMetadataForNamespace(namespace);
moduleName = ModuleRenaming.getGlobalName(m, namespace);
checkState(m.isEs6Module() || m.isGoogModule() || m.isGoogProvide());
} }

moduleName = ModuleRenaming.getGlobalName(m, namespace);
checkState(m.isEs6Module() || m.isGoogModule() || m.isGoogProvide());
} else { } else {
ModuleLoader.ModulePath modulePath = ModuleLoader.ModulePath modulePath =
t.getInput() t.getInput()
Expand Down Expand Up @@ -825,6 +826,24 @@ private void visitGoogModuleGet(NodeTraversal t, Node getCall, Node parent) {
visitRequireOrGet(t, getCall, parent, /* isRequire= */ false); visitRequireOrGet(t, getCall, parent, /* isRequire= */ false);
} }


/**
* Gets some made-up metadata for the given Closure namespace.
*
* <p>This is used when the namespace is not part of the input so that this pass can be fault
* tolerant and still rewrite to something. Some tools don't care about rewriting correctly and
* just want the type information of this module (e.g. clutz).
*/
private ModuleMetadata getFallbackMetadataForNamespace(String namespace) {
// Assume a provide'd file to be consistent with goog.module rewriting.
ModuleMetadata.Builder builder =
ModuleMetadata.builder()
.moduleType(ModuleMetadataMap.ModuleType.GOOG_PROVIDE)
.usesClosure(true)
.isTestOnly(false);
builder.googNamespacesBuilder().add(namespace);
return builder.build();
}

private void visitRequireOrGet( private void visitRequireOrGet(
NodeTraversal t, Node requireCall, Node parent, boolean isRequire) { NodeTraversal t, Node requireCall, Node parent, boolean isRequire) {
if (!requireCall.hasTwoChildren() || !requireCall.getLastChild().isString()) { if (!requireCall.hasTwoChildren() || !requireCall.getLastChild().isString()) {
Expand All @@ -850,7 +869,7 @@ private void visitRequireOrGet(


if (m == null) { if (m == null) {
t.report(requireCall, MISSING_MODULE_OR_PROVIDE, namespace); t.report(requireCall, MISSING_MODULE_OR_PROVIDE, namespace);
return; m = getFallbackMetadataForNamespace(namespace);
} }


if (isStoredInDeclaration) { if (isStoredInDeclaration) {
Expand Down
16 changes: 10 additions & 6 deletions src/com/google/javascript/jscomp/RewriteGoogJsImports.java
Expand Up @@ -155,9 +155,11 @@ public boolean shouldTraverse(NodeTraversal nodeTraversal, Node n, Node parent)
private class ReferenceReplacer extends AbstractPostOrderCallback { private class ReferenceReplacer extends AbstractPostOrderCallback {
private boolean hasBadExport; private boolean hasBadExport;
private final Node googImportNode; private final Node googImportNode;
private final boolean globalizeAllReferences;


public ReferenceReplacer(Node script, Node googImportNode) { public ReferenceReplacer(Node script, Node googImportNode, boolean globalizeAllReferences) {
this.googImportNode = googImportNode; this.googImportNode = googImportNode;
this.globalizeAllReferences = globalizeAllReferences;
NodeTraversal.traverse(compiler, script, this); NodeTraversal.traverse(compiler, script, this);


if (googImportNode.getSecondChild().isImportStar()) { if (googImportNode.getSecondChild().isImportStar()) {
Expand All @@ -184,7 +186,8 @@ private void maybeRewriteBadGoogJsImportRef(NodeTraversal t, Node nameNode, Node
return; return;
} }


if (exportsFinder.exportedNames.contains(parent.getSecondChild().getString())) { if (globalizeAllReferences
|| exportsFinder.exportedNames.contains(parent.getSecondChild().getString())) {
return; return;
} }


Expand Down Expand Up @@ -311,10 +314,11 @@ public void hotSwapScript(Node scriptRoot, Node originalRoot) {
Node googImportNode = findGoogImportNode(scriptRoot); Node googImportNode = findGoogImportNode(scriptRoot);
NodeTraversal.traverse(compiler, scriptRoot, new FindReexports(googImportNode != null)); NodeTraversal.traverse(compiler, scriptRoot, new FindReexports(googImportNode != null));


// exportsFinder can be null in LINT_AND_REWRITE which indicates that goog.js is not part of if (googImportNode != null && mode == Mode.LINT_AND_REWRITE) {
// the input. Meaning we should just lint and do not do any rewriting. // If exportsFinder is null then goog.js was not part of the input. Try to be fault tolerant
if (exportsFinder != null && googImportNode != null && mode == Mode.LINT_AND_REWRITE) { // and just assume that everything exported is on the global goog.
new ReferenceReplacer(scriptRoot, googImportNode); new ReferenceReplacer(
scriptRoot, googImportNode, /* globalizeAllReferences= */ exportsFinder == null);
} }
} }
} }
Expand Down
Expand Up @@ -971,4 +971,25 @@ public void testForwardDeclareEs6ModuleInEs6Module() {
"alias = goog.modle.get('es6');"))), "alias = goog.modle.get('es6');"))),
FORWARD_DECLARE_FOR_ES6_SHOULD_BE_CONST); FORWARD_DECLARE_FOR_ES6_SHOULD_BE_CONST);
} }

@Test
public void testMissingRequireAssumesGoogProvide() {
ignoreWarnings(MISSING_MODULE_OR_PROVIDE);

test(
srcs(
lines(
"const missing = goog.require('is.missing');", //
"use(missing);",
"export {};")),
expected(lines("use(is.missing);", "/** @const */ var module$testcode = {};")));

test(
srcs(
lines(
"import missing from 'goog:is.missing';", //
"use(missing);",
"export {};")),
expected(lines("use(is.missing);", "/** @const */ var module$testcode = {};")));
}
} }
13 changes: 8 additions & 5 deletions test/com/google/javascript/jscomp/RewriteGoogJsImportsTest.java
Expand Up @@ -65,17 +65,20 @@ public void testBaseAndGoogUntouched() {
} }


@Test @Test
public void testIfCannotDetectGoogJsThenDoesNotRewrite() { public void testIfCannotDetectGoogJsThenGlobalizesAll() {
SourceFile testcode = SourceFile testcode =
SourceFile.fromCode( SourceFile.fromCode(
"testcode", "import * as goog from './closure/goog.js'; use(goog.bad);"); "testcode", "import * as goog from './closure/goog.js'; use(goog.bad);");


SourceFile expected =
SourceFile.fromCode("testcode", "import './closure/goog.js'; use(goog.bad);");

// No base.js = no detecting goog.js // No base.js = no detecting goog.js
testSame(srcs(GOOG, testcode)); test(srcs(GOOG, testcode), expected(GOOG, expected));


// No goog.js // No goog.js
testSame(srcs(BASE, testcode)); test(srcs(BASE, testcode), expected(BASE, expected));
testSame(srcs(testcode)); test(srcs(testcode), expected(expected));


// Linting still happens. // Linting still happens.
testError("import * as notgoog from './goog.js';", GOOG_JS_IMPORT_MUST_BE_GOOG_STAR); testError("import * as notgoog from './goog.js';", GOOG_JS_IMPORT_MUST_BE_GOOG_STAR);
Expand Down Expand Up @@ -121,7 +124,7 @@ public void testGoogAndBaseInExterns() {
} }


@Test @Test
public void testBadPropertyAccess() { public void testKnownBadPropertyAccess() {
test( test(
srcs( srcs(
BASE, BASE,
Expand Down

0 comments on commit e6fbcfd

Please sign in to comment.