Support two-step method calls and allow retrieving built-in methods

using getattr.

It's now possible to call methods in two steps `y = x.f; y()`

src/main/java/com/google/devtools/build/lib/syntax/BuiltinCallable.java

@@ -43,14 +43,25 @@ public Object call(
       Environment env)
       throws EvalException, InterruptedException {
     MethodDescriptor methodDescriptor = getMethodDescriptor(env.getSemantics());
+    Class<?> clazz;
+    Object objValue;
+
+    if (obj instanceof String) {
+      args.add(0, obj);
+      clazz = StringModule.class;
+      objValue = StringModule.INSTANCE;
+    } else {
+      clazz = obj.getClass();
+      objValue = obj;
+    }
 
     // TODO(cparsons): Profiling should be done at the MethodDescriptor level.
     try (SilentCloseable c =
         Profiler.instance().profile(ProfilerTask.STARLARK_BUILTIN_FN, methodName)) {
       Object[] javaArguments =
           ast.convertStarlarkArgumentsToJavaMethodArguments(
-              methodDescriptor, obj.getClass(), args, kwargs, env);
-      return methodDescriptor.call(obj, javaArguments, ast.getLocation(), env);
+              methodDescriptor, clazz, args, kwargs, env);
+      return methodDescriptor.call(objValue, javaArguments, ast.getLocation(), env);
     }
   }
 

src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java

@@ -154,7 +154,9 @@ public static Object eval(Object objValue, String name,
         return result;
       }
     }
-
+    if (method != null) {
+      return new BuiltinCallable(objValue, name);
+    }
     return null;
   }
 

src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java

@@ -742,10 +742,8 @@ public Boolean hasAttr(Object obj, String name, Environment env) throws EvalExce
       name = "getattr",
       doc =
           "Returns the struct's field of the given name if it exists. If not, it either returns "
-              + "<code>default</code> (if specified) or raises an error. Built-in methods cannot "
-              + "currently be retrieved in this way; doing so will result in an error if a "
-              + "<code>default</code> is not given. <code>getattr(x, \"foobar\")</code> is "
-              + "equivalent to <code>x.foobar</code>."
+              + "<code>default</code> (if specified) or raises an error. "
+              + "<code>getattr(x, \"foobar\")</code> is equivalent to <code>x.foobar</code>."
               + "<pre class=\"language-python\">getattr(ctx.attr, \"myattr\")\n"
               + "getattr(ctx.attr, \"myattr\", \"mydefault\")</pre>",
       parameters = {

src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java

@@ -211,12 +211,10 @@ public void testGetAttrMissingField_typoDetection() throws Exception {
 
   @Test
   public void testGetAttrWithMethods() throws Exception {
-    String msg =
-        "object of type 'string' has no attribute 'count', however, "
-            + "a method of that name exists";
+    String msg = "object of type 'string' has no attribute 'cnt'";
     new SkylarkTest()
-        .testIfExactError(msg, "getattr('a string', 'count')")
-        .testStatement("getattr('a string', 'count', 'default')", "default");
+        .testIfExactError(msg, "getattr('a string', 'cnt')")
+        .testStatement("getattr('a string', 'cnt', 'default')", "default");
   }
 
   @Test
@@ -731,7 +729,7 @@ public void testLegacyNamed() throws Exception {
         .testEval("range(start_or_stop=3, stop_or_none=9, step=2)", "range(3, 9, 2)")
         .testStatement("hasattr(x=depset(), name='union')", Boolean.TRUE)
         .testStatement("bool(x=3)", Boolean.TRUE)
-        .testStatement("getattr(x='hello', name='count', default='default')", "default")
+        .testStatement("getattr(x='hello', name='cnt', default='default')", "default")
         .testEval(
             "dir(x={})",
             "[\"clear\", \"get\", \"items\", \"keys\","

src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java

@@ -1448,9 +1448,7 @@ public void testProxyMethodsObjectWithArgsAndKwargs() throws Exception {
   public void testStructAccessOfMethod() throws Exception {
     new SkylarkTest()
         .update("mock", new Mock())
-        .testIfExactError(
-            "object of type 'Mock' has no field 'function', however, a method of that name exists",
-            "v = mock.function");
+        .testStatement("v = mock.function", null);
   }
 
   @Test
@@ -1894,15 +1892,15 @@ public void testHasattrMethods() throws Exception {
   public void testGetattrMethods() throws Exception {
     new SkylarkTest()
         .update("mock", new Mock())
-        .setUp("a = getattr(mock, 'struct_field', 'no')",
-            "b = getattr(mock, 'function', 'no')",
-            "c = getattr(mock, 'is_empty', 'no')",
-            "d = getattr('str', 'replace', 'no')",
-            "e = getattr(mock, 'other', 'no')\n")
+        .setUp("a = str(getattr(mock, 'struct_field', 'no'))",
+            "b = str(getattr(mock, 'function', 'no'))",
+            "c = str(getattr(mock, 'is_empty', 'no'))",
+            "d = str(getattr('str', 'replace', 'no'))",
+            "e = str(getattr(mock, 'other', 'no'))\n")
         .testLookup("a", "a")
-        .testLookup("b", "no")
-        .testLookup("c", "no")
-        .testLookup("d", "no")
+        .testLookup("b", "<built-in function function>")
+        .testLookup("c", "<built-in function is_empty>")
+        .testLookup("d", "<built-in function replace>")
         .testLookup("e", "no");
   }
 

src/test/starlark/testdata/function.sky

@@ -0,0 +1,60 @@
+# 2-step function call
+
+x = [1, 2, 3]
+y = x.clear
+assert_eq(x, [1, 2, 3])
+z = y()
+assert_eq(z, None)
+assert_eq(x, [])
+---
+
+x = {1: 2}
+y = x.pop
+assert_eq(x, {1: 2})
+z = y(1)
+assert_eq(z, 2)
+assert_eq(x, {})
+---
+
+x = "hello"
+y = x.upper
+z = y()
+assert_eq(z, "HELLO")
+assert_eq(x, "hello")
+---
+
+x = "abc"
+y = x.index
+z = y("b")
+assert_eq(z, 1)
+assert_eq(x, "abc")
+---
+
+y = {}
+assert_eq(y.clear == y.clear, False)
+assert_eq([].clear == [].clear, False)
+assert_eq(type([].clear), "function")
+assert_eq(str([].clear), "<built-in function clear>")
+
+---
+x = {}.pop
+x() ###  parameter 'key' has no default value
+---
+
+# getattr
+
+assert_eq(getattr("abc", "upper")(), "ABC")
+assert_eq(getattr({'a': True}, "pop")('a'), True)
+assert_eq(getattr({}, "hello", "default"), "default")
+
+y = [1, 2, 3]
+x = getattr(y, "clear")
+assert_eq(y, [1, 2, 3])
+x()
+assert_eq(y, [])
+
+---
+getattr("", "abc") ### object of type 'string' has no attribute 'abc'
+---
+x = getattr("", "pop", "clear")
+x() ### 'string' object is not callable