Eliminate PackageArgument

The package() function is now implemented by a @StarlarkMethod rather than an anonymous subclass of StarlarkCallable.

The PackageArgument machinery is deleted. The convert-and-process approach is instead inlined into a series of if-elses for each argument (with the convert step using a helper to reduce repetition). The big if-else is factored out into an overridable method, so other implementations of package() can add their own parameters on top.

The package() symbol is now registered on the ConfiguredRuleClassProvider like any other BUILD toplevel. Removed its special casing in BazelStarlarkEnvironment, and the ability to register PackageArguments with the CRCP.

Fixed a copy-paste error where the "default_applicable_licenses" param would claim to be "default_package_metadata" for the purposes of error messages.

PiperOrigin-RevId: 532911239
Change-Id: I957354e0a4e32f7922b510c5537f56c6aff72ab9
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
index 2518797..8f8e5d6 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
@@ -49,8 +49,6 @@
 import com.google.devtools.build.lib.graph.Node;
 import com.google.devtools.build.lib.packages.BazelStarlarkEnvironment;
 import com.google.devtools.build.lib.packages.NativeAspectClass;
-import com.google.devtools.build.lib.packages.PackageArgument;
-import com.google.devtools.build.lib.packages.PackageCallable;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleClassProvider;
 import com.google.devtools.build.lib.packages.RuleFactory;
@@ -157,7 +155,6 @@
     private final Map<String, RuleClass> ruleClassMap = new HashMap<>();
     private final Map<String, RuleDefinition> ruleDefinitionMap = new HashMap<>();
     private final Map<String, NativeAspectClass> nativeAspectClassMap = new HashMap<>();
-    private final List<PackageArgument<?>> packageArgs = new ArrayList<>();
     private final Map<Class<? extends RuleDefinition>, RuleClass> ruleMap = new HashMap<>();
     private final Digraph<Class<? extends RuleDefinition>> dependencyGraph = new Digraph<>();
     private final List<Class<? extends Fragment>> universalFragments = new ArrayList<>();
@@ -306,12 +303,6 @@
       return this;
     }
 
-    @CanIgnoreReturnValue
-    public Builder addPackageArg(PackageArgument<?> arg) {
-      packageArgs.add(arg);
-      return this;
-    }
-
     /**
      * Adds a configuration fragment and all build options required by its fragment.
      *
@@ -608,7 +599,6 @@
           ImmutableMap.copyOf(ruleClassMap),
           ImmutableMap.copyOf(ruleDefinitionMap),
           ImmutableMap.copyOf(nativeAspectClassMap),
-          ImmutableList.copyOf(packageArgs),
           FragmentRegistry.create(
               configurationFragmentClasses, universalFragments, configurationOptions),
           defaultWorkspaceFilePrefix.toString(),
@@ -728,7 +718,6 @@
       ImmutableMap<String, RuleClass> ruleClassMap,
       ImmutableMap<String, RuleDefinition> ruleDefinitionMap,
       ImmutableMap<String, NativeAspectClass> nativeAspectClassMap,
-      ImmutableList<PackageArgument<?>> packageArgs,
       FragmentRegistry fragmentRegistry,
       String defaultWorkspaceFilePrefix,
       String defaultWorkspaceFileSuffix,
@@ -780,10 +769,6 @@
     this.bazelStarlarkEnvironment =
         new BazelStarlarkEnvironment(
             /* ruleFunctions= */ RuleFactory.buildRuleFunctions(ruleClassMap),
-            // TODO(b/280446865): Instead of exposing the {@code package()} symbol separately, just
-            // keep it part of the BUILD/native environments. Requires migrating {@code package()}
-            // to be a @StarlarkMethod that gets registered as a BUILD toplevel.
-            /* packageCallable= */ PackageCallable.newPackageCallable(packageArgs),
             buildFileToplevels,
             /* bzlToplevels= */ environment,
             /* nativeRuleSpecificBindings= */ nativeRuleSpecificBindings,
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/rules/BUILD
index 158b025..ddb8968 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BUILD
@@ -64,6 +64,7 @@
         "//src/main/java/com/google/devtools/build/lib/exec:module_action_context_registry",
         "//src/main/java/com/google/devtools/build/lib/exec:spawn_cache",
         "//src/main/java/com/google/devtools/build/lib/exec:spawn_strategy_registry",
