Remove flag `--incompatible_strict_argument_ordering`.
RELNOTES: None.
PiperOrigin-RevId: 240453542
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
index 1a2c2e9..b534ba5 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
@@ -477,20 +477,6 @@
+ "will be available")
public boolean incompatibleRemoveNativeMavenJar;
- @Option(
- name = "incompatible_strict_argument_ordering",
- defaultValue = "true",
- documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
- effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
- metadataTags = {
- OptionMetadataTag.INCOMPATIBLE_CHANGE,
- OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
- },
- help =
- "If set to true, the order of arguments is stricter in function calls, see "
- + "https://github.com/bazelbuild/bazel/issues/6611")
- public boolean incompatibleStricArgumentOrdering;
-
/** Used in an integration test to confirm that flags are visible to the interpreter. */
@Option(
name = "internal_skylark_flag_test_canary",
@@ -566,7 +552,6 @@
.incompatibleNoTransitiveLoads(incompatibleNoTransitiveLoads)
.incompatibleRemapMainRepo(incompatibleRemapMainRepo)
.incompatibleRemoveNativeMavenJar(incompatibleRemoveNativeMavenJar)
- .incompatibleStricArgumentOrdering(incompatibleStricArgumentOrdering)
.incompatibleUseToolchainProvidersInJavaCommon(
incompatibleUseToolchainProvidersInJavaCommon)
.internalSkylarkFlagTestCanary(internalSkylarkFlagTestCanary)
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Argument.java b/src/main/java/com/google/devtools/build/lib/syntax/Argument.java
index 8854adf..f988fab 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Argument.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Argument.java
@@ -187,42 +187,6 @@
/**
* Validate that the list of Argument's, whether gathered by the Parser or from annotations,
- * satisfies the requirements of the Python calling conventions: all Positional's first, at most
- * one Star, at most one StarStar, at the end only.
- *
- * <p>TODO(laurentlb): remove this function and use only validateFuncallArguments.
- */
- public static void legacyValidateFuncallArguments(List<Passed> arguments)
- throws ArgumentException {
- boolean hasNamed = false;
- boolean hasStar = false;
- boolean hasKwArg = false;
- for (Passed arg : arguments) {
- if (hasKwArg) {
- throw new ArgumentException(arg.getLocation(), "argument after **kwargs");
- }
- if (arg.isPositional()) {
- if (hasNamed) {
- throw new ArgumentException(arg.getLocation(), "non-keyword arg after keyword arg");
- } else if (arg.isStar()) {
- throw new ArgumentException(
- arg.getLocation(), "only named arguments may follow *expression");
- }
- } else if (arg.isKeyword()) {
- hasNamed = true;
- } else if (arg.isStar()) {
- if (hasStar) {
- throw new ArgumentException(arg.getLocation(), "more than one *stararg");
- }
- hasStar = true;
- } else {
- hasKwArg = true;
- }
- }
- }
-
- /**
- * Validate that the list of Argument's, whether gathered by the Parser or from annotations,
* satisfies the requirements: first Positional arguments, then Keyword arguments, then an
* optional *arg argument, finally an optional **kwarg argument.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
index 000c6de..81f34c3 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
@@ -543,7 +543,7 @@
private ImmutableList<Argument.Passed> parseFuncallArguments() {
ImmutableList<Argument.Passed> arguments = parseFunctionArguments(this::parseFuncallArgument);
try {
- Argument.legacyValidateFuncallArguments(arguments);
+ Argument.validateFuncallArguments(arguments);
} catch (Argument.ArgumentException e) {
reportError(e.getLocation(), e.getMessage());
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
index abddc4f..0039631 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
@@ -174,8 +174,6 @@
public abstract boolean incompatibleRemoveNativeMavenJar();
- public abstract boolean incompatibleStricArgumentOrdering();
-
public abstract boolean internalSkylarkFlagTestCanary();
public abstract boolean incompatibleUseToolchainProvidersInJavaCommon();
@@ -229,7 +227,6 @@
.incompatibleNoTransitiveLoads(true)
.incompatibleRemapMainRepo(false)
.incompatibleRemoveNativeMavenJar(false)
- .incompatibleStricArgumentOrdering(true)
.internalSkylarkFlagTestCanary(false)
.incompatibleDoNotSplitLinkingCmdline(false)
.build();
@@ -301,8 +298,6 @@
public abstract Builder incompatibleRemoveNativeMavenJar(boolean value);
- public abstract Builder incompatibleStricArgumentOrdering(boolean value);
-
public abstract Builder incompatibleUseToolchainProvidersInJavaCommon(boolean value);
public abstract Builder internalSkylarkFlagTestCanary(boolean value);
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java b/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
index e71670d..8c28293 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
@@ -187,18 +187,6 @@
}
@Override
- public void visit(FuncallExpression node) {
- super.visit(node);
- try {
- if (env.getSemantics().incompatibleStricArgumentOrdering()) {
- Argument.validateFuncallArguments(node.getArguments());
- }
- } catch (Argument.ArgumentException e) {
- throw new ValidationException(e.getLocation(), e.getMessage());
- }
- }
-
- @Override
public void visit(ReturnStatement node) {
if (block.scope != Scope.Local) {
throw new ValidationException(
diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
index ff7874f..ebe9004 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
@@ -155,7 +155,6 @@
"--incompatible_no_transitive_loads=" + rand.nextBoolean(),
"--incompatible_remap_main_repo=" + rand.nextBoolean(),
"--incompatible_remove_native_maven_jar=" + rand.nextBoolean(),
- "--incompatible_strict_argument_ordering=" + rand.nextBoolean(),
"--incompatible_use_toolchain_providers_in_java_common=" + rand.nextBoolean(),
"--internal_skylark_flag_test_canary=" + rand.nextBoolean(),
"--incompatible_do_not_split_linking_cmdline=" + rand.nextBoolean());
@@ -201,7 +200,6 @@
.incompatibleNoTransitiveLoads(rand.nextBoolean())
.incompatibleRemapMainRepo(rand.nextBoolean())
.incompatibleRemoveNativeMavenJar(rand.nextBoolean())
- .incompatibleStricArgumentOrdering(rand.nextBoolean())
.incompatibleUseToolchainProvidersInJavaCommon(rand.nextBoolean())
.internalSkylarkFlagTestCanary(rand.nextBoolean())
.incompatibleDoNotSplitLinkingCmdline(rand.nextBoolean())
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ASTPrettyPrintTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ASTPrettyPrintTest.java
index 7310754..262e4e4 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ASTPrettyPrintTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ASTPrettyPrintTest.java
@@ -147,7 +147,7 @@
public void funcallExpression() {
assertExprBothRoundTrip("f()");
assertExprBothRoundTrip("f(a)");
- assertExprBothRoundTrip("f(a, b = B, *c, d = D, **e)");
+ assertExprBothRoundTrip("f(a, b = B, c = C, *d, **e)");
assertExprBothRoundTrip("o.f()");
}
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
index 4e59cdd..d2c6e44 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
@@ -1495,7 +1495,7 @@
" *[2],",
" *[3],", // error on this line
")\n");
- assertContainsError(":4:5: more than one *stararg");
+ assertContainsError(":4:5: *arg argument is misplaced");
}
@Test
@@ -1507,7 +1507,7 @@
" a = 4,",
" 3,", // error on this line
")\n");
- assertContainsError(":4:5: non-keyword arg after keyword arg");
+ assertContainsError(":4:5: positional argument is misplaced (positional arguments come first)");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java
index ea58193..ec47e1d 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java
@@ -241,7 +241,6 @@
@Test
public void testPositionalAfterStarArg() throws Exception {
- env = newEnvironmentWithSkylarkOptions("--incompatible_strict_argument_ordering=true");
checkError(
"positional argument is misplaced (positional arguments come first)",
"def fct(*args, **kwargs): pass",
@@ -250,7 +249,6 @@
@Test
public void testTwoStarArgs() throws Exception {
- env = newEnvironmentWithSkylarkOptions("--incompatible_strict_argument_ordering=true");
checkError(
"*arg argument is misplaced",
"def fct(*args, **kwargs):",
@@ -260,7 +258,6 @@
@Test
public void testKeywordArgAfterStarArg() throws Exception {
- env = newEnvironmentWithSkylarkOptions("--incompatible_strict_argument_ordering=true");
checkError(
"keyword argument is misplaced (keyword arguments must be before any *arg or **kwarg)",
"def fct(*args, **kwargs): pass",
diff --git a/src/test/starlark/testdata/string_format.sky b/src/test/starlark/testdata/string_format.sky
index 58c4288..8393482 100644
--- a/src/test/starlark/testdata/string_format.sky
+++ b/src/test/starlark/testdata/string_format.sky
@@ -74,9 +74,9 @@
---
'{} and {}'.format('this') ### No replacement found for index 1
---
-'{test} and {}'.format(test = 1, 2) ### non-keyword arg after keyword arg
+'{test} and {}'.format(test = 1, 2) ### positional argument is misplaced (positional arguments come first)
---
-'{test} and {0}'.format(test = 1, 2) ### non-keyword arg after keyword arg
+'{test} and {0}'.format(test = 1, 2) ### positional argument is misplaced (positional arguments come first)
---
'{} and {1}'.format(1, 2) ### Cannot mix manual and automatic numbering of positional fields
---
