Skip to content

Commit

Permalink
Use ModuleMetadata in the AbstractModuleCallback.
Browse files Browse the repository at this point in the history 
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=223077471
  • Loading branch information
@tjgq
johnplaisted authored and tjgq committed Nov 28, 2018
1 parent 4f95766 commit 12c9f6b
Show file tree
Hide file tree
Showing 9 changed files with 342 additions and 99 deletions.
85 changes: 37 additions & 48 deletions src/com/google/javascript/jscomp/ClosureCheckModule.java
View file
Expand Up @@ -23,6 +23,8 @@
import static com.google.javascript.jscomp.ClosurePrimitiveErrors.INVALID_DESTRUCTURING_FORWARD_DECLARE;
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.rhino.JSDocInfo;
import com.google.javascript.rhino.Node;
Expand All @@ -31,6 +33,7 @@
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import javax.annotation.Nullable;

/**
* Checks that goog.module() is used correctly.
Expand Down Expand Up @@ -79,18 +82,13 @@ public final class ClosureCheckModule extends AbstractModuleCallback
static final DiagnosticType LET_GOOG_REQUIRE =
DiagnosticType.disabled(
"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 =
DiagnosticType.error(
"JSC_MULTIPLE_MODULES_IN_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 =
DiagnosticType.error(
"JSC_ONE_REQUIRE_PER_DECLARATION",
Expand Down Expand Up @@ -149,8 +147,6 @@ public final class ClosureCheckModule extends AbstractModuleCallback
DiagnosticType.error(
"JSC_REQUIRE_NOT_AT_TOP_LEVEL", "goog.require() must be called at file scope.");

private final AbstractCompiler compiler;

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

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

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

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

@Override
public void enterModule(NodeTraversal t, Node scopeRoot) {
Node firstStatement = scopeRoot.getFirstChild();
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);
}
}
public void enterModule(ModuleMetadata currentModule, Node moduleScopeRoot) {
if (!currentModule.isGoogModule()) {
return;
}

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

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

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

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

public boolean getProcessCommonJSModules() {
return processCommonJSModules;
}

/**
* How ES6 modules should be transformed.
*/
Expand Down
14 changes: 10 additions & 4 deletions src/com/google/javascript/jscomp/DefaultPassConfig.java
View file
Expand Up @@ -1091,6 +1091,12 @@ private void assertValidOrderForChecks(List<PassFactory> checks) {
checkVariableReferences,
closureGoogScopeAliases,
"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 @@ -1472,7 +1478,7 @@ protected FeatureSet featureSet() {
new HotSwapPassFactory("closureCheckModule") {
@Override
protected HotSwapCompilerPass create(AbstractCompiler compiler) {
return new ClosureCheckModule(compiler);
return new ClosureCheckModule(compiler, compiler.getModuleMetadataMap());
}

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

private final PassFactory gatherModuleMetadataPass =
new PassFactory(PassNames.GATHER_MODULE_METADATA, /* isOneTimePass= */ true) {
private final HotSwapPassFactory gatherModuleMetadataPass =
new HotSwapPassFactory(PassNames.GATHER_MODULE_METADATA) {
@Override
protected CompilerPass create(AbstractCompiler compiler) {
protected HotSwapCompilerPass create(AbstractCompiler compiler) {
return new GatherModuleMetadata(
compiler, options.processCommonJSModules, options.moduleResolutionMode);
}
Expand Down
28 changes: 27 additions & 1 deletion 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
* {@link ModuleMetadataMap}.
*/
public final class GatherModuleMetadata implements CompilerPass {
public final class GatherModuleMetadata implements HotSwapCompilerPass {
static final DiagnosticType MIXED_MODULE_TYPE =
DiagnosticType.error("JSC_MIXED_MODULE_TYPE", "A file cannot be both {0} and {1}.");

Expand Down Expand Up @@ -420,4 +420,30 @@ public void process(Node externs, Node root) {
NodeTraversal.traverse(compiler, root, new Finder());
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: 19 additions & 1 deletion src/com/google/javascript/jscomp/LintPassConfig.java
View file
Expand Up @@ -16,6 +16,7 @@
package com.google.javascript.jscomp;

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.CheckEmptyStatements;
import com.google.javascript.jscomp.lint.CheckEnums;
Expand Down Expand Up @@ -45,6 +46,7 @@ class LintPassConfig extends PassConfig.PassConfigDelegate {
@Override
protected List<PassFactory> getChecks() {
return ImmutableList.of(
gatherModuleMetadataPass,
earlyLintChecks,
checkRequires,
variableReferenceCheck,
Expand All @@ -57,6 +59,22 @@ protected List<PassFactory> getOptimizations() {
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 =
new PassFactory("earlyLintChecks", true) {
@Override
Expand All @@ -72,7 +90,7 @@ protected CompilerPass create(AbstractCompiler compiler) {
new CheckMissingSemicolon(compiler),
new CheckSuper(compiler),
new CheckPrimitiveAsObject(compiler),
new ClosureCheckModule(compiler),
new ClosureCheckModule(compiler, compiler.getModuleMetadataMap()),
new CheckNullabilityModifiers(compiler),
new CheckRequiresAndProvidesSorted(compiler),
new CheckSideEffects(
Expand Down

0 comments on commit 12c9f6b

Please sign in to comment.