+        "//src/main/java/com/google/devtools/build/lib/packages",
         "//src/main/java/com/google/devtools/build/lib/packages/semantics",
         "//src/main/java/com/google/devtools/build/lib/remote",
         "//src/main/java/com/google/devtools/build/lib/remote/options",
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
index b1b3ee4..0b64005 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
@@ -59,6 +59,7 @@
 import com.google.devtools.build.lib.bazel.rules.python.BazelPythonConfiguration;
 import com.google.devtools.build.lib.cmdline.LabelConstants;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
+import com.google.devtools.build.lib.packages.PackageCallable;
 import com.google.devtools.build.lib.rules.android.AarImportBaseRule;
 import com.google.devtools.build.lib.rules.android.AndroidApplicationResourceInfo;
 import com.google.devtools.build.lib.rules.android.AndroidAssetsInfo;
@@ -308,6 +309,15 @@
           builder.addStarlarkBuiltinsInternal(CcStarlarkInternal.NAME, new CcStarlarkInternal());
           builder.addStarlarkBuiltinsInternal(
               BazelObjcStarlarkInternal.NAME, new BazelObjcStarlarkInternal());
+
+          // Add the package() function.
+          // TODO(bazel-team): Factor this into a group of similar BUILD definitions, or add a more
+          // convenient way of obtaining a BuiltinFunction than addMethods().
+          ImmutableMap.Builder<String, Object> symbols = ImmutableMap.builder();
+          Starlark.addMethods(symbols, PackageCallable.INSTANCE);
+          for (Map.Entry<String, Object> entry : symbols.buildOrThrow().entrySet()) {
+            builder.addBuildFileToplevel(entry.getKey(), entry.getValue());
+          }
         }
       };
 
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 f17325d..b8b5f08 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
@@ -81,7 +81,6 @@
    *
    * @param ruleFunctions a map from a rule class name (e.g. "java_library") to the (uninjected)
    *     Starlark callable that instantiates it
-   * @param packageCallable the symbol implementing the {@code package()} function in BUILD files
    * @param buildFileToplevels the map of non-{@link Starlark#UNIVERSE} top-level symbols available
    *     to BUILD files, prior to builtins injection
    * @param bzlToplevels the map of non-universe top-level symbols available to .bzl files, prior to
@@ -97,7 +96,6 @@
    */
   public BazelStarlarkEnvironment(
       ImmutableMap<String, ?> ruleFunctions,
-      Object packageCallable,
       ImmutableMap<String, Object> buildFileToplevels,
       ImmutableMap<String, Object> bzlToplevels,
       ImmutableMap<String, Object> nativeRuleSpecificBindings,
@@ -110,7 +108,7 @@
     this.workspaceBzlNativeBindings = workspaceBzlNativeBindings;
 
     this.uninjectedBuildBzlNativeBindings =
-        createUninjectedBuildBzlNativeBindings(ruleFunctions, packageCallable, buildFileToplevels);
+        createUninjectedBuildBzlNativeBindings(ruleFunctions, buildFileToplevels);
     this.uninjectedBuildBzlEnv =
         createUninjectedBuildBzlEnv(bzlToplevels, uninjectedBuildBzlNativeBindings);
     this.uninjectedWorkspaceBzlEnv =
@@ -126,8 +124,7 @@
             builtinsInternals,
             uninjectedBuildBzlNativeBindings,
             uninjectedBuildBzlEnv);
-    this.uninjectedBuildEnv =
-        createUninjectedBuildEnv(ruleFunctions, packageCallable, buildFileToplevels);
+    this.uninjectedBuildEnv = createUninjectedBuildEnv(ruleFunctions, buildFileToplevels);
   }
 
   /**
@@ -186,13 +183,10 @@
    * injection didn't happen.
    */
   private static ImmutableMap<String, Object> createUninjectedBuildBzlNativeBindings(
-      Map<String, ?> ruleFunctions,
-      Object packageCallable,
-      Map<String, Object> buildFileToplevels) {
+      Map<String, ?> ruleFunctions, Map<String, Object> buildFileToplevels) {
     ImmutableMap.Builder<String, Object> env = new ImmutableMap.Builder<>();
     env.putAll(StarlarkNativeModule.BINDINGS_FOR_BUILD_FILES);
     env.putAll(ruleFunctions);
-    env.put("package", packageCallable);
     env.putAll(buildFileToplevels);
     return env.buildOrThrow();
   }
