Improvements to NoSuchMethodError.toString

This makes the following improvements:

* Always show the call that was made, with arguments.
* ...and show the actual arguments in more cases.
* Be less confusing in the case of a call on the null receiver.
* Be less redundant in the output.
* Report when a constructor is called with the wrong number of arguments.

R=rmacnak@google.com

Review URL: https://codereview.chromium.org/2405353002 .
This commit is contained in:
John McCutchan 2016-10-12 09:52:51 -07:00
parent 3066d28c0d
commit 7fd9df4d91
3 changed files with 96 additions and 105 deletions

View file

@ -166,14 +166,13 @@ class _InternalError {
int numPositionalArguments = arguments == null ? 0 : arguments.length;
numPositionalArguments -= numNamedArguments;
List positionalArguments;
if (numPositionalArguments == 0) {
// Differ between no arguments specified and 0 arguments.
// TODO(srdjan): This can currently occur for unresolvable static methods.
// In that case, the arguments are evaluated but not passed to the
if (numPositionalArguments > 0) {
// TODO(srdjan): Unresolvable static methods sometimes do not provide the
// arguments, because the arguments are evaluated but not passed to the
// throwing stub (see EffectGraphVisitor::BuildThrowNoSuchMethodError and
// Parser::ThrowNoSuchMethodError)).
positionalArguments = argumentNames == null ? null : [];
} else {
// Parser::ThrowNoSuchMethodError)). There is no way to distinguish the
// case of no arguments from the case of the arguments not being passed
// in here, though. See https://github.com/dart-lang/sdk/issues/27572
positionalArguments = arguments.sublist(0, numPositionalArguments);
}
Map<Symbol, dynamic> namedArguments = new Map<Symbol, dynamic>();
@ -234,127 +233,119 @@ class _InternalError {
value: (k) => namedArguments[k]),
this._existingArgumentNames = existingArgumentNames;
String _developerMessage(args_mismatch) {
if (_invocation_type < 0) {
return "";
}
var type = _invocation_type & _InvocationMirror._TYPE_MASK;
@patch String toString() {
var level = (_invocation_type >> _InvocationMirror._CALL_SHIFT) &
_InvocationMirror._CALL_MASK;
var type_str =
(const ["method", "getter", "setter", "getter or setter", "variable"])[type];
var args_message = args_mismatch ? " with matching arguments" : "";
var msg;
var memberName =
(_memberName == null) ? "" : internal.Symbol.getUnmangledName(_memberName);
_InvocationMirror._CALL_MASK;
var type = _invocation_type & _InvocationMirror._TYPE_MASK;
String memberName = (_memberName == null) ? "" :
internal.Symbol.getUnmangledName(_memberName);
if (type == _InvocationMirror._LOCAL_VAR) {
return "cannot assign to final variable '$memberName'.\n\n";
return "NoSuchMethodError: Cannot assign to final variable '$memberName'";
}
StringBuffer arguments = new StringBuffer();
int argumentCount = 0;
if (_arguments != null) {
for (; argumentCount < _arguments.length; argumentCount++) {
if (argumentCount > 0) {
arguments.write(", ");
}
arguments.write(Error.safeToString(_arguments[argumentCount]));
}
}
if (_namedArguments != null) {
_namedArguments.forEach((Symbol key, var value) {
if (argumentCount > 0) {
arguments.write(", ");
}
arguments.write(internal.Symbol.getUnmangledName(key));
arguments.write(": ");
arguments.write(Error.safeToString(value));
argumentCount++;
});
}
bool args_mismatch = _existingArgumentNames != null;
String args_message = args_mismatch ? " with matching arguments" : "";
String type_str;
if (type >= 0 && type < 5) {
type_str = (const ["method", "getter", "setter", "getter or setter",
"variable"])[type];
}
StringBuffer msg_buf = new StringBuffer("NoSuchMethodError: ");
switch (level) {
case _InvocationMirror._DYNAMIC: {
if (_receiver == null) {
msg = "The null object does not have a $type_str '$memberName'"
"$args_message.";
if (args_mismatch) {
msg_buf.writeln("The null object does not have a $type_str "
"'$memberName'$args_message.");
} else {
msg_buf.writeln("The $type_str '$memberName' was called on null.");
}
} else {
if (_receiver is Function) {
msg = "Closure call with mismatched arguments: "
"function '$memberName'";
msg_buf.writeln("Closure call with mismatched arguments: "
"function '$memberName'");
} else {
msg = "Class '${_receiver.runtimeType}' has no instance $type_str "
"'$memberName'$args_message.";
msg_buf.writeln("Class '${_receiver.runtimeType}' has no instance "
"$type_str '$memberName'$args_message.");
}
}
break;
}
case _InvocationMirror._SUPER: {
msg = "Super class of class '${_receiver.runtimeType}' has no instance "
"$type_str '$memberName'$args_message.";
msg_buf.writeln("Super class of class '${_receiver.runtimeType}' has "
"no instance $type_str '$memberName'$args_message.");
memberName = "super.$memberName";
break;
}
case _InvocationMirror._STATIC: {
msg = "No static $type_str '$memberName' declared in class "
"'$_receiver'.";
msg_buf.writeln("No static $type_str '$memberName'$args_message "
"declared in class '$_receiver'.");
break;
}
case _InvocationMirror._CONSTRUCTOR: {
msg = "No constructor '$memberName'$args_message declared in class '$_receiver'.";
msg_buf.writeln("No constructor '$memberName'$args_message declared "
"in class '$_receiver'.");
memberName = "new $memberName";
break;
}
case _InvocationMirror._TOP_LEVEL: {
msg = "No top-level $type_str '$memberName'$args_message declared.";
msg_buf.writeln("No top-level $type_str '$memberName'$args_message "
"declared.");
break;
}
}
return "$msg\n\n";
}
@patch String toString() {
StringBuffer actual_buf = new StringBuffer();
int i = 0;
if (_arguments == null) {
// Actual arguments unknown.
// TODO(srdjan): Remove once arguments are passed for unresolvable
// static methods.
actual_buf.write("...");
if (level == _InvocationMirror._TOP_LEVEL) {
msg_buf.writeln("Receiver: top-level");
} else {
for (; i < _arguments.length; i++) {
if (i > 0) {
actual_buf.write(", ");
}
actual_buf.write(Error.safeToString(_arguments[i]));
}
msg_buf.writeln("Receiver: ${Error.safeToString(_receiver)}");
}
if (_namedArguments != null) {
_namedArguments.forEach((Symbol key, var value) {
if (i > 0) {
actual_buf.write(", ");
}
actual_buf.write(internal.Symbol.getUnmangledName(key));
actual_buf.write(": ");
actual_buf.write(Error.safeToString(value));
i++;
});
}
var args_mismatch = _existingArgumentNames != null;
StringBuffer msg_buf = new StringBuffer(_developerMessage(args_mismatch));
String receiver_str;
var level = (_invocation_type >> _InvocationMirror._CALL_SHIFT) &
_InvocationMirror._CALL_MASK;
if ( level == _InvocationMirror._TOP_LEVEL) {
receiver_str = "top-level";
if (type == _InvocationMirror._METHOD) {
msg_buf.write("Tried calling: $memberName($arguments)");
} else if (argumentCount == 0) {
msg_buf.write("Tried calling: $memberName");
} else if (type == _InvocationMirror._SETTER) {
msg_buf.write("Tried calling: $memberName$arguments");
} else {
receiver_str = Error.safeToString(_receiver);
msg_buf.write("Tried calling: $memberName = $arguments");
}
var memberName =
(_memberName == null) ? "" : internal.Symbol.getUnmangledName(_memberName);
var type = _invocation_type & _InvocationMirror._TYPE_MASK;
if (type == _InvocationMirror._LOCAL_VAR) {
msg_buf.write(
"NoSuchMethodError: cannot assign to final variable '$memberName'");
} else if (!args_mismatch) {
msg_buf.write(
"NoSuchMethodError: method not found: '$memberName'\n"
"Receiver: $receiver_str\n"
"Arguments: [$actual_buf]");
} else {
String actualParameters = actual_buf.toString();
StringBuffer formal_buf = new StringBuffer();
if (args_mismatch) {
StringBuffer formalParameters = new StringBuffer();
for (int i = 0; i < _existingArgumentNames.length; i++) {
if (i > 0) {
formal_buf.write(", ");
formalParameters.write(", ");
}
formal_buf.write(_existingArgumentNames[i]);
formalParameters.write(_existingArgumentNames[i]);
}
String formalParameters = formal_buf.toString();
msg_buf.write(
"NoSuchMethodError: incorrect number of arguments passed to "
"method named '$memberName'\n"
"Receiver: $receiver_str\n"
"Tried calling: $memberName($actualParameters)\n"
"Found: $memberName($formalParameters)");
msg_buf.write("\nFound: $memberName($formalParameters)");
}
return msg_buf.toString();
}
}

View file

@ -1536,7 +1536,7 @@ TEST_CASE(IsolateReload_DanglingGetter_Instance) {
TestCase::SetReloadTestScript(kReloadScript);
EXPECT_STREQ("4 Class 'C' has no instance getter 'y'.",
EXPECT_STREQ("4 NoSuchMethodError: Class 'C' has no instance getter 'y'.",
SimpleInvokeStr(lib, "main"));
lib = TestCase::GetReloadErrorOrRootLibrary();
@ -1595,8 +1595,8 @@ TEST_CASE(IsolateReload_DanglingGetter_Class) {
TestCase::SetReloadTestScript(kReloadScript);
EXPECT_STREQ("4 No static getter 'y' declared in class 'C'.",
SimpleInvokeStr(lib, "main"));
EXPECT_STREQ("4 NoSuchMethodError: No static getter 'y' declared "
"in class 'C'.", SimpleInvokeStr(lib, "main"));
lib = TestCase::GetReloadErrorOrRootLibrary();
EXPECT_VALID(lib);
@ -1653,7 +1653,7 @@ TEST_CASE(IsolateReload_DanglingGetter_Library) {
TestCase::SetReloadTestScript(kScript); // Root library does not change.
EXPECT_STREQ("4 No top-level getter 'y' declared.",
EXPECT_STREQ("4 NoSuchMethodError: No top-level getter 'y' declared.",
SimpleInvokeStr(lib, "main"));
lib = TestCase::GetReloadErrorOrRootLibrary();
@ -1710,7 +1710,7 @@ TEST_CASE(IsolateReload_DanglingSetter_Instance) {
TestCase::SetReloadTestScript(kReloadScript);
EXPECT_STREQ("null Class 'C' has no instance setter 'y='.",
EXPECT_STREQ("null NoSuchMethodError: Class 'C' has no instance setter 'y='.",
SimpleInvokeStr(lib, "main"));
lib = TestCase::GetReloadErrorOrRootLibrary();
@ -1769,8 +1769,8 @@ TEST_CASE(IsolateReload_DanglingSetter_Class) {
TestCase::SetReloadTestScript(kReloadScript);
EXPECT_STREQ("5 No static setter 'y=' declared in class 'C'.",
SimpleInvokeStr(lib, "main"));
EXPECT_STREQ("5 NoSuchMethodError: No static setter 'y=' declared in "
"class 'C'.", SimpleInvokeStr(lib, "main"));
lib = TestCase::GetReloadErrorOrRootLibrary();
EXPECT_VALID(lib);
@ -1827,7 +1827,7 @@ TEST_CASE(IsolateReload_DanglingSetter_Library) {
TestCase::SetReloadTestScript(kScript); // Root library does not change.
EXPECT_STREQ("5 No top-level setter 'y=' declared.",
EXPECT_STREQ("5 NoSuchMethodError: No top-level setter 'y=' declared.",
SimpleInvokeStr(lib, "main"));
lib = TestCase::GetReloadErrorOrRootLibrary();
@ -1883,8 +1883,8 @@ TEST_CASE(IsolateReload_TearOff_AddArguments) {
TestCase::SetReloadTestScript(kReloadScript);
EXPECT_STREQ("1 Class 'C' has no instance method 'foo' with matching"
" arguments.", SimpleInvokeStr(lib, "main"));
EXPECT_STREQ("1 NoSuchMethodError: Class 'C' has no instance method "
"'foo' with matching arguments.", SimpleInvokeStr(lib, "main"));
lib = TestCase::GetReloadErrorOrRootLibrary();
EXPECT_VALID(lib);
@ -1937,8 +1937,8 @@ TEST_CASE(IsolateReload_TearOff_AddArguments2) {
TestCase::SetReloadTestScript(kReloadScript);
EXPECT_STREQ("1 Closure call with mismatched arguments: function 'C.foo'",
SimpleInvokeStr(lib, "main"));
EXPECT_STREQ("1 NoSuchMethodError: Closure call with mismatched arguments: "
"function 'C.foo'", SimpleInvokeStr(lib, "main"));
lib = TestCase::GetReloadErrorOrRootLibrary();
EXPECT_VALID(lib);

View file

@ -6,7 +6,7 @@
import "package:expect/expect.dart";
// Test error message with noSuchMethodError: non-existent names
// should result in a "method not found" message.
// should result in a message that reports the missing method.
call_bar(x) => x.bar();
@ -14,7 +14,7 @@ testMessage() {
try {
call_bar(5);
} catch (e) {
Expect.isTrue(e.toString().indexOf("method not found") != -1);
Expect.isTrue(e.toString().indexOf("has no instance method 'bar'") != -1);
}
}