Skip to content

Commit

Permalink
Rollback of "Use ModuleMetadata in the AbstractModuleCallback".
Browse files Browse the repository at this point in the history 
Reason: broke internal tests.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=223087417
  • Loading branch information
@tjgq
johnplaisted authored and tjgq committed Nov 28, 2018
1 parent 12c9f6b commit 407418d
Show file tree
Hide file tree
Showing 9 changed files with 99 additions and 342 deletions.
85 changes: 48 additions & 37 deletions src/com/google/javascript/jscomp/ClosureCheckModule.java
View file
Expand Up @@ -23,8 +23,6 @@
import static com.google.javascript.jscomp.ClosurePrimitiveErrors.INVALID_DESTRUCTURING_FORWARD_DECLARE; import static com.google.javascript.jscomp.ClosurePrimitiveErrors.INVALID_DESTRUCTURING_FORWARD_DECLARE;
import static com.google.javascript.jscomp.ClosurePrimitiveErrors.MODULE_USES_GOOG_MODULE_GET; import static com.google.javascript.jscomp.ClosurePrimitiveErrors.MODULE_USES_GOOG_MODULE_GET;


import com.google.common.collect.Iterables;
import com.google.javascript.jscomp.ModuleMetadataMap.ModuleMetadata;
import com.google.javascript.jscomp.NodeTraversal.AbstractModuleCallback; import com.google.javascript.jscomp.NodeTraversal.AbstractModuleCallback;
import com.google.javascript.rhino.JSDocInfo; import com.google.javascript.rhino.JSDocInfo;
import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Node;
Expand All @@ -33,7 +31,6 @@
import java.util.HashSet; import java.util.HashSet;
import java.util.Map; import java.util.Map;
import java.util.Set; import java.util.Set;
import javax.annotation.Nullable;