@@ -217,13 +211,10 @@
   }
 
   private static ImmutableMap<String, Object> createUninjectedBuildEnv(
-      Map<String, ?> ruleFunctions,
-      Object packageCallable,
-      Map<String, Object> buildFileToplevels) {
+      Map<String, ?> ruleFunctions, Map<String, Object> buildFileToplevels) {
     ImmutableMap.Builder<String, Object> env = ImmutableMap.builder();
     env.putAll(StarlarkLibrary.BUILD); // e.g. select, depset
     env.putAll(StarlarkNativeModule.BINDINGS_FOR_BUILD_FILES);
-    env.put("package", packageCallable);
     env.putAll(ruleFunctions);
     env.putAll(buildFileToplevels);
     return env.buildOrThrow();
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageArgument.java b/src/main/java/com/google/devtools/build/lib/packages/PackageArgument.java
deleted file mode 100644
index a23150a..0000000
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageArgument.java
+++ /dev/null
@@ -1,57 +0,0 @@
-// Copyright 2020 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//    http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.devtools.build.lib.packages;
-
-import net.starlark.java.eval.EvalException;
-import net.starlark.java.syntax.Location;
-
-/** Defines an argument to the {@code package()} function. */
-public abstract class PackageArgument<T> {
-  private final String name;
-  private final Type<T> type;
-
-  protected PackageArgument(String name, Type<T> type) {
-    this.name = name;
-    this.type = type;
-  }
-
-  String getName() {
-    return name;
-  }
-
-  /**
-   * Converts an untyped argument to a typed one, then calls the user-provided {@link #process}.
-   *
-   * Note that the location is used not just for exceptions (for which null would do), but also for
-   * reporting events.
-   */
-  final void convertAndProcess(
-      Package.Builder pkgBuilder, Location location, Object value)
-      throws EvalException {
-    T typedValue = type.convert(value, "'package' argument", pkgBuilder.getLabelConverter());
-    process(pkgBuilder, location, typedValue);
-  }
-
-  /**
-   * Processes an argument.
-   *
-   * @param pkgBuilder the package builder to be mutated
-   * @param location the location of the {@code package} function for error reporting
-   * @param value the value of the argument. Typically passed to {@link Type#convert}
-   */
-  protected abstract void process(
-      Package.Builder pkgBuilder, Location location, T value)
-      throws EvalException;
-}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageCallable.java b/src/main/java/com/google/devtools/build/lib/packages/PackageCallable.java
index 6390926..3f8db53 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageCallable.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageCallable.java
@@ -14,276 +14,124 @@
 
 package com.google.devtools.build.lib.packages;
 
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.License.DistributionType;
 import com.google.devtools.build.lib.server.FailureDetails.PackageLoading.Code;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import net.starlark.java.eval.Dict;
+import net.starlark.java.annot.Param;
+import net.starlark.java.annot.StarlarkMethod;
 import net.starlark.java.eval.EvalException;
-import net.starlark.java.eval.Printer;
 import net.starlark.java.eval.Starlark;
-import net.starlark.java.eval.StarlarkCallable;
 import net.starlark.java.eval.StarlarkThread;
-import net.starlark.java.eval.Tuple;
 import net.starlark.java.syntax.Location;
 
 /**
- * Utility class encapsulating the definition of the {@code package()} function of BUILD files.
- *
- * <p>Also includes the definitions of those arguments to {@code package()} that are available in
- * all Bazel environments.
+ * Utility class encapsulating the standard definition of the {@code package()} function of BUILD
+ * files.
  */
 public class PackageCallable {
 
-  private PackageCallable() {}
+  protected PackageCallable() {}
+
+  public static final PackageCallable INSTANCE = new PackageCallable();
+
+  @StarlarkMethod(
+      name = "package",
+      documented = false, // documented in docgen/templates/be/functions.vm
+      extraKeywords = @Param(name = "kwargs", defaultValue = "{}"),
+      useStarlarkThread = true)
+  public Object packageCallable(Map<String, Object> kwargs, StarlarkThread thread)
+      throws EvalException {
+    Package.Builder pkgBuilder = PackageFactory.getContext(thread).pkgBuilder;
+    if (pkgBuilder.isPackageFunctionUsed()) {
+      throw new EvalException("'package' can only be used once per BUILD file");
+    }
+    pkgBuilder.setPackageFunctionUsed();
+
+    if (kwargs.isEmpty()) {
+      throw new EvalException("at least one argument must be given to the 'package' function");
+    }
+
+    Location loc = thread.getCallerLocation();
+    for (Map.Entry<String, Object> kwarg : kwargs.entrySet()) {
+      String name = kwarg.getKey();
+      Object rawValue = kwarg.getValue();
+      processParam(name, rawValue, pkgBuilder, loc);
+    }
+    return Starlark.NONE;
+  }
 
   /**
-   * Returns a {@link StarlarkCallable} that implements the {@code package()} functionality.
-   *
-   * @param packageArgs a list of {@link PackageArgument}s to support, beyond the standard ones
-   *     included in every Bazel environment
+   * Handles one parameter. Subclasses can add new parameters by overriding this method and falling
+   * back on the super method when the parameter does not match.
    */
-  // TODO(b/280446865): Consider eliminating the package() extensibility mechanism altogether.
-  // There is currently only one use case: distinguishing the set of package() arguments available
-  // in OSS Bazel vs internally to Google. Instead of registering these arguments and passing them
-  // to this factory method to obtain a package() callable, we could instead define two
-  // @StarlarkMethod-annotated Java functions implementing the two versions of package(), and
-  // register the appropriate one with the ConfiguredRuleClassProvider.Builder.
-  public static StarlarkCallable newPackageCallable(List<PackageArgument<?>> packageArgs) {
-    // Construct a map of PackageArguments, which the returned callable closes over.
-    ImmutableMap.Builder<String, PackageArgument<?>> argsBuilder = ImmutableMap.builder();
-    for (PackageArgument<?> arg : getCommonArguments()) {
-      argsBuilder.put(arg.getName(), arg);
-    }
-    for (PackageArgument<?> arg : packageArgs) {
-      argsBuilder.put(arg.getName(), arg);
-    }
-    final ImmutableMap<String, PackageArgument<?>> packageArguments = argsBuilder.buildOrThrow();
-
-    return new StarlarkCallable() {
-      @Override
-      public String getName() {
-        return "package";
-      }
-
-      @Override
-      public String toString() {
-        return "package(...)";
-      }
-
-      @Override
-      public boolean isImmutable() {
-        return true;
-      }
-
-      @Override
-      public void repr(Printer printer) {
-        printer.append("<built-in function package>");
-      }
-
-      @Override
-      public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwargs)
-          throws EvalException {
-        if (!args.isEmpty()) {
-          throw new EvalException("unexpected positional arguments");
-        }
-        Package.Builder pkgBuilder = PackageFactory.getContext(thread).pkgBuilder;
-
-        // Validate parameter list
-        if (pkgBuilder.isPackageFunctionUsed()) {
-          throw new EvalException("'package' can only be used once per BUILD file");
-        }
-        pkgBuilder.setPackageFunctionUsed();
-
-        // Each supplied argument must name a PackageArgument.
-        if (kwargs.isEmpty()) {
-          throw new EvalException("at least one argument must be given to the 'package' function");
-        }
-        Location loc = thread.getCallerLocation();
-        for (Map.Entry<String, Object> kwarg : kwargs.entrySet()) {
-          String name = kwarg.getKey();
-          PackageArgument<?> pkgarg = packageArguments.get(name);
-          if (pkgarg == null) {
-            throw Starlark.errorf("unexpected keyword argument: %s", name);
-          }
-          pkgarg.convertAndProcess(pkgBuilder, loc, kwarg.getValue());
-        }
-        return Starlark.NONE;
-      }
-    };
-  }
-
-  /** Returns the basic set of {@link PackageArgument}s. */
-  private static ImmutableList<PackageArgument<?>> getCommonArguments() {
-    return ImmutableList.of(
-        new DefaultDeprecation(),
-        new DefaultDistribs(),
-        new DefaultApplicableLicenses(),
-        new DefaultPackageMetadata(),
-        new DefaultLicenses(),
-        new DefaultTestOnly(),
-        new DefaultVisibility(),
-        new Features(),
-        new DefaultCompatibleWith(),
-        new DefaultRestrictedTo());
-  }
-
-  private static class DefaultVisibility extends PackageArgument<List<Label>> {
-    private DefaultVisibility() {
-      super("default_visibility", BuildType.LABEL_LIST);
-    }
-
-    @Override
-    protected void process(Package.Builder pkgBuilder, Location location, List<Label> value)
-        throws EvalException {
+  protected void processParam(
+      String name, Object rawValue, Package.Builder pkgBuilder, Location loc) throws EvalException {
+    if (name.equals("default_visibility")) {
+      List<Label> value = convert(BuildType.LABEL_LIST, rawValue, pkgBuilder);
       pkgBuilder.setDefaultVisibility(RuleVisibility.parse(value));
-    }
-  }
 
-  private static class DefaultTestOnly extends PackageArgument<Boolean> {
-    private DefaultTestOnly() {
-      super("default_testonly", Type.BOOLEAN);
-    }
-
-    @Override
-    protected void process(Package.Builder pkgBuilder, Location location, Boolean value) {
+    } else if (name.equals("default_testonly")) {
+      Boolean value = convert(Type.BOOLEAN, rawValue, pkgBuilder);
       pkgBuilder.setDefaultTestonly(value);
-    }
-  }
 
-  private static class DefaultDeprecation extends PackageArgument<String> {
-    private DefaultDeprecation() {
-      super("default_deprecation", Type.STRING);
-    }
-
-    @Override
-    protected void process(Package.Builder pkgBuilder, Location location, String value) {
+    } else if (name.equals("default_deprecation")) {
+      String value = convert(Type.STRING, rawValue, pkgBuilder);
       pkgBuilder.setDefaultDeprecation(value);
-    }
-  }
 
-  private static class Features extends PackageArgument<List<String>> {
-    private Features() {
-      super("features", Type.STRING_LIST);
-    }
-
-    @Override
-    protected void process(Package.Builder pkgBuilder, Location location, List<String> value) {
+    } else if (name.equals("features")) {
+      List<String> value = convert(Type.STRING_LIST, rawValue, pkgBuilder);
       pkgBuilder.addFeatures(value);
-    }
-  }
 
-  /**
-   * Declares the package() attribute specifying the default value for {@link
-   * com.google.devtools.build.lib.packages.RuleClass#APPLICABLE_LICENSES_ATTR} when not explicitly
-   * specified.
-   */
-  private static class DefaultApplicableLicenses extends PackageArgument<List<Label>> {
-    private DefaultApplicableLicenses() {
-      super("default_applicable_licenses", BuildType.LABEL_LIST);
-    }
-
-    @Override
-    protected void process(Package.Builder pkgBuilder, Location location, List<Label> value) {
-      if (!pkgBuilder.getDefaultPackageMetadata().isEmpty()) {
-        pkgBuilder.addEvent(
-            Package.error(
-                location,
-                "Can not set both default_package_metadata and default_applicable_licenses."
-                    + " Move all declarations to default_package_metadata.",
-                Code.INVALID_PACKAGE_SPECIFICATION));
-      }
-
-      pkgBuilder.setDefaultPackageMetadata(value, "default_package_metadata", location);
-    }
-  }
-
-  /**
-   * Declares the package() attribute specifying the default value for {@link
-   * com.google.devtools.build.lib.packages.RuleClass#APPLICABLE_LICENSES_ATTR} when not explicitly
-   * specified.
-   */
-  private static class DefaultPackageMetadata extends PackageArgument<List<Label>> {
-    private static final String DEFAULT_PACKAGE_METADATA_ATTRIBUTE = "default_package_metadata";
-
-    private DefaultPackageMetadata() {
-      super(DEFAULT_PACKAGE_METADATA_ATTRIBUTE, BuildType.LABEL_LIST);
-    }
-
-    @Override
-    protected void process(Package.Builder pkgBuilder, Location location, List<Label> value) {
-      if (!pkgBuilder.getDefaultPackageMetadata().isEmpty()) {
-        pkgBuilder.addEvent(
-            Package.error(
-                location,
-                "Can not set both default_package_metadata and default_applicable_licenses."
-                    + " Move all declarations to default_package_metadata.",
-                Code.INVALID_PACKAGE_SPECIFICATION));
-      }
-      pkgBuilder.setDefaultPackageMetadata(value, DEFAULT_PACKAGE_METADATA_ATTRIBUTE, location);
-    }
-  }
-
-  private static class DefaultLicenses extends PackageArgument<License> {
-    private DefaultLicenses() {
-      super("licenses", BuildType.LICENSE);
-    }
-
-    @Override
-    protected void process(Package.Builder pkgBuilder, Location location, License value) {
+    } else if (name.equals("licenses")) {
+      License value = convert(BuildType.LICENSE, rawValue, pkgBuilder);
       pkgBuilder.setDefaultLicense(value);
-    }
-  }
 
-  private static class DefaultDistribs extends PackageArgument<Set<DistributionType>> {
-    private DefaultDistribs() {
-      super("distribs", BuildType.DISTRIBUTIONS);
-    }
-
-    @Override
-    protected void process(
-        Package.Builder pkgBuilder, Location location, Set<DistributionType> value) {
+    } else if (name.equals("distribs")) {
+      Set<DistributionType> value = convert(BuildType.DISTRIBUTIONS, rawValue, pkgBuilder);
       pkgBuilder.setDefaultDistribs(value);
+
+    } else if (name.equals("default_compatible_with")) {
+      List<Label> value = convert(BuildType.LABEL_LIST, rawValue, pkgBuilder);
+      pkgBuilder.setDefaultCompatibleWith(value, name, loc);
+
+    } else if (name.equals("default_restricted_to")) {
+      List<Label> value = convert(BuildType.LABEL_LIST, rawValue, pkgBuilder);
+      pkgBuilder.setDefaultRestrictedTo(value, name, loc);
+
+    } else if (name.equals("default_applicable_licenses")) {
+      List<Label> value = convert(BuildType.LABEL_LIST, rawValue, pkgBuilder);
+      if (!pkgBuilder.getDefaultPackageMetadata().isEmpty()) {
+        pkgBuilder.addEvent(
+            Package.error(
+                loc,
+                "Can not set both default_package_metadata and default_applicable_licenses."
+                    + " Move all declarations to default_package_metadata.",
+                Code.INVALID_PACKAGE_SPECIFICATION));
+      }
+      pkgBuilder.setDefaultPackageMetadata(value, name, loc);
+
+    } else if (name.equals("default_package_metadata")) {
+      List<Label> value = convert(BuildType.LABEL_LIST, rawValue, pkgBuilder);
+      if (!pkgBuilder.getDefaultPackageMetadata().isEmpty()) {
+        pkgBuilder.addEvent(
+            Package.error(
+                loc,
+                "Can not set both default_package_metadata and default_applicable_licenses."
+                    + " Move all declarations to default_package_metadata.",
+                Code.INVALID_PACKAGE_SPECIFICATION));
+      }
+      pkgBuilder.setDefaultPackageMetadata(value, name, loc);
+
+    } else {
+      throw Starlark.errorf("unexpected keyword argument: %s", name);
     }
   }
 
-  /**
-   * Declares the package() attribute specifying the default value for {@link
-   * com.google.devtools.build.lib.packages.RuleClass#COMPATIBLE_ENVIRONMENT_ATTR} when not
-   * explicitly specified.
-   */
-  private static class DefaultCompatibleWith extends PackageArgument<List<Label>> {
-    private static final String DEFAULT_COMPATIBLE_WITH_ATTRIBUTE = "default_compatible_with";
-
-    private DefaultCompatibleWith() {
-      super(DEFAULT_COMPATIBLE_WITH_ATTRIBUTE, BuildType.LABEL_LIST);
-    }
-
-    @Override
-    protected void process(Package.Builder pkgBuilder, Location location, List<Label> value) {
-      pkgBuilder.setDefaultCompatibleWith(value, DEFAULT_COMPATIBLE_WITH_ATTRIBUTE, location);
-    }
-  }
-
-  /**
-   * Declares the package() attribute specifying the default value for {@link
-   * com.google.devtools.build.lib.packages.RuleClass#RESTRICTED_ENVIRONMENT_ATTR} when not
-   * explicitly specified.
-   */
-  private static class DefaultRestrictedTo extends PackageArgument<List<Label>> {
-    private static final String DEFAULT_RESTRICTED_TO_ATTRIBUTE = "default_restricted_to";
-
-    private DefaultRestrictedTo() {
-      super(DEFAULT_RESTRICTED_TO_ATTRIBUTE, BuildType.LABEL_LIST);
-    }
-
-    @Override
-    protected void process(Package.Builder pkgBuilder, Location location, List<Label> value) {
-      pkgBuilder.setDefaultRestrictedTo(value, DEFAULT_RESTRICTED_TO_ATTRIBUTE, location);
-    }
+  protected static <T> T convert(Type<T> type, Object value, Package.Builder pkgBuilder)
+      throws EvalException {
+    return type.convert(value, "'package' argument", pkgBuilder.getLabelConverter());
   }
 }