diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java index bfb3a1e36c2343..c8ea38de63192e 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java @@ -627,8 +627,7 @@ public StarlarkAspect aspect( * *

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 diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD index 2af591bc6b634c..8f141dda4ee201 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD +++ b/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", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java index a45f85f6344dc6..2c9a8734023881 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java +++ b/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; diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java index 35dd758fc17854..eb4baa011f3621 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java @@ -429,7 +429,7 @@ private static ImmutableMap 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) { diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleFunction.java b/src/main/java/com/google/devtools/build/lib/packages/RuleFunction.java index 1be4d07454f593..3d6aab70744642 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/RuleFunction.java +++ b/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(); } diff --git a/src/main/java/net/starlark/java/eval/BuiltinCallable.java b/src/main/java/net/starlark/java/eval/BuiltinCallable.java index 7967affea59f91..c1f7d4021e26af 100644 --- a/src/main/java/net/starlark/java/eval/BuiltinCallable.java +++ b/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; diff --git a/src/main/java/net/starlark/java/eval/Starlark.java b/src/main/java/net/starlark/java/eval/Starlark.java index 84764e2162de7f..25c76fec005850 100644 --- a/src/main/java/net/starlark/java/eval/Starlark.java +++ b/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), but it also means "any" when used // as an argument to Sequence.cast, and more generally it means "value". diff --git a/src/main/java/net/starlark/java/eval/StarlarkFunction.java b/src/main/java/net/starlark/java/eval/StarlarkFunction.java index 57292427aed28d..0b54775e3df3b0 100644 --- a/src/main/java/net/starlark/java/eval/StarlarkFunction.java +++ b/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; diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java index dd9639a18a2ea8..2c34b5a0e10c47 100644 --- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java +++ b/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"); } } diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleImplementationFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleImplementationFunctionsTest.java index 7cbae46446b7de..66cd2124d38ff9 100644 --- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleImplementationFunctionsTest.java +++ b/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"); } diff --git a/src/test/java/net/starlark/java/eval/MethodLibraryTest.java b/src/test/java/net/starlark/java/eval/MethodLibraryTest.java index 22fb066bb7c233..9c694def4590a7 100644 --- a/src/test/java/net/starlark/java/eval/MethodLibraryTest.java +++ b/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 diff --git a/src/test/java/net/starlark/java/eval/StarlarkEvaluationTest.java b/src/test/java/net/starlark/java/eval/StarlarkEvaluationTest.java index 57263186e15394..775ebab52470a5 100644 --- a/src/test/java/net/starlark/java/eval/StarlarkEvaluationTest.java +++ b/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"); } diff --git a/src/test/java/net/starlark/java/eval/testdata/dict.star b/src/test/java/net/starlark/java/eval/testdata/dict.star index c34bdef3291408..9d87fff72a6644 100644 --- a/src/test/java/net/starlark/java/eval/testdata/dict.star +++ b/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. diff --git a/src/test/java/net/starlark/java/eval/testdata/function.star b/src/test/java/net/starlark/java/eval/testdata/function.star index 71c596836ee6fd..ffcad4eba1546b 100644 --- a/src/test/java/net/starlark/java/eval/testdata/function.star +++ b/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), "") assert_eq(str({}.clear), "") assert_eq(str(len), "") diff --git a/src/test/java/net/starlark/java/eval/testdata/json.star b/src/test/java/net/starlark/java/eval/testdata/json.star index 415d9c1aa55795..e30a1639dfb8f2 100644 --- a/src/test/java/net/starlark/java/eval/testdata/json.star +++ b/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):