bazel syntax: enforce that ClassObject.getValue returns legal Starlark values
EvalUtils.getAttr now enforces that the value returned by
ClassObject.getValue is a legal value.
Previously, only the ternary SkylarkClassObject.getValue method
offered this postcondition, and it wasn't enforced.
This turned up BuildConfiguration.Fragment,
which was used as a value (e.g. in ctx.fragments.android)
but had forgotten to declare its relationship to StarlarkValue.
Also:
- remove obsolete 'instanceof Class' check.
- remove try/catch of IllegalArgumentException.
Instead do this check with narrower scope in the fragment code.
We will fix other cases as they appear.
PiperOrigin-RevId: 284644428
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index f167e8a..0810a03 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -475,18 +475,25 @@
}
@Nullable
- public Fragment getSkylarkFragment(String name, ConfigurationTransition transition) {
+ public Fragment getSkylarkFragment(String name, ConfigurationTransition transition)
+ throws EvalException {
Class<? extends Fragment> fragmentClass =
getConfiguration(transition).getSkylarkFragmentByName(name);
if (fragmentClass == null) {
return null;
}
- return getFragment(fragmentClass, name,
- String.format(
- " Please update the '%1$sfragments' argument of the rule definition "
- + "(for example: %1$sfragments = [\"%2$s\"])",
- (transition.isHostTransition()) ? "host_" : "", name),
- transition);
+ try {
+ return getFragment(
+ fragmentClass,
+ name,
+ String.format(
+ " Please update the '%1$sfragments' argument of the rule definition "
+ + "(for example: %1$sfragments = [\"%2$s\"])",
+ transition.isHostTransition() ? "host_" : "", name),
+ transition);
+ } catch (IllegalArgumentException ex) { // fishy
+ throw new EvalException(null, ex.getMessage());
+ }
}
public ImmutableCollection<String> getSkylarkFragmentNames(ConfigurationTransition transition) {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
index 9b07ff9..5fbd7bd 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -48,6 +48,7 @@
import com.google.devtools.build.lib.skylarkbuildapi.BuildConfigurationApi;
import com.google.devtools.build.lib.skylarkinterface.SkylarkInterfaceUtils;
import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
+import com.google.devtools.build.lib.syntax.StarlarkValue;
import com.google.devtools.build.lib.util.OS;
import com.google.devtools.build.lib.util.RegexFilter;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -110,8 +111,10 @@
* <p>All implementations must be immutable and communicate this as clearly as possible (e.g.
* declare {@link ImmutableList} signatures on their interfaces vs. {@link List}). This is because
* fragment instances may be shared across configurations.
+ *
+ * <p>Fragments are Starlark values, as returned by {@code ctx.fragments.android}, for example.
*/
- public abstract static class Fragment {
+ public abstract static class Fragment implements StarlarkValue {
/**
* Validates the options for this Fragment. Issues warnings for the use of deprecated options,
* and warnings or errors for any option settings that conflict.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/FragmentCollection.java b/src/main/java/com/google/devtools/build/lib/analysis/config/FragmentCollection.java
index d396bcc..11f2ab7 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/FragmentCollection.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/FragmentCollection.java
@@ -19,6 +19,7 @@
import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.skylarkbuildapi.FragmentCollectionApi;
+import com.google.devtools.build.lib.syntax.EvalException;
import javax.annotation.Nullable;
/** Represents a collection of configuration fragments in Skylark. */
@@ -35,7 +36,7 @@
@Override
@Nullable
- public Object getValue(String name) {
+ public Object getValue(String name) throws EvalException {
return ruleContext.getSkylarkFragment(name, transition);
}
