treat rule toolchain reqs as query dependencies

With this change, as far as "query" is concerned: rules with toolchain-type requirements now have an implicit dependency on these toolchain types. (In Starlark rule definitions, these are specified using the `toolchains` parameter of the `rule()` function)

RELNOTES: toolchain type requirements of rules are now treated as implicit dependencies in `bazel query`. This includes those defined via the `toolchains` parameter of starlark `rule()` definitions.
PiperOrigin-RevId: 623197554
Change-Id: I3f2f546a6a19d5bfb94407b56c4d4bf48b3c0c77

site/en/extending/rules.md

@@ -171,7 +171,7 @@ dependency graph of targets.
 
 <a name="private-attributes"></a>
 
-### Private attributes and implicit dependencies
+### Private attributes and implicit dependencies {:#private_attributes_and_implicit_dependencies}
 
 A dependency attribute with a default value creates an *implicit dependency*. It
 is implicit because it's a part of the target graph that the user doesn't

site/en/query/language.md

@@ -163,15 +163,20 @@ cycles are treated are not specified and should not be relied upon.
 ### Implicit dependencies {:#implicit-dependencies}
 
 In addition to build dependencies that are defined explicitly in `BUILD` files,
-Bazel adds additional _implicit_ dependencies to rules. For example
-every Java rule implicitly depends on the JavaBuilder. Implicit dependencies
-are established using attributes that start with `$` and they
-cannot be overridden in `BUILD` files.
+Bazel adds additional _implicit_ dependencies to rules. Implicit dependencies
+may be defined by:
 
