Fix code warnings under src/main/java/com/google/devtools/build/lib/packages... PiperOrigin-RevId: 381904325

diff --git a/src/main/java/com/google/devtools/build/lib/packages/AbstractAttributeMapper.java b/src/main/java/com/google/devtools/build/lib/packages/AbstractAttributeMapper.java
index 8f4e9ce..4a002c4 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AbstractAttributeMapper.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AbstractAttributeMapper.java

@@ -32,7 +32,7 @@
 public abstract class AbstractAttributeMapper implements AttributeMap {
   private final RuleClass ruleClass;
   private final Label ruleLabel;
-  protected final Rule rule;
+  final Rule rule;
 
   protected AbstractAttributeMapper(Rule rule) {
     this.ruleClass = rule.getRuleClassObject();
@@ -66,7 +66,7 @@
     if (value instanceof Attribute.ComputedDefault) {
       value = ((Attribute.ComputedDefault) value).getDefault(this);
     } else if (value instanceof Attribute.LateBoundDefault) {
-      value = ((Attribute.LateBoundDefault) value).getDefault();
+      value = ((Attribute.LateBoundDefault<?, ?>) value).getDefault();
     } else if (value instanceof SelectorList) {
       throw new IllegalArgumentException(
           String.format(
@@ -194,7 +194,7 @@
   }
 
   /** Visits all labels reachable from the given attribute. */
-  protected void visitLabels(Attribute attribute, Type.LabelVisitor<Attribute> visitor) {
+  void visitLabels(Attribute attribute, Type.LabelVisitor<Attribute> visitor) {
     Type<?> type = attribute.getType();
     Object value = get(attribute.getName(), type);
     if (value != null) { // null values are particularly possible for computed defaults.
@@ -231,7 +231,7 @@
    * Helper routine that just checks the given attribute has the given type for this rule and throws
    * an IllegalException if not.
    */
-  protected void checkType(String attrName, Type<?> type) {
+  void checkType(String attrName, Type<?> type) {
     Integer index = ruleClass.getAttributeIndex(attrName);
     if (index == null) {
       throw new IllegalArgumentException(

diff --git a/src/main/java/com/google/devtools/build/lib/packages/AdvertisedProviderSet.java b/src/main/java/com/google/devtools/build/lib/packages/AdvertisedProviderSet.java
index 6705a21..d627456 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AdvertisedProviderSet.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AdvertisedProviderSet.java

@@ -48,11 +48,9 @@
   }
 
   public static final AdvertisedProviderSet ANY =
-      new AdvertisedProviderSet(
-          true, ImmutableSet.<Class<?>>of(), ImmutableSet.<StarlarkProviderIdentifier>of());
+      new AdvertisedProviderSet(true, ImmutableSet.of(), ImmutableSet.of());
   public static final AdvertisedProviderSet EMPTY =
-      new AdvertisedProviderSet(
-          false, ImmutableSet.<Class<?>>of(), ImmutableSet.<StarlarkProviderIdentifier>of());
+      new AdvertisedProviderSet(false, ImmutableSet.of(), ImmutableSet.of());
 
   public static AdvertisedProviderSet create(
       ImmutableSet<Class<?>> builtinProviders,
@@ -79,7 +77,7 @@
     }
 
     AdvertisedProviderSet that = (AdvertisedProviderSet) obj;
-    return Objects.equals(this.canHaveAnyProvider, that.canHaveAnyProvider)
+    return this.canHaveAnyProvider == that.canHaveAnyProvider
         && Objects.equals(this.builtinProviders, that.builtinProviders)
         && Objects.equals(this.starlarkProviders, that.starlarkProviders);
   }
@@ -91,7 +89,7 @@
     }
     return String.format(
         "allowed built-in providers=%s, allowed Starlark providers=%s",
-        getBuiltinProviders(), getStarlarkProviders());
+        builtinProviders, starlarkProviders);
   }
 
   /** Checks whether the rule can have any provider.
@@ -124,9 +122,9 @@
    *       fingerprint (except for unintentional digest collisions).
    * </ul>
    *
-   * <p>In other words, {@link #fingerprint} is a proxy for {@link #equals}. These properties *do
-   * not* need to be maintained across Blaze versions (e.g. there's no need to worry about
-   * historical serialized fingerprints).
+   * <p>In other words, this method is a proxy for {@link #equals}. These properties *do not* need
+   * to be maintained across Blaze versions (e.g. there's no need to worry about historical
+   * serialized fingerprints).
    */
   public void fingerprint(Fingerprint fp) {
     fp.addBoolean(canHaveAnyProvider);
@@ -142,17 +140,6 @@
 
   /**
    * Returns {@code true} if this provider set can have any provider, or if it advertises the
-   * specific built-in provider requested.
-   */
-  public boolean advertises(Class<?> builtinProviderClass) {
-    if (canHaveAnyProvider()) {
-      return true;
-    }
-    return builtinProviders.contains(builtinProviderClass);
-  }
-
-  /**
-   * Returns {@code true} if this provider set can have any provider, or if it advertises the
    * specific Starlark provider requested.
    */
   public boolean advertises(StarlarkProviderIdentifier starlarkProvider) {
@@ -213,10 +200,5 @@
       starlarkProviders.add(id);
       return this;
     }
-
-    public Builder addStarlark(Provider.Key id) {
-      starlarkProviders.add(StarlarkProviderIdentifier.forKey(id));
-      return this;
-    }
   }
 }

diff --git a/src/main/java/com/google/devtools/build/lib/packages/Aspect.java b/src/main/java/com/google/devtools/build/lib/packages/Aspect.java
index 217d674..bdd9ec6 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Aspect.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Aspect.java

@@ -153,7 +153,8 @@
   }
 
   /** {@link ObjectCodec} for {@link Aspect}. */
-  static class AspectCodec implements ObjectCodec<Aspect> {
+  @SuppressWarnings("unused") // Used reflectively.
+  private static final class AspectCodec implements ObjectCodec<Aspect> {
     @Override
     public Class<Aspect> getEncodedClass() {
       return Aspect.class;

diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java b/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java
index 1298594..cbe417f 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java

@@ -133,10 +133,6 @@
     return aspectClass.getName();
   }
 
-  public AspectClass getAspectClass() {
-    return aspectClass;
-  }
-
   /**
    * Returns the attributes of the aspect in the form of a String -&gt; {@link Attribute} map.
    *
@@ -220,7 +216,7 @@
   }
 
   public static boolean satisfies(Aspect aspect, AdvertisedProviderSet advertisedProviderSet) {
-    return aspect.getDefinition().getRequiredProviders().isSatisfiedBy(advertisedProviderSet);
+    return aspect.getDefinition().requiredProviders.isSatisfiedBy(advertisedProviderSet);
   }
 
   /** Checks if the given {@code maybeRequiredAspect} is required by this aspect definition */
@@ -255,7 +251,7 @@
           }
           consumer.accept(aspectAttribute, repositoryRelativeLabel);
         };
-    for (Attribute aspectAttribute : aspect.getDefinition().getAttributes().values()) {
+    for (Attribute aspectAttribute : aspect.getDefinition().attributes.values()) {
       if (!dependencyFilter.apply(aspect, aspectAttribute)) {
         continue;
       }
@@ -277,10 +273,11 @@
     private final Map<String, Attribute> attributes = new LinkedHashMap<>();
     private final AdvertisedProviderSet.Builder advertisedProviders =
         AdvertisedProviderSet.builder();
-    private RequiredProviders.Builder requiredProviders = RequiredProviders.acceptAnyBuilder();
+    private final RequiredProviders.Builder requiredProviders =
+        RequiredProviders.acceptAnyBuilder();
     private BiPredicate<Object, String> propagateViaAttribute =
         (BiPredicate<Object, String> & Serializable) (a, c) -> true;
-    private RequiredProviders.Builder requiredAspectProviders =
+    private final RequiredProviders.Builder requiredAspectProviders =
         RequiredProviders.acceptNoneBuilder();
     @Nullable private LinkedHashSet<String> propagateAlongAttributes = new LinkedHashSet<>();
     private final ConfigurationFragmentPolicy.Builder configurationFragmentPolicy =
@@ -350,17 +347,18 @@
 
     /**
      * Optional predicate to conditionally propagate down an attribute based on the {@link
-     * BuildConfiguration}.
+     * com.google.devtools.build.lib.analysis.config.BuildConfiguration}.
      *
      * <p>This is implemented specifically to support the platform-based Android toolchain
-     * migration. See {@link DexArchiveAspect} for details. Don't use this for other purposes. It
-     * introduces unfortunate API complexity and should be removed when Android no longer needs it.
+     * migration. See {@link com.google.devtools.build.lib.rules.android.DexArchiveAspect} for
+     * details. Don't use this for other purposes. It introduces unfortunate API complexity and
+     * should be removed when Android no longer needs it.
      *
-     * @param propagateFunction {@link BiPredicate} that takes the aspect's {@link
-     *     BuildConfiguration} and name of the attribute to propagate. If it returns true,
-     *     propagates down this attribute in this configuration. We don't explicitly type with
-     *     {@link BuildConfiguration} because {@link AspectDefinition} is a loading phase class,
-     *     with no access to config symbols.
+     * @param propagateFunction {@link BiPredicate} that takes the aspect's build configuration and
+     *     name of the attribute to propagate. If it returns true, propagates down this attribute in
+     *     this configuration. We don't explicitly type with {@link
+     *     com.google.devtools.build.lib.analysis.config.BuildConfiguration} because {@link
+     *     AspectDefinition} is a loading phase class, with no access to config symbols.
      */
     public Builder propagateViaAttribute(BiPredicate<Object, String> propagateFunction) {
       propagateViaAttribute = propagateFunction;
@@ -403,13 +401,12 @@
      * Declares that this aspect propagates along an {@code attribute} on the target associated with
      * this aspect.
      *
-     * <p>Specify multiple attributes by calling {@link #propagateAlongAttribute(String)}
-     * repeatedly.
+     * <p>Specify multiple attributes by calling this method repeatedly.
      *
      * <p>Aspect can also declare to propagate along all attributes with {@link
      * #propagateAlongAttributes}.
      */
-    public final Builder propagateAlongAttribute(String attribute) {
+    public Builder propagateAlongAttribute(String attribute) {
       Preconditions.checkNotNull(attribute);
       Preconditions.checkState(
           this.propagateAlongAttributes != null,
@@ -426,7 +423,7 @@
      *
      * <p>Specify either this or {@link #propagateAlongAttribute(String)}, not both.
      */
-    public final Builder propagateAlongAllAttributes() {
+    public Builder propagateAlongAllAttributes() {
       Preconditions.checkState(
           this.propagateAlongAttributes != null,
           "Aspects for all attributes must only be specified once");

diff --git a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java
index a3bd3fb..d4b7adb 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java

@@ -80,15 +80,14 @@
   abstract static class RuleAspect<C extends AspectClass> {
     private static final ImmutableList<String> ALL_ATTR_ASPECTS = ImmutableList.of("*");
 
-    protected final C aspectClass;
-    protected final Function<Rule, AspectParameters> parametersExtractor;
+    final C aspectClass;
+    final Function<Rule, AspectParameters> parametersExtractor;
 
-    protected String baseAspectName;
-    protected ImmutableList.Builder<ImmutableSet<StarlarkProviderIdentifier>>
-        inheritedRequiredProviders;
-    protected ImmutableList.Builder<String> inheritedAttributeAspects;
-    protected boolean inheritedAllProviders = false;
-    protected boolean inheritedAllAttributes = false;
+    String baseAspectName;
+    ImmutableList.Builder<ImmutableSet<StarlarkProviderIdentifier>> inheritedRequiredProviders;
+    ImmutableList.Builder<String> inheritedAttributeAspects;
+    boolean inheritedAllProviders = false;
+    boolean inheritedAllAttributes = false;
 
     private RuleAspect(C aspectClass, Function<Rule, AspectParameters> parametersExtractor) {
       this.aspectClass = aspectClass;
@@ -124,21 +123,21 @@
       }
     }
 
-    public String getName() {
+    String getName() {
       return this.aspectClass.getName();
     }
 
-    public ImmutableSet<String> getRequiredParameters() {
-      return ImmutableSet.<String>of();
+    ImmutableSet<String> getRequiredParameters() {
+      return ImmutableSet.of();
     }
 
-    public abstract Aspect getAspect(Rule rule);
+    protected abstract Aspect getAspect(Rule rule);
 
-    public C getAspectClass() {
+    C getAspectClass() {
       return aspectClass;
     }
 
-    protected void updateInheritedRequiredProviders(
+    void updateInheritedRequiredProviders(
         ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> requiredProviders) {
       if (!inheritedAllProviders && !requiredProviders.isEmpty()) {
         if (inheritedRequiredProviders == null) {
@@ -151,7 +150,7 @@
       }
     }
 
-    protected void updateInheritedAttributeAspects(ImmutableList<String> attributeAspects) {
+    void updateInheritedAttributeAspects(ImmutableList<String> attributeAspects) {
       if (!inheritedAllAttributes && !ALL_ATTR_ASPECTS.equals(attributeAspects)) {
         if (inheritedAttributeAspects == null) {
           inheritedAttributeAspects = ImmutableList.builder();
@@ -163,7 +162,7 @@
       }
     }
 
-    protected RequiredProviders buildInheritedRequiredProviders() {
+    RequiredProviders buildInheritedRequiredProviders() {
       if (baseAspectName == null) {
         return RequiredProviders.acceptNoneBuilder().build();
       } else if (inheritedAllProviders) {
@@ -184,13 +183,13 @@
     }
 
     @Nullable
-    protected ImmutableSet<String> buildInheritedAttributeAspects() {
+    ImmutableSet<String> buildInheritedAttributeAspects() {
       if (baseAspectName == null) {
         return ImmutableSet.of();
       } else if (inheritedAllAttributes) {
         return null;
       } else {
-        return ImmutableSet.<String>copyOf(inheritedAttributeAspects.build());
+        return ImmutableSet.copyOf(inheritedAttributeAspects.build());
       }
     }
 
@@ -204,11 +203,6 @@
     public ImmutableList<String> getInheritedAttributeAspectsList() {
       return inheritedAttributeAspects == null ? null : inheritedAttributeAspects.build();
     }
-
-    @VisibleForSerialization
-    public String getBaseAspectName() {
-      return baseAspectName;
-    }
   }
 
   private static class NativeRuleAspect extends RuleAspect<NativeAspectClass> {
@@ -626,7 +620,7 @@
      *     set.
      */
     public Builder<TYPE> setPropertyFlag(String propertyName) throws EvalException {
-      PropertyFlag flag = null;
+      PropertyFlag flag;
       try {
         flag = PropertyFlag.valueOf(propertyName);
       } catch (IllegalArgumentException e) {
@@ -1049,7 +1043,7 @@
      * <p>This only works on a per-target basis, not on a per-file basis; with other words, it works
      * for 'deps' attributes, but not 'srcs' attributes.
      */
-    public Builder<TYPE> allowedRuleClassesWithWarning(RuleClassNamePredicate allowedRuleClasses) {
+    Builder<TYPE> allowedRuleClassesWithWarning(RuleClassNamePredicate allowedRuleClasses) {
       Preconditions.checkState(
           type.getLabelClass() == LabelClass.DEPENDENCY, "must be a label-valued type");
       propertyFlags.add(PropertyFlag.STRICT_LABEL_CHECKING);
@@ -1081,7 +1075,7 @@
      * this label type attribute has to provide all the providers from one of those lists, otherwise
      * an error is produced during the analysis phase.
      */
-    public final Builder<TYPE> mandatoryBuiltinProvidersList(
+    final Builder<TYPE> mandatoryBuiltinProvidersList(
         Iterable<? extends Iterable<Class<? extends TransitiveInfoProvider>>> providersList) {
       Preconditions.checkState(
           type.getLabelClass() == LabelClass.DEPENDENCY, "must be a label-valued type");
@@ -1115,16 +1109,6 @@
       return this;
     }
 
-    public Builder<TYPE> legacyMandatoryProviders(String... ids) {
-      return mandatoryProviders(
-          Iterables.transform(
-              Arrays.asList(ids),
-              s -> {
-                Preconditions.checkNotNull(s);
-                return StarlarkProviderIdentifier.forLegacy(s);
-              }));
-    }
-
     public Builder<TYPE> mandatoryProviders(Iterable<StarlarkProviderIdentifier> providers) {
       if (providers.iterator().hasNext()) {
         mandatoryProvidersList(ImmutableList.of(providers));
@@ -1362,11 +1346,7 @@
    * natively-defined {@link ComputedDefault}s, which are limited in the number of configurable
    * attributes they depend on, not on the number of different combinations of possible inputs.
    */
-  private static final ComputationLimiter<RuntimeException> NULL_COMPUTATION_LIMITER =
-      new ComputationLimiter<RuntimeException>() {
-        @Override
-        public void onComputationCount(int count) throws RuntimeException {}
-      };
+  private static final ComputationLimiter<RuntimeException> NULL_COMPUTATION_LIMITER = count -> {};
 
   /** Exception for computed default attributes that depend on too many configurable attributes. */
   private static class TooManyConfigurableAttributesException extends Exception {
@@ -1505,15 +1485,15 @@
      * Create a computed default that can read all non-configurable attribute values and no
      * configurable attribute values.
      */
-    public ComputedDefault() {
-      this(ImmutableList.<String>of());
+    protected ComputedDefault() {
+      this(ImmutableList.of());
     }
 
     /**
      * Create a computed default that can read all non-configurable attributes values and one
      * explicitly specified configurable attribute value
      */
-    public ComputedDefault(String depAttribute) {
+    protected ComputedDefault(String depAttribute) {
       this(ImmutableList.of(depAttribute));
     }
 
@@ -1521,7 +1501,7 @@
      * Create a computed default that can read all non-configurable attributes values and two
      * explicitly specified configurable attribute values.
      */
-    public ComputedDefault(String depAttribute1, String depAttribute2) {
+    protected ComputedDefault(String depAttribute1, String depAttribute2) {
       this(ImmutableList.of(depAttribute1, depAttribute2));
     }
 
@@ -1534,7 +1514,7 @@
      * growth of possible values. {@link StarlarkComputedDefault} uses this, but is limited by
      * {@link FixedComputationLimiter#COMPUTED_DEFAULT_MAX_COMBINATIONS}.
      */
-    protected ComputedDefault(ImmutableList<String> dependencies) {
+    ComputedDefault(ImmutableList<String> dependencies) {
       // Order is important for #createDependencyAssignmentTuple.
       this.dependencies = Ordering.natural().immutableSortedCopy(dependencies);
     }
@@ -1637,7 +1617,7 @@
           };
 
       ImmutableList.Builder<Type<?>> dependencyTypesBuilder = ImmutableList.builder();
-      Map<List<Object>, Object> lookupTable = new HashMap<>();
+      Map<List<Object>, Object> lookupTable;
       try {
         for (String dependency : dependencies) {
           Attribute attribute = rule.getRuleClassObject().getAttributeByNameMaybe(dependency);
@@ -1647,9 +1627,10 @@
           }
           dependencyTypesBuilder.add(attribute.getType());
         }
-        lookupTable.putAll(
-            strategy.computeValuesForAllCombinations(
-                dependencies, attr.getType(), rule, FixedComputationLimiter.INSTANCE));
+        lookupTable =
+            new HashMap<>(
+                strategy.computeValuesForAllCombinations(
+                    dependencies, attr.getType(), rule, FixedComputationLimiter.INSTANCE));
         if (caughtEvalExceptionIfAny.get() != null) {
           throw caughtEvalExceptionIfAny.get();
         }
@@ -2003,10 +1984,9 @@
   public static class LabelListLateBoundDefault<FragmentT>
       extends SimpleLateBoundDefault<FragmentT, List<Label>> {
     private LabelListLateBoundDefault(
-        boolean useHostConfiguration,
         Class<FragmentT> fragmentClass,
         Resolver<FragmentT, List<Label>> resolver) {
-      super(useHostConfiguration, fragmentClass, ImmutableList.of(), resolver);
+      super(/*useHostConfiguration=*/ false, fragmentClass, ImmutableList.of(), resolver);
     }
 
     public static <FragmentT> LabelListLateBoundDefault<FragmentT> fromTargetConfiguration(
@@ -2015,7 +1995,7 @@
           !fragmentClass.equals(Void.class),
           "Use fromRuleAndAttributesOnly to specify a LateBoundDefault which does not use "
               + "configuration.");
-      return new LabelListLateBoundDefault<>(false, fragmentClass, resolver);
+      return new LabelListLateBoundDefault<>(fragmentClass, resolver);
     }
 
     /**
@@ -2032,7 +2012,7 @@
      */
     public static LabelListLateBoundDefault<Void> fromRuleAndAttributesOnly(
         Resolver<Void, List<Label>> resolver) {
-      return new LabelListLateBoundDefault<>(false, Void.class, resolver);
+      return new LabelListLateBoundDefault<>(Void.class, resolver);
     }
   }
 
@@ -2104,7 +2084,7 @@
    * @param transitionFactory the configuration transition for this attribute (which must be of type
    *     LABEL, LABEL_LIST, NODEP_LABEL or NODEP_LABEL_LIST).
    */
-  Attribute(
+  private Attribute(
       String name,
       String doc,
       Type<?> type,
@@ -2183,7 +2163,7 @@
    * of '$' or ':').
    */
   public String getPublicName() {
-    return getStarlarkName(getName());
+    return getStarlarkName(name);
   }
 
   /**
@@ -2309,8 +2289,8 @@
   /** Returns true if this attribute's value can be influenced by the build configuration. */
   public boolean isConfigurable() {
     // Output types are excluded because of Rule#populateExplicitOutputFiles.
-    return !(type.getLabelClass() == LabelClass.OUTPUT
-        || getPropertyFlag(PropertyFlag.NONCONFIGURABLE));
+    return type.getLabelClass() != LabelClass.OUTPUT
+        && !getPropertyFlag(PropertyFlag.NONCONFIGURABLE);
   }
 
   /**
@@ -2321,13 +2301,13 @@
    * <p>Non-dependency attributes will always return {@code false}.
    */
   public boolean isToolDependency() {
-    if (getType().getLabelClass() != LabelClass.DEPENDENCY) {
+    if (type.getLabelClass() != LabelClass.DEPENDENCY) {
       return false;
     }
     if (getPropertyFlag(PropertyFlag.IS_TOOL_DEPENDENCY)) {
       return true;
     }
-    return getTransitionFactory().isTool();
+    return transitionFactory.isTool();
   }
   /**
    * Returns a predicate that evaluates to true for rule classes that are allowed labels in this
@@ -2364,7 +2344,7 @@
   }
 
   public Predicate<AttributeMap> getCondition() {
-    return condition == null ? Predicates.<AttributeMap>alwaysTrue() : condition;
+    return condition == null ? Predicates.alwaysTrue() : condition;
   }
 
   public PredicateWithMessage<Object> getAllowedValues() {
@@ -2454,7 +2434,7 @@
    * designates implicit attributes.
    */
   public boolean isImplicit() {
-    return isImplicit(getName());
+    return isImplicit(name);
   }
 
   /**
@@ -2470,7 +2450,7 @@
    * late-bound attributes.
    */
   public boolean isLateBound() {
-    return isLateBound(getName());
+    return isLateBound(name);
   }
 
   /**
@@ -2592,12 +2572,12 @@
   public static Object valueToStarlark(Object x) {
     // Is x a non-empty string_list_dict?
     if (x instanceof Map) {
-      Map<?, ?> map = (Map) x;
+      Map<?, ?> map = (Map<?, ?>) x;
       if (!map.isEmpty() && map.values().iterator().next() instanceof List) {
         // Recursively convert subelements.
         Dict.Builder<Object, Object> dict = Dict.builder();
         for (Map.Entry<?, ?> e : map.entrySet()) {
-          dict.put((String) e.getKey(), Starlark.fromJava(e.getValue(), null));
+          dict.put(e.getKey(), Starlark.fromJava(e.getValue(), null));
         }
         return dict.buildImmutable();
       }

diff --git a/src/main/java/com/google/devtools/build/lib/packages/AttributeFormatter.java b/src/main/java/com/google/devtools/build/lib/packages/AttributeFormatter.java
index 28df7b5..4ce98be 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AttributeFormatter.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AttributeFormatter.java

@@ -57,7 +57,7 @@
 public class AttributeFormatter {
 
   private static final ImmutableSet<Type<?>> depTypes =
-      ImmutableSet.<Type<?>>of(
+      ImmutableSet.of(
           STRING,
           LABEL,
           OUTPUT,
@@ -69,7 +69,7 @@
           DISTRIBUTIONS);
 
   private static final ImmutableSet<Type<?>> noDepTypes =
-      ImmutableSet.<Type<?>>of(NODEP_LABEL_LIST, NODEP_LABEL);
+      ImmutableSet.of(NODEP_LABEL_LIST, NODEP_LABEL);
 
   private AttributeFormatter() {}
 

diff --git a/src/main/java/com/google/devtools/build/lib/packages/AttributeMap.java b/src/main/java/com/google/devtools/build/lib/packages/AttributeMap.java
index e958be7..7b8412b 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AttributeMap.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AttributeMap.java

@@ -74,8 +74,7 @@
    */
   default <T> T getOrDefault(String attributeName, Type<T> type, T defaultValue) {
     if (has(attributeName)) {
-      T value = get(attributeName, type);
-      return value;
+      return get(attributeName, type);
     }
     return defaultValue;
   }

diff --git a/src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkContext.java b/src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkContext.java
index e066387..28a6619 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkContext.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkContext.java

@@ -103,11 +103,6 @@
     this.analysisRuleLabel = analysisRuleLabel;
   }
 
-  /** Returns the phase to which this Starlark thread belongs. */
-  public Phase getPhase() {
-    return phase;
-  }
-
   /** Returns the name of the tools repository, such as "@bazel_tools". */
   @Nullable
   @Override

diff --git a/src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkEnvironment.java b/src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkEnvironment.java
index 46039c6..b7213e7 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkEnvironment.java

@@ -157,8 +157,7 @@
 
   private static ImmutableMap<String, Object> createUninjectedBuildBzlEnv(
       RuleClassProvider ruleClassProvider, Map<String, Object> uninjectedBuildBzlNativeBindings) {
-    Map<String, Object> env = new HashMap<>();
-    env.putAll(ruleClassProvider.getEnvironment());
+    Map<String, Object> env = new HashMap<>(ruleClassProvider.getEnvironment());
 
     // Determine the "native" module.
     // TODO(#11954): Use the same "native" object for both BUILD- and WORKSPACE-loaded .bzls, and
@@ -186,8 +185,7 @@
 
   private static ImmutableMap<String, Object> createWorkspaceBzlEnv(
       RuleClassProvider ruleClassProvider, Map<String, Object> workspaceBzlNativeBindings) {
-    Map<String, Object> env = new HashMap<>();
-    env.putAll(ruleClassProvider.getEnvironment());
+    Map<String, Object> env = new HashMap<>(ruleClassProvider.getEnvironment());
 
     // See above comments for native in BUILD bzls.
     env.put("native", createNativeModule(workspaceBzlNativeBindings));
@@ -199,8 +197,7 @@
       RuleClassProvider ruleClassProvider,
       ImmutableMap<String, Object> uninjectedBuildBzlNativeBindings,
       ImmutableMap<String, Object> uninjectedBuildBzlEnv) {
-    Map<String, Object> env = new HashMap<>();
-    env.putAll(ruleClassProvider.getEnvironment());
+    Map<String, Object> env = new HashMap<>(ruleClassProvider.getEnvironment());
 
     // Clear out rule-specific symbols like CcInfo.
     env.keySet().removeAll(ruleClassProvider.getNativeRuleSpecificBindings().keySet());
@@ -339,8 +336,7 @@
     Map<String, Boolean> overridesMap = parseInjectionOverridesList(overridesList);
 
     // Determine top-level symbols.
-    Map<String, Object> env = new HashMap<>();
-    env.putAll(uninjectedBuildBzlEnv);
+    Map<String, Object> env = new HashMap<>(uninjectedBuildBzlEnv);
     for (Map.Entry<String, Object> entry : exportedToplevels.entrySet()) {
       String key = entry.getKey();
       String name = getKeySuffix(key);
@@ -356,8 +352,7 @@
 
     // Determine "native" bindings.
     // TODO(#11954): See above comment in createUninjectedBuildBzlEnv.
-    Map<String, Object> nativeBindings = new HashMap<>();
-    nativeBindings.putAll(uninjectedBuildBzlNativeBindings);
+    Map<String, Object> nativeBindings = new HashMap<>(uninjectedBuildBzlNativeBindings);
     for (Map.Entry<String, Object> entry : exportedRules.entrySet()) {
       String key = entry.getKey();
       String name = getKeySuffix(key);
@@ -407,7 +402,7 @@
 
   /** Indicates a problem performing builtins injection. */
   public static final class InjectionException extends Exception {
-    public InjectionException(String message) {
+    InjectionException(String message) {
       super(message);
     }
   }

diff --git a/src/main/java/com/google/devtools/build/lib/packages/BuildFileName.java b/src/main/java/com/google/devtools/build/lib/packages/BuildFileName.java
index 627ff36..b2ff9f2 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/BuildFileName.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/BuildFileName.java

@@ -43,11 +43,9 @@
     }
   };
 
-  private static final BuildFileName[] VALUES = BuildFileName.values();
-
   private final PathFragment filenameFragment;
 
-  private BuildFileName(String filename) {
+  BuildFileName(String filename) {
     this.filenameFragment = PathFragment.create(filename);
   }
 
@@ -61,8 +59,4 @@
    * @param packageIdentifier the identifier for this package
    */
   public abstract PathFragment getBuildFileFragment(PackageIdentifier packageIdentifier);
-
-  public static BuildFileName lookupByOrdinal(int ordinal) {
-    return VALUES[ordinal];
-  }
 }

diff --git a/src/main/java/com/google/devtools/build/lib/packages/BuildType.java b/src/main/java/com/google/devtools/build/lib/packages/BuildType.java
index 940f06c..5feb884 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/BuildType.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/BuildType.java

@@ -82,7 +82,7 @@
   @AutoCodec public static final Type<License> LICENSE = new LicenseType();
   /** The type of a single distribution. Only used internally, as a type symbol, not a converter. */
   @AutoCodec
-  public static final Type<DistributionType> DISTRIBUTION =
+  static final Type<DistributionType> DISTRIBUTION =
       new Type<DistributionType>() {
         @Override
         public DistributionType cast(Object value) {
@@ -147,7 +147,7 @@
       Type<T> type, Object x, Object what, LabelConversionContext context)
       throws ConversionException {
     if (x instanceof com.google.devtools.build.lib.packages.SelectorList) {
-      return new SelectorList<T>(
+      return new SelectorList<>(
           ((com.google.devtools.build.lib.packages.SelectorList) x).getElements(),
           what,
           context,
@@ -211,7 +211,7 @@
       this.convertedLabelsInPackage = convertedLabelsInPackage;
     }
 
-    public Label getLabel() {
+    Label getLabel() {
       return label;
     }
 
@@ -221,16 +221,15 @@
       // construction, and global Interner lookup. This approach tends to be very profitable
       // overall, since it's common for the targets in a single package to have duplicate
       // label-strings across all their attribute values.
-      Label label = convertedLabelsInPackage.get(input);
-      if (label == null) {
-        label = getLabel().getRelativeWithRemapping(input, getRepositoryMapping());
-        convertedLabelsInPackage.put(input, label);
+      Label converted = convertedLabelsInPackage.get(input);
+      if (converted == null) {
+        converted = label.getRelativeWithRemapping(input, repositoryMapping);
+        convertedLabelsInPackage.put(input, converted);
       }
-
-      return label;
+      return converted;
     }
 
-    public ImmutableMap<RepositoryName, RepositoryName> getRepositoryMapping() {
+    ImmutableMap<RepositoryName, RepositoryName> getRepositoryMapping() {
       return repositoryMapping;
     }
 
@@ -319,7 +318,7 @@
       super(LABEL, valueType, LabelClass.DEPENDENCY);
     }
 
-    public static <ValueT> LabelKeyedDictType<ValueT> create(Type<ValueT> valueType) {
+    static <ValueT> LabelKeyedDictType<ValueT> create(Type<ValueT> valueType) {
       Preconditions.checkArgument(
           valueType.getLabelClass() == LabelClass.NONE
               || valueType.getLabelClass() == LabelClass.DEPENDENCY,
@@ -343,8 +342,7 @@
       Map<Label, List<Object>> convertedFrom = new LinkedHashMap<>();
       for (Object original : input.keySet()) {
         Label label = LABEL.convert(original, what, context);
-        convertedFrom.computeIfAbsent(label, k -> new ArrayList<Object>());
-        convertedFrom.get(label).add(original);
+        convertedFrom.computeIfAbsent(label, k -> new ArrayList<>()).add(original);
       }
       Printer errorMessage = new Printer();
       errorMessage.append("duplicate labels");
@@ -373,9 +371,8 @@
   }
 
   /**
-   * Like Label, LicenseType is a derived type, which is declared specially
-   * in order to allow syntax validation. It represents the licenses, as
-   * described in {@ref License}.
+   * Like Label, LicenseType is a derived type, which is declared specially in order to allow syntax
+   * validation. It represents the licenses, as described in {@link License}.
    */
   public static class LicenseType extends Type<License> {
     @Override
@@ -409,12 +406,11 @@
   }
 
   /**
-   * Like Label, Distributions is a derived type, which is declared specially
-   * in order to allow syntax validation. It represents the declared distributions
-   * of a target, as described in {@ref License}.
+   * Like Label, Distributions is a derived type, which is declared specially in order to allow
+   * syntax validation. It represents the declared distributions of a target, as described in {@link
+   * License}.
    */
-  private static class Distributions extends
-      Type<Set<DistributionType>> {
+  private static class Distributions extends Type<Set<DistributionType>> {
     @SuppressWarnings("unchecked")
     @Override
     public Set<DistributionType> cast(Object value) {
@@ -491,7 +487,7 @@
       try {
         // Enforce value is relative to the context.
         Label currentRule;
-        ImmutableMap<RepositoryName, RepositoryName> repositoryMapping = ImmutableMap.of();
+        ImmutableMap<RepositoryName, RepositoryName> repositoryMapping;
         if (context instanceof LabelConversionContext) {
           currentRule = ((LabelConversionContext) context).getLabel();
           repositoryMapping = ((LabelConversionContext) context).getRepositoryMapping();
@@ -525,7 +521,7 @@
     SelectorList(
         List<Object> x, Object what, @Nullable LabelConversionContext context, Type<T> originalType)
         throws ConversionException {
-      if (x.size() > 1 && originalType.concat(ImmutableList.<T>of()) == null) {
+      if (x.size() > 1 && originalType.concat(ImmutableList.of()) == null) {
         throw new ConversionException(
             String.format("type '%s' doesn't support select concatenation", originalType));
       }
@@ -570,7 +566,7 @@
      */
     public Set<Label> getKeyLabels() {
       ImmutableSet.Builder<Label> keys = ImmutableSet.builder();
-      for (Selector<T> selector : getSelectors()) {
+      for (Selector<T> selector : elements) {
         for (Label label : selector.getEntries().keySet()) {
           if (!Selector.isReservedLabel(label)) {
             keys.add(label);
@@ -627,7 +623,7 @@
     @VisibleForTesting
     public static final String DEFAULT_CONDITION_KEY = "//conditions:default";
 
-    public static final Label DEFAULT_CONDITION_LABEL =
+    static final Label DEFAULT_CONDITION_LABEL =
         Label.parseAbsoluteUnchecked(DEFAULT_CONDITION_KEY);
 
     private final Type<T> originalType;

diff --git a/src/main/java/com/google/devtools/build/lib/packages/BuiltinProvider.java b/src/main/java/com/google/devtools/build/lib/packages/BuiltinProvider.java
index 33f430c..f04475a 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/BuiltinProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/BuiltinProvider.java

@@ -42,7 +42,7 @@
   private final String name;
   private final Class<T> valueClass;
 
-  public BuiltinProvider(String name, Class<T> valueClass) {
+  protected BuiltinProvider(String name, Class<T> valueClass) {
     this.key = new Key(name, getClass());
     this.name = name;
     this.valueClass = valueClass;
@@ -89,12 +89,12 @@
   @Override
   public void repr(Printer printer) {
     // TODO(adonovan): change to '<provider name>'.
-    printer.append("<function " + getPrintableName() + ">");
+    printer.append("<function " + name + ">");
   }
 
   /** Returns the identifier of this provider. */
   public StarlarkProviderIdentifier id() {
-    return StarlarkProviderIdentifier.forKey(getKey());
+    return StarlarkProviderIdentifier.forKey(key);
   }
 
   /**

diff --git a/src/main/java/com/google/devtools/build/lib/packages/ConstantRuleVisibility.java b/src/main/java/com/google/devtools/build/lib/packages/ConstantRuleVisibility.java
index 50378c4..f9b80fd 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/ConstantRuleVisibility.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/ConstantRuleVisibility.java

@@ -50,7 +50,7 @@
 
   private final boolean result;
 
-  public ConstantRuleVisibility(boolean result) {
+  private ConstantRuleVisibility(boolean result) {
     this.result = result;
   }
 
@@ -91,7 +91,7 @@
     return null;
   }
 
-  public static ConstantRuleVisibility tryParse(Label label) {
+  private static ConstantRuleVisibility tryParse(Label label) {
     if (PUBLIC_LABEL.equals(label)) {
       return PUBLIC;
     } else if (PRIVATE_LABEL.equals(label)) {

diff --git a/src/main/java/com/google/devtools/build/lib/packages/DependencyFilter.java b/src/main/java/com/google/devtools/build/lib/packages/DependencyFilter.java
index 754847f..f12cfb6 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/DependencyFilter.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/DependencyFilter.java

@@ -72,14 +72,6 @@
               || attribute.getName().equals("visibility");
         }
       };
-  /** Checks to see if the attribute has the isDirectCompileTimeInput property. */
-  public static final DependencyFilter DIRECT_COMPILE_TIME_INPUT =
-      new DependencyFilter() {
-        @Override
-        public boolean apply(AttributeInfoProvider infoProvider, Attribute attribute) {
-          return attribute.isDirectCompileTimeInput();
-        }
-      };
 
   /**
    * Returns true if a given attribute should be processed.

diff --git a/src/main/java/com/google/devtools/build/lib/packages/EnumFilterConverter.java b/src/main/java/com/google/devtools/build/lib/packages/EnumFilterConverter.java
index 8ce7ecf..e3f2c2f 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/EnumFilterConverter.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/EnumFilterConverter.java

@@ -16,7 +16,6 @@
 import com.google.devtools.build.lib.util.StringUtil;
 import com.google.devtools.common.options.Converter;
 import com.google.devtools.common.options.OptionsParsingException;
-
 import java.util.Collections;
 import java.util.EnumSet;
 import java.util.LinkedHashSet;
@@ -44,7 +43,7 @@
    * @param typeClass this should be E.class (Java generics can't infer that directly)
    * @param userFriendlyName a user-friendly description of this enum type
    */
-  public EnumFilterConverter(Class<E> typeClass, String userFriendlyName) {
+  EnumFilterConverter(Class<E> typeClass, String userFriendlyName) {
     this.typeClass = typeClass;
     this.prettyEnumName = userFriendlyName;
     for (E value : EnumSet.allOf(typeClass)) {

diff --git a/src/main/java/com/google/devtools/build/lib/packages/EnvironmentLabels.java b/src/main/java/com/google/devtools/build/lib/packages/EnvironmentLabels.java
index 4bbef07..e301f49 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/EnvironmentLabels.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/EnvironmentLabels.java

@@ -47,7 +47,7 @@
    * can't set this map until all Target instances for member environments have been initialized,
    * which occurs after group instantiation (this makes the class mutable).
    */
-  private Map<Label, NestedSet<Label>> fulfillersMap = null;
+  private Map<Label, NestedSet<Label>> fulfillersMap;
 
   EnvironmentLabels(Label label, Collection<Label> environments, Collection<Label> defaults) {
     this(label, environments, defaults, null);

diff --git a/src/main/java/com/google/devtools/build/lib/packages/FileTarget.java b/src/main/java/com/google/devtools/build/lib/packages/FileTarget.java
index a539e67..72bb473 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/FileTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/FileTarget.java

@@ -27,7 +27,7 @@
  */
 @Immutable
 public abstract class FileTarget implements Target, FileType.HasFileType {
-  protected final Label label;
+  final Label label;
 
   /** Constructs a file with the given label, which must be in the given package. */
   FileTarget(Package pkg, Label label) {
@@ -61,7 +61,7 @@
 
   @Override
   public String toString() {
-    return getTargetKind() + "(" + getLabel() + ")"; // Just for debugging
+    return getTargetKind() + "(" + label + ")"; // Just for debugging
   }
 
   @Override

diff --git a/src/main/java/com/google/devtools/build/lib/packages/FilesetEntry.java b/src/main/java/com/google/devtools/build/lib/packages/FilesetEntry.java
index cc625fa..b02e40c 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/FilesetEntry.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/FilesetEntry.java

@@ -25,7 +25,6 @@
 import com.google.devtools.build.lib.starlarkbuildapi.FilesetEntryApi;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Locale;
@@ -41,7 +40,7 @@
 @ThreadSafe
 public final class FilesetEntry implements StarlarkValue, FilesetEntryApi {
 
-  public static final SymlinkBehavior DEFAULT_SYMLINK_BEHAVIOR = SymlinkBehavior.COPY;
+  private static final SymlinkBehavior DEFAULT_SYMLINK_BEHAVIOR = SymlinkBehavior.COPY;
   public static final String DEFAULT_STRIP_PREFIX = ".";
   public static final String STRIP_PREFIX_WORKSPACE = "%workspace%";
 
@@ -50,35 +49,20 @@
     return true;
   }
 
-  public static List<String> makeStringList(List<Label> labels) {
-    if (labels == null) {
-      return Collections.emptyList();
-    }
-    List<String> strings = Lists.newArrayListWithCapacity(labels.size());
-    for (Label label : labels) {
-      strings.add(label.toString());
-    }
-    return strings;
-  }
-
-  public static List<?> makeList(Collection<?> list) {
-    return list == null ? Lists.newArrayList() : Lists.newArrayList(list);
-  }
-
   @Override
   public void repr(Printer printer) {
     printer.append("FilesetEntry(srcdir = ");
-    printer.repr(getSrcLabel().toString());
+    printer.repr(srcLabel.toString());
     printer.append(", files = ");
-    printer.repr(makeStringList(getFiles()));
+    printer.repr(files == null ? ImmutableList.of() : Lists.transform(files, Label::toString));
     printer.append(", excludes = ");
-    printer.repr(makeList(getExcludes()));
+    printer.repr(excludes == null ? ImmutableList.of() : excludes.asList());
     printer.append(", destdir = ");
-    printer.repr(getDestDir().getPathString());
+    printer.repr(destDir.getPathString());
     printer.append(", strip_prefix = ");
-    printer.repr(getStripPrefix());
+    printer.repr(stripPrefix);
     printer.append(", symlinks = ");
-    printer.repr(getSymlinkBehavior().toString());
+    printer.repr(symlinkBehavior.toString());
     printer.append(")");
   }
 

diff --git a/src/main/java/com/google/devtools/build/lib/packages/GlobCache.java b/src/main/java/com/google/devtools/build/lib/packages/GlobCache.java
index b314d1d..b289dcb 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/GlobCache.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/GlobCache.java

@@ -72,10 +72,9 @@
    */
   private final Predicate<Path> childDirectoryPredicate;
 
-  /**
-   * System call caching layer.
-   */
-  private AtomicReference<? extends UnixGlob.FilesystemCalls> syscalls;
+  /** System call caching layer. */
+  private final AtomicReference<? extends UnixGlob.FilesystemCalls> syscalls;
+
   private final int maxDirectoriesToEagerlyVisit;
 
   /** The thread pool for glob evaluation. */

diff --git a/src/main/java/com/google/devtools/build/lib/packages/ImplicitOutputsFunction.java b/src/main/java/com/google/devtools/build/lib/packages/ImplicitOutputsFunction.java
index 2f63af9..a5e23fe 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/ImplicitOutputsFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/ImplicitOutputsFunction.java

@@ -303,7 +303,7 @@
    */
   // It would be nice to unify this with fromTemplates above, but that's not possible because
   // substitutePlaceholderIntoUnsafeTemplate can throw an exception.
-  public static ImplicitOutputsFunction fromUnsafeTemplates(Iterable<String> templates) {
+  private static ImplicitOutputsFunction fromUnsafeTemplates(Iterable<String> templates) {
     return new UnsafeTemplatesImplicitOutputsFunction(templates);
   }
 
@@ -457,7 +457,7 @@
     abstract List<String> attributeNames();
 
     static ParsedTemplate parse(String rawTemplate) {
-      List<String> placeholders = Lists.<String>newArrayList();
+      List<String> placeholders = Lists.newArrayList();
       String formatStr = createPlaceholderSubstitutionFormatString(rawTemplate, placeholders);
       if (placeholders.isEmpty()) {
         placeholders = ImmutableList.of();
@@ -476,7 +476,7 @@
       for (String placeholder : attributeNames()) {
         Set<String> attrValues = attributeGetter.get(attributeMap, placeholder);
         if (attrValues.isEmpty()) {
-          return ImmutableList.<String>of();
+          return ImmutableList.of();
         }
         values.add(attrValues);
       }

diff --git a/src/main/java/com/google/devtools/build/lib/packages/LabelVisitationUtils.java b/src/main/java/com/google/devtools/build/lib/packages/LabelVisitationUtils.java
index 8d3c21c..0eff74a 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/LabelVisitationUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/LabelVisitationUtils.java

@@ -91,7 +91,6 @@
 
     if (target instanceof PackageGroup) {
       visitPackageGroup((PackageGroup) target, labelProcessor);
-      return;
     }
   }
 

diff --git a/src/main/java/com/google/devtools/build/lib/packages/License.java b/src/main/java/com/google/devtools/build/lib/packages/License.java
index 8bbfa39..3a514e1 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/License.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/License.java

@@ -44,7 +44,7 @@
    * The error that's thrown if a build file contains an invalid license string.
    */
   public static class LicenseParsingException extends Exception {
-    public LicenseParsingException(String s) {
+    LicenseParsingException(String s) {
       super(s);
     }
   }

diff --git a/src/main/java/com/google/devtools/build/lib/packages/NoSuchPackageException.java b/src/main/java/com/google/devtools/build/lib/packages/NoSuchPackageException.java
index a7ef140..56f5758 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/NoSuchPackageException.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/NoSuchPackageException.java

@@ -58,7 +58,7 @@
     return packageId;
   }
 
-  public String getRawMessage() {
+  String getRawMessage() {
     return super.getMessage();
   }
 

diff --git a/src/main/java/com/google/devtools/build/lib/packages/NoSuchTargetException.java b/src/main/java/com/google/devtools/build/lib/packages/NoSuchTargetException.java
index 37c2890..0ba199a 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/NoSuchTargetException.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/NoSuchTargetException.java

@@ -51,9 +51,10 @@
         /*hasTarget=*/ true);
   }
 
-  public NoSuchTargetException(String message, @Nullable Label label, boolean hasTarget) {
+  private NoSuchTargetException(String message, @Nullable Label label, boolean hasTarget) {
     // TODO(bazel-team): Does the exception matter?
-    super(message,
+    super(
+        message,
         hasTarget ? new BuildFileContainsErrorsException(label.getPackageIdentifier()) : null);
     this.label = label;
     this.hasTarget = hasTarget;

diff --git a/src/main/java/com/google/devtools/build/lib/packages/NoSuchThingException.java b/src/main/java/com/google/devtools/build/lib/packages/NoSuchThingException.java
index d4de044..032bbba 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/NoSuchThingException.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/NoSuchThingException.java

@@ -33,17 +33,17 @@
     this.detailedExitCode = null;
   }
 
-  public NoSuchThingException(String message, Throwable cause) {
+  NoSuchThingException(String message, Throwable cause) {
     super(message, cause);
     this.detailedExitCode = null;
   }
 
-  public NoSuchThingException(String message, DetailedExitCode detailedExitCode) {
+  NoSuchThingException(String message, DetailedExitCode detailedExitCode) {
     super(message);
     this.detailedExitCode = detailedExitCode;
   }
 
-  public NoSuchThingException(String message, Throwable cause, DetailedExitCode detailedExitCode) {
+  NoSuchThingException(String message, Throwable cause, DetailedExitCode detailedExitCode) {
     super(message, cause);
     this.detailedExitCode = detailedExitCode;
   }

diff --git a/src/main/java/com/google/devtools/build/lib/packages/OutputFile.java b/src/main/java/com/google/devtools/build/lib/packages/OutputFile.java
index 6a3c679..058c57e 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/OutputFile.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/OutputFile.java

@@ -78,7 +78,7 @@
 
   @Override
   public Rule getAssociatedRule() {
-    return getGeneratingRule();
+    return generatingRule;
   }
 
   @Override

diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java
index bf73595..6d55c567d 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Package.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java

@@ -868,7 +868,7 @@
     return error.withProperty(DetailedExitCode.class, createDetailedCode(error.toString(), code));
   }
 
-  public static DetailedExitCode createDetailedCode(String errorMessage, Code code) {
+  private static DetailedExitCode createDetailedCode(String errorMessage, Code code) {
     return DetailedExitCode.of(
         FailureDetail.newBuilder()
             .setMessage(errorMessage)
@@ -1324,7 +1324,7 @@
     }
 
     @Nullable
-    public FailureDetail getFailureDetail() {
+    FailureDetail getFailureDetail() {
       if (failureDetailOverride != null) {
         return failureDetailOverride;
       }

diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageCodecDependencies.java b/src/main/java/com/google/devtools/build/lib/packages/PackageCodecDependencies.java
index fd5a5f2..04213aa 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageCodecDependencies.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageCodecDependencies.java

@@ -20,7 +20,7 @@
   PackageSerializerInterface getPackageSerializer();
 
   /** Simplest implementation of PackageCodecDependencies. */
-  public static class SimplePackageCodecDependencies implements PackageCodecDependencies {
+  class SimplePackageCodecDependencies implements PackageCodecDependencies {
     private final PackageSerializerInterface packageSerializer;
 
     public SimplePackageCodecDependencies(PackageSerializerInterface packageSerializer) {

diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
index b1c0054..9da6e4a 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java

@@ -111,7 +111,6 @@
   }
 
   private final RuleFactory ruleFactory;
-  private final ImmutableMap<String, BuiltinRuleFunction> ruleFunctions;
   private final RuleClassProvider ruleClassProvider;
 
   private AtomicReference<? extends UnixGlob.FilesystemCalls> syscalls;
@@ -133,7 +132,7 @@
   /** Builder for {@link PackageFactory} instances. Intended to only be used by unit tests. */
   @VisibleForTesting
   public abstract static class BuilderForTesting {
-    protected final String version = "test";
+    protected static final String VERSION = "test";
     protected Iterable<EnvironmentExtension> environmentExtensions = ImmutableList.of();
     protected PackageValidator packageValidator = PackageValidator.NOOP_VALIDATOR;
     protected PackageOverheadEstimator packageOverheadEstimator =
@@ -193,7 +192,6 @@
       PackageOverheadEstimator packageOverheadEstimator,
       PackageLoadingListener packageLoadingListener) {
     this.ruleFactory = new RuleFactory(ruleClassProvider);
-    this.ruleFunctions = buildRuleFunctions(ruleFactory);
     this.ruleClassProvider = ruleClassProvider;
     this.executor = executorForGlobbing;
     this.environmentExtensions = ImmutableList.copyOf(environmentExtensions);
@@ -205,7 +203,7 @@
     this.bazelStarlarkEnvironment =
         new BazelStarlarkEnvironment(
             ruleClassProvider,
-            ruleFunctions,
+            buildRuleFunctions(ruleFactory),
             this.environmentExtensions,
             newPackageFunction(packageArguments),
             version);

diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageGroupsRuleVisibility.java b/src/main/java/com/google/devtools/build/lib/packages/PackageGroupsRuleVisibility.java
index a90289e..dbf46cb 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageGroupsRuleVisibility.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageGroupsRuleVisibility.java

@@ -27,7 +27,7 @@
   private final PackageGroupContents directPackages;
   private final List<Label> declaredLabels;
 
-  public PackageGroupsRuleVisibility(Label ruleLabel, List<Label> labels) {
+  private PackageGroupsRuleVisibility(Label ruleLabel, List<Label> labels) {
     declaredLabels = ImmutableList.copyOf(labels);
     ImmutableList.Builder<PackageSpecification> directPackageBuilder = ImmutableList.builder();
     ImmutableList.Builder<Label> packageGroupBuilder = ImmutableList.builder();

diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageSpecification.java b/src/main/java/com/google/devtools/build/lib/packages/PackageSpecification.java
index c103514..f682618 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageSpecification.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageSpecification.java

@@ -161,7 +161,7 @@
   @AutoCodec
   @VisibleForSerialization
   static final class SinglePackage extends PackageSpecification {
-    private PackageIdentifier singlePackageName;
+    private final PackageIdentifier singlePackageName;
 
     @VisibleForSerialization
     SinglePackage(PackageIdentifier singlePackageName) {
@@ -204,7 +204,7 @@
   @AutoCodec
   @VisibleForSerialization
   static final class AllPackagesBeneath extends PackageSpecification {
-    private PackageIdentifier prefix;
+    private final PackageIdentifier prefix;
 
     @VisibleForSerialization
     AllPackagesBeneath(PackageIdentifier prefix) {
@@ -348,10 +348,6 @@
       this.allSpecifications = allSpecifications;
     }
 
-    public ImmutableList<PackageSpecification> getPackageSpecifications() {
-      throw new UnsupportedOperationException();
-    }
-
     /**
      * Creates a {@link PackageGroupContents} representing a collection of {@link
      * PackageSpecification}s.

diff --git a/src/main/java/com/google/devtools/build/lib/packages/PredicateWithMessage.java b/src/main/java/com/google/devtools/build/lib/packages/PredicateWithMessage.java
index c479788..f04fb27 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PredicateWithMessage.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PredicateWithMessage.java

@@ -22,9 +22,8 @@
 public interface PredicateWithMessage<T> extends Predicate<T> {
 
   /**
-   * The error message to display when predicate checks param. Only makes sense to
-   * call this method is apply(param) returns false.
+   * The error message to display when predicate checks param. Only makes sense to call this method
+   * if apply(param) returns false.
    */
-  public String getErrorReason(T param);
-
+  String getErrorReason(T param);
 }

diff --git a/src/main/java/com/google/devtools/build/lib/packages/ProtoUtils.java b/src/main/java/com/google/devtools/build/lib/packages/ProtoUtils.java
index 0080e1a..4b82982 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/ProtoUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/ProtoUtils.java

@@ -40,11 +40,8 @@
 
 /** Shared code used in proto buffer output for rules and rule classes. */
 public class ProtoUtils {
-  /**
-   * This map contains all attribute types that are recognized by the protocol
-   * output formatter.
-   */
-  static final ImmutableMap<Type<?>, Discriminator> TYPE_MAP =
+  /** This map contains all attribute types that are recognized by the protocol output formatter. */
+  private static final ImmutableMap<Type<?>, Discriminator> TYPE_MAP =
       new ImmutableMap.Builder<Type<?>, Discriminator>()
           .put(INTEGER, Discriminator.INTEGER)
           .put(DISTRIBUTIONS, Discriminator.DISTRIBUTION_SET)

diff --git a/src/main/java/com/google/devtools/build/lib/packages/Provider.java b/src/main/java/com/google/devtools/build/lib/packages/Provider.java
index b5984fd..35da1d9f 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Provider.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Provider.java

@@ -61,7 +61,7 @@
   Location getLocation();
 
   /** A serializable and fingerprintable representation of {@link Provider}. */
-  public abstract static class Key {
+  abstract class Key {
     abstract void fingerprint(Fingerprint fp);
   }
 }

diff --git a/src/main/java/com/google/devtools/build/lib/packages/RawAttributeMapper.java b/src/main/java/com/google/devtools/build/lib/packages/RawAttributeMapper.java
index b217481..0884f79 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RawAttributeMapper.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RawAttributeMapper.java

@@ -32,7 +32,7 @@
  */
 public class RawAttributeMapper extends AbstractAttributeMapper {
 
-  RawAttributeMapper(Rule rule) {
+  private RawAttributeMapper(Rule rule) {
     super(rule);
   }
 

diff --git a/src/main/java/com/google/devtools/build/lib/packages/RequiredProviders.java b/src/main/java/com/google/devtools/build/lib/packages/RequiredProviders.java
index 0bf6a48d..554bef3 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RequiredProviders.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RequiredProviders.java

@@ -144,7 +144,7 @@
       }
 
       @Override
-      public boolean satisfies(
+      boolean satisfies(
           Predicate<Class<? extends TransitiveInfoProvider>> hasBuiltinProvider,
           Predicate<StarlarkProviderIdentifier> hasStarlarkProvider,
           RequiredProviders requiredProviders,
@@ -204,7 +204,7 @@
     };
 
     /** Checks if {@code advertisedProviderSet} satisfies these {@code RequiredProviders} */
-    public abstract boolean satisfies(
+    protected abstract boolean satisfies(
         AdvertisedProviderSet advertisedProviderSet,
         RequiredProviders requiredProviders,
         Builder missing);

diff --git a/src/main/java/com/google/devtools/build/lib/packages/Rule.java b/src/main/java/com/google/devtools/build/lib/packages/Rule.java
index 5fb73a4..3dd861d 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Rule.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Rule.java

@@ -32,6 +32,7 @@
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.events.NullEventHandler;
 import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.StarlarkImplicitOutputsFunction;
 import com.google.devtools.build.lib.packages.License.DistributionType;
 import com.google.devtools.build.lib.packages.Package.ConfigSettingVisibilityPolicy;
@@ -257,10 +258,9 @@
     Attribute attribute = ruleClass.getAttributeByNameMaybe(attributeName);
     // TODO(murali): This method should be property of ruleclass not rule instance.
     // Further, this call to AbstractAttributeMapper.isConfigurable is delegated right back
-    // to this instance!.
+    // to this instance!
     return attribute != null
-        ? AbstractAttributeMapper.isConfigurable(this, attributeName, attribute.getType())
-        : false;
+        && AbstractAttributeMapper.isConfigurable(this, attributeName, attribute.getType());
   }
 
   /**
@@ -470,7 +470,7 @@
     // TODO(b/151165647): this logic has always been wrong:
     // it spuriously matches occurrences of the package name earlier in the path.
     String absolutePath = location.toString();
-    int pos = absolutePath.indexOf(getLabel().getPackageName());
+    int pos = absolutePath.indexOf(label.getPackageName());
     return (pos < 0) ? null : absolutePath.substring(pos);
   }
 
@@ -496,7 +496,7 @@
     Integer index = ruleClass.getAttributeIndex(attrName);
     if (index == null) {
       throw new IllegalArgumentException(
-          "No such attribute " + attrName + " in " + ruleClass + " rule " + getLabel());
+          "No such attribute " + attrName + " in " + ruleClass + " rule " + label);
     }
     Attribute attr = ruleClass.getAttribute(index);
     if (attr.getType() != type) {
@@ -510,7 +510,7 @@
               + " in "
               + ruleClass
               + " rule "
-              + getLabel());
+              + label);
     }
     return getAttrWithIndex(index);
   }
@@ -539,7 +539,7 @@
               + " in "
               + ruleClass
               + " rule "
-              + getLabel());
+              + label);
     }
     return (BuildType.SelectorList<T>) attrValue;
   }
@@ -644,7 +644,7 @@
    * Check if this rule is valid according to the validityPredicate of its RuleClass.
    */
   void checkValidityPredicate(EventHandler eventHandler) {
-    PredicateWithMessage<Rule> predicate = getRuleClassObject().getValidityPredicate();
+    PredicateWithMessage<Rule> predicate = ruleClass.getValidityPredicate();
     if (!predicate.apply(this)) {
       reportError(predicate.getErrorReason(this), eventHandler);
     }
@@ -660,11 +660,10 @@
         eventHandler, pkgBuilder.getPackageIdentifier(), /*checkLabels=*/ true);
   }
 
-  void populateOutputFilesUnchecked(EventHandler eventHandler, Package.Builder pkgBuilder)
-      throws InterruptedException {
+  void populateOutputFilesUnchecked(Package.Builder pkgBuilder) throws InterruptedException {
     try {
       populateOutputFilesInternal(
-          eventHandler, pkgBuilder.getPackageIdentifier(), /*checkLabels=*/ false);
+          NullEventHandler.INSTANCE, pkgBuilder.getPackageIdentifier(), /*checkLabels=*/ false);
     } catch (LabelSyntaxException e) {
       throw new IllegalStateException(e);
     }
@@ -672,12 +671,12 @@
 
   @FunctionalInterface
   private interface ExplicitOutputHandler {
-    public void accept(Attribute attribute, Label outputLabel) throws LabelSyntaxException;
+    void accept(Attribute attribute, Label outputLabel) throws LabelSyntaxException;
   }
 
   @FunctionalInterface
   private interface ImplicitOutputHandler {
-    public void accept(String outputKey, String outputName);
+    void accept(String outputKey, String outputName);
   }
 
   private void populateOutputFilesInternal(
@@ -711,7 +710,7 @@
               reportError(
                   String.format(
                       "illegal output file name '%s' in rule %s due to: %s",
-                      outputName, getLabel(), e.getMessage()),
+                      outputName, this.label, e.getMessage()),
                   eventHandler);
               return;
             }
@@ -745,8 +744,7 @@
         }
       }
     } catch (EvalException e) {
-      reportError(
-          String.format("In rule %s: %s", getLabel(), e.getMessageWithStack()), eventHandler);
+      reportError(String.format("In rule %s: %s", label, e.getMessageWithStack()), eventHandler);
     }
 
     ExplicitOutputHandler explicitOutputHandler =
@@ -830,7 +828,7 @@
   /** Returns a string of the form "cc_binary rule //foo:foo" */
   @Override
   public String toString() {
-    return getRuleClass() + " rule " + getLabel();
+    return getRuleClass() + " rule " + label;
   }
 
  /**
@@ -847,7 +845,7 @@
     // enforced). See https://github.com/bazelbuild/bazel/issues/12669. Ultimately this entire
     // conditional should be removed.
     if (ruleClass.getName().equals("config_setting")) {
-      ConfigSettingVisibilityPolicy policy = getPackage().getConfigSettingVisibilityPolicy();
+      ConfigSettingVisibilityPolicy policy = pkg.getConfigSettingVisibilityPolicy();
       if (visibility != null) {
         return visibility; // Use explicitly set visibility
       } else if (policy == ConfigSettingVisibilityPolicy.DEFAULT_PUBLIC) {
@@ -879,7 +877,7 @@
         && isAttributeValueExplicitlySpecified("distribs")) {
       return NonconfigurableAttributeMapper.of(this).get("distribs", BuildType.DISTRIBUTIONS);
     } else {
-      return getPackage().getDefaultDistribs();
+      return pkg.getDefaultDistribs();
     }
   }
 
@@ -894,10 +892,10 @@
     } else if (isAttrDefined("licenses", BuildType.LICENSE)
         && isAttributeValueExplicitlySpecified("licenses")) {
       return NonconfigurableAttributeMapper.of(this).get("licenses", BuildType.LICENSE);
-    } else if (getRuleClassObject().ignoreLicenses()) {
+    } else if (ruleClass.ignoreLicenses()) {
       return License.NO_LICENSE;
     } else {
-      return getPackage().getDefaultLicense();
+      return pkg.getDefaultLicense();
     }
   }
 
@@ -919,7 +917,7 @@
    */
   public Set<String> getRuleTags() {
     Set<String> ruleTags = new LinkedHashSet<>();
-    for (Attribute attribute : getRuleClassObject().getAttributes()) {
+    for (Attribute attribute : ruleClass.getAttributes()) {
       if (attribute.isTaggable()) {
         Type<?> attrType = attribute.getType();
         String name = attribute.getName();
@@ -933,10 +931,10 @@
   }
 
   /**
-   * Computes labels of additional dependencies that can be provided by aspects that this rule
-   * can require from its direct dependencies.
+   * Computes labels of additional dependencies that can be provided by aspects that this rule can
+   * require from its direct dependencies.
    */
-  public Collection<? extends Label> getAspectLabelsSuperset(DependencyFilter predicate) {
+  public Collection<Label> getAspectLabelsSuperset(DependencyFilter predicate) {
     if (!hasAspects()) {
       return ImmutableList.of();
     }
@@ -953,7 +951,7 @@
    * @return The repository name.
    */
   public RepositoryName getRepository() {
-    return getLabel().getPackageIdentifier().getRepository();
+    return label.getPackageIdentifier().getRepository();
   }
 
   /** Returns the suffix of target kind for all rules. */

diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
index a3b628a..0e586b0 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java

@@ -222,7 +222,7 @@
     INHERIT;
 
     /** Determine the correct value to use based on the current setting and the parent's value. */
-    public ToolchainResolutionMode apply(String name, ToolchainResolutionMode parent) {
+    ToolchainResolutionMode apply(String name, ToolchainResolutionMode parent) {
       if (this == INHERIT) {
         return parent;
       } else if (parent == INHERIT) {
@@ -237,7 +237,7 @@
       return this;
     }
 
-    public boolean isActive() {
+    boolean isActive() {
       switch (this) {
         case ENABLED:
           return true;
@@ -259,7 +259,7 @@
     INHERIT;
 
     /** Determine the correct value to use based on the current setting and the parent's value. */
-    public ToolchainTransitionMode apply(String name, ToolchainTransitionMode parent) {
+    ToolchainTransitionMode apply(String name, ToolchainTransitionMode parent) {
       if (this == INHERIT) {
         return parent;
       } else if (parent == INHERIT) {
@@ -274,7 +274,7 @@
       return this;
     }
 
-    public boolean isActive() {
+    boolean isActive() {
       switch (this) {
         case ENABLED:
           return true;
@@ -549,12 +549,12 @@
       public abstract void checkName(String name);
 
       /**
-       * Checks whether the given set of attributes contains all the required
-       * attributes for the current rule class type.
+       * Checks whether the given set of attributes contains all the required attributes for the
+       * current rule class type.
        *
        * @throws IllegalArgumentException if a required attribute is missing
        */
-      public abstract void checkAttributes(Map<String, Attribute> attributes);
+      protected abstract void checkAttributes(Map<String, Attribute> attributes);
     }
 
     /** A predicate that filters rule classes based on their names. */
@@ -723,17 +723,17 @@
      */
     public static final String STARLARK_BUILD_SETTING_DEFAULT_ATTR_NAME = "build_setting_default";
 
-    public static final String STARLARK_BUILD_SETTING_HELP_ATTR_NAME = "help";
+    static final String STARLARK_BUILD_SETTING_HELP_ATTR_NAME = "help";
 
-    public static final String BUILD_SETTING_DEFAULT_NONCONFIGURABLE =
+    static final String BUILD_SETTING_DEFAULT_NONCONFIGURABLE =
         "Build setting defaults are referenced during analysis.";
 
     /** List of required attributes for normal rules, name and type. */
-    public static final ImmutableList<Attribute> REQUIRED_ATTRIBUTES_FOR_NORMAL_RULES =
+    static final ImmutableList<Attribute> REQUIRED_ATTRIBUTES_FOR_NORMAL_RULES =
         ImmutableList.of(attr("tags", Type.STRING_LIST).build());
 
     /** List of required attributes for test rules, name and type. */
-    public static final ImmutableList<Attribute> REQUIRED_ATTRIBUTES_FOR_TESTS =
+    static final ImmutableList<Attribute> REQUIRED_ATTRIBUTES_FOR_TESTS =
         ImmutableList.of(
             attr("tags", Type.STRING_LIST).build(),
             attr("size", Type.STRING).build(),
@@ -759,8 +759,7 @@
     private ImplicitOutputsFunction implicitOutputsFunction = ImplicitOutputsFunction.NONE;
     private TransitionFactory<Rule> transitionFactory;
     private ConfiguredTargetFactory<?, ?, ?> configuredTargetFactory = null;
-    private PredicateWithMessage<Rule> validityPredicate =
-        PredicatesWithMessage.<Rule>alwaysTrue();
+    private PredicateWithMessage<Rule> validityPredicate = PredicatesWithMessage.alwaysTrue();
     private Predicate<String> preferredDependencyPredicate = Predicates.alwaysFalse();
     private final AdvertisedProviderSet.Builder advertisedProviders =
         AdvertisedProviderSet.builder();
@@ -1035,7 +1034,7 @@
      */
     public Builder requiresConfigurationFragments(Class<?>... configurationFragments) {
       configurationFragmentPolicy.requiresConfigurationFragments(
-          ImmutableSet.<Class<?>>copyOf(configurationFragments));
+          ImmutableSet.copyOf(configurationFragments));
       return this;
     }
 
@@ -1053,8 +1052,7 @@
     public Builder requiresConfigurationFragments(ConfigurationTransition transition,
         Class<?>... configurationFragments) {
       configurationFragmentPolicy.requiresConfigurationFragments(
-          transition,
-          ImmutableSet.<Class<?>>copyOf(configurationFragments));
+          transition, ImmutableSet.copyOf(configurationFragments));
       return this;
     }
 
@@ -1175,7 +1173,7 @@
      * #cfg(TransitionFactory)}.
      */
     public Builder cfg(PatchTransition transition) {
-      return cfg((TransitionFactory<Rule>) unused -> transition);
+      return cfg(unused -> transition);
     }
 
     /**
@@ -2112,7 +2110,7 @@
     Rule rule =
         pkgBuilder.createRule(ruleLabel, this, location, callstack, implicitOutputsFunction);
     populateRuleAttributeValues(rule, pkgBuilder, attributeValues, NullEventHandler.INSTANCE);
-    rule.populateOutputFilesUnchecked(NullEventHandler.INSTANCE, pkgBuilder);
+    rule.populateOutputFilesUnchecked(pkgBuilder);
     return rule;
   }
 
@@ -2335,7 +2333,7 @@
     // attribute gets an '$implicit_tests' attribute, whose value is a shared per-package list of
     // all test labels, populated later.
     // TODO(blaze-rules-team): This should be in test_suite's implementation, not here.
-    if (this.name.equals("test_suite") && !this.isStarlark()) {
+    if (this.name.equals("test_suite") && !this.isStarlark) {
       Attribute implicitTests = this.getAttributeByName("$implicit_tests");
       NonconfigurableAttributeMapper attributeMapper = NonconfigurableAttributeMapper.of(rule);
       if (implicitTests != null && attributeMapper.get("tests", BuildType.LABEL_LIST).isEmpty()) {
@@ -2780,6 +2778,6 @@
   // https://docs.google.com/document/d/1uwBuhAoBNrw8tmFs-NxlssI6VRolidGYdYqagLqHWt8/edit#
   // TODO(b/183637322) consider this further
   public boolean isBazelLicense() {
-    return getName().equals("_license") && hasAttr("license_kinds", BuildType.LABEL_LIST);
+    return name.equals("_license") && hasAttr("license_kinds", BuildType.LABEL_LIST);
   }
 }

diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleCodec.java b/src/main/java/com/google/devtools/build/lib/packages/RuleCodec.java
index 18597b5..8b1bd3c 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleCodec.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleCodec.java

@@ -21,7 +21,6 @@
 import com.google.devtools.build.lib.skyframe.serialization.SerializationException;
 import com.google.protobuf.CodedInputStream;
 import com.google.protobuf.CodedOutputStream;
-import java.io.IOException;
 
 /**
  * Codec for {@link Rule} that throws. We expect never to serialize Rule except for in PackageCodec,
@@ -45,13 +44,13 @@
 
   @Override
   public void serialize(SerializationContext context, Rule obj, CodedOutputStream codedOut)
-      throws SerializationException, IOException {
+      throws SerializationException {
     throw new SerializationException(String.format(SERIALIZATION_ERROR_TEMPLATE, obj));
   }
 
   @Override
   public Rule deserialize(DeserializationContext context, CodedInputStream codedIn)
-      throws SerializationException, IOException {
+      throws SerializationException {
     throw new SerializationException(DESERIALIZATION_ERROR_TEMPLATE);
   }
 }

diff --git a/src/main/java/com/google/devtools/build/lib/packages/SelectorList.java b/src/main/java/com/google/devtools/build/lib/packages/SelectorList.java
index 7a66a7c..799b050 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SelectorList.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SelectorList.java

@@ -131,7 +131,7 @@
 
     for (Object value : values) {
       if (value instanceof SelectorList) {
-        elements.addAll(((SelectorList) value).getElements());
+        elements.addAll(((SelectorList) value).elements);
       } else {
         elements.add(value);
       }
@@ -152,7 +152,7 @@
 
   private static String getTypeName(Object x) {
     if (x instanceof SelectorList) {
-      return "select of " + Depset.ElementType.of(((SelectorList) x).getType());
+      return "select of " + Depset.ElementType.of(((SelectorList) x).type);
     } else if (x instanceof SelectorValue) {
       return "select of " + Depset.ElementType.of(((SelectorValue) x).getType());
     } else {
@@ -162,7 +162,7 @@
 
   private static Class<?> getNativeType(Object value) {
     if (value instanceof SelectorList) {
-      return ((SelectorList) value).getType();
+      return ((SelectorList) value).type;
     } else if (value instanceof SelectorValue) {
       return ((SelectorValue) value).getType();
     } else {
@@ -177,11 +177,8 @@
   private static boolean canConcatenate(Class<?> type1, Class<?> type2) {
     if (type1 == type2) {
       return true;
-    } else if (isListType(type1) && isListType(type2)) {
-      return true;
-    } else {
-      return false;
     }
+    return isListType(type1) && isListType(type2);
   }
 
   @Override

diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkAspectClass.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkAspectClass.java
index 9bde830..bf0b414 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkAspectClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkAspectClass.java

@@ -40,12 +40,12 @@
   }
 
   @Override
-  public final String getName() {
-    return getExtensionLabel() + "%" + getExportedName();
+  public String getName() {
+    return extensionLabel + "%" + exportedName;
   }
 
   @Override
-  public final boolean equals(Object o) {
+  public boolean equals(Object o) {
     if (this == o) {
       return true;
     }
@@ -61,8 +61,8 @@
   }
 
   @Override
-  public final int hashCode() {
-    return Objects.hash(getExtensionLabel(), getExportedName());
+  public int hashCode() {
+    return Objects.hash(extensionLabel, exportedName);
   }
 
   @Override

diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkDefinedAspect.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkDefinedAspect.java
index 4562a99..77bd274 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkDefinedAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkDefinedAspect.java

@@ -342,7 +342,7 @@
         && Objects.equals(hostTransition, that.hostTransition)
         && Objects.equals(hostFragments, that.hostFragments)
         && Objects.equals(requiredToolchains, that.requiredToolchains)
-        && Objects.equals(useToolchainTransition, that.useToolchainTransition)
+        && useToolchainTransition == that.useToolchainTransition
         && Objects.equals(aspectClass, that.aspectClass);
   }
 

diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkInfo.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkInfo.java
index 55aae7f..7a973de 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkInfo.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkInfo.java

@@ -264,7 +264,7 @@
   @Override
   public boolean isImmutable() {
     // If the provider is not yet exported, the hash code of the object is subject to change.
-    if (!getProvider().isExported()) {
+    if (!provider.isExported()) {
       return false;
     }
     for (int i = table.length / 2; i < table.length; i++) {
@@ -350,8 +350,8 @@
   }
 
   private static StarlarkInfo plus(StarlarkInfo x, StarlarkInfo y) throws EvalException {
-    Provider xprov = x.getProvider();
-    Provider yprov = y.getProvider();
+    Provider xprov = x.provider;
+    Provider yprov = y.provider;
     if (!xprov.equals(yprov)) {
       throw Starlark.errorf(
           "Cannot use '+' operator on instances of different providers (%s and %s)",

diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkLibrary.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkLibrary.java
index 5f25557..07f255f 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkLibrary.java

@@ -169,7 +169,7 @@
       private void field(String name, Object v) throws EvalException {
         // dict?
         if (v instanceof Dict) {
-          Dict<?, ?> dict = (Dict) v;
+          Dict<?, ?> dict = (Dict<?, ?>) v;
           for (Map.Entry<?, ?> entry : dict.entrySet()) {
             Object key = entry.getKey();
             if (!(key instanceof String || key instanceof StarlarkInt)) {
@@ -194,7 +194,7 @@
         // list or tuple?
         if (v instanceof Sequence) {
           int i = 0;
-          for (Object item : (Sequence) v) {
+          for (Object item : (Sequence<?>) v) {
             try {
               fieldElement(name, item);
             } catch (EvalException ex) {

diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java
index bc1e62b..438b6b2 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java

@@ -359,7 +359,7 @@
 
     if (val instanceof List) {
       List<Object> l = new ArrayList<>();
-      for (Object o : (List) val) {
+      for (Object o : (List<?>) val) {
         Object elt = starlarkifyValue(mu, o, pkg);
         if (elt == null) {
           continue;

diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkProvider.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkProvider.java
index 520054b..1256093 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkProvider.java

@@ -126,7 +126,7 @@
 
   @Override
   public Object fastcall(StarlarkThread thread, Object[] positional, Object[] named)
-      throws EvalException, InterruptedException {
+      throws EvalException {
     if (positional.length > 0) {
       throw Starlark.errorf("%s: unexpected positional arguments", getName());
     }

diff --git a/src/main/java/com/google/devtools/build/lib/packages/StructImpl.java b/src/main/java/com/google/devtools/build/lib/packages/StructImpl.java
index cf30856..282e5e0 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StructImpl.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StructImpl.java

@@ -220,7 +220,7 @@
     } else if (value instanceof List) {
       sb.append("[");
       String join = "";
-      for (Object item : ((List) value)) {
+      for (Object item : ((List<?>) value)) {
         sb.append(join);
         join = ",";
         printJson(item, sb, "list element in struct field", key);

diff --git a/src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java b/src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java
index 5221315..e05ac93 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java

@@ -290,9 +290,8 @@
                     String.class,
                     "execution_requirements"));
 
-    Map<String, String> executionInfoBuilder = new HashMap<>();
     // adding filtered execution requirements to the execution info map
-    executionInfoBuilder.putAll(checkedExecutionRequirements);
+    Map<String, String> executionInfoBuilder = new HashMap<>(checkedExecutionRequirements);
 
     if (allowTagsPropagation) {
       Map<String, String> checkedTags = getExecutionInfo(rule);
@@ -307,7 +306,7 @@
    * Returns the execution info. These include execution requirement tags ('block-*', 'requires-*',
    * 'no-*', 'supports-*', 'disable-*', 'local', and 'cpu:*') as keys with empty values.
    */
-  public static Map<String, String> filter(Map<String, String> executionInfo) {
+  private static Map<String, String> filter(Map<String, String> executionInfo) {
     return Maps.filterKeys(executionInfo, TargetUtils::legalExecInfoKeys);
   }
 

diff --git a/src/main/java/com/google/devtools/build/lib/packages/TestSize.java b/src/main/java/com/google/devtools/build/lib/packages/TestSize.java
index b43f60f..a9cd8f8 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/TestSize.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/TestSize.java

@@ -48,7 +48,7 @@
   private final TestTimeout timeout;
   private final int defaultShards;
 
-  private TestSize(TestTimeout defaultTimeout, int defaultShards) {
+  TestSize(TestTimeout defaultTimeout, int defaultShards) {
     this.timeout = defaultTimeout;
     this.defaultShards = defaultShards;
   }

diff --git a/src/main/java/com/google/devtools/build/lib/packages/TestTargetUtils.java b/src/main/java/com/google/devtools/build/lib/packages/TestTargetUtils.java
index 245177a..d6e2d2e 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/TestTargetUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/TestTargetUtils.java

@@ -128,11 +128,9 @@
         excludedTags.add(tag.substring(1));
       } else if (tag.startsWith("+")) {
         requiredTags.add(tag.substring(1));
-      } else if (tag.equals("manual")) {
-        // Ignore manual attribute because it is an exception: it is not a filter
-        // but a property of test_suite
-        continue;
-      } else {
+      } else if (!tag.equals("manual")) {
+        // Ignore manual attribute because it is an exception: it is not a filter but a property of
+        // test_suite.
         requiredTags.add(tag);
       }
     }

diff --git a/src/main/java/com/google/devtools/build/lib/packages/TestTimeout.java b/src/main/java/com/google/devtools/build/lib/packages/TestTimeout.java
index 1c24197..0b17a47 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/TestTimeout.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/TestTimeout.java

@@ -110,7 +110,7 @@
 
   private final int timeout;
 
-  private TestTimeout(int timeout) {
+  TestTimeout(int timeout) {
     this.timeout = timeout;
   }
 
@@ -202,7 +202,7 @@
         // so we can't fully emulate String.split(String, 0).
         if (!token.isEmpty() || values.size() > 1) {
           try {
-            values.add(Duration.ofSeconds(Integer.valueOf(token)));
+            values.add(Duration.ofSeconds(Integer.parseInt(token)));
           } catch (NumberFormatException e) {
             throw new OptionsParsingException("'" + input + "' is not an int");
           }

diff --git a/src/main/java/com/google/devtools/build/lib/packages/Type.java b/src/main/java/com/google/devtools/build/lib/packages/Type.java
index 4f66ce3..8bdd1b2 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Type.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Type.java

@@ -86,7 +86,7 @@
 // types, both Starlark and native.
 public abstract class Type<T> {
 
-  protected Type() {}
+  Type() {}
 
   /**
    * Converts a legal Starlark value x into an Java value of type T.
@@ -285,7 +285,7 @@
     }
 
     /** Contructs a conversion error. Throws NullPointerException if value is null. */
-    public ConversionException(Type<?> type, Object value, @Nullable Object what) {
+    ConversionException(Type<?> type, Object value, @Nullable Object what) {
       super(message(type, Preconditions.checkNotNull(value), what));
     }
 
@@ -523,7 +523,7 @@
       return new DictType<>(keyType, valueType, labelClass);
     }
 
-    protected DictType(Type<KeyT> keyType, Type<ValueT> valueType, LabelClass labelClass) {
+    DictType(Type<KeyT> keyType, Type<ValueT> valueType, LabelClass labelClass) {
       this.keyType = keyType;
       this.valueType = valueType;
       this.labelClass = labelClass;
@@ -738,7 +738,7 @@
         throws ConversionException {
       // TODO(adonovan): converge on Starlark.toIterable.
       if (x instanceof Sequence) {
-        return ((Sequence) x).getImmutableList();
+        return ((Sequence<Object>) x).getImmutableList();
       } else if (x instanceof List) {
         return (List<Object>) x;
       } else if (x instanceof Iterable) {

diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
index c7b60c9..f46a53f 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java

@@ -56,8 +56,6 @@
   private final Path defaultSystemJavabaseDir;
   private final Mutability mutability;
 
-  private final RuleFactory ruleFactory;
-
   private final WorkspaceGlobals workspaceGlobals;
   private final StarlarkSemantics starlarkSemantics;
   private final ImmutableMap<String, Object> workspaceFunctions;
@@ -93,7 +91,7 @@
     this.workspaceDir = workspaceDir;
     this.defaultSystemJavabaseDir = defaultSystemJavabaseDir;
     this.environmentExtensions = environmentExtensions;
-    this.ruleFactory = new RuleFactory(ruleClassProvider);
+    RuleFactory ruleFactory = new RuleFactory(ruleClassProvider);
     this.workspaceGlobals = new WorkspaceGlobals(allowOverride, ruleFactory);
     this.starlarkSemantics = starlarkSemantics;
     this.workspaceFunctions =

diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java
index 39997b6..233dd0c 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java

@@ -35,7 +35,6 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeMap;
-import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import javax.annotation.Nullable;
 import net.starlark.java.eval.Dict;
@@ -86,7 +85,7 @@
     Package.Builder builder = PackageFactory.getContext(thread).pkgBuilder;
     RuleClass localRepositoryRuleClass = ruleFactory.getRuleClass("local_repository");
     RuleClass bindRuleClass = ruleFactory.getRuleClass("bind");
-    Map<String, Object> kwargs = ImmutableMap.<String, Object>of("name", name, "path", ".");
+    Map<String, Object> kwargs = ImmutableMap.of("name", name, "path", ".");
     try {
       // This effectively adds a "local_repository(name = "<ws>", path = ".")"
       // definition to the WORKSPACE file.
@@ -110,7 +109,7 @@
 
   @Override
   public void dontSymlinkDirectoriesInExecroot(Sequence<?> paths, StarlarkThread thread)
-      throws EvalException, InterruptedException {
+      throws EvalException {
     List<String> pathsList = Sequence.cast(paths, String.class, "paths");
     Set<String> set = Sets.newHashSet();
     for (String path : pathsList) {
@@ -245,7 +244,7 @@
 
   @Override
   public void registerExecutionPlatforms(Sequence<?> platformLabels, StarlarkThread thread)
-      throws EvalException, InterruptedException {
+      throws EvalException {
     // Add to the package definition for later.
     Package.Builder builder = PackageFactory.getContext(thread).pkgBuilder;
     List<String> patterns = Sequence.cast(platformLabels, String.class, "platform_labels");
@@ -254,7 +253,7 @@
 
   @Override
   public void registerToolchains(Sequence<?> toolchainLabels, StarlarkThread thread)
-      throws EvalException, InterruptedException {
+      throws EvalException {
     // Add to the package definition for later.
     Package.Builder builder = PackageFactory.getContext(thread).pkgBuilder;
     List<String> patterns = Sequence.cast(toolchainLabels, String.class, "toolchain_labels");
@@ -289,7 +288,6 @@
    * Returns true if the given name is a valid workspace name.
    */
   public static boolean isLegalWorkspaceName(String name) {
-    Matcher matcher = LEGAL_WORKSPACE_NAME.matcher(name);
-    return matcher.matches();
+    return LEGAL_WORKSPACE_NAME.matcher(name).matches();
   }
 }

diff --git a/src/main/java/com/google/devtools/build/lib/packages/metrics/ExtremaPackageMetricsRecorder.java b/src/main/java/com/google/devtools/build/lib/packages/metrics/ExtremaPackageMetricsRecorder.java
index dc1a3bf..475344a 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/metrics/ExtremaPackageMetricsRecorder.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/metrics/ExtremaPackageMetricsRecorder.java

@@ -173,7 +173,7 @@
             largestPackages.getExtremeElements().stream(),
             packagesWithMostTransitiveLoads.getExtremeElements().stream(),
             packagesWithMostOverhead.getExtremeElements().stream())
-        .map(c -> c.getPackageMetrics())
+        .map(PackageMetricsContainer::getPackageMetrics)
         .collect(toImmutableSet());
   }
 

diff --git a/src/main/java/com/google/devtools/build/lib/packages/metrics/PackageMetricsPackageLoadingListener.java b/src/main/java/com/google/devtools/build/lib/packages/metrics/PackageMetricsPackageLoadingListener.java
index 038801e..053acc4 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/metrics/PackageMetricsPackageLoadingListener.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/metrics/PackageMetricsPackageLoadingListener.java

@@ -24,7 +24,7 @@
 public class PackageMetricsPackageLoadingListener implements PackageLoadingListener {
 
   @GuardedBy("this")
-  private PackageMetricsRecorder recorder = null;
+  private PackageMetricsRecorder recorder;
 
   @GuardedBy("PackageMetricsPackageLoadingListener.class")
   private static PackageMetricsPackageLoadingListener instance = null;

diff --git a/src/main/java/com/google/devtools/build/lib/packages/metrics/PackageMetricsRecorder.java b/src/main/java/com/google/devtools/build/lib/packages/metrics/PackageMetricsRecorder.java
index b2c8819..79ea6c6 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/metrics/PackageMetricsRecorder.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/metrics/PackageMetricsRecorder.java

@@ -22,7 +22,7 @@
 public interface PackageMetricsRecorder {
 
   /** What type of packages are metrics being recorded for? */
-  public enum Type {
+  enum Type {
     ONLY_EXTREMES,
     ALL,
   }