From 012f789c249b00f318d1e9e7e3817531f6dc34e4 Mon Sep 17 00:00:00 2001 From: Francois-Rene Rideau Date: Tue, 31 Mar 2015 17:27:01 +0000 Subject: 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 --- .../devtools/build/lib/syntax/BaseFunctionTest.java | 21 ++++++++++++--------- .../devtools/build/lib/syntax/FunctionTest.java | 2 +- .../devtools/build/lib/syntax/ParserTest.java | 4 ++-- 3 files changed, 15 insertions(+), 12 deletions(-) (limited to 'src/test/java/com/google/devtools/build/lib') 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 6833622e28..9deb6244ac 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 @@ public class BaseFunctionTest extends AbstractEvaluationTestCase { "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 @@ public class BaseFunctionTest extends AbstractEvaluationTestCase { "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 @@ public class BaseFunctionTest extends AbstractEvaluationTestCase { @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 2eea35a1d9..a55fc01401 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 @@ public class FunctionTest extends AbstractEvaluationTestCase { 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 683f2c7f48..9157a09e9c 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 @@ public class ParserTest extends AbstractParserTestCase { 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.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); -- cgit v1.2.3