Create flag to turn down named-ness of certain Starlark params

This change creates --experimental_restrict_named_params to make all builtin Starlark parameters which are named via legacyNamed = true to instead be treated as positional-only.

This is a step on the path to cleaning up namedness of builtin Starlark parameters.

When we finalize the list of parameters which should truly be positional-only, we can change this flag to be prefixed with --incompatible instead of --experimental, as per the incompatible change policies.

Progress toward #5010.

RELNOTES: None.
PiperOrigin-RevId: 236898018

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

@@ -126,6 +126,19 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl
               + "debugging.")
   public boolean experimentalPlatformsApi;
 
+  // TODO(cparsons): Change this flag to --incompatible instead of --experimental when it is
+  // fully implemented.
+  @Option(
+      name = "experimental_restrict_named_params",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
+      effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
+      metadataTags = {OptionMetadataTag.EXPERIMENTAL},
+      help =
+          "If set to true, restricts a number of Starlark built-in function parameters to be "
+              + "only specifiable positionally (and not by keyword).")
+  public boolean experimentalRestrictNamedParams;
+
   // TODO(cparsons): Resolve and finalize the transition() API. The transition implementation
   // function should accept two mandatory parameters, 'settings' and 'attr'.
   @Option(
@@ -551,6 +564,7 @@ public StarlarkSemantics toSkylarkSemantics() {
         .experimentalJavaCommonCreateProviderEnabledPackages(
             experimentalJavaCommonCreateProviderEnabledPackages)
         .experimentalPlatformsApi(experimentalPlatformsApi)
+        .experimentalRestrictNamedParams(experimentalRestrictNamedParams)
         .experimentalStarlarkConfigTransitions(experimentalStarlarkConfigTransitions)
         .experimentalTransitionWhitelistLocation(experimentalTransitionWhitelistLocation)
         .incompatibleBzlDisallowLoadAfterStatement(incompatibleBzlDisallowLoadAfterStatement)

src/main/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessor.java

@@ -221,7 +221,7 @@ private void verifyParamSemantics(ExecutableElement methodElement, SkylarkCallab
     boolean allowNonDefaultPositionalNext = true;
 
     for (Param parameter : annotation.parameters()) {
-      if ((!parameter.positional()) && (!isParamNamed(parameter))) {
+      if (!parameter.positional() && !parameter.named()) {
         throw new SkylarkCallableProcessorException(
             methodElement,
             String.format("Parameter '%s' must be either positional or named",

src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java

@@ -553,10 +553,7 @@ public Object[] convertStarlarkArgumentsToJavaMethodArguments(
           }
         } else { // Param not specified by user. Use default value.
           if (param.getDefaultValue().isEmpty()) {
-            throw argumentMismatchException(
-                String.format("parameter '%s' has no default value", param.getName()),
-                method,
-                objClass);
+            throw unspecifiedParameterException(param, method, objClass, kwargs);
           }
           value =
               SkylarkSignatureProcessor.getDefaultValue(
@@ -613,6 +610,22 @@ public Object[] convertStarlarkArgumentsToJavaMethodArguments(
     return builder.build().toArray();
   }
 
+  private EvalException unspecifiedParameterException(
+      ParamDescriptor param,
+      MethodDescriptor method,
+      Class<?> objClass,
+      Map<String, Object> kwargs) {
+    if (kwargs.containsKey(param.getName())) {
+      return argumentMismatchException(
+          String.format("parameter '%s' may not be specified by name", param.getName()),
+          method,
+          objClass);
+    } else {
+      return argumentMismatchException(
+          String.format("parameter '%s' has no default value", param.getName()), method, objClass);
+    }
+  }
+
   private EvalException unexpectedKeywordArgumentException(
       Set<String> unexpectedKeywords,
       MethodDescriptor method,

src/main/java/com/google/devtools/build/lib/syntax/ParamDescriptor.java

@@ -56,7 +56,6 @@ private ParamDescriptor(
       Class<?> generic1,
       boolean noneable,
       boolean named,
-      boolean legacyNamed,
       boolean positional,
       SkylarkType skylarkType,
       @Nullable String valueOverride,
@@ -67,7 +66,7 @@ private ParamDescriptor(
     this.allowedTypes = allowedTypes;
     this.generic1 = generic1;
     this.noneable = noneable;
-    this.named = named || legacyNamed;
+    this.named = named;
     this.positional = positional;
     this.skylarkType = skylarkType;
     this.valueOverride = valueOverride;
@@ -103,14 +102,20 @@ static ParamDescriptor of(Param param, StarlarkSemantics starlarkSemantics) {
         allowedTypes,
         generic,
         noneable,
-        param.named(),
-        param.legacyNamed(),
+        isNamed(param, starlarkSemantics),
         param.positional(),
         getType(type, generic, allowedTypes, noneable),
         valueOverride,
         flagResponsibleForDisable);
   }
 
+  private static boolean isNamed(Param param, StarlarkSemantics starlarkSemantics) {
+    if (param.named()) {
+      return true;
+    }
+    return param.legacyNamed() && !starlarkSemantics.experimentalRestrictNamedParams();
+  }
+
   /** @see Param#name() */
   public String getName() {
     return name;

src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java

@@ -127,6 +127,8 @@ public boolean flagValue(FlagIdentifier flagIdentifier) {
 
   public abstract boolean experimentalPlatformsApi();
 
+  public abstract boolean experimentalRestrictNamedParams();
+
   public abstract boolean experimentalStarlarkConfigTransitions();
 
   public abstract String experimentalTransitionWhitelistLocation();
@@ -208,6 +210,7 @@ public static Builder builderWithDefaults() {
           .experimentalEnableAndroidMigrationApis(false)
           .experimentalJavaCommonCreateProviderEnabledPackages(ImmutableList.of())
           .experimentalPlatformsApi(false)
+          .experimentalRestrictNamedParams(false)
           .experimentalStarlarkConfigTransitions(false)
           .experimentalTransitionWhitelistLocation("")
           .incompatibleUseToolchainProvidersInJavaCommon(false)
@@ -257,6 +260,8 @@ public abstract static class Builder {
 
     public abstract Builder experimentalPlatformsApi(boolean value);
 
+    public abstract Builder experimentalRestrictNamedParams(boolean value);
+
     public abstract Builder experimentalStarlarkConfigTransitions(boolean value);
 
     public abstract Builder experimentalTransitionWhitelistLocation(String value);

src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java

@@ -130,6 +130,7 @@ private static StarlarkSemanticsOptions buildRandomOptions(Random rand) throws E
             + ","
             + rand.nextDouble(),
         "--experimental_platforms_api=" + rand.nextBoolean(),
+        "--experimental_restrict_named_params=" + rand.nextBoolean(),
         "--experimental_starlark_config_transitions=" + rand.nextBoolean(),
         "--experimental_transition_whitelist_location=" + rand.nextDouble(),
         "--incompatible_bzl_disallow_load_after_statement=" + rand.nextBoolean(),
@@ -177,6 +178,7 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) {
         .experimentalJavaCommonCreateProviderEnabledPackages(
             ImmutableList.of(String.valueOf(rand.nextDouble()), String.valueOf(rand.nextDouble())))
         .experimentalPlatformsApi(rand.nextBoolean())
+        .experimentalRestrictNamedParams(rand.nextBoolean())
         .experimentalStarlarkConfigTransitions(rand.nextBoolean())
         .experimentalTransitionWhitelistLocation(String.valueOf(rand.nextDouble()))
         .incompatibleBzlDisallowLoadAfterStatement(rand.nextBoolean())

src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java

@@ -745,4 +745,13 @@ public void testLegacyNamed() throws Exception {
         .testIfErrorContains("None", "fail(msg=None)")
         .testEval("type(None)", "'NoneType'");
   }
+
+  @Test
+  public void testExperimentalStarlarkConfig() throws Exception {
+    new SkylarkTest("--experimental_restrict_named_params")
+        .testIfErrorContains(
+            "parameter 'elements' may not be specified by name, "
+                + "for call to method join(elements) of 'string'",
+            "','.join(elements=['foo', 'bar'])");
+  }
 }