Fix code warnings around rule/package factories.

Mostly "unused parameter" and "field can be converted to local variable".

PiperOrigin-RevId: 518961876
Change-Id: I0b0dd5e6a1af69577fcb2ac27c603c1e0314d4fd

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java

@@ -308,7 +308,6 @@ public StarlarkRuleFunction rule(
         executable,
         outputToGenfiles,
         fragments,
-        hostFragments,
         starlarkTestable,
         toolchains,
         doc,
@@ -331,7 +330,6 @@ public static StarlarkRuleFunction createRule(
       boolean executable,
       boolean outputToGenfiles,
       Sequence<?> fragments,
-      Sequence<?> hostFragments,
       boolean starlarkTestable,
       Sequence<?> toolchains,
       Object doc,
@@ -824,7 +822,6 @@ public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwarg
             ruleClass,
             attributeValues,
             pkgContext.getEventHandler(),
-            thread.getSemantics(),
             thread.getCallStack());
       } catch (InvalidRuleException | NameConflictException e) {
         throw new EvalException(e);

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java

@@ -76,7 +76,7 @@ public static Rule createRule(
     try {
       rule =
           RuleFactory.createAndAddRule(
-              packageBuilder, ruleClass, attributeValues, eventHandler, semantics, callStack);
+              packageBuilder, ruleClass, attributeValues, eventHandler, callStack);
     } catch (NameConflictException e) {
       // This literally cannot happen -- we just created the package!
       throw new IllegalStateException(e);

src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java

@@ -39,7 +39,6 @@
 import com.google.devtools.build.lib.packages.Package.NameConflictException;
 import com.google.devtools.build.lib.packages.PackageFactory;
 import com.google.devtools.build.lib.packages.PackageFactory.PackageContext;
-import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
 import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException;
@@ -225,17 +224,14 @@ private Object createRuleLegacy(StarlarkThread thread, Dict<String, Object> kwar
 
         // TODO(adonovan): is this cast safe? Check.
         String name = (String) kwargs.get("name");
-        WorkspaceFactoryHelper.addMainRepoEntry(packageBuilder, name, thread.getSemantics());
+        WorkspaceFactoryHelper.addMainRepoEntry(packageBuilder, name);
         WorkspaceFactoryHelper.addRepoMappings(packageBuilder, kwargs, name);
-        Rule rule =
-            WorkspaceFactoryHelper.createAndAddRepositoryRule(
-                context.getBuilder(),
-                ruleClass,
-                /* bindRuleClass= */ null,
-                WorkspaceFactoryHelper.getFinalKwargs(kwargs),
-                thread.getSemantics(),
-                thread.getCallStack());
-        return rule;
+        return WorkspaceFactoryHelper.createAndAddRepositoryRule(
+            context.getBuilder(),
+            ruleClass,
+            /* bindRuleClass= */ null,
+            WorkspaceFactoryHelper.getFinalKwargs(kwargs),
+            thread.getCallStack());
       } catch (InvalidRuleException | NameConflictException | LabelSyntaxException e) {
         throw Starlark.errorf("%s", e.getMessage());
       }

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

@@ -123,8 +123,6 @@ public interface EnvironmentExtension {
   private int maxDirectoriesToEagerlyVisitInGlobbing;
 
   private final ImmutableList<EnvironmentExtension> environmentExtensions;
-  private final ImmutableMap<String, PackageArgument<?>> packageArguments;
-
   private final PackageSettings packageSettings;
   private final PackageValidator packageValidator;
   private final PackageOverheadEstimator packageOverheadEstimator;
@@ -202,7 +200,6 @@ public PackageFactory(
     this.ruleClassProvider = ruleClassProvider;
     this.executor = executorForGlobbing;
     this.environmentExtensions = ImmutableList.copyOf(environmentExtensions);
-    this.packageArguments = createPackageArguments(this.environmentExtensions);
     this.packageSettings = packageSettings;
     this.packageValidator = packageValidator;
     this.packageOverheadEstimator = packageOverheadEstimator;
@@ -212,7 +209,7 @@ public PackageFactory(
             ruleClassProvider,
             buildRuleFunctions(ruleFactory),
             this.environmentExtensions,
-            newPackageFunction(packageArguments),
+            newPackageFunction(createPackageArguments(this.environmentExtensions)),
             version);
   }
 
@@ -415,7 +412,6 @@ public NoneType call(StarlarkThread thread, Tuple args, Dict<String, Object> kwa
             ruleClass,
             new BuildLangTypedAttributeValuesMap(kwargs),
             context.eventHandler,
-            thread.getSemantics(),
             thread.getCallStack());
       } catch (RuleFactory.InvalidRuleException | Package.NameConflictException e) {
         throw new EvalException(e);
@@ -773,7 +769,7 @@ void extractGlobPatterns(CallExpression call) {
 
               Expression excludeDirectories = null;
               Expression include = null;
-              List<Argument> arguments = call.getArguments();
+              ImmutableList<Argument> arguments = call.getArguments();
               for (int i = 0; i < arguments.size(); i++) {
                 Argument arg = arguments.get(i);
                 String name = arg.getName();

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

@@ -22,9 +22,9 @@
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.packages.Attribute.StarlarkComputedDefaultTemplate.CannotPrecomputeDefaultsException;
 import com.google.devtools.build.lib.packages.Package.NameConflictException;
+import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import java.util.Map;
 import java.util.Set;
-import net.starlark.java.eval.StarlarkSemantics;
 import net.starlark.java.eval.StarlarkThread;
 import net.starlark.java.eval.StarlarkThread.CallStackEntry;
 import net.starlark.java.syntax.Location;
@@ -70,7 +70,6 @@ public static Rule createRule(
       RuleClass ruleClass,
       BuildLangTypedAttributeValuesMap attributeValues,
       EventHandler eventHandler,
-      StarlarkSemantics semantics,
       ImmutableList<StarlarkThread.CallStackEntry> callstack)
       throws InvalidRuleException, InterruptedException {
     Preconditions.checkNotNull(ruleClass);
@@ -131,24 +130,22 @@ public static Rule createRule(
    *     a map entry for each non-optional attribute of this class of rule.
    * @param eventHandler a eventHandler on which errors and warnings are reported during rule
    *     creation
-   * @param semantics the Starlark semantics
    * @param callstack the stack of active calls in the Starlark thread
    * @throws InvalidRuleException if the rule could not be constructed for any reason (e.g. no
    *     {@code name} attribute is defined)
    * @throws NameConflictException if the rule's name or output files conflict with others in this
    *     package
    * @throws InterruptedException if interrupted
    */
+  @CanIgnoreReturnValue
   public static Rule createAndAddRule(
       Package.Builder pkgBuilder,
       RuleClass ruleClass,
       BuildLangTypedAttributeValuesMap attributeValues,
       EventHandler eventHandler,
-      StarlarkSemantics semantics,
       ImmutableList<StarlarkThread.CallStackEntry> callstack)
       throws InvalidRuleException, NameConflictException, InterruptedException {
-    Rule rule =
-        createRule(pkgBuilder, ruleClass, attributeValues, eventHandler, semantics, callstack);
+    Rule rule = createRule(pkgBuilder, ruleClass, attributeValues, eventHandler, callstack);
     pkgBuilder.addRule(rule);
     return rule;
   }
@@ -218,7 +215,7 @@ public boolean valuesAreBuildLanguageTyped() {
     }
 
     @Override
-    public Iterable<Map.Entry<String, Object>> getAttributeAccessors() {
+    public Set<Map.Entry<String, Object>> getAttributeAccessors() {
       return attributeValues.entrySet();
     }
 

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

@@ -56,7 +56,6 @@ public class WorkspaceFactory {
   private final Path defaultSystemJavabaseDir;
   private final Mutability mutability;
 
-  private final WorkspaceGlobals workspaceGlobals;
   private final StarlarkSemantics starlarkSemantics;
   private final ImmutableMap<String, Object> workspaceFunctions;
   private final ImmutableList<EnvironmentExtension> environmentExtensions;
@@ -91,12 +90,14 @@ public WorkspaceFactory(
     this.workspaceDir = workspaceDir;
     this.defaultSystemJavabaseDir = defaultSystemJavabaseDir;
     this.environmentExtensions = environmentExtensions;
-    RuleFactory ruleFactory = new RuleFactory(ruleClassProvider);
-    this.workspaceGlobals = new WorkspaceGlobals(allowOverride, ruleFactory);
     this.starlarkSemantics = starlarkSemantics;
+    RuleFactory ruleFactory = new RuleFactory(ruleClassProvider);
     this.workspaceFunctions =
         createWorkspaceFunctions(
-            allowOverride, ruleFactory, this.workspaceGlobals, starlarkSemantics);
+            allowOverride,
+            ruleFactory,
+            new WorkspaceGlobals(allowOverride, ruleFactory),
+            starlarkSemantics);
   }
 
   /**
@@ -281,7 +282,7 @@ public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwarg
           // Add an entry in every repository from @<mainRepoName> to "@" to avoid treating
           // @<mainRepoName> as a separate repository. This will be overridden if the main
           // repository has a repo_mapping entry from <mainRepoName> to something.
-          WorkspaceFactoryHelper.addMainRepoEntry(builder, externalRepoName, thread.getSemantics());
+          WorkspaceFactoryHelper.addMainRepoEntry(builder, externalRepoName);
           WorkspaceFactoryHelper.addRepoMappings(builder, kwargs, externalRepoName);
           RuleClass ruleClass = ruleFactory.getRuleClass(ruleClassName);
           RuleClass bindRuleClass = ruleFactory.getRuleClass("bind");
@@ -291,7 +292,6 @@ public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwarg
                   ruleClass,
                   bindRuleClass,
                   WorkspaceFactoryHelper.getFinalKwargs(kwargs),
-                  thread.getSemantics(),
                   thread.getCallStack());
           RepositoryName.validateUserProvidedRepoName(rule.getName());
         } catch (RuleFactory.InvalidRuleException

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

@@ -26,36 +26,37 @@
 import com.google.devtools.build.lib.cmdline.TargetPattern;
 import com.google.devtools.build.lib.events.StoredEventHandler;
 import com.google.devtools.build.lib.packages.RuleFactory.BuildLangTypedAttributeValuesMap;
+import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import java.util.Map;
 import java.util.stream.Collectors;
 import net.starlark.java.eval.Dict;
 import net.starlark.java.eval.EvalException;
-import net.starlark.java.eval.StarlarkSemantics;
 import net.starlark.java.eval.StarlarkThread;
 
 /** A helper for the {@link WorkspaceFactory} to create repository rules */
 public final class WorkspaceFactoryHelper {
 
+  @CanIgnoreReturnValue
   public static Rule createAndAddRepositoryRule(
       Package.Builder pkg,
       RuleClass ruleClass,
       RuleClass bindRuleClass,
       Map<String, Object> kwargs,
-      StarlarkSemantics semantics,
       ImmutableList<StarlarkThread.CallStackEntry> callstack)
-      throws RuleFactory.InvalidRuleException, Package.NameConflictException, LabelSyntaxException,
+      throws RuleFactory.InvalidRuleException,
+          Package.NameConflictException,
+          LabelSyntaxException,
           InterruptedException {
     StoredEventHandler eventHandler = new StoredEventHandler();
     BuildLangTypedAttributeValuesMap attributeValues = new BuildLangTypedAttributeValuesMap(kwargs);
-    Rule rule =
-        RuleFactory.createRule(pkg, ruleClass, attributeValues, eventHandler, semantics, callstack);
+    Rule rule = RuleFactory.createRule(pkg, ruleClass, attributeValues, eventHandler, callstack);
     pkg.addEvents(eventHandler.getEvents());
     pkg.addPosts(eventHandler.getPosts());
     overwriteRule(pkg, rule);
     for (Map.Entry<String, Label> entry :
         ruleClass.getExternalBindingsFunction().apply(rule).entrySet()) {
       Label nameLabel = Label.parseCanonical("//external:" + entry.getKey());
-      addBindRule(pkg, bindRuleClass, nameLabel, entry.getValue(), semantics, callstack);
+      addBindRule(pkg, bindRuleClass, nameLabel, entry.getValue(), callstack);
     }
     // NOTE(wyv): What is this madness?? This is the only instance where a repository rule can
     // register toolchains upon being instantiated. We should look into converting
@@ -73,7 +74,7 @@ public static Rule createAndAddRepositoryRule(
   }
 
   /**
-   * Updates the map of attributes specified by the user to match the set of attributes decared in
+   * Updates the map of attributes specified by the user to match the set of attributes declared in
    * the rule definition.
    */
   public static Map<String, Object> getFinalKwargs(Map<String, Object> kwargs) {
@@ -96,8 +97,7 @@ public static Map<String, Object> getFinalKwargs(Map<String, Object> kwargs) {
    * Additionally, the labels {@code @foo//:bar} and {@code @//:bar} from an external repository
    * should also evaluate to the same thing.
    */
-  public static void addMainRepoEntry(
-      Package.Builder builder, String externalRepoName, StarlarkSemantics semantics)
+  public static void addMainRepoEntry(Package.Builder builder, String externalRepoName)
       throws LabelSyntaxException {
     if (!Strings.isNullOrEmpty(builder.getPackageWorkspaceName())) {
       // Create repository names with validation, LabelSyntaxException is thrown is the name
@@ -151,7 +151,6 @@ static void addBindRule(
       RuleClass bindRuleClass,
       Label virtual,
       Label actual,
-      StarlarkSemantics semantics,
       ImmutableList<StarlarkThread.CallStackEntry> callstack)
       throws RuleFactory.InvalidRuleException, Package.NameConflictException, InterruptedException {
 
@@ -165,8 +164,7 @@ static void addBindRule(
     StoredEventHandler handler = new StoredEventHandler();
     BuildLangTypedAttributeValuesMap attributeValues =
         new BuildLangTypedAttributeValuesMap(attributes);
-    Rule rule =
-        RuleFactory.createRule(pkg, bindRuleClass, attributeValues, handler, semantics, callstack);
+    Rule rule = RuleFactory.createRule(pkg, bindRuleClass, attributeValues, handler, callstack);
     overwriteRule(pkg, rule);
   }
 

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

@@ -31,7 +31,6 @@
 import com.google.devtools.build.lib.starlarkbuildapi.WorkspaceGlobalsApi;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.List;
-import java.util.Map;
 import javax.annotation.Nullable;
 import net.starlark.java.eval.EvalException;
 import net.starlark.java.eval.Module;
@@ -68,7 +67,7 @@ public void workspace(
     Package.Builder builder = PackageFactory.getContext(thread).pkgBuilder;
     RuleClass localRepositoryRuleClass = ruleFactory.getRuleClass("local_repository");
     RuleClass bindRuleClass = ruleFactory.getRuleClass("bind");
-    Map<String, Object> kwargs = ImmutableMap.of("name", name, "path", ".");
+    ImmutableMap<String, Object> kwargs = ImmutableMap.of("name", name, "path", ".");
     try {
       // This effectively adds a "local_repository(name = "<ws>", path = ".")"
       // definition to the WORKSPACE file.
@@ -77,7 +76,6 @@ public void workspace(
           localRepositoryRuleClass,
           bindRuleClass,
           kwargs,
-          thread.getSemantics(),
           thread.getCallStack());
     } catch (InvalidRuleException | NameConflictException | LabelSyntaxException e) {
       throw Starlark.errorf("%s", e.getMessage());
@@ -93,7 +91,7 @@ private static RepositoryName getRepositoryName(@Nullable Label label) {
       return RepositoryName.MAIN;
     }
 
-    // registeration happened in a loaded bzl file
+    // registration happened in a loaded bzl file
     return label.getRepository();
   }
 
@@ -151,7 +149,6 @@ public void bind(String name, Object actual, StarlarkThread thread)
           ruleClass,
           nameLabel,
           actual == NONE ? null : Label.parseCanonical((String) actual),
-          thread.getSemantics(),
           thread.getCallStack());
     } catch (InvalidRuleException | Package.NameConflictException | LabelSyntaxException e) {
       throw Starlark.errorf("%s", e.getMessage());

src/main/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModule.java

@@ -80,7 +80,6 @@ public void analysisTest(
             /* executable= */ false,
             /* outputToGenfiles= */ false,
             /* fragments= */ fragments,
-            /* hostFragments= */ StarlarkList.empty(),
             /* starlarkTestable= */ false,
             /* toolchains= */ toolchains,
             /* doc= */ Starlark.NONE,