starlark: change type(len) to "builtin_function_or_method"

Before, any Callable had a type of 'function', but this was a mistake.

The Callable interface itself is now described as "callable"
(e.g. in type error messages), but implementations must define their
own type string. Bazel now uses "rule" and "repository_rule" for its two.

This is technically a breaking change, though I expect the fallout to be small.
(I found no problematic uses in Google's code base.)

PiperOrigin-RevId: 339098833

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java

@@ -627,8 +627,7 @@ public StarlarkAspect aspect(
  *
  * <p>Exactly one of {@link #builder} or {@link #ruleClass} is null except inside {@link #export}.
  */
- public static final class StarlarkRuleFunction
- implements StarlarkCallable, StarlarkExportable, RuleFunction {
+ public static final class StarlarkRuleFunction implements StarlarkExportable, RuleFunction {
  private RuleClass.Builder builder;
 
  private RuleClass ruleClass;
@@ -637,6 +636,10 @@ public static final class StarlarkRuleFunction
  private final Location definitionLocation;
  private Label starlarkLabel;
 
+ // TODO(adonovan): merge {Starlark,Builtin}RuleFunction and RuleClass,
+ // making the latter a callable, StarlarkExportable value.
+ // (Making RuleClasses first-class values will help us to build a
+ // rich query output mode that includes values from loaded .bzl files.)
  public StarlarkRuleFunction(
  RuleClass.Builder builder,
  RuleClassType type,
@@ -665,7 +668,7 @@ private StarlarkRuleFunction(
 
  @Override
  public String getName() {
- return "rule";
+ return ruleClass != null ? ruleClass.getName() : "unexported rule";
  }
 
  @Override

src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD

@@ -12,6 +12,7 @@ java_library(
  name = "starlark",
  srcs = glob(["*.java"]),
  deps = [
+ "//src/main/java/com/google/devtools/build/docgen/annot",
  "//src/main/java/com/google/devtools/build/lib:runtime",
  "//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
  "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
@@ -45,6 +46,7 @@ java_library(
  "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
  "//src/main/java/com/google/devtools/build/skyframe",
  "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
+ "//src/main/java/net/starlark/java/annot",
  "//src/main/java/net/starlark/java/eval",
  "//third_party:guava",
  "//third_party:java-diff-utils",

src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java

@@ -20,6 +20,7 @@
 import static com.google.devtools.build.lib.packages.Type.STRING_LIST;
 
 import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.docgen.annot.DocCategory;
 import com.google.devtools.build.lib.analysis.BaseRuleClasses;
 import com.google.devtools.build.lib.analysis.starlark.StarlarkAttrModule.Descriptor;
 import com.google.devtools.build.lib.cmdline.Label;
@@ -40,6 +41,7 @@
 import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
 import com.google.devtools.build.lib.starlarkbuildapi.repository.RepositoryModuleApi;
 import java.util.Map;
+import net.starlark.java.annot.StarlarkBuiltin;
 import net.starlark.java.eval.Dict;
 import net.starlark.java.eval.EvalException;
 import net.starlark.java.eval.Module;
@@ -110,6 +112,12 @@ public StarlarkCallable repositoryRule(
 
  // RepositoryRuleFunction is the result of repository_rule(...).
  // It is a callable value; calling it yields a Rule instance.
+ @StarlarkBuiltin(
+ name = "repository_rule",
+ category = DocCategory.BUILTIN,
+ doc =
+ "A callable value that may be invoked during evaluation of the WORKSPACE file to"
+ + " instantiate and return a repository rule.")
  private static final class RepositoryRuleFunction
  implements StarlarkCallable, StarlarkExportable {
  private final RuleClass.Builder builder;

src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java

@@ -429,7 +429,7 @@ private static ImmutableMap<String, BuiltinRuleFunction> buildRuleFunctions(
  /** A callable Starlark value that creates Rules for native RuleClasses. */
  // TODO(adonovan): why is this distinct from RuleClass itself?
  // Make RuleClass implement StarlarkCallable directly.
- private static class BuiltinRuleFunction implements StarlarkCallable, RuleFunction {
+ private static class BuiltinRuleFunction implements RuleFunction {
  private final RuleClass ruleClass;
 
  BuiltinRuleFunction(RuleClass ruleClass) {

src/main/java/com/google/devtools/build/lib/packages/RuleFunction.java

@@ -14,7 +14,18 @@
 
 package com.google.devtools.build.lib.packages;
 
+import com.google.devtools.build.docgen.annot.DocCategory;
+import net.starlark.java.annot.StarlarkBuiltin;
+import net.starlark.java.eval.StarlarkCallable;
+
 /** Interface for a native or Starlark rule function. */
-public interface RuleFunction {
+@StarlarkBuiltin(
+ name = "rule",
+ category = DocCategory.BUILTIN,
+ doc =
+ "A callable value representing the type of a native or Starlark rule. Calling the value"
+ + " during evaluation of a package's BUILD file creates an instance of the rule and"
+ + " adds it to the package's target set.")
+public interface RuleFunction extends StarlarkCallable {
  RuleClass getRuleClass();
 }

src/main/java/net/starlark/java/eval/BuiltinCallable.java

@@ -21,6 +21,7 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 import javax.annotation.Nullable;
+import net.starlark.java.annot.StarlarkBuiltin;
 import net.starlark.java.annot.StarlarkMethod;
 import net.starlark.java.spelling.SpellChecker;
 
@@ -30,8 +31,11 @@
  * Starlark value. BuiltinCallables are not produced for Java methods for which {@link
  * StarlarkMethod#structField} is true.
  */
-// TODO(adonovan): rename AnnotatedMethod?
-// TODO(adonovan): annotate with type="builtin_function_or_method"
+// TODO(adonovan): rename to BuiltinFunction. Also, support annotated static methods.
+@StarlarkBuiltin(
+ name = "builtin_function_or_method", // (following Python)
+ category = "core",
+ doc = "The type of a built-in function, defined by Java code.")
 public final class BuiltinCallable implements StarlarkCallable {
 
  private final Object obj;

src/main/java/net/starlark/java/eval/Starlark.java

@@ -329,16 +329,8 @@ public static String classType(Class<?> c) {
  return module.name();
  }
 
- if (StarlarkCallable.class.isAssignableFrom(c)) {
- // All callable values have historically been lumped together as "function".
- // TODO(adonovan): eliminate this case.
- // Built-in types that don't use StarlarkModule should report
- // their own type string, but this is a breaking change as users often
- // use type(x)=="function" for Starlark and built-in functions.
- return "function";
-
- } else if (c.equals(Object.class)) {
- // "Unknown" is another unfortunate choice.
+ if (c.equals(Object.class)) {
+ // "unknown" is another unfortunate choice.
  // Object.class does mean "unknown" when talking about the type parameter
  // of a collection (List<Object>), but it also means "any" when used
  // as an argument to Sequence.cast, and more generally it means "value".

src/main/java/net/starlark/java/eval/StarlarkFunction.java

@@ -19,6 +19,7 @@
 import java.util.Arrays;
 import java.util.List;
 import javax.annotation.Nullable;
+import net.starlark.java.annot.StarlarkBuiltin;
 import net.starlark.java.spelling.SpellChecker;
 import net.starlark.java.syntax.Expression;
 import net.starlark.java.syntax.ExpressionStatement;
@@ -28,7 +29,10 @@
 import net.starlark.java.syntax.StringLiteral;
 
 /** A StarlarkFunction is a function value created by a Starlark {@code def} statement. */
-// TODO(adonovan): annotate with type="function"
+@StarlarkBuiltin(
+ name = "function",
+ category = "core",
+ doc = "The type of functions declared in Starlark.")
 public final class StarlarkFunction implements StarlarkCallable {
 
  private final Resolver.Function rfn;

src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java

@@ -1090,7 +1090,7 @@ public void testTextMessageInvalidStructure() throws Exception {
 
  // callable in field
  ev.checkEvalErrorContains(
- "in struct field .a: got function, want string, int, bool, or struct",
+ "in struct field .a: got builtin_function_or_method, want string, int, bool, or struct",
  "struct(a=rule).to_proto()");
  }
 
@@ -1166,7 +1166,7 @@ public void testJsonNestedListStructure() throws Exception {
  public void testJsonInvalidStructure() throws Exception {
  ev.checkEvalErrorContains(
  "Invalid text format, expected a struct, a string, a bool, or an int but got a "
- + "function for struct field 'a'",
+ + "builtin_function_or_method for struct field 'a'",
  "struct(a=rule).to_json()");
  }
 
@@ -2047,6 +2047,7 @@ public void ruleClassDefinedInBuildFile_fails() throws Exception {
 
  getConfiguredTarget("//r:r");
 
- ev.assertContainsError("Error in rule: Invalid rule class hasn't been exported by a bzl file");
+ ev.assertContainsError(
+ "Error in unexported rule: Invalid rule class hasn't been exported by a bzl file");
  }
 }

src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleImplementationFunctionsTest.java

@@ -1974,11 +1974,9 @@ public void testBuiltInFunctionAsRuleImplementation() throws Exception {
  "silly_rule(name = 'silly')");
  thrown.handleAssertionErrors(); // Compatibility with JUnit 4.11
  thrown.expect(AssertionError.class);
- // This confusing message shows why we should distinguish
- // built-ins and Starlark functions in their repr strings.
  thrown.expectMessage(
- "in call to rule(), parameter 'implementation' got value of type 'function', want"
- + " 'function'");
+ "in call to rule(), parameter 'implementation' got value of type"
+ + " 'builtin_function_or_method', want 'function'");
  getConfiguredTarget("//test:silly");
  }
 

src/test/java/net/starlark/java/eval/MethodLibraryTest.java

@@ -287,7 +287,8 @@ ev.new Scenario()
  .testEval("sorted([[1], [], [1, 2]], key=len, reverse=True)", "[[1, 2], [1], []]")
  .testEval("sorted([[0, 5], [4, 1], [1, 7]], key=max)", "[[4, 1], [0, 5], [1, 7]]")
  .testIfExactError(
- "unsupported comparison: function <=> function", "sorted([sorted, sorted])");
+ "unsupported comparison: builtin_function_or_method <=> builtin_function_or_method",
+ "sorted([sorted, sorted])");
  }
 
  @Test
@@ -622,8 +623,8 @@ public void testLenOnBadType() throws Exception {
  @Test
  public void testIndexOnFunction() throws Exception {
  ev.new Scenario()
- .testIfErrorContains("type 'function' has no operator [](int)", "len[1]")
- .testIfErrorContains("invalid slice operand: function", "len[1:4]");
+ .testIfErrorContains("type 'builtin_function_or_method' has no operator [](int)", "len[1]")
+ .testIfErrorContains("invalid slice operand: builtin_function_or_method", "len[1:4]");
  }
 
  @Test
@@ -656,13 +657,15 @@ public void testStrFunction() throws Exception {
  @Test
  public void testType() throws Exception {
  ev.new Scenario()
+ .setUp("def f(): pass")
  .testExpression("type(1)", "int")
  .testExpression("type('a')", "string")
  .testExpression("type([1, 2])", "list")
  .testExpression("type((1, 2))", "tuple")
  .testExpression("type(True)", "bool")
  .testExpression("type(None)", "NoneType")
- .testExpression("type(str)", "function");
+ .testExpression("type(f)", "function")
+ .testExpression("type(str)", "builtin_function_or_method");
  }
 
  @Test

src/test/java/net/starlark/java/eval/StarlarkEvaluationTest.java

@@ -1353,7 +1353,9 @@ ev.new Scenario()
 
  @Test
  public void testStructAccessOfMethod() throws Exception {
- ev.new Scenario().update("mock", new Mock()).testExpression("type(mock.function)", "function");
+ ev.new Scenario()
+ .update("mock", new Mock())
+ .testExpression("type(mock.function)", "builtin_function_or_method");
  ev.new Scenario().update("mock", new Mock()).testExpression("mock.function()", "a");
  }
 

src/test/java/net/starlark/java/eval/testdata/dict.star

@@ -95,19 +95,22 @@ assert_eq("a" in dict(a=1), True)
 # 'len in {}' and '{}.get(len, False)' should both successfully evaluate to False.
 # TODO(adonovan): clarify spec and fix this test (https://github.com/bazelbuild/starlark/issues/65)
 
+# Starlark functions are already hashable:
+def f(): pass
+f in {} # no error
 # unhashable types
 
 {} in {} ### unhashable type: 'dict'
 ---
 [] in {} ### unhashable type: 'list'
 ---
-len in {} ### unhashable type: 'function'
+len in {} ### unhashable type: 'builtin_function_or_method'
 ---
 {}.get([]) ### unhashable type: 'list'
 ---
 dict().get({}) ### unhashable type: 'dict'
 ---
-{1: 2}.get(len) ### unhashable type: 'function'
+{1: 2}.get(len) ### unhashable type: 'builtin_function_or_method'
 ---
 # For composite keys, the error message relates to the
 # unhashable subelement of the key, not the key itself.

src/test/java/net/starlark/java/eval/testdata/function.star

@@ -33,7 +33,7 @@ assert_eq(x, "abc")
 y = {}
 assert_eq(y.clear == y.clear, False)
 assert_eq([].clear == [].clear, False)
-assert_eq(type([].clear), "function")
+assert_eq(type([].clear), "builtin_function_or_method")
 assert_eq(str([].clear), "<built-in method clear of list value>")
 assert_eq(str({}.clear), "<built-in method clear of dict value>")
 assert_eq(str(len), "<built-in function len>")

src/test/java/net/starlark/java/eval/testdata/json.star

@@ -39,11 +39,11 @@ json.encode(float("NaN")) ### cannot encode non-finite float nan
 ---
 json.encode({1: "two"}) ### dict has int key, want string
 ---
-json.encode(len) ### cannot encode function as JSON
+json.encode(len) ### cannot encode builtin_function_or_method as JSON
 ---
-json.encode(struct(x = [1, len])) ### in struct field .x: at list index 1: cannot encode function as JSON
+json.encode(struct(x = [1, len])) ### in struct field .x: at list index 1: cannot encode builtin_function_or_method as JSON
 ---
-json.encode(struct(x = [1, {"x": len}])) ### in struct field .x: at list index 1: in dict key "x": cannot encode function as JSON
+json.encode(struct(x = [1, {"x": len}])) ### in struct field .x: at list index 1: in dict key "x": cannot encode builtin_function_or_method as JSON
 ---
 def f(deep):
  for x in range(10000):