Save memory by using a single constant NestedSet to represent publi…

…c visibility.

Also made the private `NestedSet` a constant too for consistency, although this doesn't save memory since the empty set is a singleton anyway.

Also fix other code warnings in the file.

PiperOrigin-RevId: 516948710
Change-Id: I9a764c6aaa8f2ae4b602f4c0a91eec138210de35

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

@@ -92,6 +92,14 @@
 @ThreadSafe
 public final class ConfiguredTargetFactory {
 
+  private static final NestedSet<PackageGroupContents> PUBLIC_VISIBILITY =
+      NestedSetBuilder.create(
+          Order.STABLE_ORDER,
+          PackageGroupContents.create(ImmutableList.of(PackageSpecification.everything())));
+
+  private static final NestedSet<PackageGroupContents> PRIVATE_VISIBILITY =
+      NestedSetBuilder.emptySet(Order.STABLE_ORDER);
+
   // This class is not meant to be outside of the analysis phase machinery and is only public
   // in order to be accessible from the .view.skyframe package.
 
@@ -112,10 +120,8 @@ private static NestedSet<PackageGroupContents> convertVisibility(
     RuleVisibility ruleVisibility = target.getVisibility();
     if (ruleVisibility instanceof ConstantRuleVisibility) {
       return ((ConstantRuleVisibility) ruleVisibility).isPubliclyVisible()
-          ? NestedSetBuilder.create(
-              Order.STABLE_ORDER,
-              PackageGroupContents.create(ImmutableList.of(PackageSpecification.everything())))
-          : NestedSetBuilder.emptySet(Order.STABLE_ORDER);
+          ? PUBLIC_VISIBILITY
+          : PRIVATE_VISIBILITY;
     } else if (ruleVisibility instanceof PackageGroupsRuleVisibility) {
       PackageGroupsRuleVisibility packageGroupsVisibility =
           (PackageGroupsRuleVisibility) ruleVisibility;
@@ -321,7 +327,7 @@ private ConfiguredTarget createRule(
             .setTransitivePackagesForRunfileRepoMappingManifest(transitivePackages)
             .build();
 
-    List<NestedSet<AnalysisFailure>> analysisFailures =
+    ImmutableList<NestedSet<AnalysisFailure>> analysisFailures =
         depAnalysisFailures(ruleContext, ImmutableList.of());
     if (!analysisFailures.isEmpty()) {
       return erroredConfiguredTargetWithFailures(ruleContext, analysisFailures);
@@ -582,7 +588,7 @@ public ConfiguredAspect createAspect(
     // will be propagated via a hook elsewhere as AnalysisFailureInfo.
     boolean allowAnalysisFailures = ruleContext.getConfiguration().allowAnalysisFailures();
 
-    List<NestedSet<AnalysisFailure>> analysisFailures =
+    ImmutableList<NestedSet<AnalysisFailure>> analysisFailures =
         depAnalysisFailures(ruleContext, ImmutableList.of(configuredTarget));
     if (!analysisFailures.isEmpty()) {
       return erroredConfiguredAspectWithFailures(ruleContext, analysisFailures);
@@ -672,15 +678,12 @@ private static ImmutableMap<String, Attribute> mergeAspectAttributes(
     } else if (aspectPath.size() == 1) {
       return aspectPath.get(0).getDefinition().getAttributes();
     } else {
-
       LinkedHashMap<String, Attribute> aspectAttributes = new LinkedHashMap<>();
       for (Aspect underlyingAspect : aspectPath) {
-        ImmutableMap<String, Attribute> currentAttributes = underlyingAspect.getDefinition()
-            .getAttributes();
+        ImmutableMap<String, Attribute> currentAttributes =
+            underlyingAspect.getDefinition().getAttributes();
         for (Map.Entry<String, Attribute> kv : currentAttributes.entrySet()) {
-          if (!aspectAttributes.containsKey(kv.getKey())) {
-            aspectAttributes.put(kv.getKey(), kv.getValue());
-          }
+          aspectAttributes.putIfAbsent(kv.getKey(), kv.getValue());
         }
       }
       return ImmutableMap.copyOf(aspectAttributes);