Skip to content

Commit

Permalink
Minor code simplification in SkylarkImports
Browse files Browse the repository at this point in the history
RELNOTES: None.
PiperOrigin-RevId: 243616522
  • Loading branch information
laurentlb authored and copybara-github committed Apr 15, 2019
1 parent c07188a commit 4333692
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -52,18 +52,14 @@ public Label getLabel(Label containingFileLabel) {
// to the repo of the containing file.
return containingFileLabel.resolveRepositoryRelative(importLabel);
}

}

@VisibleForSerialization
@AutoCodec
static final class RelativeLabelImport extends SkylarkImport {
private final String importTarget;

@VisibleForSerialization
RelativeLabelImport(String importString, String importTarget) {
RelativeLabelImport(String importString) {
super(importString);
this.importTarget = importTarget;
}

@Override
Expand All @@ -73,14 +69,13 @@ public Label getLabel(Label containingFileLabel) {
try {
// This is for imports relative to the current repository, so repositoryMapping can be
// empty
return containingFileLabel.getRelativeWithRemapping(importTarget, ImmutableMap.of());
return containingFileLabel.getRelativeWithRemapping(getImportString(), ImmutableMap.of());
} catch (LabelSyntaxException e) {
// shouldn't happen because the parent label is assumed validated and the target string is
// validated on construction
throw new IllegalStateException(e);
}
}

}

/**
Expand Down Expand Up @@ -136,6 +131,10 @@ public static SkylarkImport create(String importString) throws SkylarkImportSynt
public static SkylarkImport create(
String importString, ImmutableMap<RepositoryName, RepositoryName> repositoryMapping)
throws SkylarkImportSyntaxException {
if (!importString.endsWith(".bzl")) {
throw new SkylarkImportSyntaxException(MUST_HAVE_BZL_EXT_MSG);
}

if (importString.startsWith("//") || importString.startsWith("@")) {
// Absolute label.
Label importLabel;
Expand All @@ -144,10 +143,6 @@ public static SkylarkImport create(
} catch (LabelSyntaxException e) {
throw new SkylarkImportSyntaxException(INVALID_LABEL_PREFIX + e.getMessage());
}
String targetName = importLabel.getName();
if (!targetName.endsWith(".bzl")) {
throw new SkylarkImportSyntaxException(MUST_HAVE_BZL_EXT_MSG);
}
PackageIdentifier packageId = importLabel.getPackageIdentifier();
if (packageId.equals(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER)) {
throw new SkylarkImportSyntaxException(EXTERNAL_PKG_NOT_ALLOWED_MSG);
Expand All @@ -156,15 +151,12 @@ public static SkylarkImport create(
} else if (importString.startsWith(":")) {
// Relative label. We require that relative labels use an explicit ':' prefix.
String importTarget = importString.substring(1);
if (!importTarget.endsWith(".bzl")) {
throw new SkylarkImportSyntaxException(MUST_HAVE_BZL_EXT_MSG);
}
String maybeErrMsg = LabelValidator.validateTargetName(importTarget);
if (maybeErrMsg != null) {
// Null indicates successful target validation.
throw new SkylarkImportSyntaxException(INVALID_TARGET_PREFIX + maybeErrMsg);
}
return new RelativeLabelImport(importString, importTarget);
return new RelativeLabelImport(importString);
}

throw new SkylarkImportSyntaxException(INVALID_PATH_SYNTAX);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1124,7 +1124,7 @@ private void invalidImportTest(String importString, String expectedMsg) {

@Test
public void testRelativeImportPathInIsInvalid() throws Exception {
invalidImportTest("file", SkylarkImports.INVALID_PATH_SYNTAX);
invalidImportTest("file", SkylarkImports.MUST_HAVE_BZL_EXT_MSG);
}

@Test
Expand All @@ -1134,12 +1134,12 @@ public void testInvalidRelativePathBzlExtImplicit() throws Exception {

@Test
public void testInvalidRelativePathNoSubdirs() throws Exception {
invalidImportTest("path/to/file", SkylarkImports.INVALID_PATH_SYNTAX);
invalidImportTest("path/to/file.bzl", SkylarkImports.INVALID_PATH_SYNTAX);
}

@Test
public void testInvalidRelativePathInvalidFilename() throws Exception {
invalidImportTest("\tfile", SkylarkImports.INVALID_PATH_SYNTAX);
invalidImportTest("\tfile.bzl", SkylarkImports.INVALID_PATH_SYNTAX);
}

private void validAbsoluteImportLabelTest(String importString)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2215,14 +2215,14 @@ public void testListComprehensionsShadowGlobalVariable() throws Exception {
public void testLoadStatementWithAbsolutePath() throws Exception {
checkEvalErrorContains(
"First argument of 'load' must be a label and start with either '//', ':', or '@'",
"load('/tmp/foo', 'arg')");
"load('/tmp/foo.bzl', 'arg')");
}

@Test
public void testLoadStatementWithRelativePath() throws Exception {
checkEvalErrorContains(
"First argument of 'load' must be a label and start with either '//', ':', or '@'",
"load('foo', 'arg')");
"load('foo.bzl', 'arg')");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,13 +174,13 @@ public void testInvalidRelativePathBzlExtImplicit() throws Exception {

@Test
public void testInvalidRelativePathNoSubdirs() throws Exception {
invalidImportTest("path/to/file", SkylarkImports.INVALID_PATH_SYNTAX);
invalidImportTest("path/to/file.bzl", SkylarkImports.INVALID_PATH_SYNTAX);
}

@Test
public void testInvalidRelativePathInvalidFilename() throws Exception {
// tab character is invalid
invalidImportTest("\tfile", SkylarkImports.INVALID_PATH_SYNTAX);
invalidImportTest("\tfile.bzl", SkylarkImports.INVALID_PATH_SYNTAX);
}

@Test
Expand Down

0 comments on commit 4333692

Please sign in to comment.