/** /**
* Checks that goog.module() is used correctly. * Checks that goog.module() is used correctly.
Expand Down Expand Up @@ -82,13 +79,18 @@ public final class ClosureCheckModule extends AbstractModuleCallback
static final DiagnosticType LET_GOOG_REQUIRE = static final DiagnosticType LET_GOOG_REQUIRE =
DiagnosticType.disabled( DiagnosticType.disabled(
"JSC_LET_GOOG_REQUIRE", "JSC_LET_GOOG_REQUIRE",
"Module imports must be constant. Please use ''const'' instead of ''let''."); "Module imports must be constant. Please use 'const' instead of 'let'.");


static final DiagnosticType MULTIPLE_MODULES_IN_FILE = static final DiagnosticType MULTIPLE_MODULES_IN_FILE =
DiagnosticType.error( DiagnosticType.error(
"JSC_MULTIPLE_MODULES_IN_FILE", "JSC_MULTIPLE_MODULES_IN_FILE",
"There should only be a single goog.module() statement per file."); "There should only be a single goog.module() statement per file.");


static final DiagnosticType MODULE_AND_PROVIDES =
DiagnosticType.error(
"JSC_MODULE_AND_PROVIDES",
"A file using goog.module() may not also use goog.provide() statements.");

static final DiagnosticType ONE_REQUIRE_PER_DECLARATION = static final DiagnosticType ONE_REQUIRE_PER_DECLARATION =
DiagnosticType.error( DiagnosticType.error(
"JSC_ONE_REQUIRE_PER_DECLARATION", "JSC_ONE_REQUIRE_PER_DECLARATION",
Expand Down Expand Up @@ -147,6 +149,8 @@ public final class ClosureCheckModule extends AbstractModuleCallback
DiagnosticType.error( DiagnosticType.error(
"JSC_REQUIRE_NOT_AT_TOP_LEVEL", "goog.require() must be called at file scope."); "JSC_REQUIRE_NOT_AT_TOP_LEVEL", "goog.require() must be called at file scope.");


private final AbstractCompiler compiler;

private static class ModuleInfo { private static class ModuleInfo {
// Name of the module in question (i.e. the argument to goog.module) // Name of the module in question (i.e. the argument to goog.module)
private final String name; private final String name;
Expand All @@ -164,10 +168,10 @@ private static class ModuleInfo {
} }
} }


private ModuleInfo currentModuleInfo = null; private ModuleInfo currentModule = null;


public ClosureCheckModule(AbstractCompiler compiler, ModuleMetadataMap moduleMetadataMap) { public ClosureCheckModule(AbstractCompiler compiler) {
super(compiler, moduleMetadataMap); this.compiler = compiler;
} }


@Override @Override
Expand All @@ -181,29 +185,31 @@ public void hotSwapScript(Node scriptRoot, Node originalRoot) {
} }


@Override @Override
public void enterModule(ModuleMetadata currentModule, Node moduleScopeRoot) { public void enterModule(NodeTraversal t, Node scopeRoot) {
if (!currentModule.isGoogModule()) { Node firstStatement = scopeRoot.getFirstChild();
return; if (NodeUtil.isExprCall(firstStatement)) {
Node call = firstStatement.getFirstChild();
Node callee = call.getFirstChild();
if (callee.matchesQualifiedName("goog.module")) {
checkState(currentModule == null);
String moduleName = extractFirstArgumentName(call);
if (moduleName == null) {
t.report(scopeRoot, ClosureRewriteModule.INVALID_MODULE_NAMESPACE);
} else {
currentModule = new ModuleInfo(moduleName);
}
}
} }

checkState(currentModuleInfo == null);
checkState(!currentModule.googNamespaces().isEmpty());
currentModuleInfo = new ModuleInfo(Iterables.getFirst(currentModule.googNamespaces(), ""));
} }


@Override @Override
public void exitModule(@Nullable ModuleMetadata currentModule, @Nullable Node moduleScopeRoot) { public void exitModule(NodeTraversal t, Node scopeRoot) {
currentModuleInfo = null; currentModule = null;
} }


@Override @Override
protected void visit( public void visit(NodeTraversal t, Node n, Node parent) {
NodeTraversal t, if (currentModule == null) {
Node n,
@Nullable ModuleMetadata currentModule,
@Nullable Node moduleScopeRoot) {
Node parent = n.getParent();
if (currentModuleInfo == null) {
if (NodeUtil.isCallTo(n, "goog.module")) { if (NodeUtil.isCallTo(n, "goog.module")) {
t.report(n, GOOG_MODULE_IN_NON_MODULE); t.report(n, GOOG_MODULE_IN_NON_MODULE);
} else if (NodeUtil.isGoogModuleDeclareLegacyNamespaceCall(n)) { } else if (NodeUtil.isGoogModuleDeclareLegacyNamespaceCall(n)) {
Expand All @@ -219,8 +225,10 @@ protected void visit(
case CALL: case CALL:
Node callee = n.getFirstChild(); Node callee = n.getFirstChild();
if (callee.matchesQualifiedName("goog.module") if (callee.matchesQualifiedName("goog.module")
&& !currentModuleInfo.name.equals(extractFirstArgumentName(n))) { && !currentModule.name.equals(extractFirstArgumentName(n))) {
t.report(n, MULTIPLE_MODULES_IN_FILE); t.report(n, MULTIPLE_MODULES_IN_FILE);
} else if (callee.matchesQualifiedName("goog.provide")) {
t.report(n, MODULE_AND_PROVIDES);
} else if (callee.matchesQualifiedName("goog.require") } else if (callee.matchesQualifiedName("goog.require")
|| callee.matchesQualifiedName("goog.forwardDeclare")) { || callee.matchesQualifiedName("goog.forwardDeclare")) {
checkRequireCall(t, n, parent); checkRequireCall(t, n, parent);
Expand Down Expand Up @@ -264,10 +272,10 @@ protected void visit(
} }
break; break;
case GETPROP: case GETPROP:
if (n.matchesQualifiedName(currentModuleInfo.name)) { if (n.matchesQualifiedName(currentModule.name)) {
t.report(n, REFERENCE_TO_MODULE_GLOBAL_NAME); t.report(n, REFERENCE_TO_MODULE_GLOBAL_NAME);
} else if (currentModuleInfo.importsByLongRequiredName.containsKey(n.getQualifiedName())) { } else if (currentModule.importsByLongRequiredName.containsKey(n.getQualifiedName())) {
Node importLhs = currentModuleInfo.importsByLongRequiredName.get(n.getQualifiedName()); Node importLhs = currentModule.importsByLongRequiredName.get(n.getQualifiedName());
if (importLhs == null) { if (importLhs == null) {
t.report(n, REFERENCE_TO_FULLY_QUALIFIED_IMPORT_NAME, n.getQualifiedName()); t.report(n, REFERENCE_TO_FULLY_QUALIFIED_IMPORT_NAME, n.getQualifiedName());
} else if (importLhs.isName()) { } else if (importLhs.isName()) {
Expand Down Expand Up @@ -336,8 +344,8 @@ public void visit(Node node) {
} }
String type = node.getString(); String type = node.getString();
while (true) { while (true) {
if (currentModuleInfo.importsByLongRequiredName.containsKey(type)) { if (currentModule.importsByLongRequiredName.containsKey(type)) {
Node importLhs = currentModuleInfo.importsByLongRequiredName.get(type); Node importLhs = currentModule.importsByLongRequiredName.get(type);
if (importLhs == null || !importLhs.isName()) { if (importLhs == null || !importLhs.isName()) {
t.report(node, JSDOC_REFERENCE_TO_FULLY_QUALIFIED_IMPORT_NAME, type); t.report(node, JSDOC_REFERENCE_TO_FULLY_QUALIFIED_IMPORT_NAME, type);
} else if (!importLhs.getString().equals(type)) { } else if (!importLhs.getString().equals(type)) {
Expand Down Expand Up @@ -393,18 +401,18 @@ private void checkModuleExport(NodeTraversal t, Node n, Node parent) {
Node lhs = n.getFirstChild(); Node lhs = n.getFirstChild();
checkState(isExportLhs(lhs)); checkState(isExportLhs(lhs));
// Check multiple exports of the same name // Check multiple exports of the same name
Node previousDefinition = currentModuleInfo.exportNodesByName.get(lhs.getQualifiedName()); Node previousDefinition = currentModule.exportNodesByName.get(lhs.getQualifiedName());
if (previousDefinition != null if (previousDefinition != null
&& !isPermittedTypeScriptMultipleExportPattern(previousDefinition, lhs)) { && !isPermittedTypeScriptMultipleExportPattern(previousDefinition, lhs)) {
int previousLine = previousDefinition.getLineno(); int previousLine = previousDefinition.getLineno();
t.report(n, EXPORT_REPEATED_ERROR, String.valueOf(previousLine)); t.report(n, EXPORT_REPEATED_ERROR, String.valueOf(previousLine));
} }
// Check exports in invalid program position // Check exports in invalid program position
Node defaultExportNode = currentModuleInfo.exportNodesByName.get("exports"); Node defaultExportNode = currentModule.exportNodesByName.get("exports");
// If we have never seen an `exports =` default export assignment, or this is the // 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. // default export, then treat this assignment as an export and do the checks it is well formed.
if (defaultExportNode == null || lhs.matchesQualifiedName("exports")) { if (defaultExportNode == null || lhs.matchesQualifiedName("exports")) {
currentModuleInfo.exportNodesByName.put(lhs.getQualifiedName(), lhs); currentModule.exportNodesByName.put(lhs.getQualifiedName(), lhs);
if (!t.inModuleScope()) { if (!t.inModuleScope()) {
t.report(n, EXPORT_NOT_AT_MODULE_SCOPE); t.report(n, EXPORT_NOT_AT_MODULE_SCOPE);
} else if (!parent.isExprResult()) { } else if (!parent.isExprResult()) {
Expand All @@ -431,12 +439,15 @@ private String extractFirstArgumentName(Node callNode) {


private void checkRequireCall(NodeTraversal t, Node callNode, Node parent) { private void checkRequireCall(NodeTraversal t, Node callNode, Node parent) {
checkState(callNode.isCall()); checkState(callNode.isCall());
checkState(callNode.getLastChild().isString()); if (!callNode.getLastChild().isString()) {
t.report(callNode, ProcessClosurePrimitives.INVALID_ARGUMENT_ERROR, "goog.require");
return;
}
switch (parent.getToken()) { switch (parent.getToken()) {
case EXPR_RESULT: case EXPR_RESULT:
String key = extractFirstArgumentName(callNode); String key = extractFirstArgumentName(callNode);
if (!currentModuleInfo.importsByLongRequiredName.containsKey(key)) { if (!currentModule.importsByLongRequiredName.containsKey(key)) {
currentModuleInfo.importsByLongRequiredName.put(key, parent); currentModule.importsByLongRequiredName.put(key, parent);
} }
return; return;
case NAME: case NAME:
Expand Down Expand Up @@ -470,10 +481,10 @@ private void checkShortGoogRequireCall(NodeTraversal t, Node callNode, Node decl
checkState(lhs.isName()); checkState(lhs.isName());
checkShortName(t, lhs, callNode.getLastChild().getString()); checkShortName(t, lhs, callNode.getLastChild().getString());
} }
currentModuleInfo.importsByLongRequiredName.put(extractFirstArgumentName(callNode), lhs); currentModule.importsByLongRequiredName.put(extractFirstArgumentName(callNode), lhs);
for (Node nameNode : NodeUtil.findLhsNodesInNode(declaration)) { for (Node nameNode : NodeUtil.findLhsNodesInNode(declaration)) {
String name = nameNode.getString(); String name = nameNode.getString();
if (!currentModuleInfo.shortImportNames.add(name)) { if (!currentModule.shortImportNames.add(name)) {
t.report(nameNode, DUPLICATE_NAME_SHORT_REQUIRE, name); t.report(nameNode, DUPLICATE_NAME_SHORT_REQUIRE, name);
} }
} }
Expand Down
4 changes: 0 additions & 4 deletions src/com/google/javascript/jscomp/CompilerOptions.java
View file
Expand Up @@ -2721,10 +2721,6 @@ public void setProcessCommonJSModules(boolean processCommonJSModules) {
this.processCommonJSModules = processCommonJSModules; this.processCommonJSModules = processCommonJSModules;
} }


public boolean getProcessCommonJSModules() {
return processCommonJSModules;
}

/** /**
* How ES6 modules should be transformed. * How ES6 modules should be transformed.
*/ */
Expand Down
14 changes: 4 additions & 10 deletions src/com/google/javascript/jscomp/DefaultPassConfig.java
View file
Expand Up @@ -1091,12 +1091,6 @@ private void assertValidOrderForChecks(List<PassFactory> checks) {
checkVariableReferences, checkVariableReferences,
closureGoogScopeAliases, closureGoogScopeAliases,
"Variable checking must happen before goog.scope processing."); "Variable checking must happen before goog.scope processing.");

