Disallow field syntax to access a target's providers
The provider-key syntax should be used instead.
This is an incompatible change attached to new flag --incompatible_disable_target_provider_fields.
See https://github.com/bazelbuild/bazel/issues/9014 for details.
RELNOTES: New incompatible flag --incompatible_disable_target_provider_fields removes the ability (in Starlark) to access a target's providers via the field syntax (for example, `ctx.attr.dep.my_provider`). The provider-key syntax should be used instead (for example, `ctx.attr.dep[MyProvider]`). See https://github.com/bazelbuild/bazel/issues/9014 for details.
PiperOrigin-RevId: 260920114
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/AbstractConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/AbstractConfiguredTarget.java
index 3027a99..799b6d1 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/AbstractConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/AbstractConfiguredTarget.java
@@ -16,6 +16,7 @@
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.analysis.AnalysisUtils;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.DefaultInfo;
@@ -36,20 +37,23 @@
import com.google.devtools.build.lib.skyframe.BuildConfigurationValue;
import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
import com.google.devtools.build.lib.skylarkinterface.StarlarkContext;
-import com.google.devtools.build.lib.syntax.ClassObject;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.EvalUtils;
+import com.google.devtools.build.lib.syntax.Mutability;
import com.google.devtools.build.lib.syntax.Printer;
+import com.google.devtools.build.lib.syntax.SkylarkClassObject;
+import com.google.devtools.build.lib.syntax.SkylarkType;
+import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Consumer;
import javax.annotation.Nullable;
/**
- * An abstract implementation of ConfiguredTarget in which all properties are
- * assigned trivial default values.
+ * An abstract implementation of ConfiguredTarget in which all properties are assigned trivial
+ * default values.
*/
public abstract class AbstractConfiguredTarget
- implements ConfiguredTarget, VisibilityProvider, ClassObject {
+ implements ConfiguredTarget, VisibilityProvider, SkylarkClassObject {
private final Label label;
private final BuildConfigurationValue.Key configurationKey;
@@ -62,6 +66,18 @@
private static final String DATA_RUNFILES_FIELD = "data_runfiles";
private static final String DEFAULT_RUNFILES_FIELD = "default_runfiles";
+ // A set containing all field names which may be specially handled (and thus may not be
+ // attributed to normal user-specified providers).
+ private static final ImmutableSet<String> SPECIAL_FIELD_NAMES =
+ ImmutableSet.of(
+ LABEL_FIELD,
+ FILES_FIELD,
+ DEFAULT_RUNFILES_FIELD,
+ DATA_RUNFILES_FIELD,
+ FilesToRunProvider.SKYLARK_NAME,
+ OutputGroupInfo.SKYLARK_NAME,
+ RuleConfiguredTarget.ACTIONS_FIELD_NAME);
+
public AbstractConfiguredTarget(Label label, BuildConfigurationValue.Key configurationKey) {
this(label, configurationKey, NestedSetBuilder.emptySet(Order.STABLE_ORDER));
}
@@ -106,10 +122,33 @@
}
@Override
+ public Object getValue(Location loc, StarlarkSemantics semantics, String name)
+ throws EvalException {
+ if (semantics.incompatibleDisableTargetProviderFields()
+ && !SPECIAL_FIELD_NAMES.contains(name)) {
+ throw new EvalException(
+ loc,
+ "Accessing providers via the field syntax on structs is "
+ + "deprecated and will be removed soon. It may be temporarily re-enabled by setting "
+ + "--incompatible_disable_target_provider_fields=false. See "
+ + "https://github.com/bazelbuild/bazel/issues/9014 for details.");
+ }
+ return getValue(name);
+ }
+
+ @Override
public Object getValue(String name) {
switch (name) {
case LABEL_FIELD:
return getLabel();
+ case RuleConfiguredTarget.ACTIONS_FIELD_NAME:
+ // Depending on subclass, the 'actions' field will either be unsupported or of type
+ // java.util.List, which needs to be converted to SkylarkList before being returned.
+ Object result = get(name);
+ if (result != null) {
+ result = SkylarkType.convertToSkylark(result, (Mutability) null);
+ }
+ return result;
default:
return get(name);
}
@@ -207,9 +246,6 @@
*/
@Override
public final Object get(String providerKey) {
- if (OutputGroupInfo.SKYLARK_NAME.equals(providerKey)) {
- return get(OutputGroupInfo.SKYLARK_CONSTRUCTOR);
- }
switch (providerKey) {
case FILES_FIELD:
return getDefaultProvider().getFiles();
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkInfo.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkInfo.java
index 0837b1f..c18438f 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkInfo.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkInfo.java
@@ -32,6 +32,7 @@
import com.google.devtools.build.lib.syntax.EvalUtils;
import com.google.devtools.build.lib.syntax.SkylarkClassObject;
import com.google.devtools.build.lib.syntax.SkylarkType;
+import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
@@ -97,6 +98,13 @@
return getLayout() != null;
}
+ @Override
+ public Object getValue(Location loc, StarlarkSemantics starlarkSemantics, String name)
+ throws EvalException {
+ // By default, a SkylarkInfo's field values are not affected by the Starlark semantics.
+ return getValue(name);
+ }
+
/**
* Creates a schemaless (map-based) provider instance with the given provider type and field
* values.
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
index 65c65e4..f588a4e 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
@@ -233,6 +233,23 @@
public boolean incompatibleDepsetIsNotIterable;
@Option(
+ name = "incompatible_disable_target_provider_fields",
+ defaultValue = "false",
+ documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
+ effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
+ metadataTags = {
+ OptionMetadataTag.INCOMPATIBLE_CHANGE,
+ OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+ },
+ help =
+ "If set to true, disable the ability to access providers on 'target' objects via field "
+ + "syntax. Use provider-key syntax instead. For example, instead of using "
+ + "`ctx.attr.dep.my_info` to access `my_info` from inside a rule implementation "
+ + "function, use `ctx.attr.dep[MyInfo]`. See "
+ + "https://github.com/bazelbuild/bazel/issues/9014 for details.")
+ public boolean incompatibleDisableTargetProviderFields;
+
+ @Option(
name = "incompatible_disable_deprecated_attr_params",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
@@ -702,6 +719,7 @@
.incompatibleBzlDisallowLoadAfterStatement(incompatibleBzlDisallowLoadAfterStatement)
.incompatibleDepsetIsNotIterable(incompatibleDepsetIsNotIterable)
.incompatibleDepsetUnion(incompatibleDepsetUnion)
+ .incompatibleDisableTargetProviderFields(incompatibleDisableTargetProviderFields)
.incompatibleDisableThirdPartyLicenseChecking(
incompatibleDisableThirdPartyLicenseChecking)
.incompatibleDisableDeprecatedAttrParams(incompatibleDisableDeprecatedAttrParams)
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java
index 07c00f1..64701b5 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java
@@ -133,7 +133,7 @@
if (objValue instanceof SkylarkClassObject) {
try {
- return ((SkylarkClassObject) objValue).getValue(name);
+ return ((SkylarkClassObject) objValue).getValue(loc, env.getSemantics(), name);
} catch (IllegalArgumentException ex) {
throw new EvalException(loc, ex);
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkClassObject.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkClassObject.java
index 03c9f88..64e6fc6 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkClassObject.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkClassObject.java
@@ -13,6 +13,9 @@
// limitations under the License.
package com.google.devtools.build.lib.syntax;
+import com.google.devtools.build.lib.events.Location;
+import javax.annotation.Nullable;
+
/**
* A marker interface for a {@link ClassObject} whose {@link #getValue} always returns a
* Skylark-friendly value, with no defensive conversion required.
@@ -26,4 +29,17 @@
* Skylark-friendly values.
*/
public interface SkylarkClassObject extends ClassObject {
+
+ /**
+ * Returns the value of the field with the given name, or null if the field does not exist.
+ *
+ * @param loc the location in the current Starlark evaluation context
+ * @param starlarkSemantics the current starlark semantics, which may affect which fields are
+ * available, or the semantics of the available fields
+ * @param name the name of the field to return the value of
+ * @throws EvalException if a user-visible error occurs (other than non-existent field).
+ */
+ @Nullable
+ Object getValue(Location loc, StarlarkSemantics starlarkSemantics, String name)
+ throws EvalException;
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
index 7527c6c..c3e1143 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
@@ -148,6 +148,8 @@
public abstract boolean incompatibleDepsetUnion();
+ public abstract boolean incompatibleDisableTargetProviderFields();
+
public abstract boolean incompatibleDisableThirdPartyLicenseChecking();
public abstract boolean incompatibleDisableDeprecatedAttrParams();
@@ -263,6 +265,7 @@
.incompatibleBzlDisallowLoadAfterStatement(true)
.incompatibleDepsetIsNotIterable(true)
.incompatibleDepsetUnion(true)
+ .incompatibleDisableTargetProviderFields(false)
.incompatibleDisableThirdPartyLicenseChecking(true)
.incompatibleDisableDeprecatedAttrParams(true)
.incompatibleDisableObjcProviderResources(true)
@@ -331,6 +334,8 @@
public abstract Builder incompatibleDepsetUnion(boolean value);
+ public abstract Builder incompatibleDisableTargetProviderFields(boolean value);
+
public abstract Builder incompatibleDisableThirdPartyLicenseChecking(boolean value);
public abstract Builder incompatibleDisableDeprecatedAttrParams(boolean value);
diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
index e77ed2d..a154354 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
@@ -138,6 +138,7 @@
"--incompatible_depset_for_libraries_to_link_getter=" + rand.nextBoolean(),
"--incompatible_depset_is_not_iterable=" + rand.nextBoolean(),
"--incompatible_depset_union=" + rand.nextBoolean(),
+ "--incompatible_disable_target_provider_fields=" + rand.nextBoolean(),
"--incompatible_disable_deprecated_attr_params=" + rand.nextBoolean(),
"--incompatible_disable_objc_provider_resources=" + rand.nextBoolean(),
"--incompatible_disable_third_party_license_checking=" + rand.nextBoolean(),
@@ -196,6 +197,7 @@
.incompatibleDepsetForLibrariesToLinkGetter(rand.nextBoolean())
.incompatibleDepsetIsNotIterable(rand.nextBoolean())
.incompatibleDepsetUnion(rand.nextBoolean())
+ .incompatibleDisableTargetProviderFields(rand.nextBoolean())
.incompatibleDisableDeprecatedAttrParams(rand.nextBoolean())
.incompatibleDisableObjcProviderResources(rand.nextBoolean())
.incompatibleDisableThirdPartyLicenseChecking(rand.nextBoolean())
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 38f7a3a..11b0f10 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
@@ -2909,6 +2909,98 @@
}
@Test
+ public void testDisableTargetProviderFields() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_disable_target_provider_fields=true");
+ scratch.file(
+ "test/skylark/rule.bzl",
+ "MyProvider = provider()",
+ "",
+ "def _my_rule_impl(ctx): ",
+ " print(ctx.attr.dep.my_info)",
+ "def _dep_rule_impl(ctx): ",
+ " my_info = MyProvider(foo = 'bar')",
+ " return struct(my_info = my_info, providers = [my_info])",
+ "my_rule = rule(",
+ " implementation = _my_rule_impl,",
+ " attrs = {",
+ " 'dep': attr.label(),",
+ " })",
+ "dep_rule = rule(implementation = _dep_rule_impl)");
+ scratch.file(
+ "test/skylark/BUILD",
+ "load(':rule.bzl', 'my_rule', 'dep_rule')",
+ "",
+ "my_rule(name = 'r', dep = ':d')",
+ "dep_rule(name = 'd')");
+
+ reporter.removeHandler(failFastHandler);
+ getConfiguredTarget("//test/skylark:r");
+ assertContainsEvent(
+ "Accessing providers via the field syntax on structs is deprecated and will be removed "
+ + "soon. It may be temporarily re-enabled by setting "
+ + "--incompatible_disable_target_provider_fields=false. "
+ + "See https://github.com/bazelbuild/bazel/issues/9014 for details.");
+ }
+
+ // Verifies that non-provider fields on the 'target' type are still available even with
+ // --incompatible_disable_target_provider_fields.
+ @Test
+ public void testDisableTargetProviderFields_actionsField() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_disable_target_provider_fields=true");
+ scratch.file(
+ "test/skylark/rule.bzl",
+ "MyProvider = provider()",
+ "",
+ "def _my_rule_impl(ctx): ",
+ " print(ctx.attr.dep.actions)",
+ "def _dep_rule_impl(ctx): ",
+ " my_info = MyProvider(foo = 'bar')",
+ " return struct(my_info = my_info, providers = [my_info])",
+ "my_rule = rule(",
+ " implementation = _my_rule_impl,",
+ " attrs = {",
+ " 'dep': attr.label(),",
+ " })",
+ "dep_rule = rule(implementation = _dep_rule_impl)");
+ scratch.file(
+ "test/skylark/BUILD",
+ "load(':rule.bzl', 'my_rule', 'dep_rule')",
+ "",
+ "my_rule(name = 'r', dep = ':d')",
+ "dep_rule(name = 'd')");
+
+ assertThat(getConfiguredTarget("//test/skylark:r")).isNotNull();
+ }
+
+ @Test
+ public void testDisableTargetProviderFields_disabled() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_disable_target_provider_fields=false");
+ scratch.file(
+ "test/skylark/rule.bzl",
+ "MyProvider = provider()",
+ "",
+ "def _my_rule_impl(ctx): ",
+ " print(ctx.attr.dep.my_info)",
+ "def _dep_rule_impl(ctx): ",
+ " my_info = MyProvider(foo = 'bar')",
+ " return struct(my_info = my_info, providers = [my_info])",
+ "my_rule = rule(",
+ " implementation = _my_rule_impl,",
+ " attrs = {",
+ " 'dep': attr.label(),",
+ " })",
+ "dep_rule = rule(implementation = _dep_rule_impl)");
+ scratch.file(
+ "test/skylark/BUILD",
+ "load(':rule.bzl', 'my_rule', 'dep_rule')",
+ "",
+ "my_rule(name = 'r', dep = ':d')",
+ "dep_rule(name = 'd')");
+
+ assertThat(getConfiguredTarget("//test/skylark:r")).isNotNull();
+ }
+
+ @Test
public void testNoRuleOutputsParam() throws Exception {
setSkylarkSemanticsOptions("--incompatible_no_rule_outputs_param=true");
scratch.file(
