Convert MethodLibrary's SkylarkSignature methods to SkylarkCallable

Note that converted parameter @param annotations need legacyNamed = true (and in some places, noneable = True) to retain compatibility with the bugginess of SkylarkSignature. This facilitates followup to clean up these methods / parameters -- given the refactor, we can enforce some of these parameters are positional-only (subject to an --incompatible flag) Progress toward bazelbuild#5010 RELNOTES: None. PiperOrigin-RevId: 230975480
weixiao-huang · Jan 25, 2019 · d3dd9d1 · d3dd9d1
1 parent 6b96ff7
commit d3dd9d1
Show file tree

Hide file tree

Showing 16 changed files with 517 additions and 594 deletions.
diff --git a/src/main/java/com/google/devtools/build/docgen/ApiExporter.java b/src/main/java/com/google/devtools/build/docgen/ApiExporter.java
@@ -27,6 +27,7 @@
 import com.google.devtools.build.lib.skylarkinterface.SkylarkInterfaceUtils;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
 import com.google.devtools.build.lib.syntax.BaseFunction;
+import com.google.devtools.build.lib.syntax.BuiltinCallable;
 import com.google.devtools.build.lib.syntax.FuncallExpression;
 import com.google.devtools.build.lib.syntax.FunctionSignature;
 import com.google.devtools.build.lib.syntax.MethodDescriptor;
@@ -93,6 +94,15 @@ private static void appendGlobals(Builtins.Builder builtins, Map<String, Object>
       Value.Builder value = Value.newBuilder();
       if (obj instanceof BaseFunction) {
         value = collectFunctionInfo((BaseFunction) obj);
+      } else if (obj instanceof BuiltinCallable) {
+        BuiltinCallable builtinCallable = (BuiltinCallable) obj;
+        MethodDescriptor descriptor =
+            builtinCallable.getMethodDescriptor(SkylarkSemantics.DEFAULT_SEMANTICS);
+        value =
+            collectFunctionInfo(
+                descriptor.getName(),
+                SkylarkSignatureProcessor.getSignatureForCallable(
+                    descriptor.getName(), descriptor, null, null));
       } else {
         value.setName(entry.getKey());
       }

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BuiltinCallable.java b/src/main/java/com/google/devtools/build/lib/syntax/BuiltinCallable.java
@@ -42,8 +42,7 @@ public Object call(
       FuncallExpression ast,
       Environment env)
       throws EvalException, InterruptedException {
-    MethodDescriptor methodDescriptor =
-        FuncallExpression.getMethod(env.getSemantics(), obj.getClass(), methodName);
+    MethodDescriptor methodDescriptor = getMethodDescriptor(env.getSemantics());
 
     // TODO(cparsons): Profiling should be done at the MethodDescriptor level.
     try (SilentCloseable c =
@@ -55,6 +54,10 @@ public Object call(
     }
   }
 
+  public MethodDescriptor getMethodDescriptor(SkylarkSemantics semantics) {
+    return FuncallExpression.getMethod(semantics, obj.getClass(), methodName);
+  }
+
   @Override
   public void repr(SkylarkPrinter printer) {
     printer.append("<built-in function " + methodName + ">");

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
@@ -647,7 +647,7 @@ private EvalException unexpectedKeywordArgumentException(
         String.format(
             "unexpected keyword%s %s",
             unexpectedKeywords.size() > 1 ? "s" : "",
-            Joiner.on(",").join(Iterables.transform(unexpectedKeywords, s -> "'" + s + "'"))),
+            Joiner.on(", ").join(Iterables.transform(unexpectedKeywords, s -> "'" + s + "'"))),
         method,
         objClass);
   }