Improve error message when calling a builtin function with wrong type
Error message is simpler and doesn't show the type of all arguments.
RELNOTES: None.
PiperOrigin-RevId: 161187134
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java
index b3c742f..5852386 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java
@@ -186,17 +186,15 @@
if (args[i] != null && !types[i].isAssignableFrom(args[i].getClass())) {
String paramName =
i < len ? signature.getSignature().getNames().get(i) : extraArgs[i - len].name();
- int extraArgsCount = (extraArgs == null) ? 0 : extraArgs.length;
throw new EvalException(
loc,
String.format(
- "method %s is not applicable for arguments %s: "
- + "'%s' is '%s', but should be '%s'",
- getShortSignature(),
- printTypeString(args, args.length - extraArgsCount),
+ "argument '%s' has type '%s', but should be '%s'\nin call to builtin %s %s",
paramName,
EvalUtils.getDataTypeName(args[i]),
- EvalUtils.getDataTypeNameFromClass(types[i])));
+ EvalUtils.getDataTypeNameFromClass(types[i]),
+ hasSelfArgument() ? "method" : "function",
+ getShortSignature()));
}
}
throw badCallException(loc, e, args);
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FunctionSignature.java b/src/main/java/com/google/devtools/build/lib/syntax/FunctionSignature.java
index 3b12fd5..5dc2a6a 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FunctionSignature.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FunctionSignature.java
@@ -391,7 +391,7 @@
public StringBuilder toStringBuilder(
final StringBuilder sb,
final boolean showDefaults,
- final boolean skipMissingTypeNames,
+ final boolean showTypes,
final boolean skipFirstMandatory) {
FunctionSignature signature = getSignature();
Shape shape = signature.getShape();
@@ -431,9 +431,9 @@
// a) there is no type defined (such as in user-defined functions) or
// b) the type is java.lang.Object.
boolean typeDefined = types != null && types.get(i) != null;
- if (typeDefined || !skipMissingTypeNames) {
+ if (typeDefined && showTypes) {
printer.append(": ");
- printer.append(typeDefined ? types.get(i).toString() : "object");
+ printer.append(types.get(i).toString());
}
}
public void mandatory(int i) {
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 a469bc5..465ff44 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
@@ -715,9 +715,8 @@
events.setFailFast(false);
assertGlobFails(
"glob(1, exclude=2)",
- "method glob(include: sequence of strings, *, exclude: sequence of strings, "
- + "exclude_directories: int) is not applicable for arguments (int, int, int): "
- + "'include' is 'int', but should be 'sequence'");
+ "argument 'include' has type 'int', but should be 'sequence'\n"
+ + "in call to builtin function glob(include, *, exclude, exclude_directories)");
}
@Test
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 1f77337..6e4a576 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
@@ -337,9 +337,8 @@
"str",
"\t\tstr.index(1)"
+ System.lineSeparator()
- + "method string.index(sub: string, start: int, end: int or NoneType) is not "
- + "applicable for arguments (int, int, NoneType): 'sub' is 'int', "
- + "but should be 'string'");
+ + "argument 'sub' has type 'int', but should be 'string'\n"
+ + "in call to builtin method string.index(sub, start, end)");
}
@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 4fa16cd..a0f6316 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
@@ -492,9 +492,8 @@
@Test
public void testAttrDefaultValueBadType() throws Exception {
checkErrorContains(
- "method attr.string(*, default: string, mandatory: bool, values: sequence of strings) "
- + "is not applicable for arguments (int, bool, list): 'default' is 'int', "
- + "but should be 'string'",
+ "argument 'default' has type 'int', but should be 'string'\n"
+ + "in call to builtin function attr.string(*, default, mandatory, values)",
"attr.string(default = 1)");
}
@@ -569,9 +568,8 @@
@Test
public void testLateBoundAttrWorksWithOnlyLabel() throws Exception {
checkEvalError(
- "method attr.string(*, default: string, mandatory: bool, values: sequence of strings) "
- + "is not applicable for arguments (function, bool, list): 'default' is 'function', "
- + "but should be 'string'",
+ "argument 'default' has type 'function', but should be 'string'\n"
+ + "in call to builtin function attr.string(*, default, mandatory, values)",
"def attr_value(cfg): return 'a'",
"attr.string(default=attr_value)");
}
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 a76bbc6..85979f4 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
@@ -397,8 +397,8 @@
// only one built-in function.
new BothModesTest()
.testIfExactError(
- "method string.index(sub: string, start: int, end: int or NoneType) is not applicable "
- + "for arguments (int, int, NoneType): 'sub' is 'int', but should be 'string'",
+ "argument 'sub' has type 'int', but should be 'string'\n"
+ + "in call to builtin method string.index(sub, start, end)",
"'test'.index(1)");
}
@@ -422,9 +422,8 @@
+ LINE_SEPARATOR
+ "\t\t\"test\".index(x)"
+ LINE_SEPARATOR
- + "method string.index(sub: string, start: int, end: int or NoneType) "
- + "is not applicable "
- + "for arguments (int, int, NoneType): 'sub' is 'int', but should be 'string'",
+ + "argument 'sub' has type 'int', but should be 'string'\n"
+ + "in call to builtin method string.index(sub, start, end)",
"def foo():",
" bar(1)",
"def bar(x):",
@@ -436,13 +435,10 @@
@Test
public void testBuiltinFunctionErrorMessage() throws Exception {
new BothModesTest()
+ .testIfErrorContains("substring \"z\" not found in \"abc\"", "'abc'.index('z')")
.testIfErrorContains(
- "substring \"z\" not found in \"abc\"",
- "'abc'.index('z')")
- .testIfErrorContains(
- "method string.startswith(sub: string, start: int, end: int or NoneType) is not "
- + "applicable for arguments (int, int, NoneType): 'sub' is 'int', "
- + "but should be 'string'",
+ "argument 'sub' has type 'int', but should be 'string'\n"
+ + "in call to builtin method string.startswith(sub, start, end)",
"'test'.startswith(1)")
.testIfErrorContains(
"expected value of type 'list(object)' for parameter args in dict(), "
@@ -1440,8 +1436,8 @@
.testStatement("hash('skylark')", "skylark".hashCode())
.testStatement("hash('google')", "google".hashCode())
.testIfErrorContains(
- "method hash(value: string) is not applicable for arguments (NoneType): "
- + "'value' is 'NoneType', but should be 'string'",
+ "argument 'value' has type 'NoneType', but should be 'string'\n"
+ + "in call to builtin function hash(value)",
"hash(None)");
}
@@ -1479,8 +1475,8 @@
public void testEnumerateBadArg() throws Exception {
new BothModesTest()
.testIfErrorContains(
- "method enumerate(list: sequence) is not applicable for arguments (string): "
- + "'list' is 'string', but should be 'sequence'",
+ "argument 'list' has type 'string', but should be 'sequence'\n"
+ + "in call to builtin function enumerate(list)",
"enumerate('a')");
}
@@ -1515,8 +1511,8 @@
.testLookup("FOO", MutableList.of(env, "a", "b", "c", "d", "e", "f"))
.testIfErrorContains("type 'tuple' has no method extend(list)", "(1, 2).extend([3, 4])")
.testIfErrorContains(
- "method list.extend(items: sequence) is not applicable for arguments "
- + "(int): 'items' is 'int', but should be 'sequence'",
+ "argument 'items' has type 'int', but should be 'sequence'\n"
+ + "in call to builtin method list.extend(items)",
"[1, 2].extend(3)");
}
