Migrate a few trivial uses of @SkylarkSignature to use @SkylarkCallable instead

RELNOTES: None.
PiperOrigin-RevId: 187531006

7 files changed, 84 insertions, 146 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/TransitiveInfoCollection.java b/src/main/java/com/google/devtools/build/lib/analysis/TransitiveInfoCollection.java
index 5cb79fda16..e557cc211f 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/TransitiveInfoCollection.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/TransitiveInfoCollection.java
@@ -14,12 +14,19 @@
 
 package com.google.devtools.build.lib.analysis;
 
+import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
+import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.packages.RequiredProviders;
+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.SkylarkModuleCategory;
 import com.google.devtools.build.lib.syntax.SkylarkIndexable;
+import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
 import javax.annotation.Nullable;
 
 /**
@@ -62,6 +69,20 @@ import javax.annotation.Nullable;
 )
 public interface TransitiveInfoCollection extends SkylarkIndexable, SkylarkProviderCollection {
 
+  @SkylarkCallable(name = "output_group",
+    documented = false, //  TODO(dslomov): document.
+    parameters = {
+      @Param(name = "group_name", type = String.class, doc = "Output group name")
+    }
+  )
+  default SkylarkNestedSet outputGroup(String group) {
+    OutputGroupInfo provider = OutputGroupInfo.get(this);
+    NestedSet<Artifact> result = provider != null
+        ? provider.getOutputGroup(group)
+        : NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER);
+    return SkylarkNestedSet.of(Artifact.class, result);
+  }
+
   /**
    * Returns the transitive information provider requested, or null if the provider is not found.
    * The provider has to be a TransitiveInfoProvider Java class.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkCommandLine.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkCommandLine.java
index 0bbc29ae1a..6dd7b7f258 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkCommandLine.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkCommandLine.java
@@ -17,12 +17,10 @@ package com.google.devtools.build.lib.analysis.skylark;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 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.SkylarkModuleCategory;
-import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature;
-import com.google.devtools.build.lib.syntax.BuiltinFunction;
 import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
-import com.google.devtools.build.lib.syntax.SkylarkSignatureProcessor;
 
 /** A Skylark module class to create memory efficient command lines. */
 @SkylarkModule(
@@ -33,10 +31,8 @@ import com.google.devtools.build.lib.syntax.SkylarkSignatureProcessor;
 )
 public class SkylarkCommandLine {
 
-  @SkylarkSignature(
+  @SkylarkCallable(
     name = "join_paths",
-    objectType = SkylarkCommandLine.class,
-    returnType = String.class,
     doc =
         "Deprecated. Creates a single command line argument joining the paths of a set "
             + "of files on the separator string.",
@@ -50,16 +46,9 @@ public class SkylarkCommandLine {
       )
     }
   )
-  private static BuiltinFunction joinPaths =
-      new BuiltinFunction("join_paths") {
-        public String invoke(String separator, SkylarkNestedSet files) {
-          NestedSet<Artifact> artifacts = files.getSet(Artifact.class);
-          // TODO(bazel-team): lazy evaluate
-          return Artifact.joinExecPaths(separator, artifacts);
-        }
-      };
-
-  static {
-    SkylarkSignatureProcessor.configureSkylarkFunctions(SkylarkCommandLine.class);
+  public String joinPaths(String separator, SkylarkNestedSet files) {
+    NestedSet<Artifact> artifacts = files.getSet(Artifact.class);
+    // TODO(bazel-team): lazy evaluate
+    return Artifact.joinExecPaths(separator, artifacts);
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
index 5e30bb4abe..b58432c1ef 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
@@ -32,12 +32,10 @@ import com.google.common.cache.LoadingCache;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
-import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.ActionsProvider;
 import com.google.devtools.build.lib.analysis.BaseRuleClasses;
 import com.google.devtools.build.lib.analysis.DefaultInfo;
 import com.google.devtools.build.lib.analysis.OutputGroupInfo;
-import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.config.ConfigAwareRuleClassBuilder;
 import com.google.devtools.build.lib.analysis.config.HostTransition;
 import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
@@ -46,9 +44,6 @@ import com.google.devtools.build.lib.analysis.test.TestConfiguration;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.cmdline.LabelValidator;
-import com.google.devtools.build.lib.collect.nestedset.NestedSet;
-import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
-import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.AttributeMap;
@@ -91,7 +86,6 @@ import com.google.devtools.build.lib.syntax.Runtime;
 import com.google.devtools.build.lib.syntax.SkylarkCallbackFunction;
 import com.google.devtools.build.lib.syntax.SkylarkDict;
 import com.google.devtools.build.lib.syntax.SkylarkList;
-import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
 import com.google.devtools.build.lib.syntax.SkylarkSignatureProcessor;
 import com.google.devtools.build.lib.syntax.SkylarkType;
 import com.google.devtools.build.lib.syntax.SkylarkUtils;
@@ -1248,29 +1242,6 @@ public class SkylarkRuleClassFunctions {
         }
       };
 
-  @SkylarkSignature(name = "output_group",
-      documented = false, //  TODO(dslomov): document.
-      objectType =  TransitiveInfoCollection.class,
-      returnType = SkylarkNestedSet.class,
-      parameters = {
-          @Param(name = "self", type = TransitiveInfoCollection.class, doc =
-              "this target"
-          ),
-          @Param(name = "group_name", type = String.class, doc =
-              "Output group name"
-          )
-      }
-  )
-  private static final BuiltinFunction output_group = new BuiltinFunction("output_group") {
-    public SkylarkNestedSet invoke(TransitiveInfoCollection self, String group) {
-      OutputGroupInfo provider = OutputGroupInfo.get(self);
-      NestedSet<Artifact> result = provider != null
-          ? provider.getOutputGroup(group)
-          : NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER);
-      return SkylarkNestedSet.of(Artifact.class, result);
-    }
-  };
-
   static {
     SkylarkSignatureProcessor.configureSkylarkFunctions(SkylarkRuleClassFunctions.class);
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleSkylarkCommon.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleSkylarkCommon.java
index d72a550377..5e8881adff 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleSkylarkCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleSkylarkCommon.java
@@ -403,13 +403,10 @@ public class AppleSkylarkCommon {
         }
       };
 
-  @SkylarkSignature(
+  @SkylarkCallable(
     name = "new_dynamic_framework_provider",
-    objectType = AppleSkylarkCommon.class,
-    returnType = AppleDynamicFrameworkInfo.class,
     doc = "Creates a new AppleDynamicFramework provider instance.",
     parameters = {
-      @Param(name = "self", type = AppleSkylarkCommon.class, doc = "The apple_common instance."),
       @Param(
         name = AppleDynamicFrameworkInfo.DYLIB_BINARY_FIELD_NAME,
         type = Artifact.class,
@@ -452,35 +449,29 @@ public class AppleSkylarkCommon {
       )
     }
   )
-  public static final BuiltinFunction NEW_DYNAMIC_FRAMEWORK_PROVIDER =
-      new BuiltinFunction("new_dynamic_framework_provider") {
-        @SuppressWarnings("unused")
-        // This method is registered statically for skylark, and never called directly.
-        public AppleDynamicFrameworkInfo invoke(
-            AppleSkylarkCommon self,
-            Artifact dylibBinary,
-            ObjcProvider depsObjcProvider,
-            Object dynamicFrameworkDirs,
-            Object dynamicFrameworkFiles) {
-          NestedSet<PathFragment> frameworkDirs;
-          if (dynamicFrameworkDirs == Runtime.NONE) {
-            frameworkDirs = NestedSetBuilder.<PathFragment>emptySet(Order.STABLE_ORDER);
-          } else {
-            Iterable<String> pathStrings =
-                ((SkylarkNestedSet) dynamicFrameworkDirs).getSet(String.class);
-            frameworkDirs =
-                NestedSetBuilder.<PathFragment>stableOrder()
-                    .addAll(Iterables.transform(pathStrings, PathFragment::create))
-                    .build();
-          }
-          NestedSet<Artifact> frameworkFiles =
-              dynamicFrameworkFiles != Runtime.NONE
-                  ? ((SkylarkNestedSet) dynamicFrameworkFiles).getSet(Artifact.class)
-                  : NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER);
-          return new AppleDynamicFrameworkInfo(
-              dylibBinary, depsObjcProvider, frameworkDirs, frameworkFiles);
-        }
-      };
+  public AppleDynamicFrameworkInfo newDynamicFrameworkProvider(
+      Artifact dylibBinary,
+      ObjcProvider depsObjcProvider,
+      Object dynamicFrameworkDirs,
+      Object dynamicFrameworkFiles) {
+    NestedSet<PathFragment> frameworkDirs;
+    if (dynamicFrameworkDirs == Runtime.NONE) {
+      frameworkDirs = NestedSetBuilder.<PathFragment>emptySet(Order.STABLE_ORDER);
+    } else {
+      Iterable<String> pathStrings =
+          ((SkylarkNestedSet) dynamicFrameworkDirs).getSet(String.class);
+      frameworkDirs =
+          NestedSetBuilder.<PathFragment>stableOrder()
+              .addAll(Iterables.transform(pathStrings, PathFragment::create))
+              .build();
+    }
+    NestedSet<Artifact> frameworkFiles =
+        dynamicFrameworkFiles != Runtime.NONE
+            ? ((SkylarkNestedSet) dynamicFrameworkFiles).getSet(Artifact.class)
+            : NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER);
+    return new AppleDynamicFrameworkInfo(
+        dylibBinary, depsObjcProvider, frameworkDirs, frameworkFiles);
+  }
 
   @SkylarkCallable(
       name = "link_multi_arch_binary",
@@ -503,31 +494,20 @@ public class AppleSkylarkCommon {
     }
   }
 
-  @SkylarkSignature(
+  @SkylarkCallable(
     name = "dotted_version",
-    objectType = AppleSkylarkCommon.class,
-    returnType = DottedVersion.class,
     doc = "Creates a new <a href=\"DottedVersion.html\">DottedVersion</a> instance.",
     parameters = {
-      @Param(name = "self", type = AppleSkylarkCommon.class, doc = "The apple_common instance."),
       @Param(
         name = "version",
         type = String.class,
-        named = false,
-        positional = false,
         doc = "The string representation of the DottedVersion."
       )
     }
   )
-  public static final BuiltinFunction DOTTED_VERSION =
-      new BuiltinFunction("dotted_version") {
-        @SuppressWarnings("unused")
-        // This method is registered statically for skylark, and never called directly.
-        public DottedVersion invoke(
-            AppleSkylarkCommon self, String version) {
-          return DottedVersion.fromString(version);
-        }
-      };
+  public DottedVersion dottedVersion(String version) {
+    return DottedVersion.fromString(version);
+  }
 
   @SkylarkCallable(
     name = "objc_proto_aspect",
diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/SkylarkTestingModule.java b/src/main/java/com/google/devtools/build/lib/rules/test/SkylarkTestingModule.java
index d95302634f..a2fa110a85 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/test/SkylarkTestingModule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/test/SkylarkTestingModule.java
@@ -16,11 +16,9 @@ package com.google.devtools.build.lib.rules.test;
 import com.google.devtools.build.lib.analysis.test.ExecutionInfo;
 import com.google.devtools.build.lib.analysis.test.TestEnvironmentInfo;
 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.BuiltinFunction;
 import com.google.devtools.build.lib.syntax.SkylarkDict;
-import com.google.devtools.build.lib.syntax.SkylarkSignatureProcessor;
 
 /** A class that exposes testing infrastructure to skylark. */
 @SkylarkModule(
@@ -29,17 +27,13 @@ import com.google.devtools.build.lib.syntax.SkylarkSignatureProcessor;
 )
 public class SkylarkTestingModule {
 
-  // TODO(bazel-team): Change this BuiltinFunction to be the actual
-  // ExecutionInfo.PROVIDER.
-  @SkylarkSignature(
+  // TODO(bazel-team): Change this BuiltinFunction to be the actual ExecutionInfo.PROVIDER.
+  @SkylarkCallable(
     name = "ExecutionInfo",
-    objectType = SkylarkTestingModule.class,
-    returnType = ExecutionInfo.class,
     doc =
         "Creates a new execution info provider. Use this provider to specify special"
             + "environments requirements needed to run tests.",
     parameters = {
-      @Param(name = "self", type = SkylarkTestingModule.class, doc = "The 'testing' instance."),
       @Param(
         name = "requirements",
         type = SkylarkDict.class,
@@ -52,26 +46,17 @@ public class SkylarkTestingModule {
       )
     }
   )
-  public static final BuiltinFunction NEW_EXECUTION_INFO_PROVIDER =
-      new BuiltinFunction("ExecutionInfo") {
-        @SuppressWarnings("unused")
-        // This method is registered statically for skylark, and never called directly.
-        public ExecutionInfo invoke(SkylarkTestingModule self, SkylarkDict requirements) {
-          return new ExecutionInfo(requirements);
-        }
-      };
+  public ExecutionInfo executionInfo(SkylarkDict<String, String> requirements) {
+    return new ExecutionInfo(requirements);
+  }
 
-  // TODO(bazel-team): Change this BuiltinFunction to be the actual
-  // TestEnvironmentInfo.PROVIDER.
-  @SkylarkSignature(
+  // TODO(bazel-team): Change this BuiltinFunction to be the actual TestEnvironmentInfo.PROVIDER.
+  @SkylarkCallable(
     name = "TestEnvironment",
-    objectType = SkylarkTestingModule.class,
-    returnType = TestEnvironmentInfo.class,
     doc =
         "Creates a new test environment provider. Use this provider to specify extra"
             + "environment variables to be made available during test execution.",
     parameters = {
-      @Param(name = "self", type = SkylarkTestingModule.class, doc = "The 'testing' instance."),
       @Param(
         name = "environment",
         type = SkylarkDict.class,
@@ -83,16 +68,7 @@ public class SkylarkTestingModule {
       )
     }
   )
-  public static final BuiltinFunction NEW_TEST_ENVIRONMENT_PROVIDER =
-      new BuiltinFunction("TestEnvironment") {
-        @SuppressWarnings("unused")
-        // This method is registered statically for skylark, and never called directly.
-        public TestEnvironmentInfo invoke(SkylarkTestingModule self, SkylarkDict environment) {
-          return new TestEnvironmentInfo(environment);
-        }
-      };
-
-  static {
-    SkylarkSignatureProcessor.configureSkylarkFunctions(SkylarkTestingModule.class);
+  public TestEnvironmentInfo testEnvironment(SkylarkDict<String, String> environment) {
+    return new TestEnvironmentInfo(environment);
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
index 55781cef82..59135375e4 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
@@ -1547,24 +1547,6 @@ public class MethodLibrary {
     }
   };
 
-  @SkylarkSignature(name = "get", objectType = SkylarkDict.class,
-      doc = "Returns the value for <code>key</code> if <code>key</code> is in the dictionary, "
-          + "else <code>default</code>. If <code>default</code> is not given, it defaults to "
-          + "<code>None</code>, so that this method never throws an error.",
-      parameters = {
-        @Param(name = "self", doc = "This dict."),
-        @Param(name = "key", doc = "The key to look for."),
-        @Param(name = "default", defaultValue = "None",
-            doc = "The default value to use (instead of None) if the key is not found.")})
-  private static final BuiltinFunction get = new BuiltinFunction("get") {
-    public Object invoke(SkylarkDict<?, ?> self, Object key, Object defaultValue) {
-      if (self.containsKey(key)) {
-        return self.get(key);
-      }
-      return defaultValue;
-    }
-  };
-
   @SkylarkSignature(
     name = "tuple",
     returnType = Tuple.class,
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkDict.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkDict.java
index 3cd3f999bb..8c951cbb44 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkDict.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkDict.java
@@ -16,6 +16,8 @@ package com.google.devtools.build.lib.syntax;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.devtools.build.lib.events.Location;
+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.SkylarkModuleCategory;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
@@ -70,6 +72,23 @@ public final class SkylarkDict<K, V> extends MutableMap<K, V>
     this.mutability = env == null ? Mutability.IMMUTABLE : env.mutability();
   }
 
+  @SkylarkCallable(name = "get",
+    doc = "Returns the value for <code>key</code> if <code>key</code> is in the dictionary, "
+        + "else <code>default</code>. If <code>default</code> is not given, it defaults to "
+        + "<code>None</code>, so that this method never throws an error.",
+    parameters = {
+      @Param(name = "key", noneable = true, doc = "The key to look for."),
+      @Param(name = "default", defaultValue = "None", noneable = true, named = true,
+          doc = "The default value to use (instead of None) if the key is not found.")},
+    allowReturnNones = true
+  )
+  public Object get(Object key, Object defaultValue) {
+    if (this.containsKey(key)) {
+      return this.get(key);
+    }
+    return defaultValue;
+  }
+
   private static final SkylarkDict<?, ?> EMPTY = withMutability(Mutability.IMMUTABLE);
 
   /** Returns an immutable empty dict. */