assertPassOrder(
checks,
gatherModuleMetadataPass,
closureCheckModule,
"Need to gather module metadata before checking closure modules.");
} }


/** /**
Expand Down Expand Up @@ -1478,7 +1472,7 @@ protected FeatureSet featureSet() {
new HotSwapPassFactory("closureCheckModule") { new HotSwapPassFactory("closureCheckModule") {
@Override @Override
protected HotSwapCompilerPass create(AbstractCompiler compiler) { protected HotSwapCompilerPass create(AbstractCompiler compiler) {
return new ClosureCheckModule(compiler, compiler.getModuleMetadataMap()); return new ClosureCheckModule(compiler);
} }


@Override @Override
Expand Down Expand Up @@ -3438,10 +3432,10 @@ protected FeatureSet featureSet() {
} }
}; };


private final HotSwapPassFactory gatherModuleMetadataPass = private final PassFactory gatherModuleMetadataPass =
new HotSwapPassFactory(PassNames.GATHER_MODULE_METADATA) { new PassFactory(PassNames.GATHER_MODULE_METADATA, /* isOneTimePass= */ true) {
@Override @Override
protected HotSwapCompilerPass create(AbstractCompiler compiler) { protected CompilerPass create(AbstractCompiler compiler) {
return new GatherModuleMetadata( return new GatherModuleMetadata(
compiler, options.processCommonJSModules, options.moduleResolutionMode); compiler, options.processCommonJSModules, options.moduleResolutionMode);
} }
Expand Down
28 changes: 1 addition & 27 deletions src/com/google/javascript/jscomp/GatherModuleMetadata.java
View file
Expand Up @@ -37,7 +37,7 @@
* Gathers metadata around modules that is useful for checking imports / requires and creates a * Gathers metadata around modules that is useful for checking imports / requires and creates a
* {@link ModuleMetadataMap}. * {@link ModuleMetadataMap}.
*/ */
public final class GatherModuleMetadata implements HotSwapCompilerPass { public final class GatherModuleMetadata implements CompilerPass {
static final DiagnosticType MIXED_MODULE_TYPE = static final DiagnosticType MIXED_MODULE_TYPE =
DiagnosticType.error("JSC_MIXED_MODULE_TYPE", "A file cannot be both {0} and {1}."); DiagnosticType.error("JSC_MIXED_MODULE_TYPE", "A file cannot be both {0} and {1}.");


Expand Down Expand Up @@ -420,30 +420,4 @@ public void process(Node externs, Node root) {
NodeTraversal.traverse(compiler, root, new Finder()); NodeTraversal.traverse(compiler, root, new Finder());
compiler.setModuleMetadataMap(new ModuleMetadataMap(modulesByPath, modulesByGoogNamespace)); compiler.setModuleMetadataMap(new ModuleMetadataMap(modulesByPath, modulesByGoogNamespace));
} }

@Override
public void hotSwapScript(Node scriptRoot, Node originalRoot) {
// This pass is run as either a hot swap or full pass. So if we're running in hot swap this is
// a different instance from the full pass, and we need to populate these again.
modulesByPath.putAll(compiler.getModuleMetadataMap().getModulesByPath());
modulesByGoogNamespace.putAll(compiler.getModuleMetadataMap().getModulesByGoogNamespace());

ModuleMetadata oldMetadata =
modulesByPath.remove(compiler.getInput(originalRoot.getInputId()).getPath().toString());

if (oldMetadata != null) {
for (String namespace : oldMetadata.googNamespaces()) {
modulesByGoogNamespace.remove(namespace);
}

for (ModuleMetadata nestedMetadata : oldMetadata.nestedModules()) {
for (String namespace : nestedMetadata.googNamespaces()) {
modulesByGoogNamespace.remove(namespace);
}
}
}

NodeTraversal.traverse(compiler, scriptRoot, new Finder());
compiler.setModuleMetadataMap(new ModuleMetadataMap(modulesByPath, modulesByGoogNamespace));
}
} }
20 changes: 1 addition & 19 deletions src/com/google/javascript/jscomp/LintPassConfig.java
View file
Expand Up @@ -16,7 +16,6 @@
package com.google.javascript.jscomp; package com.google.javascript.jscomp;


