For aspect evaluation, only check visibility of aspect's implicit dependencies

mai93 · copybara-github · commit bc1df9180748 · 2023-10-16T10:16:41.000-07:00
Move this check to also cover native aspects evaluation, otherwise it evaluates the visibility of the tools of the underlying target rule against the target itself.

PiperOrigin-RevId: 573850253
Change-Id: I69beb6542128338fe859d2978d29018731fc1258
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/CommonPrerequisiteValidator.java b/src/main/java/com/google/devtools/build/lib/analysis/CommonPrerequisiteValidator.java
@@ -99,6 +99,17 @@ private void validateDirectPrerequisiteVisibility(
       return;
     }
 
+    // Only verify visibility of implicit dependencies of the current aspect.
+    // Dependencies of other aspects as well as the rule itself are checked when they are
+    // evaluated.
+    Aspect mainAspect = context.getMainAspect();
+    if (mainAspect != null) {
+      if (!attribute.isImplicit()
+          || !mainAspect.getDefinition().getAttributes().containsKey(attrName)) {
+        return;
+      }
+    }
+
     // Determine if we should use the new visibility rules for tools.
     boolean toolCheckAtDefinition =
         context
@@ -118,16 +129,10 @@ private void validateDirectPrerequisiteVisibility(
       // from the location of the definition that declares the attribute. Only perform this check
       // for the current aspect.
       Label implicitDefinition = null;
-      Aspect mainAspect = context.getMainAspect();
       if (mainAspect != null) {
-        // Only verify visibility of implicit dependencies of the current aspect. Implicit
-        // dependencies of other aspects as well as the rule itself are checked when they are
-        // evaluated.
-        if (mainAspect.getDefinition().getAttributes().containsKey(attrName)) {
-          StarlarkAspectClass aspectClass = (StarlarkAspectClass) mainAspect.getAspectClass();
-          // Never null since we already checked that the aspect is Starlark-defined.
-          implicitDefinition = checkNotNull(aspectClass.getExtensionLabel());
-        }
+        StarlarkAspectClass aspectClass = (StarlarkAspectClass) mainAspect.getAspectClass();
+        // Never null since we already checked that the aspect is Starlark-defined.
+        implicitDefinition = checkNotNull(aspectClass.getExtensionLabel());
       } else {
         // Never null since we already checked that the rule is a Starlark rule.
         implicitDefinition =
