bazel analysis/skylark: fix crash in attr.int(default=<function>)
The type of the default=... parameter of each of the attr.<type> functions
was inconsistent with the Param.type or Param.allowed_types annotations,
causing the annotation-based type-check to accept a StarlarkFunction value,
but the reflective call to then fail.
This CL removes StarlarkFunction from Param.allowed_types for these parameters:
attr.int(default=function)
attr.int_list(default=function)
attr.string(default=function)
attr.string_list(default=function)
attr.bool(default=function)
attr.string_dict(default=function)
attr.string_list_dict(default=function)
Only these label-oriented attribute types accept a function:
attr.label,
attr.label_list
attr.label_keyed_string_dict
attr.output \ the default parameter
attt.output_list / is deprecated for these
CL 285849424 fixes the annotation processor to detect this and similar errors.
It catches other instances and does other checks, so it is split off as a separate change.
Also:
- move the nasty little optionMap function out of EvalUtils (where its
generic type forces it to abuse unsafe casts) and into SkylarkAttr.
(The only other use, in a test, was trivially replaced by simpler code.)
- use Map not Dict for the map produced by optionMap.
See github.com/bazelbuild/bazel/issues/9463.
PiperOrigin-RevId: 286037108
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
index 710d364..2301288 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
@@ -671,6 +671,13 @@
@Test
public void testLateBoundAttrWorksWithOnlyLabel() throws Exception {
+ // Late-bound attributes, which are computed during analysis as a function
+ // of the configuration, are only available for attributes involving labels:
+ // attr.label
+ // attr.label_list
+ // attr.label_keyed_string_dict
+ // attr.output,
+ // attr.output_list
checkEvalErrorContains(
"expected value of type 'string' for parameter 'default', for call to method "
+ "string(default = '', doc = '', mandatory = False, values = []) "
@@ -679,6 +686,42 @@
"attr.string(default=attr_value)");
}
+ @Test
+ public void testNoComputedAttrDefaults() throws Exception {
+ // This is a regression test for github.com/bazelbuild/bazel/issues/9463.
+ // The loading-phase feature, computed attribute defaults, is not exposed
+ // to Starlark.
+ // (Not to be confused with "late-bound defaults", an analysis-phase
+ // mechanism only for attributes involving labels; see test above.)
+ //
+ // Most attributes like attr.string should not accept a function as
+ // their default value. (The bug was that the @SkylarkCallable
+ // annotation was more permissive than the method declaration.)
+ exec("def f(): pass");
+ checkEvalErrorContains(
+ "expected value of type 'string' for parameter 'default'", "attr.string(default=f)");
+ checkEvalErrorContains(
+ "expected value of type 'sequence of strings' for parameter 'default'",
+ "attr.string_list(default=f)");
+ checkEvalErrorContains(
+ "expected value of type 'int' for parameter 'default'", "attr.int(default=f)");
+ checkEvalErrorContains(
+ "expected value of type 'sequence of ints' for parameter 'default'",
+ "attr.int_list(default=f)");
+ checkEvalErrorContains(
+ "expected value of type 'bool' for parameter 'default'", "attr.bool(default=f)");
+ checkEvalErrorContains(
+ "expected value of type 'dict' for parameter 'default'", "attr.string_dict(default=f)");
+ checkEvalErrorContains(
+ "expected value of type 'dict' for parameter 'default'",
+ "attr.string_list_dict(default=f)");
+ // Also:
+ // - The attr.output(default=...) parameter is deprecated
+ // (see --incompatible_no_output_attr_default)
+ // - attr.license appears to be disabled already.
+ // (see --incompatible_no_attr_license)
+ }
+
private static final Label FAKE_LABEL = Label.parseAbsoluteUnchecked("//fake/label.bzl");
@Test