import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
import com.google.javascript.jscomp.PassFactory.HotSwapPassFactory;
import com.google.javascript.jscomp.lint.CheckDuplicateCase; import com.google.javascript.jscomp.lint.CheckDuplicateCase;
import com.google.javascript.jscomp.lint.CheckEmptyStatements; import com.google.javascript.jscomp.lint.CheckEmptyStatements;
import com.google.javascript.jscomp.lint.CheckEnums; import com.google.javascript.jscomp.lint.CheckEnums;
Expand Down Expand Up @@ -46,7 +45,6 @@ class LintPassConfig extends PassConfig.PassConfigDelegate {
@Override @Override
protected List<PassFactory> getChecks() { protected List<PassFactory> getChecks() {
return ImmutableList.of( return ImmutableList.of(
gatherModuleMetadataPass,
earlyLintChecks, earlyLintChecks,
checkRequires, checkRequires,
variableReferenceCheck, variableReferenceCheck,
Expand All @@ -59,22 +57,6 @@ protected List<PassFactory> getOptimizations() {
return ImmutableList.of(); return ImmutableList.of();
} }


private final HotSwapPassFactory gatherModuleMetadataPass =
new HotSwapPassFactory(PassNames.GATHER_MODULE_METADATA) {
@Override
protected HotSwapCompilerPass create(AbstractCompiler compiler) {
return new GatherModuleMetadata(
compiler,
compiler.getOptions().getProcessCommonJSModules(),
compiler.getOptions().getModuleResolutionMode());
}

@Override
protected FeatureSet featureSet() {
return FeatureSet.latest().withoutTypes();
}
};

private final PassFactory earlyLintChecks = private final PassFactory earlyLintChecks =
new PassFactory("earlyLintChecks", true) { new PassFactory("earlyLintChecks", true) {
@Override @Override
Expand All @@ -90,7 +72,7 @@ protected CompilerPass create(AbstractCompiler compiler) {
new CheckMissingSemicolon(compiler), new CheckMissingSemicolon(compiler),
new CheckSuper(compiler), new CheckSuper(compiler),
new CheckPrimitiveAsObject(compiler), new CheckPrimitiveAsObject(compiler),
new ClosureCheckModule(compiler, compiler.getModuleMetadataMap()), new ClosureCheckModule(compiler),
new CheckNullabilityModifiers(compiler), new CheckNullabilityModifiers(compiler),
new CheckRequiresAndProvidesSorted(compiler), new CheckRequiresAndProvidesSorted(compiler),
new CheckSideEffects( new CheckSideEffects(
Expand Down

0 comments on commit 407418d

Please sign in to comment.