Minor code simplification in SkylarkImports
RELNOTES: None.
PiperOrigin-RevId: 243616522
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImports.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImports.java
index 5cc4665..27aa931 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImports.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImports.java
@@ -52,18 +52,14 @@
// 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
@@ -73,14 +69,13 @@
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);
}
}
-
}
/**
@@ -136,6 +131,10 @@
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;
@@ -144,10 +143,6 @@
} 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);
@@ -156,15 +151,12 @@
} 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);
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
index fdb855e..50296222 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
@@ -1124,7 +1124,7 @@
@Test
public void testRelativeImportPathInIsInvalid() throws Exception {
- invalidImportTest("file", SkylarkImports.INVALID_PATH_SYNTAX);
+ invalidImportTest("file", SkylarkImports.MUST_HAVE_BZL_EXT_MSG);
}
@Test
@@ -1134,12 +1134,12 @@
@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)
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
index 2ef84a6..4f2a446 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
@@ -2215,14 +2215,14 @@
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
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkImportsTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkImportsTest.java
index 6d08c5e..124899f 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkImportsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkImportsTest.java
@@ -174,13 +174,13 @@
@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
