Skip to content

Commit

Permalink
When the module resolution is browser with tranformed prefixes:
Browse files Browse the repository at this point in the history
- Allow post-transformed prefixes to be ambiguous for better path-matching with input paths. Files are often passed to the compiler as ambiugous paths.
- Report a better error if an ambiguous path doesn't match any of the prefixes.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=229455337
  • Loading branch information
johnplaisted authored and lauraharker committed Jan 16, 2019
1 parent 8632f4e commit 4620328
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.google.javascript.jscomp.deps.ModuleLoader.PathEscaper;
import java.util.Comparator;
import java.util.Set;
import java.util.stream.Collectors;
import javax.annotation.Nullable;

/**
Expand All @@ -38,18 +39,17 @@
*/
public class BrowserWithTransformedPrefixesModuleResolver extends ModuleResolver {

static final DiagnosticType TRANSFORMED_PATH_IS_AMBIGUOUS =
static final DiagnosticType INVALID_AMBIGUOUS_PATH =
DiagnosticType.error(
"JSC_TRANSFORMED_PATH_IS_AMBIGUOUS",
"Replacing \"{0}\" with \"{1}\" in the import path \"{2}\" is an ambiguous address "
+ "(\"{3}\").");
"JSC_INVALID_AMBIGUOUS_PATH",
"The path \"{0}\" is invalid. Expected any of the following prefixes for non-relative "
+ "paths: {1}.");

/** Factory for {@link BrowserWithTransformedPrefixesModuleResolver}. */
public static final class Factory implements ModuleResolverFactory {
private final ImmutableMap<String, String> prefixReplacements;

public Factory(
ImmutableMap<String, String> prefixReplacements) {
public Factory(ImmutableMap<String, String> prefixReplacements) {
this.prefixReplacements = prefixReplacements;
}

Expand All @@ -71,6 +71,7 @@ public ModuleResolver create(
@AutoValue
abstract static class PrefixReplacement {
abstract String prefix();

abstract String replacement();

public static PrefixReplacement of(String prefix, String replacement) {
Expand All @@ -80,6 +81,7 @@ public static PrefixReplacement of(String prefix, String replacement) {
}

private final ImmutableSet<PrefixReplacement> prefixReplacements;
private final String expectedPrefixes;

public BrowserWithTransformedPrefixesModuleResolver(
ImmutableSet<String> modulePaths,
Expand All @@ -89,60 +91,54 @@ public BrowserWithTransformedPrefixesModuleResolver(
ImmutableMap<String, String> prefixReplacements) {
super(modulePaths, moduleRootPaths, errorHandler, pathEscaper);
Set<PrefixReplacement> p =
prefixReplacements
.entrySet()
.stream()
prefixReplacements.entrySet().stream()
.map(entry -> PrefixReplacement.of(entry.getKey(), entry.getValue()))
.collect(
toImmutableSortedSet(
// Sort by length in descending order to prefixes are applied most specific to
// least specific.
Comparator.<PrefixReplacement>comparingInt(r -> r.prefix().length())
.reversed()
.thenComparing(r -> r.prefix())));
.thenComparing(PrefixReplacement::prefix)));
this.prefixReplacements = ImmutableSet.copyOf(p);
this.expectedPrefixes =
this.prefixReplacements.stream()
.map(PrefixReplacement::prefix)
.sorted()
.collect(Collectors.joining(", "));
}

@Nullable
@Override
public String resolveJsModule(
String scriptAddress, String moduleAddress, String sourcename, int lineno, int colno) {
String transformedAddress = moduleAddress;

boolean replaced = false;

for (PrefixReplacement prefixReplacement : prefixReplacements) {
if (moduleAddress.startsWith(prefixReplacement.prefix())) {
transformedAddress =
prefixReplacement.replacement()
+ moduleAddress.substring(prefixReplacement.prefix().length());

if (ModuleLoader.isAmbiguousIdentifier(transformedAddress)) {
errorHandler.report(
CheckLevel.WARNING,
JSError.make(
sourcename,
lineno,
colno,
TRANSFORMED_PATH_IS_AMBIGUOUS,
prefixReplacement.prefix(),
prefixReplacement.replacement(),
moduleAddress,
transformedAddress));
}
replaced = true;
break;
}
}

// If ambiguous after the loop it was not transformed and the original moduleAddress is
// ambiguous.
if (ModuleLoader.isAmbiguousIdentifier(transformedAddress)) {
// ambiguous with an unrecognized prefix. Allow transformed paths to be ambiguous after
// transformation to maybe match how files are passed to the compiler.
if (!replaced && ModuleLoader.isAmbiguousIdentifier(transformedAddress)) {
errorHandler.report(
CheckLevel.WARNING,
JSError.make(
sourcename,
lineno,
colno,
ModuleLoader.INVALID_MODULE_PATH,
INVALID_AMBIGUOUS_PATH,
transformedAddress,
ModuleLoader.ResolutionMode.BROWSER_WITH_TRANSFORMED_PREFIXES.toString()));
expectedPrefixes));
return null;
}

Expand Down
25 changes: 25 additions & 0 deletions test/com/google/javascript/jscomp/deps/ModuleLoaderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,15 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.javascript.jscomp.CheckLevel;
import com.google.javascript.jscomp.CompilerInput;
import com.google.javascript.jscomp.ErrorHandler;
import com.google.javascript.jscomp.JSError;
import com.google.javascript.jscomp.SourceFile;
import com.google.javascript.jscomp.deps.ModuleLoader.PathEscaper;
import com.google.javascript.jscomp.deps.ModuleLoader.PathResolver;
import java.util.ArrayList;
import java.util.List;
import javax.annotation.Nullable;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -563,6 +567,27 @@ public void testBrowserWithPrefixReplacementAppliedMostSpecificToLeast() {
loader.resolve("fake.js").resolveJsModule("0/1/2/file.js"));
}

@Test
public void testBrowserWithPrefixReplacementInvalidPrefix() {
List<JSError> errors = new ArrayList<>();

ModuleLoader loader =
new ModuleLoader(
(CheckLevel level, JSError error) -> errors.add(error),
ImmutableList.of("."),
inputs("/path/to/file.js"),
new BrowserWithTransformedPrefixesModuleResolver.Factory(
ImmutableMap.of("prefix/", "/path/to/")));

assertUri("/path/to/file.js", loader.resolve("fake.js").resolveJsModule("prefix/file.js"));
assertThat(errors).isEmpty();

loader.resolve("fake.js").resolveJsModule("invalid/file.js");
assertThat(errors).hasSize(1);
assertThat(errors.get(0).getType())
.isSameAs(BrowserWithTransformedPrefixesModuleResolver.INVALID_AMBIGUOUS_PATH);
}

@Test
public void testCustomResolution() {
ModuleLoader loader =
Expand Down

0 comments on commit 4620328

Please sign in to comment.