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");