Change error messaging of @SkylarkCallable invocations to match @SkylarkSignature more closely.

Also clarify the method representation in these error messages is for the attempted method *call*, not the actual method signature.

RELNOTES: None.
PiperOrigin-RevId: 189935148

3 files changed, 18 insertions, 13 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
index 841a94ac13..566c9fecf9 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
@@ -440,7 +440,7 @@ public final class FuncallExpression extends Expression {
       } else {
         errorMessage =
             String.format(
-                "%s, in method %s of '%s'",
+                "%s, in method call %s of '%s'",
                 argumentListConversionResult.getError(),
                 formatMethod(methodName, args, kwargs),
                 EvalUtils.getDataTypeNameFromClass(objClass));
@@ -538,7 +538,8 @@ public final class FuncallExpression extends Expression {
         } else if (!type.contains(value)) {
           return ArgumentListConversionResult.fromError(
               String.format(
-                  "Cannot convert parameter '%s' to type %s", param.name(), type.toString()));
+                  "expected value of type '%s' for parameter '%s'",
+                  type.toString(), param.name()));
         }
         i++;
       } else if (param.named() && keys.remove(param.name())) {
@@ -547,7 +548,8 @@ public final class FuncallExpression extends Expression {
         if (!type.contains(value)) {
           return ArgumentListConversionResult.fromError(
               String.format(
-                  "Cannot convert parameter '%s' to type %s", param.name(), type.toString()));
+                  "expected value of type '%s' for parameter '%s'",
+                  type.toString(), param.name()));
         }
       } else {
         // Use default value
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 edb8d48fbb..43073e9342 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
@@ -299,7 +299,7 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase {
   public void testCreateSpawnActionArgumentsBadExecutable() throws Exception {
     checkErrorContains(
         createRuleContext("//foo:foo"),
-        "Cannot convert parameter 'executable' to type File or string, in method "
+        "expected value of type 'File or string' for parameter 'executable', in method call "
             + "run(list inputs, list outputs, list arguments, int executable) of 'actions'",
         "ruleContext.actions.run(",
         "  inputs = ruleContext.files.srcs,",
@@ -355,7 +355,7 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase {
     SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
     checkErrorContains(
         ruleContext,
-        "unexpected keyword 'bad_param', in method run("
+        "unexpected keyword 'bad_param', in method call run("
             + "list outputs, string bad_param, File executable) of 'actions'",
         "f = ruleContext.actions.declare_file('foo.sh')",
         "ruleContext.actions.run(outputs=[], bad_param = 'some text', executable = f)");
@@ -433,7 +433,8 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase {
 
     checkErrorContains(
         ruleContext,
-        "parameter 'mnemonic' has no default value, in method do_nothing(list inputs) of 'actions'",
+        "parameter 'mnemonic' has no default value, "
+        + "in method call do_nothing(list inputs) of 'actions'",
         "ruleContext.actions.do_nothing(inputs = ruleContext.files.srcs)");
   }
 
@@ -636,7 +637,7 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase {
     SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
     checkErrorContains(
         ruleContext,
-        "Cannot convert parameter 'content' to type string or Args",
+        "expected value of type 'string or Args' for parameter 'content'",
         "ruleContext.actions.write(",
         "  output = ruleContext.files.srcs[0],",
         "  content = 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 8bb82f1279..d49d0d59cf 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
@@ -937,13 +937,14 @@ public class SkylarkEvaluationTest extends EvaluationTest {
         .update("mock", new Mock())
         .setUp("")
         .testIfExactError(
-            "parameter 'named' has no default value, in method with_params(int, bool) of 'Mock'",
+            "parameter 'named' has no default value, in method call "
+                + "with_params(int, bool) of 'Mock'",
             "mock.with_params(1, True)");
     new SkylarkTest()
         .update("mock", new Mock())
         .setUp("")
         .testIfExactError(
-            "parameter 'named' has no default value, in method with_params(int, bool, bool) "
+            "parameter 'named' has no default value, in method call with_params(int, bool, bool) "
                 + "of 'Mock'",
             "mock.with_params(1, True, True)");
     new SkylarkTest()
@@ -962,14 +963,14 @@ public class SkylarkEvaluationTest extends EvaluationTest {
         .update("mock", new Mock())
         .setUp("")
         .testIfExactError(
-            "unexpected keyword 'n', in method with_params(int, bool, bool named, "
+            "unexpected keyword 'n', in method call with_params(int, bool, bool named, "
                 + "bool posOrNamed, int n) of 'Mock'",
             "mock.with_params(1, True, named=True, posOrNamed=True, n=2)");
     new SkylarkTest()
         .update("mock", new Mock())
         .setUp("")
         .testIfExactError(
-            "parameter 'nonNoneable' cannot be None, in method with_params(int, bool, bool, "
+            "parameter 'nonNoneable' cannot be None, in method call with_params(int, bool, bool, "
                 + "bool named, bool optionalNamed, NoneType nonNoneable) of 'Mock'",
             "mock.with_params(1, True, True, named=True, optionalNamed=False, nonNoneable=None)");
 
@@ -977,8 +978,9 @@ public class SkylarkEvaluationTest extends EvaluationTest {
         .update("mock", new Mock())
         .setUp("")
         .testIfExactError(
-            "Cannot convert parameter 'multi' to type string or int or sequence of ints or"
-                + " NoneType, in method with_params(int, bool, bool named, bool multi) of 'Mock'",
+            "expected value of type 'string or int or sequence of ints or NoneType' for parameter"
+                + " 'multi', in method call with_params(int, bool, bool named, bool multi)"
+                + " of 'Mock'",
             "mock.with_params(1, True, named=True, multi=False)");
 
     // We do not enforce list item parameter type constraints.