Skip to content

Commit

Permalink
Remove support for goog.require by path.
Browse files Browse the repository at this point in the history 
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=198598924
  • Loading branch information
@lauraharker
johnplaisted authored and lauraharker committed May 31, 2018
1 parent aa0e726 commit 37afb57
Show file tree
Hide file tree
Showing 9 changed files with 19 additions and 492 deletions.
106 changes: 2 additions & 104 deletions src/com/google/javascript/jscomp/Es6RewriteModules.java
View file
Expand Up @@ -16,7 +16,6 @@
package com.google.javascript.jscomp;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
import static com.google.javascript.jscomp.ClosureCheckModule.MODULE_USES_GOOG_MODULE_GET;
import static com.google.javascript.jscomp.ClosureRewriteModule.INVALID_GET_CALL_SCOPE;
Expand All @@ -26,7 +25,6 @@
import static com.google.javascript.jscomp.ProcessClosurePrimitives.INVALID_CLOSURE_CALL_ERROR;
import static com.google.javascript.jscomp.parsing.parser.FeatureSet.Feature.MODULES;

import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
import com.google.common.base.Splitter;
import com.google.common.collect.Iterables;
Expand All @@ -35,7 +33,6 @@
import com.google.javascript.jscomp.ModuleMetadata.Module;
import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback;
import com.google.javascript.jscomp.deps.ModuleLoader;
import com.google.javascript.jscomp.deps.ModuleLoader.ModulePath;
import com.google.javascript.jscomp.deps.ModuleLoader.ResolutionMode;
import com.google.javascript.rhino.IR;
import com.google.javascript.rhino.JSDocInfo;
Expand Down Expand Up @@ -85,24 +82,6 @@ public final class Es6RewriteModules extends AbstractPostOrderCallback
"JSC_PATH_REQUIRE_IN_NON_GOOG_MODULE",
"Cannot goog.require by path outside of goog.modules.");

static final DiagnosticType PATH_REQUIRE_IN_GOOG_PROVIDE_FILE =
DiagnosticType.error(
"JSC_PATH_REQUIRE_IN_GOOG_PROVIDE_FILE",
"goog.provide'd files can only goog.require namespaces. goog.require by path is only "
+ "valid in goog.modules.");

static final DiagnosticType PATH_REQUIRE_IN_ES6_MODULE =
DiagnosticType.error(
"JSC_PATH_REQUIRE_IN_ES6_MODULE",
"Import other ES6 modules and goog.require namespaces in ES6 modules. Do not "
+ "goog.require paths in ES6 modules.");

static final DiagnosticType PATH_REQUIRE_IN_GOOG_MODULE_WITH_NO_PATH =
DiagnosticType.error(
"JSC_PATH_REQUIRE_IN_GOOG_MODULE_WITH_NO_PATH",
"The goog.module {0} cannot require by path. It has been loaded without path "
+ "information (is the path argument to goog.loadModule provided?).");

private final AbstractCompiler compiler;

