Unify the handling of visibility dependencies, the generating rule de…

…pendency and dependencies through regular attributes.

Two pieces are missing:
1. Toolchain dependencies need to be special-cased somehow because we don't depend on the packages that contain them. Stay tuned!
2. The "is the visibility dependency a package group" is removed from the processing of the map. This could conceivably be done in ConfiguredTargetFunction, but that's a bit heavier refactoring and is better done by not special-casing package groups anymore anyway.

RELNOTES: None.
PiperOrigin-RevId: 232461481

src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java

@@ -54,6 +54,7 @@
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 
@@ -75,7 +76,8 @@ private interface DependencyKind {
     /**
      * The attribute through which a dependency arises.
      *
-     * <p>Should only be called for dependency kinds representing an attribute.
+     * <p>Returns {@code null} for visibility, the dependency pointing from an output file to its
+     * generating rule and toolchain dependencies.
      */
     @Nullable
     Attribute getAttribute();
@@ -95,7 +97,7 @@ private NonAttributeDependencyKind() {}
 
     @Override
     public Attribute getAttribute() {
-      throw new IllegalStateException();
+      return null;
     }
 
     @Nullable
@@ -337,6 +339,29 @@ public final OrderedSetMultimap<Attribute, Dependency> dependentNodeMap(
           PartiallyResolvedDependency.of(toLabel, attributeTransition, propagatingAspects.build()));
     }
 
+    Set<PartiallyResolvedDependency> illegalVisibilityDeps = new LinkedHashSet<>();
+    for (PartiallyResolvedDependency dep : partiallyResolvedDeps.get(VISIBILITY_DEPENDENCY)) {
+      Target toTarget = targetMap.get(dep.getLabel());
+      if (toTarget == null) {
+        // Dependency pointing to non-existent target. This error was reported above, so we can just
+        // ignore this dependency.
+      }
+
+      if (!(toTarget instanceof PackageGroup)) {
+        // Note that this error could also be caught in
+        // AbstractConfiguredTarget.convertVisibility(), but we have an
+        // opportunity here to avoid dependency cycles that result from
+        // the visibility attribute of a rule referring to a rule that
+        // depends on it (instead of its package)
+        invalidPackageGroupReferenceHook(node, dep.getLabel());
+        illegalVisibilityDeps.add(dep);
+      }
+    }
+
+    for (PartiallyResolvedDependency illegalVisibilityDep : illegalVisibilityDeps) {
+      partiallyResolvedDeps.remove(VISIBILITY_DEPENDENCY, illegalVisibilityDep);
+    }
+
     OrderedSetMultimap<Attribute, Dependency> outgoingEdges = OrderedSetMultimap.create();
 
     for (Map.Entry<DependencyKind, PartiallyResolvedDependency> entry :
@@ -361,29 +386,6 @@ public final OrderedSetMultimap<Attribute, Dependency> dependentNodeMap(
         continue;
       }
 
-      if (entry.getKey() == VISIBILITY_DEPENDENCY) {
-        if (toTarget instanceof PackageGroup) {
-          outgoingEdges.put(null, Dependency.withNullConfiguration(dep.getLabel()));
-        } else {
-          // Note that this error could also be caught in
-          // AbstractConfiguredTarget.convertVisibility(), but we have an
-          // opportunity here to avoid dependency cycles that result from
-          // the visibility attribute of a rule referring to a rule that
-          // depends on it (instead of its package)
-          invalidPackageGroupReferenceHook(node, dep.getLabel());
-        }
-
-        continue;
-      }
-
-      if (entry.getKey() == OUTPUT_FILE_RULE_DEPENDENCY) {
-        outgoingEdges.put(
-            null,
-            Dependency.withTransitionAndAspects(
-                dep.getLabel(), NoTransition.INSTANCE, AspectCollection.EMPTY));
-        continue;
-      }
-
       ConfigurationTransition transition =
           TransitionResolver.evaluateTransition(
               node.getConfiguration(), dep.getTransition(), toTarget, trimmingTransitionFactory);