dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

Fix reflective NoSuchMethodErrors to match their non-reflective counterparts when due to argument mismatches.

Browse Source 
BUG=http://dartbug.com/23266
R=asiva@google.com

Review URL: https://codereview.chromium.org//1182613004.
This commit is contained in:
Ryan Macnak
2015-06-16 09:42:11 -07:00
parent 7f8fe44067
commit 98b9ff6a30
4 changed files with 106 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
runtime/lib/mirrors.cc
+37 -22
View File
@@ -47,6 +47,7 @@ static void ThrowNoSuchMethod(const Instance& receiver,
const String& function_name,
const Function& function,
const Array& arguments,
const Array& argument_names,
const InvocationMirror::Call call,
const InvocationMirror::Type type) {
const Smi& invocation_type = Smi::Handle(Smi::New(
@@ -57,19 +58,26 @@ static void ThrowNoSuchMethod(const Instance& receiver,
args.SetAt(1, function_name);
args.SetAt(2, invocation_type);
args.SetAt(3, arguments);
// TODO(rmacnak): Argument 4 (attempted argument names).
if (!argument_names.IsNull() && (argument_names.Length() > 0)) {
// Empty and null are treated differently for some reason. Don't pass empty
// to match the non-reflective error.
args.SetAt(4, argument_names);
}
if (!function.IsNull()) {
const intptr_t total_num_parameters = function.NumParameters();
const Array& array = Array::Handle(Array::New(total_num_parameters));
String& param_name = String::Handle();
for (int i = 0; i < total_num_parameters; i++) {
param_name = function.ParameterNameAt(i);
array.SetAt(i, param_name);
}
const Array& array = Array::Handle(Array::New(1));
array.SetAt(0, String::Handle(function.UserVisibleFormalParameters()));
args.SetAt(5, array);
}
Exceptions::ThrowByType(Exceptions::kNoSuchMethod, args);
const Library& libcore = Library::Handle(Library::CoreLibrary());
const Class& NoSuchMethodError = Class::Handle(
libcore.LookupClass(Symbols::NoSuchMethodError()));
const Function& throwNew = Function::Handle(
NoSuchMethodError.LookupFunctionAllowPrivate(Symbols::ThrowNew()));
const Object& result = Object::Handle(
DartEntry::InvokeFunction(throwNew, args));
ASSERT(result.IsError());
Exceptions::PropagateError(Error::Cast(result));
UNREACHABLE();
}
@@ -672,6 +680,7 @@ static RawInstance* InvokeLibraryGetter(const Library& library,
getter_name,
getter,
Object::null_array(),
Object::null_array(),
InvocationMirror::kTopLevel,
InvocationMirror::kGetter);
UNREACHABLE();
@@ -710,6 +719,7 @@ static RawInstance* InvokeClassGetter(const Class& klass,
getter_name,
getter,
Object::null_array(),
Object::null_array(),
InvocationMirror::kStatic,
InvocationMirror::kGetter);
UNREACHABLE();
@@ -1513,7 +1523,8 @@ DEFINE_NATIVE_ENTRY(ClassMirror_invoke, 5) {
ThrowNoSuchMethod(AbstractType::Handle(klass.RareType()),
function_name,
function,
Object::null_array(),
args,
arg_names,
InvocationMirror::kStatic,
InvocationMirror::kMethod);
UNREACHABLE();
@@ -1567,6 +1578,7 @@ DEFINE_NATIVE_ENTRY(ClassMirror_invokeSetter, 4) {
internal_setter_name,
setter,
args,
Object::null_array(),
InvocationMirror::kStatic,
InvocationMirror::kSetter);
UNREACHABLE();
@@ -1587,6 +1599,7 @@ DEFINE_NATIVE_ENTRY(ClassMirror_invokeSetter, 4) {
internal_setter_name,
setter,
Object::null_array(),
Object::null_array(),
InvocationMirror::kStatic,
InvocationMirror::kSetter);
UNREACHABLE();
@@ -1611,11 +1624,13 @@ DEFINE_NATIVE_ENTRY(ClassMirror_invokeConstructor, 5) {
// unnamed constructor for class 'A' is labeled 'A.'.
// This convention prevents users from explicitly calling constructors.
const String& klass_name = String::Handle(klass.Name());
String& external_constructor_name = String::Handle(klass_name.raw());
String& internal_constructor_name =
String::Handle(String::Concat(klass_name, Symbols::Dot()));
if (!constructor_name.IsNull()) {
if (!constructor_name.IsNull() && constructor_name.Length() > 0) {
internal_constructor_name =
String::Concat(internal_constructor_name, constructor_name);
external_constructor_name = internal_constructor_name.raw();
}
Function& lookup_constructor = Function::Handle(
@@ -1624,13 +1639,11 @@ DEFINE_NATIVE_ENTRY(ClassMirror_invokeConstructor, 5) {
if (lookup_constructor.IsNull() ||
(lookup_constructor.kind() != RawFunction::kConstructor) ||
!lookup_constructor.is_reflectable()) {
// Pretend we didn't find the constructor at all when the arity is wrong
// so as to produce the same NoSuchMethodError as the non-reflective case.
lookup_constructor = Function::null();
ThrowNoSuchMethod(AbstractType::Handle(klass.RareType()),
internal_constructor_name,
external_constructor_name,
lookup_constructor,
Object::null_array(),
explicit_args,
arg_names,
InvocationMirror::kConstructor,
InvocationMirror::kMethod);
UNREACHABLE();
@@ -1702,13 +1715,12 @@ DEFINE_NATIVE_ENTRY(ClassMirror_invokeConstructor, 5) {
ArgumentsDescriptor args_descriptor(args_descriptor_array);
if (!redirected_constructor.AreValidArguments(args_descriptor, NULL) ||
!redirected_constructor.is_reflectable()) {
// Pretend we didn't find the constructor at all when the arity is wrong
// so as to produce the same NoSuchMethodError as the non-reflective case.
redirected_constructor = Function::null();
external_constructor_name = redirected_constructor.name();
ThrowNoSuchMethod(AbstractType::Handle(klass.RareType()),
internal_constructor_name,
external_constructor_name,
redirected_constructor,
Object::null_array(),
explicit_args,
arg_names,
InvocationMirror::kConstructor,
InvocationMirror::kMethod);
UNREACHABLE();
@@ -1805,7 +1817,8 @@ DEFINE_NATIVE_ENTRY(LibraryMirror_invoke, 5) {
ThrowNoSuchMethod(Instance::null_instance(),
function_name,
function,
Object::null_array(),
args,
arg_names,
InvocationMirror::kTopLevel,
InvocationMirror::kMethod);
UNREACHABLE();
@@ -1862,6 +1875,7 @@ DEFINE_NATIVE_ENTRY(LibraryMirror_invokeSetter, 4) {
internal_setter_name,
setter,
args,
Object::null_array(),
InvocationMirror::kTopLevel,
InvocationMirror::kSetter);
UNREACHABLE();
@@ -1882,6 +1896,7 @@ DEFINE_NATIVE_ENTRY(LibraryMirror_invokeSetter, 4) {
internal_setter_name,
setter,
Object::null_array(),
Object::null_array(),
InvocationMirror::kTopLevel,
InvocationMirror::kSetter);
UNREACHABLE();
tests/lib/lib.status
+2
View File
@@ -76,6 +76,7 @@ mirrors/method_mirror_source_line_ending_test : RuntimeError # Issue 6490
mirrors/method_mirror_location_test: RuntimeError # Issue 6490
mirrors/mirrors_test: RuntimeError # TODO(ahe): I'm working on fixing this. When removing this line please change the "endsWith" to "/mirrors_test.dart".
mirrors/mirrors_nsm_test/dart2js: RuntimeError # Issue 19353
mirrors/mirrors_nsm_mismatch_test: RuntimeError # Issue 19353
mirrors/mixin_test: RuntimeError # Issue 12464
mirrors/mixin_application_test: RuntimeError # Issue 12464
mirrors/other_declarations_location_test: CompileTimeError # Issue 10905
@@ -277,6 +278,7 @@ mirrors/redirecting_factory_test/none: StaticWarning # test issue X, The return
mirrors/immutable_collections_test: StaticWarning, OK # Expect failure for any type of Iterable.
mirrors/inference_and_no_such_method_test: StaticWarning, OK # Expect to trigger noSuchMethod.
mirrors/mirrors_nsm_test: StaticWarning, OK # Expect to trigger noSuchMethod.
mirrors/mirrors_nsm_mismatch_test: StaticWarning, OK # Expect to trigger noSuchMethod.
mirrors/repeated_private_anon_mixin_app_test: StaticWarning, OK # Intentional library name conflict.
mirrors/removed_api_test: StaticWarning, OK # Deliberately refers to undeclared members.
tests/lib/mirrors/mirrors_nsm_mismatch_test.dart
+54
View File
@@ -0,0 +1,54 @@
// Copyright (c) 2015, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
library test.mirrors_nsm_mistatch;
import 'dart:mirrors';
import 'mirrors_nsm_test.dart';
topLevelMethod({missing}) {}
class C {
C.constructor({missing});
factory C.redirecting({missing}) = C.constructor;
static staticMethod({missing}) {}
instanceMethod({missing}) {}
}
main() {
var mirrors = currentMirrorSystem();
var libMirror = mirrors.findLibrary(#test.mirrors_nsm_mistatch);
expectMatchingErrors(() => libMirror.invoke(#topLevelMethod, [], {#extra: 1}),
() => topLevelMethod(extra: 1));
expectMatchingErrors(() => libMirror.invoke(#topLevelMethod, ['positional']),
() => topLevelMethod('positional'));
var classMirror = reflectClass(C);
expectMatchingErrors(() => classMirror.newInstance(#constructor, [],
{#extra: 1}),
() => new C.constructor(extra: 1));
expectMatchingErrors(() => classMirror.newInstance(#redirecting, [],
{#extra: 1}),
() => new C.redirecting(extra: 1));
expectMatchingErrors(() => classMirror.invoke(#staticMethod, [],
{#extra: 1}),
() => C.staticMethod(extra: 1));
expectMatchingErrors(() => classMirror.newInstance(#constructor,
['positional']),
() => new C.constructor('positional'));
expectMatchingErrors(() => classMirror.newInstance(#redirecting,
['positional']),
() => new C.redirecting('positional'));
expectMatchingErrors(() => classMirror.invoke(#staticMethod,
['positional']),
() => C.staticMethod('positional'));
var instanceMirror = reflect(new C.constructor());
expectMatchingErrors(() => instanceMirror.invoke(#instanceMethod, [],
{#extra: 1}),
() => instanceMirror.reflectee.instanceMethod(extra: 1));
expectMatchingErrors(() => instanceMirror.invoke(#instanceMethod,
['positional']),
() => instanceMirror.reflectee
.instanceMethod('positional'));
}
tests/lib/mirrors/mirrors_nsm_test.dart
+13 -3
View File
@@ -20,6 +20,10 @@ class A {
final finalInstance = 0;
static final finalStatic = 0;
}
class B {
B(a, b);
factory B.fac(a, b) => new B(a, b);
}
testMessageContents() {
var mirrors = currentMirrorSystem();
@@ -61,14 +65,18 @@ expectMatchingErrors(reflectiveAction, baseAction) {
} catch(e) {
reflectiveError = e;
}
try {
baseAction();
} catch(e) {
baseError = e;
}
print("\n==Base==\n $baseError");
print("\n==Reflective==\n $reflectiveError");
Expect.stringEquals(baseError.toString(), reflectiveError.toString());
if (baseError.toString() != reflectiveError.toString()) {
print("\n==Base==\n $baseError");
print("\n==Reflective==\n $reflectiveError");
throw "Expected matching errors";
}
}
testMatchingMessages() {
@@ -92,6 +100,8 @@ testMatchingMessages() {
() => A.foo= null);
expectMatchingErrors(() => classMirror.setField(#finalStatic, null),
() => A.finalStatic= null);
expectMatchingErrors(() => classMirror.newInstance(#constructor, [1, 2, 3]),
() => new A.constructor(1, 2, 3));
var instanceMirror = reflect(new A());
expectMatchingErrors(() => instanceMirror.invoke(#foo, []),