Allow calling attributes of built-in objects.
If `f` is an object with an attribute `x` which is callable, it's valid now to
call `f.x()` directly (caused a "no function called x" error before because .x
is a attribute, not a method).
--
MOS_MIGRATED_REVID=137698425
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
index 4b23350..5748f63 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
@@ -361,7 +361,9 @@
ArgumentListConversionResult argumentListConversionResult = null;
if (methods != null) {
for (MethodDescriptor method : methods) {
- if (!method.getAnnotation().structField()) {
+ if (method.getAnnotation().structField()) {
+ return new Pair<>(method, null);
+ } else {
argumentListConversionResult = convertArgumentList(args, kwargs, method);
if (argumentListConversionResult.getArguments() != null) {
if (matchingMethod == null) {
@@ -410,8 +412,7 @@
/**
* Constructs the parameters list to actually pass to the method, filling with default values if
- * any. If the type does not match, returns null and fills in methodTypeMatchError with the
- * appropriate message.
+ * any. If there is a type or argument mismatch, returns a result containing an error message.
*/
private ArgumentListConversionResult convertArgumentList(
List<Object> args, Map<String, Object> kwargs, MethodDescriptor method) {
@@ -666,7 +667,7 @@
* @param positionals The first object is expected to be the object the method is called on.
* @param call the original expression that caused this call, needed for rules especially
*/
- public static Object invokeObjectMethod(
+ public Object invokeObjectMethod(
String method,
ImmutableList<Object> positionals,
ImmutableMap<String, Object> keyWordArgs,
@@ -711,6 +712,23 @@
}
Pair<MethodDescriptor, List<Object>> javaMethod =
call.findJavaMethod(objClass, method, positionalArgs, keyWordArgs);
+ if (javaMethod.first.getAnnotation().structField()) {
+ // Not a method but a callable attribute
+ try {
+ return callFunction(javaMethod.first.getMethod().invoke(obj), env);
+ } catch (IllegalAccessException e) {
+ throw new EvalException(getLocation(), "Method invocation failed: " + e);
+ } catch (InvocationTargetException e) {
+ if (e.getCause() instanceof FuncallException) {
+ throw new EvalException(getLocation(), e.getCause().getMessage());
+ } else if (e.getCause() != null) {
+ throw new EvalExceptionWithJavaCause(getLocation(), e.getCause());
+ } else {
+ // This is unlikely to happen
+ throw new EvalException(getLocation(), "Method invocation failed: " + e);
+ }
+ }
+ }
return callMethod(javaMethod.first, method, obj, javaMethod.second.toArray(), location, env);
}
}
@@ -772,13 +790,21 @@
*/
private Object invokeGlobalFunction(Environment env) throws EvalException, InterruptedException {
Object funcValue = func.eval(env);
+ return callFunction(funcValue, env);
+ }
+
+ /**
+ * Calls a function object
+ */
+ private Object callFunction(Object funcValue, Environment env)
+ throws EvalException, InterruptedException {
ImmutableList.Builder<Object> posargs = new ImmutableList.Builder<>();
// We copy this into an ImmutableMap in the end, but we can't use an ImmutableMap.Builder, or
// we'd still have to have a HashMap on the side for the sake of properly handling duplicates.
Map<String, Object> kwargs = new HashMap<>();
BaseFunction function = checkCallable(funcValue, getLocation());
evalArguments(posargs, kwargs, env);
- return function.call(posargs.build(), ImmutableMap.<String, Object>copyOf(kwargs), this, env);
+ return function.call(posargs.build(), ImmutableMap.copyOf(kwargs), this, env);
}
/**
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
index 2fee2c3..dcb927f 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
@@ -29,6 +29,7 @@
import com.google.devtools.build.lib.skylarkinterface.Param;
import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature;
import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
import com.google.devtools.build.lib.testutil.TestMode;
import org.junit.Before;
@@ -61,6 +62,13 @@
}
}
+ @SkylarkSignature(name = "foobar", returnType = String.class, documented = false)
+ static BuiltinFunction foobar = new BuiltinFunction("foobar") {
+ public String invoke() throws EvalException {
+ return "foobar";
+ }
+ };
+
@SkylarkModule(name = "Mock", doc = "")
static class Mock {
@SkylarkCallable(doc = "")
@@ -80,6 +88,10 @@
public String structField() {
return "a";
}
+ @SkylarkCallable(name = "struct_field_callable", doc = "", structField = true)
+ public BuiltinFunction structFieldCallable() {
+ return foobar;
+ }
@SkylarkCallable(name = "function", doc = "", structField = false)
public String function() {
return "a";
@@ -791,10 +803,19 @@
}
@Test
- public void testStructAccessAsFuncall() throws Exception {
+ public void testStructAccessAsFuncallNonCallable() throws Exception {
new SkylarkTest()
.update("mock", new Mock())
- .testIfExactError("Type Mock has no function struct_field()", "v = mock.struct_field()");
+ .testIfExactError("'string' object is not callable", "v = mock.struct_field()");
+ }
+
+ @Test
+ public void testStructAccessAsFuncall() throws Exception {
+ foobar.configure(getClass().getDeclaredField("foobar").getAnnotation(SkylarkSignature.class));
+ new SkylarkTest()
+ .update("mock", new Mock())
+ .setUp("v = mock.struct_field_callable()")
+ .testLookup("v", "foobar");
}
@Test
@@ -1096,6 +1117,7 @@
"string",
"string_list",
"struct_field",
+ "struct_field_callable",
"value_of",
"voidfunc",
"with_params");