Clean up Rule's API
Drop unused methods, reduce visibility where possible, get rid of unnecessary
InterruptedException declarations.
PiperOrigin-RevId: 214971136
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java
index fff662c..4c65f02 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Package.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -1347,7 +1347,7 @@
}
}
- void addRule(Rule rule) throws NameConflictException, InterruptedException {
+ void addRule(Rule rule) throws NameConflictException {
checkForConflicts(rule);
addRuleUnchecked(rule);
}
@@ -1488,7 +1488,7 @@
* <li>The generating rule of every output file in the package must itself be in the package.
* </ul>
*/
- private void checkForConflicts(Rule rule) throws NameConflictException, InterruptedException {
+ private void checkForConflicts(Rule rule) throws NameConflictException {
String name = rule.getName();
Target existing = targets.get(name);
if (existing != null) {
@@ -1540,7 +1540,7 @@
* @throws NameConflictException if a conflict is found.
*/
private void checkForInputOutputConflicts(Rule rule, Set<String> outputFiles)
- throws NameConflictException, InterruptedException {
+ throws NameConflictException {
PackageIdentifier packageIdentifier = rule.getLabel().getPackageIdentifier();
for (Label inputLabel : rule.getLabels()) {
if (packageIdentifier.equals(inputLabel.getPackageIdentifier())
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Rule.java b/src/main/java/com/google/devtools/build/lib/packages/Rule.java
index 94fbaaf..63eca18 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Rule.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Rule.java
@@ -20,7 +20,6 @@
import com.google.common.collect.HashMultimap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
-import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.LinkedHashMultimap;
import com.google.common.collect.ListMultimap;
@@ -166,13 +165,6 @@
}
/**
- * Returns the build features that apply to this rule.
- */
- public ImmutableSet<String> getFeatures() {
- return pkg.getFeatures();
- }
-
- /**
* Returns true iff the outputs of this rule should be created beneath the
* bin directory, false if beneath genfiles. For most rule
* classes, this is a constant, but for genrule, it is a property of the
@@ -203,26 +195,6 @@
}
/**
- * Returns true if this rule has any attributes that are configurable.
- *
- * <p>Note this is *not* the same as having attribute *types* that are configurable. For example,
- * "deps" is configurable, in that one can write a rule that sets "deps" to a configuration
- * dictionary. But if *this* rule's instance of "deps" doesn't do that, its instance
- * of "deps" is not considered configurable.
- *
- * <p>In other words, this method signals which rules might have their attribute values
- * influenced by the configuration.
- */
- public boolean hasConfigurableAttributes() {
- for (Attribute attribute : getAttributes()) {
- if (AbstractAttributeMapper.isConfigurable(this, attribute.getName(), attribute.getType())) {
- return true;
- }
- }
- return false;
- }
-
- /**
* Returns true if the given attribute is configurable.
*/
public boolean isConfigurableAttribute(String attributeName) {
@@ -327,7 +299,7 @@
}
/**
- * {@see #isAttributeValueExplicitlySpecified(String)}
+ * See {@link #isAttributeValueExplicitlySpecified(String)}
*/
@Override
public boolean isAttributeValueExplicitlySpecified(Attribute attribute) {
@@ -404,7 +376,7 @@
}
/** Returns a new List instance containing all direct dependencies (all types). */
- public Collection<Label> getLabels() throws InterruptedException {
+ public Collection<Label> getLabels() {
final List<Label> labels = Lists.newArrayList();
AggregatingAttributeMapper.of(this)
.visitLabels()
@@ -423,8 +395,7 @@
* label. The label will be contained in the result iff (the predicate returned {@code true}
* and the labels are not outputs)
*/
- public Collection<Label> getLabels(BinaryPredicate<? super Rule, Attribute> predicate)
- throws InterruptedException {
+ public Collection<Label> getLabels(BinaryPredicate<? super Rule, Attribute> predicate) {
return ImmutableSortedSet.copyOf(getTransitions(predicate).values());
}
@@ -438,7 +409,7 @@
* and the labels are not outputs)
*/
public Multimap<Attribute, Label> getTransitions(
- final BinaryPredicate<? super Rule, Attribute> predicate) throws InterruptedException {
+ final BinaryPredicate<? super Rule, Attribute> predicate) {
final Multimap<Attribute, Label> transitions = HashMultimap.create();
// TODO(bazel-team): move this to AttributeMap, too. Just like visitLabels, which labels should
// be visited may depend on the calling context. We shouldn't implicitly decide this for
@@ -480,7 +451,7 @@
}
}
- void populateOutputFilesInternal(
+ private void populateOutputFilesInternal(
EventHandler eventHandler, Package.Builder pkgBuilder, boolean performChecks)
throws LabelSyntaxException, InterruptedException {
Preconditions.checkState(outputFiles == null);
@@ -613,7 +584,7 @@
this.containsErrors = true;
}
- void reportWarning(String message, EventHandler eventHandler) {
+ private void reportWarning(String message, EventHandler eventHandler) {
eventHandler.handle(Event.warn(location, message));
}
@@ -701,7 +672,7 @@
// null-valued, with a packageFragment that is null...). The bug that prompted
// the introduction of this code is #2210848 (NullPointerException in
// Package.checkForConflicts() ).
- void checkForNullLabels() throws InterruptedException {
+ void checkForNullLabels() {
AggregatingAttributeMapper.of(this)
.visitLabels()
.forEach(depEdge -> checkForNullLabel(depEdge.getLabel(), depEdge.getAttribute()));
diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java
index 566f560..e2ec046 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java
@@ -101,7 +101,7 @@
}
private static void overwriteRule(Package.Builder pkg, Rule rule)
- throws Package.NameConflictException, InterruptedException {
+ throws Package.NameConflictException {
Preconditions.checkArgument(rule.getOutputFiles().isEmpty());
Target old = pkg.getTarget(rule.getName());
if (old != null) {
diff --git a/src/main/java/com/google/devtools/build/lib/query2/output/ProtoOutputFormatter.java b/src/main/java/com/google/devtools/build/lib/query2/output/ProtoOutputFormatter.java
index 4eb877b..e8d467f 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/output/ProtoOutputFormatter.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/output/ProtoOutputFormatter.java
@@ -239,7 +239,7 @@
.distinct()
.forEach(output -> rulePb.addRuleOutput(output.getLabel().toString()));
}
- for (String feature : rule.getFeatures()) {
+ for (String feature : rule.getPackage().getFeatures()) {
rulePb.addDefaultSetting(feature);
}
targetPb.setType(RULE);
diff --git a/src/main/java/com/google/devtools/build/lib/query2/output/XmlOutputFormatter.java b/src/main/java/com/google/devtools/build/lib/query2/output/XmlOutputFormatter.java
index c8f0d50..1b61882 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/output/XmlOutputFormatter.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/output/XmlOutputFormatter.java
@@ -178,7 +178,7 @@
outputElem.setAttribute("name", outputFile.getLabel().toString());
elem.appendChild(outputElem);
}
- for (String feature : rule.getFeatures()) {
+ for (String feature : rule.getPackage().getFeatures()) {
Element outputElem = doc.createElement("rule-default-setting");
outputElem.setAttribute("name", feature);
elem.appendChild(outputElem);
diff --git a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
index c67048e..44b0f58 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
@@ -1019,8 +1019,6 @@
"sh_library(name='after')");
Package pkg = packages.eval("a", file);
- assertThat(pkg.getRule("before").getFeatures()).containsExactly("b", "c");
- assertThat(pkg.getRule("after").getFeatures()).containsExactly("b", "c");
assertThat(pkg.getFeatures()).containsExactly("b", "c");
}
diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/PackageCacheTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/PackageCacheTest.java
index dc88db6..e845748 100644
--- a/src/test/java/com/google/devtools/build/lib/pkgcache/PackageCacheTest.java
+++ b/src/test/java/com/google/devtools/build/lib/pkgcache/PackageCacheTest.java
@@ -35,7 +35,6 @@
import com.google.devtools.build.lib.packages.NoSuchTargetException;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.PackageFactory;
-import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.SkylarkSemanticsOptions;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction;
@@ -570,8 +569,7 @@
"peach/BUILD",
"package(features = ['crosstool_default_false'])",
"cc_library(name = 'cc', srcs = ['cc.cc'])");
- Rule cc = (Rule) getTarget("//peach:cc");
- assertThat(cc.getFeatures()).hasSize(1);
+ assertThat(getPackage("peach").getFeatures()).hasSize(1);
}
@Test
