bazel syntax: optimize calls to built-in methods
This change rewrites CallUtils.convertStarlarkArgumentsToJavaMethodArguments,
now called BuiltinCallable.getArgumentVector, to process arguments in three
steps---positional, named, defaults---avoiding all memory allocation
except for the argument vector itself. Evaluation of parameter default
value expressions has been moved into ParamDescriptor, so that concurrent
hash map lookups are avoided.
Also, the error messages have been improved in many ways
- they are more concise, and don't dump the whole signature.
- they state facts before expectations.
- they are better phrased (subject first, emphasis last).
- the called function is always named, but simply, not in fussy detail.
- the numbers of accepted parameters are stated, when relevant.
- the incorrect type is shown after a failed type check.
- unexpected keywords are spell-checked.
- plurals are correct.
The logic is somewhat simpler.
Except in lib.syntax itself, test expectations of these error messages
have been edited down to the crucial parts, for robustness.
This is a breaking API change for copybara.
BEGIN_PUBLIC
bazel syntax: optimize calls to built-in methods
END_PUBLIC
PiperOrigin-RevId: 288725155
diff --git a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
index 4e5bf23..3b85a63 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
@@ -142,16 +142,16 @@
@Test
public void testExportsFilesVisibilityMustBeSequence() throws Exception {
expectEvalError(
- "expected value of type 'sequence or NoneType' for parameter 'visibility', "
- + "for call to method exports_files",
+ "in call to exports_files(), parameter 'visibility' got value of type 'depset', want"
+ + " 'sequence or NoneType'",
"exports_files(srcs=[], visibility=depset(['notice']))");
}
@Test
public void testExportsFilesLicensesMustBeSequence() throws Exception {
expectEvalError(
- "expected value of type 'sequence of strings or NoneType' for parameter 'licenses', "
- + "for call to method exports_files",
+ "in call to exports_files(), parameter 'licenses' got value of type 'depset', want"
+ + " 'sequence of strings or NoneType'",
"exports_files(srcs=[], licenses=depset(['notice']))");
}
@@ -647,7 +647,7 @@
events.setFailFast(false);
assertGlobFails(
"glob(['incl'],['excl'],3,True,'extraarg')",
- "expected no more than 4 positional arguments, but got 5, for call to method glob");
+ "glob() accepts no more than 4 positional arguments but got 5");
}
@Test
@@ -655,8 +655,8 @@
events.setFailFast(false);
assertGlobFails(
"glob(1, exclude=2)",
- "expected value of type 'sequence of strings' for parameter 'include', "
- + "for call to method glob");
+ "in call to glob(), parameter 'include' got value of type 'int', want 'sequence of"
+ + " strings'");
}
@Test
@@ -820,8 +820,7 @@
@Test
public void testPackageGroupNamedArguments() throws Exception {
expectEvalError(
- "expected no more than 0 positional arguments, but got 1,",
- "package_group('skin', name = 'x')");
+ "package_group() got unexpected positional argument", "package_group('skin', name = 'x')");
}
@Test
@@ -1042,7 +1041,7 @@
@Test
public void testIncompleteEnvironmentGroup() throws Exception {
expectEvalError(
- "parameter 'defaults' has no default value, for call to function environment_group",
+ "environment_group() missing 1 required named argument: defaults",
"environment(name = 'foo')",
"environment_group(name='group', environments = [':foo'])");
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java
index ac0c181..ee3be76 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java
@@ -4588,56 +4588,58 @@
AssertionError e = assertThrows(AssertionError.class, () -> getConfiguredTarget("//foo:r"));
assertThat(e)
.hasMessageThat()
- .contains("parameter 'toolchain_identifier' has no default value");
+ .contains("missing 1 required named argument: toolchain_identifier");
}
@Test
public void testCcToolchainInfoFromSkylarkRequiredHostSystemName() throws Exception {
setupSkylarkRuleForStringFieldsTesting("host_system_name");
AssertionError e = assertThrows(AssertionError.class, () -> getConfiguredTarget("//foo:r"));
- assertThat(e).hasMessageThat().contains("parameter 'host_system_name' has no default value");
+ assertThat(e).hasMessageThat().contains("missing 1 required named argument: host_system_name");
}
@Test
public void testCcToolchainInfoFromSkylarkRequiredTargetSystemName() throws Exception {
setupSkylarkRuleForStringFieldsTesting("target_system_name");
AssertionError e = assertThrows(AssertionError.class, () -> getConfiguredTarget("//foo:r"));
- assertThat(e).hasMessageThat().contains("parameter 'target_system_name' has no default value");
+ assertThat(e)
+ .hasMessageThat()
+ .contains("missing 1 required named argument: target_system_name");
}
@Test
public void testCcToolchainInfoFromSkylarkRequiredTargetCpu() throws Exception {
setupSkylarkRuleForStringFieldsTesting("target_cpu");
AssertionError e = assertThrows(AssertionError.class, () -> getConfiguredTarget("//foo:r"));
- assertThat(e).hasMessageThat().contains("parameter 'target_cpu' has no default value");
+ assertThat(e).hasMessageThat().contains("missing 1 required named argument: target_cpu");
}
@Test
public void testCcToolchainInfoFromSkylarkRequiredTargetLibc() throws Exception {
setupSkylarkRuleForStringFieldsTesting("target_libc");
AssertionError e = assertThrows(AssertionError.class, () -> getConfiguredTarget("//foo:r"));
- assertThat(e).hasMessageThat().contains("parameter 'target_libc' has no default value");
+ assertThat(e).hasMessageThat().contains("missing 1 required named argument: target_libc");
}
@Test
public void testCcToolchainInfoFromSkylarkRequiredCompiler() throws Exception {
setupSkylarkRuleForStringFieldsTesting("compiler");
AssertionError e = assertThrows(AssertionError.class, () -> getConfiguredTarget("//foo:r"));
- assertThat(e).hasMessageThat().contains("parameter 'compiler' has no default value");
+ assertThat(e).hasMessageThat().contains("missing 1 required named argument: compiler");
}
@Test
public void testCcToolchainInfoFromSkylarkRequiredAbiVersion() throws Exception {
setupSkylarkRuleForStringFieldsTesting("abi_version");
AssertionError e = assertThrows(AssertionError.class, () -> getConfiguredTarget("//foo:r"));
- assertThat(e).hasMessageThat().contains("parameter 'abi_version' has no default value");
+ assertThat(e).hasMessageThat().contains("missing 1 required named argument: abi_version");
}
@Test
public void testCcToolchainInfoFromSkylarkRequiredAbiLibcVersion() throws Exception {
setupSkylarkRuleForStringFieldsTesting("abi_libc_version");
AssertionError e = assertThrows(AssertionError.class, () -> getConfiguredTarget("//foo:r"));
- assertThat(e).hasMessageThat().contains("parameter 'abi_libc_version' has no default value");
+ assertThat(e).hasMessageThat().contains("missing 1 required named argument: abi_libc_version");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/rules/java/JavaSkylarkApiTest.java b/src/test/java/com/google/devtools/build/lib/rules/java/JavaSkylarkApiTest.java
index 9d102cf..c19f77c 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/java/JavaSkylarkApiTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/java/JavaSkylarkApiTest.java
@@ -277,7 +277,7 @@
reporter.removeHandler(failFastHandler);
getConfiguredTarget("//a:r");
- assertContainsEvent("expected value of type 'JavaRuntimeInfo' for parameter 'host_javabase'");
+ assertContainsEvent("got value of type 'Target', want 'JavaRuntimeInfo'");
}
@Test
@@ -2096,7 +2096,7 @@
reporter.removeHandler(failFastHandler);
getConfiguredTarget("//a:r");
- assertContainsEvent("expected value of type 'JavaRuntimeInfo' for parameter 'host_javabase'");
+ assertContainsEvent("got value of type 'Target', want 'JavaRuntimeInfo'");
}
@Test
@@ -2135,8 +2135,7 @@
reporter.removeHandler(failFastHandler);
getConfiguredTarget("//a:r");
- assertContainsEvent(
- "expected value of type 'JavaToolchainInfo' for parameter 'java_toolchain'");
+ assertContainsEvent("got value of type 'Target', want 'JavaToolchainInfo'");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryTest.java
index 17f54f9..ed4c9f3 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryTest.java
@@ -713,9 +713,7 @@
AssertionError expected =
assertThrows(AssertionError.class, () ->
getConfiguredTarget("//examples/apple_skylark:my_target"));
- assertThat(expected)
- .hasMessageThat()
- .contains("unexpected keyword 'foo', for call to function AppleStaticLibrary");
+ assertThat(expected).hasMessageThat().contains("unexpected keyword argument 'foo'");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/rules/python/PyInfoTest.java b/src/test/java/com/google/devtools/build/lib/rules/python/PyInfoTest.java
index 1a4b234..5613823 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/python/PyInfoTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/python/PyInfoTest.java
@@ -122,13 +122,12 @@
@Test
public void starlarkConstructorErrors_TransitiveSources() throws Exception {
checkEvalErrorContains(
- "'transitive_sources' has no default value", //
+ "missing 1 required named argument: transitive_sources", //
"PyInfo()");
checkEvalErrorContains(
- "expected value of type 'depset of Files' for parameter 'transitive_sources'",
- "PyInfo(transitive_sources = 'abc')");
+ "got value of type 'string', want 'depset of Files'", "PyInfo(transitive_sources = 'abc')");
checkEvalErrorContains(
- "expected value of type 'depset of Files' for parameter 'transitive_sources'",
+ "got value of type 'depset', want 'depset of Files'",
"PyInfo(transitive_sources = depset(direct=['abc']))");
checkEvalErrorContains(
"'transitive_sources' field should be a postorder-compatible depset of Files",
@@ -138,31 +137,31 @@
@Test
public void starlarkConstructorErrors_UsesSharedLibraries() throws Exception {
checkEvalErrorContains(
- "expected value of type 'bool' for parameter 'uses_shared_libraries'",
+ "got value of type 'string', want 'bool'",
"PyInfo(transitive_sources = depset([]), uses_shared_libraries = 'abc')");
}
@Test
public void starlarkConstructorErrors_Imports() throws Exception {
checkEvalErrorContains(
- "expected value of type 'depset of strings' for parameter 'imports'",
+ "got value of type 'string', want 'depset of strings'",
"PyInfo(transitive_sources = depset([]), imports = 'abc')");
checkEvalErrorContains(
- "expected value of type 'depset of strings' for parameter 'imports'",
+ "got value of type 'depset', want 'depset of strings'",
"PyInfo(transitive_sources = depset([]), imports = depset(direct=[123]))");
}
@Test
public void starlarkConstructorErrors_HasPy2OnlySources() throws Exception {
checkEvalErrorContains(
- "expected value of type 'bool' for parameter 'has_py2_only_sources'",
+ "got value of type 'string', want 'bool'",
"PyInfo(transitive_sources = depset([]), has_py2_only_sources = 'abc')");
}
@Test
public void starlarkConstructorErrors_HasPy3OnlySources() throws Exception {
checkEvalErrorContains(
- "expected value of type 'bool' for parameter 'has_py3_only_sources'",
+ "got value of type 'string', want 'bool'",
"PyInfo(transitive_sources = depset([]), has_py3_only_sources = 'abc')");
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/python/PyRuntimeInfoTest.java b/src/test/java/com/google/devtools/build/lib/rules/python/PyRuntimeInfoTest.java
index b4c293c..882b043 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/python/PyRuntimeInfoTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/python/PyRuntimeInfoTest.java
@@ -143,14 +143,14 @@
@Test
public void starlarkConstructorErrors_Files() throws Exception {
checkEvalErrorContains(
- "expected value of type 'depset of Files or NoneType' for parameter 'files'",
+ "got value of type 'string', want 'depset of Files or NoneType'",
"PyRuntimeInfo(",
" interpreter = dummy_interpreter,",
" files = 'abc',",
" python_version = 'PY2',",
")");
checkEvalErrorContains(
- "expected value of type 'depset of Files or NoneType' for parameter 'files'",
+ "got value of type 'depset', want 'depset of Files or NoneType'",
"PyRuntimeInfo(",
" interpreter = dummy_interpreter,",
" files = depset(['abc']),",
@@ -168,7 +168,7 @@
@Test
public void starlarkConstructorErrors_PythonVersion() throws Exception {
checkEvalErrorContains(
- "parameter 'python_version' has no default value",
+ "missing 1 required named argument: python_version",
"PyRuntimeInfo(",
" interpreter_path = '/system/interpreter',",
")");
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
index 8b9dd80..7841fdb 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
@@ -394,8 +394,7 @@
"str",
"\t\tstr.index(1)"
+ System.lineSeparator()
- + "expected value of type 'string' for parameter 'sub', for call to method "
- + "index(sub, start = 0, end = None) of 'string'");
+ + "in call to index(), parameter 'sub' got value of type 'int', want 'string'");
}
@Test
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 2301288..b9c9be4 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
@@ -458,10 +458,7 @@
@Test
public void testNonLabelAttrWithProviders() throws Exception {
checkEvalErrorContains(
- "unexpected keyword 'providers', for call to method "
- + "string(default = '', doc = '', mandatory = False, values = []) "
- + "of 'attr (a language module)'",
- "attr.string(providers = ['a'])");
+ "unexpected keyword argument 'providers'", "attr.string(providers = ['a'])");
}
private static final RuleClass.ConfiguredTargetFactory<Object, Object, Exception>
@@ -546,11 +543,7 @@
@Test
public void testAttrDefaultValueBadType() throws Exception {
- checkEvalErrorContains(
- "expected value of type 'string' for parameter 'default', for call to method "
- + "string(default = '', doc = '', mandatory = False, values = []) "
- + "of 'attr (a language module)'",
- "attr.string(default = 1)");
+ checkEvalErrorContains("got value of type 'int', want 'string'", "attr.string(default = 1)");
}
@Test
@@ -580,10 +573,7 @@
@Test
public void testAttrBadKeywordArguments() throws Exception {
checkEvalErrorContains(
- "unexpected keyword 'bad_keyword', for call to method "
- + "string(default = '', doc = '', mandatory = False, values = []) of "
- + "'attr (a language module)'",
- "attr.string(bad_keyword = '')");
+ "string() got unexpected keyword argument 'bad_keyword'", "attr.string(bad_keyword = '')");
}
@Test
@@ -650,11 +640,7 @@
@Test
public void testAttrDocValueBadType() throws Exception {
- checkEvalErrorContains(
- "expected value of type 'string' for parameter 'doc', for call to method "
- + "string(default = '', doc = '', mandatory = False, values = []) "
- + "of 'attr (a language module)'",
- "attr.string(doc = 1)");
+ checkEvalErrorContains("got value of type 'int', want 'string'", "attr.string(doc = 1)");
}
@Test
@@ -670,7 +656,9 @@
}
@Test
- public void testLateBoundAttrWorksWithOnlyLabel() throws Exception {
+ public void testFunctionAsAttrDefault() throws Exception {
+ exec("def f(): pass");
+
// Late-bound attributes, which are computed during analysis as a function
// of the configuration, are only available for attributes involving labels:
// attr.label
@@ -678,48 +666,34 @@
// 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 = []) "
- + "of 'attr (a language module)'",
- "def attr_value(cfg): return 'a'",
- "attr.string(default=attr_value)");
- }
+ // (See testRuleClassImplicitOutputFunctionDependingOnComputedAttribute
+ // for a more detailed positive test.)
+ evalAndExport(
+ "attr.label(default=f)",
+ "attr.label_list(default=f)",
+ "attr.label_keyed_string_dict(default=f)");
+ // Note: the default parameter of attr.output{,_list} is deprecated
+ // (see --incompatible_no_output_attr_default)
- @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.)
+ // For all other attribute types, the default value may not be a function.
//
- // Most attributes like attr.string should not accept a function as
- // their default value. (The bug was that the @SkylarkCallable
+ // (This is a regression test for github.com/bazelbuild/bazel/issues/9463.
+ // The loading-phase feature of "computed attribute defaults" is not exposed
+ // to Starlark; the bug was that the @SkylarkCallable
// annotation was more permissive than the method declaration.)
- exec("def f(): pass");
+ checkEvalErrorContains("got value of type 'function', want 'string'", "attr.string(default=f)");
checkEvalErrorContains(
- "expected value of type 'string' for parameter 'default'", "attr.string(default=f)");
+ "got value of type 'function', want 'sequence of strings'", "attr.string_list(default=f)");
+ checkEvalErrorContains("got value of type 'function', want 'int'", "attr.int(default=f)");
checkEvalErrorContains(
- "expected value of type 'sequence of strings' for parameter 'default'",
- "attr.string_list(default=f)");
+ "got value of type 'function', want 'sequence of ints'", "attr.int_list(default=f)");
+ checkEvalErrorContains("got value of type 'function', want 'bool'", "attr.bool(default=f)");
checkEvalErrorContains(
- "expected value of type 'int' for parameter 'default'", "attr.int(default=f)");
+ "got value of type 'function', want 'dict'", "attr.string_dict(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)
+ "got value of type 'function', want 'dict'", "attr.string_list_dict(default=f)");
+ // Note: attr.license appears to be disabled already.
+ // (see --incompatible_no_attr_license)
}
private static final Label FAKE_LABEL = Label.parseAbsoluteUnchecked("//fake/label.bzl");
@@ -812,23 +786,20 @@
public void testRuleUnknownKeyword() throws Exception {
registerDummyStarlarkFunction();
checkEvalErrorContains(
- "unexpected keyword 'bad_keyword', for call to function rule(",
- "rule(impl, bad_keyword = 'some text')");
+ "unexpected keyword argument 'bad_keyword'", "rule(impl, bad_keyword = 'some text')");
}
@Test
public void testRuleImplementationMissing() throws Exception {
checkEvalErrorContains(
- "parameter 'implementation' has no default value, for call to function rule(",
- "rule(attrs = {})");
+ "rule() missing 1 required positional argument: implementation", "rule(attrs = {})");
}
@Test
public void testRuleBadTypeForAdd() throws Exception {
registerDummyStarlarkFunction();
checkEvalErrorContains(
- "expected value of type 'dict or NoneType' for parameter 'attrs', "
- + "for call to function rule(",
+ "in call to rule(), parameter 'attrs' got value of type 'string', want 'dict or NoneType'",
"rule(impl, attrs = 'some text')");
}
@@ -843,9 +814,7 @@
@Test
public void testRuleBadTypeForDoc() throws Exception {
registerDummyStarlarkFunction();
- checkEvalErrorContains(
- "expected value of type 'string' for parameter 'doc', for call to function rule(",
- "rule(impl, doc = 1)");
+ checkEvalErrorContains("got value of type 'int', want 'string'", "rule(impl, doc = 1)");
}
@Test
@@ -1093,8 +1062,8 @@
@Test
public void testLabelAttrWrongDefault() throws Exception {
checkEvalErrorContains(
- "expected value of type 'Label or string or LateBoundDefault or function or NoneType' "
- + "for parameter 'default', for call to method label(",
+ "got value of type 'int', want 'Label or string or LateBoundDefault or function or"
+ + " NoneType'",
"attr.label(default = 123)");
}
@@ -1191,8 +1160,7 @@
@Test
public void testStructPosArgs() throws Exception {
- checkEvalErrorContains(
- "expected no more than 0 positional arguments, but got 1", "x = struct(1, b = 2)");
+ checkEvalErrorContains("struct() got unexpected positional argument", "x = struct(1, b = 2)");
}
@Test
@@ -1452,10 +1420,7 @@
@Test
public void declaredProvidersBadTypeForDoc() throws Exception {
- checkEvalErrorContains(
- "expected value of type 'string' for parameter 'doc', for call to function "
- + "provider(doc = '', fields = None)",
- "provider(doc = 1)");
+ checkEvalErrorContains("got value of type 'int', want 'string'", "provider(doc = 1)");
}
@Test
@@ -1594,9 +1559,7 @@
@Test
public void aspectBadTypeForDoc() throws Exception {
registerDummyStarlarkFunction();
- checkEvalErrorContains(
- "expected value of type 'string' for parameter 'doc', for call to function aspect(",
- "aspect(impl, doc = 1)");
+ checkEvalErrorContains("got value of type 'int', want 'string'", "aspect(impl, doc = 1)");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java
index d881afa..34d8df9 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java
@@ -222,20 +222,20 @@
@Test
public void testSkylarkFunctionTooFewArguments() throws Exception {
checkSkylarkFunctionError(
- "parameter 'mandatory' has no default value", "mock(mandatory_key='y')");
+ "missing 1 required positional argument: mandatory", "mock(mandatory_key='y')");
}
@Test
public void testSkylarkFunctionTooManyArguments() throws Exception {
checkSkylarkFunctionError(
- "expected no more than 2 positional arguments, but got 3",
+ "mock() accepts no more than 2 positional arguments but got 3",
"mock('a', 'b', 'c', mandatory_key='y')");
}
@Test
public void testSkylarkFunctionAmbiguousArguments() throws Exception {
checkSkylarkFunctionError(
- "got multiple values for keyword argument 'mandatory'",
+ "mock() got multiple values for argument 'mandatory'",
"mock('by position', mandatory='by_key', mandatory_key='c')");
}
@@ -325,8 +325,7 @@
public void testCreateSpawnActionArgumentsBadExecutable() throws Exception {
setRuleContext(createRuleContext("//foo:foo"));
checkEvalErrorContains(
- "expected value of type 'File or string or FilesToRunProvider' for parameter 'executable', "
- + "for call to method run(",
+ "got value of type 'int', want 'File or string or FilesToRunProvider'",
"ruleContext.actions.run(",
" inputs = ruleContext.files.srcs,",
" outputs = ruleContext.files.srcs,",
@@ -380,7 +379,7 @@
public void testCreateSpawnActionUnknownParam() throws Exception {
setRuleContext(createRuleContext("//foo:foo"));
checkEvalErrorContains(
- "unexpected keyword 'bad_param', for call to method run(",
+ "run() got unexpected keyword argument 'bad_param'",
"f = ruleContext.actions.declare_file('foo.sh')",
"ruleContext.actions.run(outputs=[], bad_param = 'some text', executable = f)");
}
@@ -541,8 +540,7 @@
checkEmptyAction("mnemonic = 'test', inputs = depset(ruleContext.files.srcs)");
checkEvalErrorContains(
- "parameter 'mnemonic' has no default value, for call to method "
- + "do_nothing(mnemonic, inputs = []) of 'actions'",
+ "do_nothing() missing 1 required named argument: mnemonic",
"ruleContext.actions.do_nothing(inputs = ruleContext.files.srcs)");
}
@@ -771,7 +769,7 @@
public void testBadParamTypeErrorMessage() throws Exception {
setRuleContext(createRuleContext("//foo:foo"));
checkEvalErrorContains(
- "expected value of type 'string or Args' for parameter 'content'",
+ "got value of type 'int', want 'string or Args'",
"ruleContext.actions.write(",
" output = ruleContext.files.srcs[0],",
" content = 1,",
@@ -852,8 +850,7 @@
public void testRunfilesBadSetGenericType() throws Exception {
setRuleContext(createRuleContext("//foo:foo"));
checkEvalErrorContains(
- "expected value of type 'depset of Files or NoneType' for parameter 'transitive_files', "
- + "for call to method runfiles(",
+ "got value of type 'depset', want 'depset of Files or NoneType'",
"ruleContext.runfiles(transitive_files=depset([1, 2, 3]))");
}
@@ -950,7 +947,7 @@
public void testRunfilesBadKeywordArguments() throws Exception {
setRuleContext(createRuleContext("//foo:foo"));
checkEvalErrorContains(
- "unexpected keyword 'bad_keyword', for call to method runfiles(",
+ "runfiles() got unexpected keyword argument 'bad_keyword'",
"ruleContext.runfiles(bad_keyword = '')");
}
@@ -1307,7 +1304,7 @@
assertThrows(AssertionError.class, () -> getConfiguredTarget("//test:my_rule"));
assertThat(expected)
.hasMessageThat()
- .contains("unexpected keyword 'foo', for call to function DefaultInfo(");
+ .contains("DefaultInfo() got unexpected keyword argument 'foo'");
}
@Test
@@ -1999,9 +1996,11 @@
"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(
- "expected value of type 'function' for parameter 'implementation', "
- + "for call to function rule");
+ "in call to rule(), parameter 'implementation' got value of type 'function', want"
+ + " 'function'");
getConfiguredTarget("//test:silly");
}
@@ -2365,7 +2364,8 @@
"args = ruleContext.actions.args()",
"args.add_all(1)");
checkEvalErrorContains(
- "expected value of type 'sequence or depset' for parameter 'values'",
+ "in call to add_all(), parameter 'values' got value of type 'int', want 'sequence or"
+ + " depset'",
"args = ruleContext.actions.args()",
"args.add_all('--foo', 1)");
}
@@ -2787,7 +2787,8 @@
assertThat(expected)
.hasMessageThat()
.contains(
- "expected value of type 'int' for parameter 'default', " + "for call to method int(");
+ "in call to int(), parameter 'default' got value of type 'LateBoundDefault', want"
+ + " 'int'");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/DepsetTest.java b/src/test/java/com/google/devtools/build/lib/syntax/DepsetTest.java
index d38652f..affb7ee 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/DepsetTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/DepsetTest.java
@@ -239,7 +239,8 @@
public void testTooManyPositionals() throws Exception {
new BothModesTest()
.testIfErrorContains(
- "expected no more than 2 positional arguments, but got 3", "depset([], 'default', [])");
+ "depset() accepts no more than 2 positional arguments but got 3",
+ "depset([], 'default', [])");
}
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
index 615c613..03f12f9 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
@@ -799,19 +799,14 @@
@Test
public void testDictKeysTooManyArgs() throws Exception {
- newTest()
- .testIfExactError(
- "expected no more than 0 positional arguments, but got 1, "
- + "for call to method keys() of 'dict'",
- "{'a': 1}.keys('abc')");
+ newTest().testIfExactError("keys() got unexpected positional argument", "{'a': 1}.keys('abc')");
}
@Test
public void testDictKeysTooManyKeyArgs() throws Exception {
newTest()
.testIfExactError(
- "unexpected keyword 'arg', for call to method keys() of 'dict'",
- "{'a': 1}.keys(arg='abc')");
+ "keys() got unexpected keyword argument 'arg'", "{'a': 1}.keys(arg='abc')");
}
@Test
@@ -819,17 +814,17 @@
// TODO(adonovan): when the duplication is literal, this should be caught by a static check.
newTest()
.testIfExactError(
- "duplicate argument 'arg' in call to 'keys'",
- "{'a': 1}.keys(arg='abc', arg='def', k=1, k=2)");
+ "int() got multiple values for argument 'base'", "int('1', base=10, base=16)");
+ new SkylarkTest()
+ .testIfExactError(
+ "int() got multiple values for argument 'base'", "int('1', base=10, **dict(base=16))");
}
@Test
public void testArgBothPosKey() throws Exception {
newTest()
.testIfErrorContains(
- "got multiple values for keyword argument 'base', "
- + "for call to function int(x, base = unbound)",
- "int('2', 3, base=3)");
+ "int() got multiple values for argument 'base'", "int('2', 3, base=3)");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java b/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
index 56762cc..b8af7b4 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
@@ -110,8 +110,7 @@
// only one built-in function.
new BothModesTest()
.testIfExactError(
- "expected value of type 'string' for parameter 'sub', "
- + "for call to method index(sub, start = 0, end = None) of 'string'",
+ "in call to index(), parameter 'sub' got value of type 'int', want 'string'",
"'test'.index(1)");
}
@@ -135,8 +134,7 @@
+ LINE_SEPARATOR
+ "\t\t\"test\".index(x)"
+ LINE_SEPARATOR
- + "expected value of type 'string' for parameter 'sub', "
- + "for call to method index(sub, start = 0, end = None) of 'string'",
+ + "in call to index(), parameter 'sub' got value of type 'int', want 'string'",
"def foo():",
" bar(1)",
"def bar(x):",
@@ -150,8 +148,8 @@
new BothModesTest()
.testIfErrorContains("substring \"z\" not found in \"abc\"", "'abc'.index('z')")
.testIfErrorContains(
- "expected value of type 'string or tuple of strings' for parameter 'sub', "
- + "for call to method startswith(sub, start = 0, end = None) of 'string'",
+ "in call to startswith(), parameter 'sub' got value of type 'int', want 'string or"
+ + " tuple of strings'",
"'test'.startswith(1)")
.testIfErrorContains("in dict, got string, want iterable", "dict('a')");
}
@@ -334,7 +332,8 @@
"in dict, dictionary update sequence element #0 is not iterable (string)",
"dict([('a')])")
.testIfErrorContains(
- "expected no more than 1 positional arguments, but got 3", "dict((3,4), (3,2), (1,2))")
+ "dict() accepts no more than 1 positional argument but got 3",
+ "dict((3,4), (3,2), (1,2))")
.testIfErrorContains(
"item #0 has length 3, but exactly two elements are required",
"dict([('a', 'b', 'c')])");
@@ -461,8 +460,7 @@
.testExpression("hash('skylark')", "skylark".hashCode())
.testExpression("hash('google')", "google".hashCode())
.testIfErrorContains(
- "expected value of type 'string' for parameter 'value', "
- + "for call to function hash(value)",
+ "in call to hash(), parameter 'value' got value of type 'NoneType', want 'string'",
"hash(None)");
}
@@ -759,8 +757,7 @@
public void testExperimentalStarlarkConfig() throws Exception {
new SkylarkTest("--incompatible_restrict_named_params")
.testIfErrorContains(
- "parameter 'elements' may not be specified by name, "
- + "for call to method join(elements) of 'string'",
+ "join() got named argument for positional-only parameter 'elements'",
"','.join(elements=['foo', 'bar'])");
}
@@ -796,7 +793,7 @@
"parameter 'direct' cannot be specified both positionally and by keyword",
"depset([0, 1], 'default', direct=[0,1])")
.testIfErrorContains(
- "parameter 'items' is deprecated and will be removed soon. "
+ "in call to depset(), parameter 'items' is deprecated and will be removed soon. "
+ "It may be temporarily re-enabled by setting "
+ "--incompatible_disable_depset_inputs=false",
"depset(items=[0,1])");
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
index e3b7693..10f8408 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
@@ -1115,7 +1115,8 @@
// posOrNamed by name and three positional parameters, there is a conflict.
new SkylarkTest()
.update("mock", new Mock())
- .testIfErrorContains("got multiple values for keyword argument 'posOrNamed'",
+ .testIfErrorContains(
+ "with_params() got multiple values for argument 'posOrNamed'",
"mock.with_params(1, True, True, posOrNamed=True, named=True)");
}
@@ -1123,17 +1124,20 @@
public void testTooManyPositionalArgs() throws Exception {
new SkylarkTest()
.update("mock", new Mock())
- .testIfErrorContains("expected no more than 3 positional arguments, but got 4",
+ .testIfErrorContains(
+ "with_params() accepts no more than 3 positional arguments but got 4",
"mock.with_params(1, True, True, 'toomany', named=True)");
new SkylarkTest()
.update("mock", new Mock())
- .testIfErrorContains("expected no more than 3 positional arguments, but got 5",
+ .testIfErrorContains(
+ "with_params() accepts no more than 3 positional arguments but got 5",
"mock.with_params(1, True, True, 'toomany', 'alsotoomany', named=True)");
new SkylarkTest()
.update("mock", new Mock())
- .testIfErrorContains("expected no more than 1 positional arguments, but got 2",
+ .testIfErrorContains(
+ "is_empty() accepts no more than 1 positional argument but got 2",
"mock.is_empty('a', 'b')");
}
@@ -1161,19 +1165,12 @@
.update("mock", new Mock())
.setUp("")
.testIfExactError(
- "parameter 'named' has no default value, for call to "
- + "method with_params(pos1, pos2 = False, posOrNamed = False, named, "
- + "optionalNamed = False, nonNoneable = \"a\", noneable = None, multi = None) "
- + "of 'Mock'",
- "mock.with_params(1, True)");
+ "with_params() missing 1 required named argument: named", "mock.with_params(1, True)");
new SkylarkTest()
.update("mock", new Mock())
.setUp("")
.testIfExactError(
- "parameter 'named' has no default value, for call to "
- + "method with_params(pos1, pos2 = False, posOrNamed = False, named, "
- + "optionalNamed = False, nonNoneable = \"a\", noneable = None, multi = None) "
- + "of 'Mock'",
+ "with_params() missing 1 required named argument: named",
"mock.with_params(1, True, True)");
new SkylarkTest()
.update("mock", new Mock())
@@ -1191,30 +1188,28 @@
.update("mock", new Mock())
.setUp("")
.testIfExactError(
- "unexpected keyword 'n', for call to "
- + "method with_params(pos1, pos2 = False, posOrNamed = False, named, "
- + "optionalNamed = False, nonNoneable = \"a\", noneable = None, multi = None) "
- + "of 'Mock'",
+ "with_params() got unexpected keyword argument 'posornamed' (did you mean"
+ + " 'posOrNamed'?)",
+ "mock.with_params(1, True, named=True, posornamed=True)");
+ new SkylarkTest()
+ .update("mock", new Mock())
+ .setUp("")
+ .testIfExactError(
+ "with_params() got unexpected keyword argument 'n'",
"mock.with_params(1, True, named=True, posOrNamed=True, n=2)");
new SkylarkTest()
.update("mock", new Mock())
.setUp("")
.testIfExactError(
- "parameter 'nonNoneable' cannot be None, for call to method "
- + "with_params(pos1, pos2 = False, posOrNamed = False, named, "
- + "optionalNamed = False, nonNoneable = \"a\", noneable = None, multi = None) "
- + "of 'Mock'",
+ "in call to with_params(), parameter 'nonNoneable' cannot be None",
"mock.with_params(1, True, True, named=True, optionalNamed=False, nonNoneable=None)");
new SkylarkTest()
.update("mock", new Mock())
.setUp("")
.testIfExactError(
- "expected value of type 'string or int or sequence of ints or NoneType' for parameter"
- + " 'multi', for call to method "
- + "with_params(pos1, pos2 = False, posOrNamed = False, named, "
- + "optionalNamed = False, nonNoneable = \"a\", noneable = None, multi = None) "
- + "of 'Mock'",
+ "in call to with_params(), parameter 'multi' got value of type 'bool', want 'string or"
+ + " int or sequence of ints or NoneType'",
"mock.with_params(1, True, named=True, multi=False)");
// We do not enforce list item parameter type constraints.
@@ -1261,7 +1256,7 @@
new SkylarkTest()
.update("mock", new Mock())
.testIfErrorContains(
- "expected value of type 'string' for parameter 'pos', for call to function MockFn(pos)",
+ "in call to MockFn(), parameter 'pos' got value of type 'int', want 'string'",
"v = mock(1)");
}
@@ -1966,8 +1961,7 @@
public void testPrintBadKwargs() throws Exception {
new SkylarkTest()
.testIfErrorContains(
- "unexpected keywords 'end', 'other', for call to function print(sep = \" \", *args)",
- "print(end='x', other='y')");
+ "print() got unexpected keyword argument 'end'", "print(end='x', other='y')");
}
// Override tests in EvaluationTest incompatible with Skylark
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/StarlarkFlagGuardingTest.java b/src/test/java/com/google/devtools/build/lib/syntax/StarlarkFlagGuardingTest.java
index 263aac4..34eac92 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/StarlarkFlagGuardingTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/StarlarkFlagGuardingTest.java
@@ -141,8 +141,8 @@
new SkylarkTest("--experimental_build_setting_api=true")
.update("mock", new Mock())
.testIfErrorContains(
- "expected value of type 'bool' for parameter 'b', "
- + "for call to method positionals_only_method(a, b, c) of 'Mock'",
+ "in call to positionals_only_method(), parameter 'b' got value of type 'int', want"
+ + " 'bool'",
"mock.positionals_only_method(1, 3)");
new SkylarkTest("--experimental_build_setting_api=false")
@@ -152,8 +152,8 @@
new SkylarkTest("--experimental_build_setting_api=false")
.update("mock", new Mock())
.testIfErrorContains(
- "expected value of type 'int' for parameter 'c', for call to method "
- + "positionals_only_method(a, c) of 'Mock'",
+ "in call to positionals_only_method(), parameter 'c' got value of type 'bool', want"
+ + " 'int'",
"mock.positionals_only_method(1, True, 3)");
}
@@ -167,8 +167,7 @@
new SkylarkTest("--experimental_build_setting_api=true")
.update("mock", new Mock())
.testIfErrorContains(
- "parameter 'b' has no default value, "
- + "for call to method keywords_only_method(a, b, c) of 'Mock'",
+ "keywords_only_method() missing 1 required named argument: b",
"mock.keywords_only_method(a=1, c=3)");
new SkylarkTest("--experimental_build_setting_api=false")
@@ -186,6 +185,7 @@
@Test
public void testMixedParamsMethod() throws Exception {
+ // def mixed_params_method(a, b, c = ?, d = ?)
new SkylarkTest("--experimental_build_setting_api=true")
.update("mock", new Mock())
.testEval(
@@ -195,10 +195,12 @@
new SkylarkTest("--experimental_build_setting_api=true")
.update("mock", new Mock())
.testIfErrorContains(
- "parameter 'b' has no default value, "
- + "for call to method mixed_params_method(a, b, c, d) of 'Mock'",
+ // Missing named arguments (d) are not reported
+ // if there are missing positional arguments.
+ "mixed_params_method() missing 1 required positional argument: b",
"mock.mixed_params_method(1, c=3)");
+ // def mixed_params_method(a, b disabled = False, c disabled = 3, d = ?)
new SkylarkTest("--experimental_build_setting_api=false")
.update("mock", new Mock())
.testEval(
@@ -207,8 +209,13 @@
new SkylarkTest("--experimental_build_setting_api=false")
.update("mock", new Mock())
.testIfErrorContains(
- "expected no more than 1 positional arguments, but got 2, "
- + "for call to method mixed_params_method(a, d) of 'Mock'",
+ "mixed_params_method() accepts no more than 1 positional argument but got 2",
+ "mock.mixed_params_method(1, True, d=True)");
+
+ new SkylarkTest("--experimental_build_setting_api=false")
+ .update("mock", new Mock())
+ .testIfErrorContains(
+ "mixed_params_method() accepts no more than 1 positional argument but got 2",
"mock.mixed_params_method(1, True, c=3, d=True)");
}
@@ -223,8 +230,7 @@
new SkylarkTest("--experimental_build_setting_api=true", "--incompatible_no_attr_license=false")
.update("mock", new Mock())
.testIfErrorContains(
- "parameter 'b' has no default value, "
- + "for call to method keywords_multiple_flags(a, b, c) of 'Mock'",
+ "keywords_multiple_flags() missing 2 required named arguments: b, c",
"mock.keywords_multiple_flags(a=42)");
new SkylarkTest("--experimental_build_setting_api=false", "--incompatible_no_attr_license=true")
