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 #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 #9014 for details.
PiperOrigin-RevId: 260920114

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 @@ public abstract class AbstractConfiguredTarget
   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));
   }
@@ -105,11 +121,34 @@ public <P extends TransitiveInfoProvider> P getProvider(Class<P> provider) {
     }
   }
 
+  @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 @@ public String getRuleClassString() {
    */
   @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();

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 @@ public boolean isCompact() {
     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.

src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java

@@ -232,6 +232,23 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl
               + "convert to a list.")
   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",
@@ -702,6 +719,7 @@ public StarlarkSemantics toSkylarkSemantics() {
             .incompatibleBzlDisallowLoadAfterStatement(incompatibleBzlDisallowLoadAfterStatement)
             .incompatibleDepsetIsNotIterable(incompatibleDepsetIsNotIterable)
             .incompatibleDepsetUnion(incompatibleDepsetUnion)
+            .incompatibleDisableTargetProviderFields(incompatibleDisableTargetProviderFields)
             .incompatibleDisableThirdPartyLicenseChecking(
                 incompatibleDisableThirdPartyLicenseChecking)
             .incompatibleDisableDeprecatedAttrParams(incompatibleDisableDeprecatedAttrParams)

src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java

@@ -133,7 +133,7 @@ public static Object eval(Object objValue, String name,
 
     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);
       }

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;
 }

src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java

@@ -148,6 +148,8 @@ public boolean flagValue(FlagIdentifier flagIdentifier) {
 
   public abstract boolean incompatibleDepsetUnion();
 
+  public abstract boolean incompatibleDisableTargetProviderFields();
+
   public abstract boolean incompatibleDisableThirdPartyLicenseChecking();
 
   public abstract boolean incompatibleDisableDeprecatedAttrParams();
@@ -263,6 +265,7 @@ public static Builder builderWithDefaults() {
           .incompatibleBzlDisallowLoadAfterStatement(true)
           .incompatibleDepsetIsNotIterable(true)
           .incompatibleDepsetUnion(true)
+          .incompatibleDisableTargetProviderFields(false)
           .incompatibleDisableThirdPartyLicenseChecking(true)
           .incompatibleDisableDeprecatedAttrParams(true)
           .incompatibleDisableObjcProviderResources(true)
@@ -331,6 +334,8 @@ public abstract static class Builder {
 
     public abstract Builder incompatibleDepsetUnion(boolean value);
 
+    public abstract Builder incompatibleDisableTargetProviderFields(boolean value);
+
     public abstract Builder incompatibleDisableThirdPartyLicenseChecking(boolean value);
 
     public abstract Builder incompatibleDisableDeprecatedAttrParams(boolean value);

src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java

@@ -138,6 +138,7 @@ private static StarlarkSemanticsOptions buildRandomOptions(Random rand) throws E
         "--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 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) {
         .incompatibleDepsetForLibrariesToLinkGetter(rand.nextBoolean())
         .incompatibleDepsetIsNotIterable(rand.nextBoolean())
         .incompatibleDepsetUnion(rand.nextBoolean())
+        .incompatibleDisableTargetProviderFields(rand.nextBoolean())
         .incompatibleDisableDeprecatedAttrParams(rand.nextBoolean())
         .incompatibleDisableObjcProviderResources(rand.nextBoolean())
         .incompatibleDisableThirdPartyLicenseChecking(rand.nextBoolean())

src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java

@@ -2908,6 +2908,98 @@ public void testDisallowStructProviderSyntax() throws Exception {
             + "--incompatible_disallow_struct_provider_syntax=false");
   }
 
+  @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");