Simplify SkylarkImports
- Merged SkylarkImport and SkylarkImportImpl
- Removed method asPathFragment
RELNOTES: None.
PiperOrigin-RevId: 243335608
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImport.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImport.java
index 40cf1a8..53c5372 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImport.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImport.java
@@ -14,29 +14,24 @@
package com.google.devtools.build.lib.syntax;
import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.vfs.PathFragment;
import java.io.Serializable;
+import java.util.Objects;
import javax.annotation.Nullable;
/**
- * Encapsulates the four syntactic variants of Skylark imports: Absolute paths, relative paths,
- * absolute labels, and relative labels.
+ * Encapsulates the two syntactic variants of Starlark imports: absolute labels and relative labels.
*/
-public interface SkylarkImport extends Serializable {
- /**
- * Returns the string originally used to specify the import (may represent a label or a path).
- */
- String getImportString();
+public abstract class SkylarkImport implements Serializable {
+ private final String importString;
- /**
- * Returns the import in the form of a path fragment for use by tools. Label-based imports are
- * converted to paths as follows: Imports using absolute labels or paths yield an absolute path
- * (whose root corresponds to the containing depot). Imports using relative labels yield a
- * package-relate path, and imports using relative paths yield a directory (of the importing-file)
- * relative path. All paths reference file names ending in '.bzl'. If there is an external
- * repository prefix, it is ignored.
- */
- PathFragment asPathFragment();
+ protected SkylarkImport(String importString) {
+ this.importString = importString;
+ }
+
+ /** Returns the string originally used to specify the import (represents a label). */
+ public String getImportString() {
+ return importString;
+ }
/**
* Given a {@link Label} representing the file that contains this import, returns a {@link Label}
@@ -44,5 +39,24 @@
*
* @throws IllegalStateException if this import takes the form of an absolute path.
*/
- Label getLabel(@Nullable Label containingFileLabel);
+ public abstract Label getLabel(@Nullable Label containingFileLabel);
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(getClass(), importString);
+ }
+
+ @Override
+ public boolean equals(Object that) {
+ if (this == that) {
+ return true;
+ }
+
+ if (!(that instanceof SkylarkImport)) {
+ return false;
+ }
+
+ return (that instanceof SkylarkImport)
+ && Objects.equals(importString, ((SkylarkImport) that).importString);
+ }
}
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 fda4e0c..5cc4665 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
@@ -23,11 +23,11 @@
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
-import com.google.devtools.build.lib.vfs.PathFragment;
-import java.util.Objects;
/**
* Factory class for creating appropriate instances of {@link SkylarkImports}.
+ *
+ * <p>TODO(laurentlb): Merge class with SkylarkImport.
*/
public class SkylarkImports {
@@ -35,49 +35,9 @@
throw new IllegalStateException("This class should not be instantiated");
}
- // Default implementation class for SkylarkImport.
- @VisibleForSerialization
- abstract static class SkylarkImportImpl implements SkylarkImport {
- private final String importString;
-
- protected SkylarkImportImpl(String importString) {
- this.importString = importString;
- }
-
- @Override
- public String getImportString() {
- return importString;
- }
-
- @Override
- public abstract PathFragment asPathFragment();
-
- @Override
- public abstract Label getLabel(Label containingFileLabel);
-
- @Override
- public int hashCode() {
- return Objects.hash(getClass(), importString);
- }
-
- @Override
- public boolean equals(Object that) {
- if (this == that) {
- return true;
- }
-
- if (!(that instanceof SkylarkImportImpl)) {
- return false;
- }
-
- return Objects.equals(getClass(), that.getClass())
- && Objects.equals(importString, ((SkylarkImportImpl) that).importString);
- }
- }
-
@VisibleForSerialization
@AutoCodec
- static final class AbsoluteLabelImport extends SkylarkImportImpl {
+ static final class AbsoluteLabelImport extends SkylarkImport {
private final Label importLabel;
@VisibleForSerialization
@@ -87,11 +47,6 @@
}
@Override
- public PathFragment asPathFragment() {
- return PathFragment.create("/").getRelative(importLabel.toPathFragment());
- }
-
- @Override
public Label getLabel(Label containingFileLabel) {
// When the import label contains no explicit repository identifier, we resolve it relative
// to the repo of the containing file.
@@ -102,7 +57,7 @@
@VisibleForSerialization
@AutoCodec
- static final class RelativeLabelImport extends SkylarkImportImpl {
+ static final class RelativeLabelImport extends SkylarkImport {
private final String importTarget;
@VisibleForSerialization
@@ -112,11 +67,6 @@
}
@Override
- public PathFragment asPathFragment() {
- return PathFragment.create(importTarget);
- }
-
- @Override
public Label getLabel(Label containingFileLabel) {
// Unlike a relative path import, the import target is relative to the containing package,
// not the containing directory within the package.
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 f22f7bb..6d08c5e 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
@@ -21,7 +21,6 @@
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.skyframe.serialization.testutils.SerializationTester;
import com.google.devtools.build.lib.syntax.SkylarkImports.SkylarkImportSyntaxException;
-import com.google.devtools.build.lib.vfs.PathFragment;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
@@ -36,8 +35,8 @@
@Rule
public ExpectedException thrown = ExpectedException.none();
- private void validAbsoluteLabelTest(String labelString, String expectedLabelString,
- String expectedPathString) throws Exception {
+ private void validAbsoluteLabelTest(String labelString, String expectedLabelString)
+ throws Exception {
SkylarkImport importForLabel = SkylarkImports.create(labelString);
assertThat(importForLabel.getImportString()).named("getImportString()").isEqualTo(labelString);
@@ -45,30 +44,23 @@
Label irrelevantContainingFile = Label.parseAbsoluteUnchecked("//another/path:BUILD");
assertThat(importForLabel.getLabel(irrelevantContainingFile)).named("getLabel()")
.isEqualTo(Label.parseAbsoluteUnchecked(expectedLabelString));
-
- assertThat(importForLabel.asPathFragment()).named("asPathFragment()")
- .isEqualTo(PathFragment.create(expectedPathString));
}
@Test
public void testValidAbsoluteLabel() throws Exception {
- validAbsoluteLabelTest("//some/skylark:file.bzl",
- /*expected label*/ "//some/skylark:file.bzl",
- /*expected path*/ "/some/skylark/file.bzl");
+ validAbsoluteLabelTest("//some/skylark:file.bzl", /*expected label*/ "//some/skylark:file.bzl");
}
@Test
public void testValidAbsoluteLabelWithRepo() throws Exception {
- validAbsoluteLabelTest("@my_repo//some/skylark:file.bzl",
- /*expected label*/ "@my_repo//some/skylark:file.bzl",
- /*expected path*/ "/some/skylark/file.bzl");
+ validAbsoluteLabelTest(
+ "@my_repo//some/skylark:file.bzl", /*expected label*/ "@my_repo//some/skylark:file.bzl");
}
@Test
public void testValidAbsoluteLabelWithRepoRemapped() throws Exception {
String labelString = "@orig_repo//some/skylark:file.bzl";
String remappedLabelString = "@new_repo//some/skylark:file.bzl";
- String expectedPath = "/some/skylark/file.bzl";
ImmutableMap<RepositoryName, RepositoryName> repositoryMapping =
ImmutableMap.of(RepositoryName.create("@orig_repo"), RepositoryName.create("@new_repo"));
SkylarkImport importForLabel = SkylarkImports.create(labelString, repositoryMapping);
@@ -79,15 +71,11 @@
assertThat(importForLabel.getLabel(irrelevantContainingFile))
.named("getLabel()")
.isEqualTo(Label.parseAbsoluteUnchecked(remappedLabelString));
-
- assertThat(importForLabel.asPathFragment())
- .named("asPathFragment()")
- .isEqualTo(PathFragment.create(expectedPath));
}
- private void validRelativeLabelTest(String labelString,
- String containingLabelString, String expectedLabelString, String expectedPathString)
- throws Exception {
+ private void validRelativeLabelTest(
+ String labelString, String containingLabelString, String expectedLabelString)
+ throws Exception {
SkylarkImport importForLabel = SkylarkImports.create(labelString);
assertThat(importForLabel.getImportString()).named("getImportString()").isEqualTo(labelString);
@@ -97,41 +85,38 @@
assertThat(importForLabel.getLabel(containingLabel))
.named("getLabel()")
.isEqualTo(Label.parseAbsolute(expectedLabelString, ImmutableMap.of()));
-
- assertThat(importForLabel.asPathFragment()).named("asPathFragment()")
- .isEqualTo(PathFragment.create(expectedPathString));
}
@Test
public void testValidRelativeSimpleLabelInPackageDir() throws Exception {
- validRelativeLabelTest(":file.bzl",
+ validRelativeLabelTest(
+ ":file.bzl",
/*containing*/ "//some/skylark:BUILD",
- /*expected label*/ "//some/skylark:file.bzl",
- /*expected path*/ "file.bzl");
+ /*expected label*/ "//some/skylark:file.bzl");
}
@Test
public void testValidRelativeSimpleLabelInPackageSubdir() throws Exception {
- validRelativeLabelTest(":file.bzl",
+ validRelativeLabelTest(
+ ":file.bzl",
/*containing*/ "//some/path/to:skylark/parent.bzl",
- /*expected label*/ "//some/path/to:file.bzl",
- /*expected path*/ "file.bzl");
+ /*expected label*/ "//some/path/to:file.bzl");
}
@Test
public void testValidRelativeComplexLabelInPackageDir() throws Exception {
- validRelativeLabelTest(":subdir/containing/file.bzl",
+ validRelativeLabelTest(
+ ":subdir/containing/file.bzl",
/*containing*/ "//some/skylark:BUILD",
- /*expected label*/ "//some/skylark:subdir/containing/file.bzl",
- /*expected path*/ "subdir/containing/file.bzl");
+ /*expected label*/ "//some/skylark:subdir/containing/file.bzl");
}
@Test
public void testValidRelativeComplexLabelInPackageSubdir() throws Exception {
- validRelativeLabelTest(":subdir/containing/file.bzl",
+ validRelativeLabelTest(
+ ":subdir/containing/file.bzl",
/*containing*/ "//some/path/to:skylark/parent.bzl",
- /*expected label*/ "//some/path/to:subdir/containing/file.bzl",
- /*expected path*/ "subdir/containing/file.bzl");
+ /*expected label*/ "//some/path/to:subdir/containing/file.bzl");
}
private void invalidImportTest(String importString, String expectedMsgPrefix) throws Exception {