-Per default `bazel query` takes implicit dependencies into account
+- [Private attributes](/extending/rules#private_attributes_and_implicit_dependencies)
+- [Toolchain requirements](/extending/toolchains#writing-rules-toolchains)
+
+By default, `bazel query` takes implicit dependencies into account
 when computing the query result. This behavior can be changed with
-the `--[no]implicit_deps` option. Note that, as query does not consider
-configurations, potential toolchains are never considered.
+the `--[no]implicit_deps` option.
+
+Note that, as query does not consider configurations, potential toolchain
+**implementations** are not considered dependencies, only the
+required toolchain types. See
+[toolchain documentation](/extending/toolchains#writing-rules-toolchains).
 
 ### Soundness {:#soundness}
 

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

@@ -13,7 +13,9 @@
 // limitations under the License.
 package com.google.devtools.build.lib.packages;
 
+import com.google.devtools.build.lib.analysis.config.ToolchainTypeRequirement;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.util.FileTypeSet;
 import javax.annotation.Nullable;
 
 /**
@@ -23,6 +25,16 @@
  */
 public final class LabelVisitationUtils {
 
+  // An attribute which symbolizes the "toolchains" parameter of rule class definitions
+  // (user-specified via the `toolchains` parameter of the starlark rule() function). This is so
+  // that labels specified in this `toolchains` parameter may be treated the same as dependencies
+  // defined on an implicit rule attribute. This "fake" attribute uses an obscure placeholder name
+  // to prevent dependencies on this implementation detail.
+  private static final Attribute TOOLCHAIN_TYPE_ATTR_FOR_FILTERING =
+      Attribute.attr("_hidden_toolchain_types", BuildType.LABEL_LIST)
+          .allowedFileTypes(FileTypeSet.NO_FILE)
+          .build();
+
   private LabelVisitationUtils() {}
 
   /** Interface for processing the {@link Label} of dep, one at a time. */
@@ -59,6 +71,7 @@ public static void visitTarget(
       Rule rule = (Rule) target;
       visitRuleVisibility(rule, edgeFilter, labelProcessor);
       visitRule(rule, edgeFilter, labelProcessor);
+      visitRuleToolchains(rule, edgeFilter, labelProcessor);
       return;
     }
 
@@ -90,6 +103,16 @@ private static void visitRuleVisibility(
     }
   }
 
+  private static void visitRuleToolchains(
+      Rule rule, DependencyFilter edgeFilter, LabelProcessor labelProcessor) {
+    RuleClass ruleClass = rule.getRuleClassObject();
+    if (edgeFilter.test(rule, TOOLCHAIN_TYPE_ATTR_FOR_FILTERING)) {
+      for (ToolchainTypeRequirement t : ruleClass.getToolchainTypes()) {
+        labelProcessor.process(rule, TOOLCHAIN_TYPE_ATTR_FOR_FILTERING, t.toolchainType());
+      }
+    }
+  }
+
   private static void visitRule(
       Rule rule, DependencyFilter edgeFilter, LabelProcessor labelProcessor) {
     AggregatingAttributeMapper.of(rule)

src/main/java/com/google/devtools/build/lib/query2/BUILD

@@ -44,6 +44,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/analysis:config/options_diff",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/starlark_defined_config_transition",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/starlark_transition_cache",
+        "//src/main/java/com/google/devtools/build/lib/analysis:config/toolchain_type_requirement",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/composing_transition",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/configuration_transition",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/no_transition",

src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java

@@ -37,6 +37,7 @@
 import com.google.common.util.concurrent.MoreExecutors;
 import com.google.common.util.concurrent.SettableFuture;
 import com.google.common.util.concurrent.ThreadFactoryBuilder;
+import com.google.devtools.build.lib.analysis.config.ToolchainTypeRequirement;
 import com.google.devtools.build.lib.bugreport.BugReport;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
@@ -160,6 +161,7 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target>
   protected final PathPackageLocator pkgPath;
   protected final int queryEvaluationParallelismLevel;
   private final boolean visibilityDepsAreAllowed;
+  private final boolean toolchainTypeDepsAreAllowed;
 
   // The following fields are set in the #beforeEvaluateQuery method.
   protected MultisetSemaphore<PackageIdentifier> packageSemaphore;
@@ -237,6 +239,9 @@ protected SkyQueryEnvironment(
     // Since this attribute is of the NODEP type, that means we need a special implementation of
     // NO_NODEP_DEPS.
     this.visibilityDepsAreAllowed = !settings.contains(Setting.NO_NODEP_DEPS);
+    // The "toolchains" parameter of rule definition should be treated as an implicit dep despite
+    // not being represented by an attribute.
+    this.toolchainTypeDepsAreAllowed = !settings.contains(Setting.NO_IMPLICIT_DEPS);
   }
 
   @Override
@@ -524,6 +529,12 @@ private Set<Label> getAllowedDeps(Rule rule) {
       // need to explicitly handle that here.
       Iterables.addAll(allowedLabels, rule.getVisibilityDependencyLabels());
     }
+    if (toolchainTypeDepsAreAllowed) {
+      for (ToolchainTypeRequirement toolchainTypeRequirement :
+          rule.getRuleClassObject().getToolchainTypes()) {
+        allowedLabels.add(toolchainTypeRequirement.toolchainType());
+      }
+    }
     // We should add deps from aspects, otherwise they are going to be filtered out.
     allowedLabels.addAll(rule.getAspectLabelsSuperset(dependencyFilter));
     return allowedLabels;

src/test/java/com/google/devtools/build/lib/query2/testutil/AbstractQueryTest.java

@@ -2588,6 +2588,88 @@ public void testLabelSettingDefaultAppearsInDepsQuery() throws Exception {
         .isEqualTo("//donut:mysetting //donut:thief");
   }
 
+  @Test
+  public void testStarlarkRuleToolchainDeps() throws Exception {
+    appendToWorkspace("register_toolchains('//bar:all')");
+    writeFile(
+        "foo/BUILD",
+        """
+        load(":foo.bzl", "my_rule")
+
+        my_rule(name = "foo")
+        """);
+    writeFile(
+        "foo/foo.bzl",
+        """
+        def noop(ctx):
+          pass
+
+        my_rule = rule(
+          implementation = noop,
+          toolchains = ['//bar:bar_type'],
+        )
+        """);
+    writeFile(
+        "bar/BUILD",
+        """
+        load(":bar.bzl", "test_toolchain")
+
+        toolchain_type(name = "bar_type")
+        toolchain_type(name = "other_type")
+        test_toolchain(name = "bar_impl")
+        test_toolchain(name = "other_impl")
+
+        toolchain(
+            name = "bar_toolchain",
+            toolchain = ":bar_impl",
+            toolchain_type = ":bar_type",
+        )
+
+        toolchain(
+            name = "other_toolchain",
+            toolchain = ":other_impl",
+            toolchain_type = ":other_type",
+        )
+        """);
+    writeFile(
+        "bar/bar.bzl",
+        """
+        def _impl(ctx):
+            toolchain = platform_common.ToolchainInfo()
+            return [toolchain]
+
+        test_toolchain = rule(
+            implementation = _impl,
+        )
+        """);
+
+    // Use contains (instead of matching full string) because post-analysis query implementation
+    // will contain resolved toolchain, whereas pre-analysis query will not.
+    assertThat(evalToString("deps(//foo, 1)")).contains("//bar:bar_type");
+    // Test unbounded deps, too.
+    assertThat(evalToString("deps(//foo)")).contains("//bar:bar_type");
+
+    helper.setQuerySettings(Setting.NO_IMPLICIT_DEPS);
+
+    assertThat(evalToString("deps(//foo, 1)")).doesNotContain("//bar:bar_type");
+    assertThat(evalToString("deps(//foo)")).doesNotContain("//bar:bar_type");
+  }
+
+  @Test
+  public void testNativeRuleToolchainDeps() throws Exception {
+    writeFile(
+        "foo/BUILD",
+        """
+        cc_library(name = "cclib")
+        """);
+
+    assertThat(evalToString("deps(//foo:cclib)")).contains("//tools/cpp:toolchain_type");
+
+    helper.setQuerySettings(Setting.NO_IMPLICIT_DEPS);
+
+    assertThat(evalToString("deps(//foo:cclib)")).doesNotContain("//tools/cpp:toolchain_type");
+  }
+
   /**
    * A helper interface that allows creating a bunch of BUILD files and running queries against
    * them. We use this rather than the existing FoundationTestCase / BuildTestCase infrastructure to