New class hierarchy for Skylark functions
* New hierarchy BaseFunction > UserModeFunction, BuiltinFunction.
The old hierarchy still exists for now, to be deleted after migration:
Function > AbstractFunction > MixedModeFunction >
(UserModeFunction, SkylarkFunction > SimpleSkylarkFunction)
(UserModeFunction is already migrated, and
BaseFunction implements Function, for now.)
* Function supports *args and **kwargs when calling functions, and
mandatory named-only parameters in the style of Python 3.
Notable difference with Python: *args binds the variable to a tuple,
because a Skylark list would have to be monomorphic.
* A better, simpler, safer FFI using reflection with BuiltinFunction.
Handles typechecking, passes parameters in a more Java style.
(Not used for now, will be used later.)
* A new annotation @SkylarkSignature, intended to replace @SkylarkBuiltin,
supports the full function call protocol, including default arguments.
* Support for annotating function Factory-s rather than functions.
--
MOS_MIGRATED_REVID=88958581
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
new file mode 100644
index 0000000..6833622
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/syntax/BaseFunctionTest.java
@@ -0,0 +1,160 @@
+// Copyright 2006-2015 Google Inc. 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.syntax;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+import java.util.Arrays;
+import java.util.Map;
+import java.util.TreeMap;
+
+/**
+ * Tests for {@link BaseFunction}.
+ * This tests the argument processing by BaseFunction
+ * between the outer call(posargs, kwargs, ast, env) and the inner call(args, ast, env).
+ */
+@RunWith(JUnit4.class)
+public class BaseFunctionTest extends AbstractEvaluationTestCase {
+
+ private Environment singletonEnv(String id, Object value) {
+ Environment env = new Environment();
+ env.update(id, value);
+ return env;
+ }
+
+ /**
+ * Handy implementation of {@link BaseFunction} that returns all its args as a list.
+ * (We'd use SkylarkList.tuple, but it can't handle null.)
+ */
+ private static class TestingBaseFunction extends BaseFunction {
+ TestingBaseFunction(FunctionSignature signature) {
+ super("mixed", signature);
+ }
+
+ @Override
+ public Object call(Object[] arguments, FuncallExpression ast, Environment env) {
+ return Arrays.asList(arguments);
+ }
+ }
+
+ private void checkBaseFunction(BaseFunction func, String callExpression, String expectedOutput)
+ throws Exception {
+ Environment env = singletonEnv(func.getName(), func);
+
+ if (expectedOutput.charAt(0) == '[') { // a tuple => expected to pass
+ assertEquals("Wrong output for " + callExpression,
+ expectedOutput, eval(callExpression, env).toString());
+
+ } else { // expected to fail with an exception
+ try {
+ eval(callExpression, env);
+ fail();
+ } catch (EvalException e) {
+ assertEquals("Wrong exception for " + callExpression,
+ expectedOutput, e.getMessage());
+ }
+ }
+ }
+
+ private static final String[] BASE_FUNCTION_EXPRESSIONS = {
+ "mixed()",
+ "mixed(1)",
+ "mixed(1, 2)",
+ "mixed(1, 2, 3)",
+ "mixed(1, 2, wiz=3, quux=4)",
+ "mixed(foo=1)",
+ "mixed(foo=1, bar=2)",
+ "mixed(bar=2, foo=1)",
+ "mixed(2, foo=1)",
+ "mixed(bar=2, foo=1, wiz=3)",
+ };
+
+ public void checkBaseFunctions(boolean onlyNamedArguments,
+ String expectedSignature, String... expectedResults) throws Exception {
+ BaseFunction func = new TestingBaseFunction(
+ onlyNamedArguments
+ ? FunctionSignature.namedOnly(1, "foo", "bar")
+ : FunctionSignature.of(1, "foo", "bar"));
+
+ assertThat(func.toString()).isEqualTo(expectedSignature);
+
+ for (int i = 0; i < BASE_FUNCTION_EXPRESSIONS.length; i++) {
+ String expr = BASE_FUNCTION_EXPRESSIONS[i];
+ String expected = expectedResults[i];
+ checkBaseFunction(func, expr, expected);
+ }
+ }
+
+ @Test
+ public void testNoSurplusArguments() throws Exception {
+ checkBaseFunctions(false, "mixed(foo, bar = ?)",
+ "insufficient arguments received by mixed(foo, bar = ?) (got 0, expected at least 1)",
+ "[1, null]",
+ "[1, 2]",
+ "too many (3) positional arguments in call to mixed(foo, bar = ?)",
+ "unexpected keywords 'quux', 'wiz' in call to mixed(foo, bar = ?)",
+ "[1, null]",
+ "[1, 2]",
+ "[1, 2]",
+ "argument 'foo' passed both by position and by name in call to mixed(foo, bar = ?)",
+ "unexpected keyword 'wiz' in call to mixed(foo, bar = ?)");
+ }
+
+ @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)",
+ "[1, 2]",
+ "[1, 2]",
+ "mixed(*, foo = ?, bar) does not accept positional arguments, but got 1",
+ "unexpected keyword 'wiz' in call to mixed(*, foo = ?, bar)");
+ }
+
+ @Test
+ @SuppressWarnings("unchecked")
+ public void testKwParam() throws Exception {
+ Environment env = new SkylarkEnvironment(syntaxEvents.collector());
+ exec(parseFileForSkylark(
+ "def foo(a, b, c=3, d=4, *args, e, f, g=7, h=8, **kwargs):\n"
+ + " return (a, b, c, d, e, f, g, h, args, kwargs)\n"
+ + "v1 = foo(1, 2, e=5, f=6)\n"
+ + "v2 = foo(1, *['x', 'y', 'z', 't'], h=9, e=5, f=6, i=0)\n"
+ + "def bar(**kwargs):\n"
+ + " return kwargs\n"
+ + "b1 = bar(name='foo', type='jpg', version=42)\n"
+ + "b2 = bar()\n"), env);
+
+ assertThat(EvalUtils.prettyPrintValue(env.lookup("v1")))
+ .isEqualTo("(1, 2, 3, 4, 5, 6, 7, 8, (), {})");
+ assertThat(EvalUtils.prettyPrintValue(env.lookup("v2")))
+ .isEqualTo("(1, \"x\", \"y\", \"z\", 5, 6, 7, 9, (\"t\",), {\"i\": 0})");
+
+ // NB: the conversion to a TreeMap below ensures the keys are sorted.
+ assertThat(EvalUtils.prettyPrintValue(
+ new TreeMap<String, Object>((Map<String, Object>) env.lookup("b1"))))
+ .isEqualTo("{\"name\": \"foo\", \"type\": \"jpg\", \"version\": 42}");
+ assertThat(EvalUtils.prettyPrintValue(env.lookup("b2"))).isEqualTo("{}");
+ }
+}
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 4ce41cb..2eea35a 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
@@ -89,7 +89,7 @@
}
private void createOuterFunction(Environment env, final List<Object> params) {
- Function outerFunc = new AbstractFunction("outer_func") {
+ BaseFunction outerFunc = new BaseFunction("outer_func") {
@Override
public Object call(List<Object> args, Map<String, Object> kwargs, FuncallExpression ast,
@@ -362,7 +362,8 @@
@Test
public void testDefaultArgumentsInsufficientArgNum() throws Exception {
- checkError("func(a, b = null, c = null) received insufficient arguments",
+ checkError("insufficient arguments received by func(a, b = \"b\", c = \"c\") "
+ + "(got 0, expected at least 1)",
"def func(a, b = 'b', c = 'c'):",
" return a + b + c",
"func()");
@@ -371,16 +372,18 @@
@Test
public void testKwargs() throws Exception {
List<Statement> input = parseFileForSkylark(
- "def foo(a, b = 'b', c = 'c'):\n"
- + " return a + b + c\n"
+ "def foo(a, b = 'b', *, c, d = 'd'):\n"
+ + " return a + b + c + d\n"
+ "args = {'a': 'x', 'c': 'z'}\n"
+ "v1 = foo(**args)\n"
- + "v2 = foo('x', **{'b': 'y'})\n"
- + "v3 = foo(c = 'z', a = 'x', **{'b': 'y'})");
+ + "v2 = foo('x', c = 'c', d = 'e', **{'b': 'y'})\n"
+ + "v3 = foo(c = 'z', a = 'x', **{'b': 'y', 'd': 'f'})");
exec(input, env);
- assertEquals("xbz", env.lookup("v1"));
- assertEquals("xyc", env.lookup("v2"));
- assertEquals("xyz", env.lookup("v3"));
+ assertEquals("xbzd", env.lookup("v1"));
+ 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());
}
@Test
@@ -401,7 +404,7 @@
@Test
public void testKwargsCollision() throws Exception {
- checkError("func(a, b) got multiple values for keyword argument 'b'",
+ checkError("argument 'b' passed both by position and by name in call to func(a, b)",
"def func(a, b):",
" return a + b",
"func('a', 'b', **{'b': 'foo'})");
@@ -452,6 +455,26 @@
assertEquals("a12", env.lookup("v4"));
}
+ @Test
+ public void testStarParam() throws Exception {
+ List<Statement> input = parseFileForSkylark(
+ "def f(name, value = '1', *rest, mandatory, optional = '2'):\n"
+ + " r = name + value + mandatory + optional + '|'\n"
+ + " for x in rest: r += x\n"
+ + " return r\n"
+ + "v1 = f('a', 'b', mandatory = 'z')\n"
+ + "v2 = f('a', 'b', 'c', 'd', mandatory = 'z')\n"
+ + "v3 = f('a', *['b', 'c', 'd'], mandatory = 'y', optional = 'z')\n"
+ + "v4 = f(*['a'], **{'value': 'b', 'mandatory': 'c'})\n"
+ + "v5 = f('a', 'b', 'c', *['d', 'e'], mandatory = 'f', **{'optional': 'g'})\n");
+ exec(input, env);
+ assertEquals("abz2|", env.lookup("v1"));
+ assertEquals("abz2|cd", env.lookup("v2"));
+ assertEquals("abyz|cd", env.lookup("v3"));
+ assertEquals("abc2|", env.lookup("v4"));
+ assertEquals("abfg|cde", env.lookup("v5"));
+ }
+
private void checkError(String msg, String... lines)
throws Exception {
try {
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 f97282e..1707b3aa 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
@@ -20,6 +20,7 @@
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
+import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.syntax.DictionaryLiteral.DictionaryEntryLiteral;
@@ -968,9 +969,20 @@
@Test
public void testUnnamedStar() throws Exception {
syntaxEvents.setFailFast(false);
- parseFileForSkylark(
- "def func(a, b1=2, b2=3, *, c1, c2, d=4): return a + b1 + b2 + c1 + c2 + d\n");
- syntaxEvents.assertContainsEvent("no star, star-star or named-only parameters (for now)");
+ List<Statement> statements = parseFileForSkylark(
+ "def func(a, b1=2, b2=3, *, c1, d=4, c2): return a + b1 + b2 + c1 + c2 + d\n");
+ assertThat(statements).hasSize(1);
+ 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.
+ assertThat(sig.getNames()).isEqualTo(ImmutableList.<String>of(
+ "a", "b1", "b2", "d", "c1", "c2"));
+ FunctionSignature.Shape shape = sig.getShape();
+ assertThat(shape.getMandatoryPositionals()).isEqualTo(1);
+ assertThat(shape.getOptionalPositionals()).isEqualTo(2);
+ assertThat(shape.getMandatoryNamedOnly()).isEqualTo(2);
+ assertThat(shape.getOptionalNamedOnly()).isEqualTo(1);
}
@Test
