Reorder arguments to BuiltinFunction-s
By popular demand from other implementers, reorder BuiltinFunction arguments
so that mandatory named-only arguments come befor optional named-only arguments
rather than after. This will make Skylark internals slightly clearer and less
surprising, at the cost of eschewing a tiny optimization.
--
MOS_MIGRATED_REVID=89978554
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/BaseFunctionTest.java b/src/test/java/com/google/devtools/build/lib/syntax/BaseFunctionTest.java
index 6833622..9deb624 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/BaseFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/BaseFunctionTest.java
@@ -80,6 +80,7 @@
"mixed(1, 2, 3)",
"mixed(1, 2, wiz=3, quux=4)",
"mixed(foo=1)",
+ "mixed(bar=2)",
"mixed(foo=1, bar=2)",
"mixed(bar=2, foo=1)",
"mixed(2, foo=1)",
@@ -111,6 +112,7 @@
"too many (3) positional arguments in call to mixed(foo, bar = ?)",
"unexpected keywords 'quux', 'wiz' in call to mixed(foo, bar = ?)",
"[1, null]",
+ "missing mandatory positional argument 'foo' while calling mixed(foo, bar = ?)",
"[1, 2]",
"[1, 2]",
"argument 'foo' passed both by position and by name in call to mixed(foo, bar = ?)",
@@ -119,17 +121,18 @@
@Test
public void testOnlyNamedArguments() throws Exception {
- checkBaseFunctions(true, "mixed(*, foo = ?, bar)",
- "missing mandatory keyword arguments in call to mixed(*, foo = ?, bar)",
- "mixed(*, foo = ?, bar) does not accept positional arguments, but got 1",
- "mixed(*, foo = ?, bar) does not accept positional arguments, but got 2",
- "mixed(*, foo = ?, bar) does not accept positional arguments, but got 3",
- "mixed(*, foo = ?, bar) does not accept positional arguments, but got 2",
- "missing mandatory named-only argument 'bar' while calling mixed(*, foo = ?, bar)",
+ checkBaseFunctions(true, "mixed(*, foo, bar = ?)",
+ "missing mandatory keyword arguments in call to mixed(*, foo, bar = ?)",
+ "mixed(*, foo, bar = ?) does not accept positional arguments, but got 1",
+ "mixed(*, foo, bar = ?) does not accept positional arguments, but got 2",
+ "mixed(*, foo, bar = ?) does not accept positional arguments, but got 3",
+ "mixed(*, foo, bar = ?) does not accept positional arguments, but got 2",
+ "[1, null]",
+ "missing mandatory named-only argument 'foo' while calling mixed(*, foo, bar = ?)",
"[1, 2]",
"[1, 2]",
- "mixed(*, foo = ?, bar) does not accept positional arguments, but got 1",
- "unexpected keyword 'wiz' in call to mixed(*, foo = ?, bar)");
+ "mixed(*, foo, bar = ?) does not accept positional arguments, but got 1",
+ "unexpected keyword 'wiz' in call to mixed(*, foo, bar = ?)");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/FunctionTest.java b/src/test/java/com/google/devtools/build/lib/syntax/FunctionTest.java
index 2eea35a..a55fc01 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/FunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/FunctionTest.java
@@ -383,7 +383,7 @@
assertEquals("xyce", env.lookup("v2"));
assertEquals("xyzf", env.lookup("v3"));
UserDefinedFunction foo = (UserDefinedFunction) env.lookup("foo");
- assertEquals("foo(a, b = \"b\", *, d = \"d\", c)", foo.toString());
+ assertEquals("foo(a, b = \"b\", *, c, d = \"d\")", foo.toString());
}
@Test
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 683f2c7..9157a09 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
@@ -1018,9 +1018,9 @@
assertThat(statements.get(0)).isInstanceOf(FunctionDefStatement.class);
FunctionDefStatement stmt = (FunctionDefStatement) statements.get(0);
FunctionSignature sig = stmt.getArgs().getSignature();
- // Note the reordering of mandatory named-only at the end.
+ // Note the reordering of optional named-only at the end.
assertThat(sig.getNames()).isEqualTo(ImmutableList.<String>of(
- "a", "b1", "b2", "d", "c1", "c2"));
+ "a", "b1", "b2", "c1", "c2", "d"));
FunctionSignature.Shape shape = sig.getShape();
assertThat(shape.getMandatoryPositionals()).isEqualTo(1);
assertThat(shape.getOptionalPositionals()).isEqualTo(2);