@Nullable private final PreprocessorSymbolTable preprocessorSymbolTable;
Expand Down Expand Up @@ -224,14 +203,6 @@ public void visit(NodeTraversal t, Node n, Node parent) {
String name = n.getLastChild().getString();
Module data = moduleMetadata.getModulesByGoogNamespace().get(name);

boolean isPathRequire = false;

// Can only goog.module.get by namespace.
if (data == null && isRequire && ModuleLoader.isRelativeIdentifier(name)) {
data = resolvePathRequire(t, n);
isPathRequire = true;
}

if (data == null || !data.isEs6Module()) {
return;
}
Expand All @@ -244,14 +215,9 @@ public void visit(NodeTraversal t, Node n, Node parent) {
Node statementNode = NodeUtil.getEnclosingStatement(n);
boolean importHasAlias = NodeUtil.isNameDeclaration(statementNode);
if (importHasAlias) {
// const module = goog.require('./es6.js');
// const module = module$es6;
//
// const module = goog.require('an.es6.namespace');
// const module = module$es6;
n.replaceWith(
IR.name(isPathRequire ? data.getGlobalName() : data.getGlobalName(name))
.useSourceInfoFromForTree(n));
n.replaceWith(IR.name(data.getGlobalName(name)).useSourceInfoFromForTree(n));
t.reportCodeChange();
} else {
if (parent.isExprResult()) {
Expand All @@ -265,70 +231,6 @@ public void visit(NodeTraversal t, Node n, Node parent) {
}
}
}

@Nullable
private Module resolvePathRequire(NodeTraversal t, Node requireCall) {
String name = requireCall.getLastChild().getString();
Module thisModule = moduleMetadata.getContainingModule(requireCall);
checkNotNull(thisModule);

if (!thisModule.isGoogModule()) {
t.report(
requireCall,
thisModule.isGoogProvide()
? PATH_REQUIRE_IN_GOOG_PROVIDE_FILE
: PATH_REQUIRE_IN_NON_GOOG_MODULE);
return null;
}

if (thisModule.getPath() == null) {
t.report(
requireCall,
PATH_REQUIRE_IN_GOOG_MODULE_WITH_NO_PATH,
Iterables.getOnlyElement(thisModule.getGoogNamespaces()));
return null;
}

ModulePath path =
thisModule
.getPath()
.resolveJsModule(
name, t.getSourceName(), requireCall.getLineno(), requireCall.getCharno());

// If path is null then resolveJsModule has logged an error.
if (path != null) {
Module requiredModule = moduleMetadata.getModulesByPath().get(path.toString());
checkNotNull(requiredModule);
if (!requiredModule.isEs6Module()) {
reportPathRequireForNonEs6Module(requiredModule, t, requireCall);
}
return requiredModule;
}
return null;
}
}

private void reportPathRequireForNonEs6Module(
Module requiredModule, NodeTraversal t, Node requireCall) {
String suggestion = "";

if (requiredModule.getGoogNamespaces().size() == 1) {
suggestion =
" Did you mean to goog.require "
+ Iterables.getOnlyElement(requiredModule.getGoogNamespaces())
+ "?";
} else if (requiredModule.getGoogNamespaces().size() > 1) {
suggestion =
" Did you mean to goog.require one of ("
+ Joiner.on(", ").join(requiredModule.getGoogNamespaces())
+ ")?";
}

t.report(
requireCall,
PATH_REQUIRE_FOR_NON_ES6_MODULE,
t.getInput().getPath().toString(),
suggestion);
}

@Override
Expand Down Expand Up @@ -740,11 +642,7 @@ private void visitRequire(NodeTraversal t, Node requireCall, Node parent, boolea
Module m = moduleMetadata.getModulesByGoogNamespace().get(namespace);

if (m == null) {
if (ModuleLoader.isRelativeIdentifier(namespace)) {
t.report(requireCall, PATH_REQUIRE_IN_ES6_MODULE, namespace);
} else {
t.report(requireCall, MISSING_MODULE_OR_PROVIDE, namespace);
}
t.report(requireCall, MISSING_MODULE_OR_PROVIDE, namespace);
return;
}

Expand Down
15 changes: 4 additions & 11 deletions src/com/google/javascript/jscomp/ModuleMetadata.java
View file
Expand Up @@ -42,12 +42,9 @@
* <p>TODO(johnplaisted): There's an opportunity for reuse here in ClosureRewriteModules, which
* would involve putting this in some common location. Currently this is only used as a helper class
* for Es6RewriteModules. CompilerInput already has some (not all) of this information but it is not
* always populated. Additionally we'd ideally unwrap the goog.loadModule calls so each becomes its
* own CompilerInput, otherwise goog.require(path) from loadModules won't work correctly. But not
* having a 1:1 mapping of actual inputs to compiler inputs may cause issues. It may also be ideal
* to include CommonJS here too as ES6 modules can import them. That would allow decoupling of how
* these modules are written; right now Es6RewriteModule only checks this for goog.requires and
* goog: imports, not for ES6 path imports.
* always populated. It may also be ideal to include CommonJS here too as ES6 modules can import
* them. That would allow decoupling of how these modules are written; right now Es6RewriteModule
* only checks this for goog.requires and goog: imports, not for ES6 path imports.
*/
final class ModuleMetadata {
/** Various types of Javascript "modules" that can be found in the JS Compiler. */
Expand Down Expand Up @@ -313,11 +310,7 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
case CALL:
if (n.isCall() && n.getFirstChild().matchesQualifiedName("goog.loadModule")) {
loadModuleCall = n;
if (n.getChildCount() > 2 && n.getChildAtIndex(2).isString()) {
enterModule(n, compiler.getModuleLoader().resolve(n.getChildAtIndex(2).getString()));
} else {
enterModule(n, null);
}
enterModule(n, null);
}
break;
default:
Expand Down
13 changes: 0 additions & 13 deletions src/com/google/javascript/jscomp/deps/DependencyInfo.java
View file
Expand Up @@ -48,11 +48,6 @@ abstract class Require implements Serializable {
public enum Type {
/** Standard goog.require call for a symbol from a goog.provide or goog.module. */
GOOG_REQUIRE_SYMBOL,
/**
* goog.require call with a path argument. Currently only supported in goog.modules for ES6
* modules.
*/
GOOG_REQUIRE_PATH,
/** ES6 import statement. */
ES6_IMPORT,
/** Parsed from an existing Closure dependency file. */
Expand All @@ -75,14 +70,6 @@ public static Require googRequireSymbol(String symbol) {
.build();
}

public static Require googRequirePath(String symbol, String rawPath) {
return builder()
.setRawText(rawPath)
.setSymbol(symbol)
.setType(Type.GOOG_REQUIRE_PATH)
.build();
}

public static Require es6Import(String symbol, String rawPath) {
return builder().setRawText(rawPath).setSymbol(symbol).setType(Type.ES6_IMPORT).build();
}
Expand Down
55 changes: 2 additions & 53 deletions src/com/google/javascript/jscomp/deps/DepsGenerator.java
View file
Expand Up @@ -78,21 +78,6 @@ public static enum InclusionStrategy {
"DEPS_ES6_IMPORT_FOR_NON_ES6_MODULE",
"Cannot import file \"{0}\" because it is not an ES6 module.");

static final DiagnosticType GOOG_REQUIRE_PATH_BETWEEN_ES6_MODULES =
DiagnosticType.warning(
"DEPS_GOOG_REQUIRE_PATH_BETWEEN_ES6_MODULES",
"Use ES6 import rather than goog.require to import ES6 module \"{0}\".");

static final DiagnosticType GOOG_REQUIRE_PATH_FOR_NON_ES6_MODULE =
DiagnosticType.warning(
"DEPS_GOOG_REQUIRE_PATH_FOR_NON_ES6_MODULE",
"Cannot goog.require \"{0}\" by path. It is not an ES6 module.");

static final DiagnosticType GOOG_REQUIRE_PATH_FROM_NON_GOOG_MODULE =
DiagnosticType.warning(
"DEPS_GOOG_REQUIRE_PATH_FROM_NON_GOOG_MODULE",
"Cannot goog.require by path outside of a goog.module.");

static final DiagnosticType UNKNOWN_PATH_IMPORT =
DiagnosticType.warning("DEPS_UNKNOWN_PATH_IMPORT", "Could not find file \"{0}\".");

Expand Down Expand Up @@ -218,8 +203,7 @@ private Map<String, DependencyInfo> removeMungedSymbols(
for (DependencyInfo dependencyInfo : jsFiles.values()) {
ArrayList<Require> newRequires = new ArrayList<>();
for (Require require : dependencyInfo.getRequires()) {
if (require.getType() == Require.Type.ES6_IMPORT
|| require.getType() == Require.Type.GOOG_REQUIRE_PATH) {
if (require.getType() == Require.Type.ES6_IMPORT) {
// Symbols are unique per file and have nothing to do with paths so map lookups are safe
// here.
DependencyInfo provider = providesMap.get(require.getSymbol());
Expand Down Expand Up @@ -303,8 +287,7 @@ private void validateDependencies(Iterable<DependencyInfo> preparsedFileDependen
} else if (provider == depInfo) {
reportSameFile(namespace, depInfo);
} else {
boolean isGoogModule = depInfo.isModule();
boolean isEs6Module = "es6".equals(depInfo.getLoadFlags().get("module"));
depInfo.isModule();
boolean providerIsEs6Module = "es6".equals(provider.getLoadFlags().get("module"));

switch (require.getType()) {
Expand All @@ -313,18 +296,6 @@ private void validateDependencies(Iterable<DependencyInfo> preparsedFileDependen
reportEs6ImportForNonEs6Module(provider, depInfo);
}
break;
case GOOG_REQUIRE_PATH:
if (isEs6Module && providerIsEs6Module) {
reportGoogRequirePathBetweenEs6Modules(provider, depInfo);
} else {
if (!isGoogModule) {
reportGoogRequirePathFromNonGoogModule(depInfo);
}
if (!providerIsEs6Module) {
reportGoogRequirePathForNonEs6Module(provider, depInfo);
}
}
break;
case GOOG_REQUIRE_SYMBOL:
case PARSED_FROM_DEPS:
break;
Expand All @@ -349,28 +320,6 @@ private void reportEs6ImportForNonEs6Module(DependencyInfo provider, DependencyI
JSError.make(depInfo.getName(), -1, -1, ES6_IMPORT_FOR_NON_ES6_MODULE, provider.getName()));
}

private void reportGoogRequirePathBetweenEs6Modules(
DependencyInfo provider, DependencyInfo depInfo) {
errorManager.report(
CheckLevel.ERROR,
JSError.make(
depInfo.getName(), -1, -1, GOOG_REQUIRE_PATH_BETWEEN_ES6_MODULES, provider.getName()));
}

private void reportGoogRequirePathForNonEs6Module(
DependencyInfo provider, DependencyInfo depInfo) {
errorManager.report(
CheckLevel.ERROR,
JSError.make(
depInfo.getName(), -1, -1, GOOG_REQUIRE_PATH_FOR_NON_ES6_MODULE, provider.getName()));
}

private void reportGoogRequirePathFromNonGoogModule(DependencyInfo depInfo) {
errorManager.report(
CheckLevel.ERROR,
JSError.make(depInfo.getName(), -1, -1, GOOG_REQUIRE_PATH_FROM_NON_GOOG_MODULE));
}

private void reportSameFile(String namespace, DependencyInfo depInfo) {
errorManager.report(CheckLevel.WARNING,
JSError.make(depInfo.getName(), -1, -1,
Expand Down
8 changes: 0 additions & 8 deletions src/com/google/javascript/jscomp/deps/JsFileParser.java
View file
Expand Up @@ -280,14 +280,6 @@ protected boolean parseLine(String line) throws ParseException {
} else if (!"goog".equals(arg)) {
// goog is always implicit.
Require require = Require.googRequireSymbol(arg);
if (ModuleLoader.isRelativeIdentifier(arg)) {
ModuleLoader.ModulePath path = file.resolveJsModule(arg);
if (path == null) {
path = file.resolveModuleAsPath(arg);
}
String symbol = path.toModuleName();
require = Require.googRequirePath(symbol, arg);
}
requires.add(require);
}
} else {
Expand Down

0 comments on commit 37afb57

Please sign in to comment.