Make explicit the contract of ConfiguredTarget builders returning null when there are rule errors.
RELNOTES: None.
PiperOrigin-RevId: 206652580
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
index 786dba9..ddc1ad6 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
@@ -49,6 +49,7 @@
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.TreeMap;
+import javax.annotation.Nullable;
/**
* Builder class for analyzed rule instances.
@@ -79,7 +80,11 @@
add(VisibilityProvider.class, new VisibilityProviderImpl(ruleContext.getVisibility()));
}
- /** Constructs the RuleConfiguredTarget instance based on the values set for this Builder. */
+ /**
+ * Constructs the RuleConfiguredTarget instance based on the values set for this Builder.
+ * Returns null if there were rule errors reported.
+ */
+ @Nullable
public ConfiguredTarget build() throws ActionConflictException {
if (ruleContext.getConfiguration().enforceConstraints()) {
checkConstraints();
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java
index 1f2a184..3b150c2 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java
@@ -74,7 +74,11 @@
private static final ImmutableSet<String> DEFAULT_PROVIDER_FIELDS =
ImmutableSet.of("files", "runfiles", "data_runfiles", "default_runfiles", "executable");
- /** Create a Rule Configured Target from the ruleContext and the ruleImplementation. */
+ /**
+ * Create a Rule Configured Target from the ruleContext and the ruleImplementation.
+ * Returns null if there were errors during target creation.
+ */
+ @Nullable
public static ConfiguredTarget buildRule(
RuleContext ruleContext,
AdvertisedProviderSet advertisedProviders,
@@ -115,8 +119,12 @@
return null;
}
ConfiguredTarget configuredTarget = createTarget(skylarkRuleContext, target);
- SkylarkProviderValidationUtil.validateArtifacts(ruleContext);
- checkDeclaredProviders(configuredTarget, advertisedProviders, location);
+ if (configuredTarget != null) {
+ // If there was error creating the ConfiguredTarget, no further validation is needed.
+ // Null will be returned and the errors thus reported.
+ SkylarkProviderValidationUtil.validateArtifacts(ruleContext);
+ checkDeclaredProviders(configuredTarget, advertisedProviders, location);
+ }
return configuredTarget;
} catch (EvalException e) {
addRuleToStackTrace(e, ruleContext.getRule(), ruleImplementation);
@@ -172,6 +180,7 @@
return ex.getMessage();
}
+ @Nullable
private static ConfiguredTarget createTarget(SkylarkRuleContext context, Object target)
throws EvalException, RuleErrorException, ActionConflictException {
RuleConfiguredTargetBuilder builder = new RuleConfiguredTargetBuilder(
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
index 75d7929..1834735 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
@@ -1731,6 +1731,39 @@
assertContainsEvent("foo/bar/baz");
}
+ @Test
+ public void testEnvironmentConstraintsFromSkylarkRule() throws Exception {
+ scratch.file(
+ "buildenv/foo/BUILD",
+ "environment_group(name = 'env_group',",
+ " defaults = [':default'],",
+ " environments = ['default', 'other'])",
+ "environment(name = 'default')",
+ "environment(name = 'other')");
+ // The example skylark rule explicitly provides the MyProvider provider as a regression test
+ // for a bug where a skylark rule with unsatisfied constraints but explicit providers would
+ // result in Bazel throwing a null pointer exception.
+ scratch.file(
+ "test/skylark/extension.bzl",
+ "MyProvider = provider()",
+ "",
+ "def _impl(ctx):",
+ " return struct(providers = [MyProvider(foo = 'bar')])",
+ "my_rule = rule(implementation = _impl,",
+ " attrs = { 'deps' : attr.label_list() },",
+ " provides = [MyProvider])");
+ scratch.file(
+ "test/skylark/BUILD",
+ "load('//test/skylark:extension.bzl', 'my_rule')",
+ "java_library(name = 'dep', srcs = ['a.java'], restricted_to = ['//buildenv/foo:other'])",
+ "my_rule(name='my', deps = [':dep'])");
+
+ reporter.removeHandler(failFastHandler);
+ assertThat(getConfiguredTarget("//test/skylark:my")).isNull();
+ assertContainsEvent(
+ "//test/skylark:dep doesn't support expected environment: //buildenv/foo:default");
+ }
+
/**
* Skylark integration test that forces inlining.
*/
