diff --git a/runtime/lib/mirrors.cc b/runtime/lib/mirrors.cc index a7c11d77a42..3ffbff8ec4f 100644 --- a/runtime/lib/mirrors.cc +++ b/runtime/lib/mirrors.cc @@ -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(); diff --git a/tests/lib/lib.status b/tests/lib/lib.status index 45e75f4ca99..293e6e83920 100644 --- a/tests/lib/lib.status +++ b/tests/lib/lib.status @@ -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. diff --git a/tests/lib/mirrors/mirrors_nsm_mismatch_test.dart b/tests/lib/mirrors/mirrors_nsm_mismatch_test.dart new file mode 100644 index 00000000000..5c63ca1dfc2 --- /dev/null +++ b/tests/lib/mirrors/mirrors_nsm_mismatch_test.dart @@ -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')); +} diff --git a/tests/lib/mirrors/mirrors_nsm_test.dart b/tests/lib/mirrors/mirrors_nsm_test.dart index f1abaa76605..2364f925b18 100644 --- a/tests/lib/mirrors/mirrors_nsm_test.dart +++ b/tests/lib/mirrors/mirrors_nsm_test.dart @@ -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, []),