From: Yuval Kogman Date: Mon, 19 Jan 2009 00:50:01 +0000 (+0000) Subject: accessors can fulfill required role methods X-Git-Tag: 0.65~19 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=f3e76c8febfc574afaaabd20d06507ed1517086f;p=gitmo%2FMoose.git accessors can fulfill required role methods --- diff --git a/lib/Moose/Meta/Role/Application/ToClass.pm b/lib/Moose/Meta/Role/Application/ToClass.pm index f61cf03..ce1f445 100644 --- a/lib/Moose/Meta/Role/Application/ToClass.pm +++ b/lib/Moose/Meta/Role/Application/ToClass.pm @@ -51,32 +51,9 @@ sub check_required_methods { push @missing, $required_method_name; } - else { - # NOTE: - # we need to make sure that the method is - # not a method modifier, because those do - # not satisfy the requirements ... - my $method = $class->find_method_by_name($required_method_name); - - # check if it is a generated accessor ... - push @is_attr, $required_method_name, - if $method->isa('Class::MOP::Method::Accessor'); - - # NOTE: - # All other tests here have been removed, they were tests - # for overriden methods and before/after/around modifiers. - # But we realized that for classes any overriden or modified - # methods would be backed by a real method of that name - # (and therefore meet the requirement). And for roles, the - # overriden and modified methods are "in statis" and so would - # not show up in this test anyway (and as a side-effect they - # would not fufill the requirement, which is exactly what we - # want them to do anyway). - # - SL - } } - return unless @missing || @is_attr; + return unless @missing; my $error = ''; @@ -94,23 +71,6 @@ sub check_required_methods { . $class->name . q{'}; } - if (@is_attr) { - my $noun = @is_attr == 1 ? 'method' : 'methods'; - - my $list - = Moose::Util::english_list( map { q{'} . $_ . q{'} } @is_attr ); - - $error .= "\n" if length $error; - - $error - .= q{'} - . $role->name - . "' requires the $noun $list " - . "to be implemented by '" - . $class->name - . "' but the method is only an attribute accessor"; - } - $class->throw_error($error); } diff --git a/t/030_roles/004_role_composition_errors.t b/t/030_roles/004_role_composition_errors.t index 7e8aba6..ce8918f 100644 --- a/t/030_roles/004_role_composition_errors.t +++ b/t/030_roles/004_role_composition_errors.t @@ -140,9 +140,8 @@ is_deeply( has 'meth2' => ( is => 'ro' ); ::throws_ok { with('Quux::Role') } - qr/\Q'Quux::Role' requires the methods 'meth3' and 'meth4' to be implemented by 'Quux::Class3'\E\n - \Q'Quux::Role' requires the methods 'meth1' and 'meth2' to be implemented by 'Quux::Class3' but the method is only an attribute accessor/x, - 'exception mentions all the require methods that are accessors at once, as well as missing methods'; + qr/'Quux::Role' requires the methods 'meth3' and 'meth4' to be implemented by 'Quux::Class3'/, + 'exception mentions all the missing methods at once, but not the accessors'; } { @@ -153,7 +152,6 @@ is_deeply( has 'meth2' => ( is => 'ro' ); ::throws_ok { with('Quux::Role') } - qr/\Q'Quux::Role' requires the methods 'meth3' and 'meth4' to be implemented by 'Quux::Class4'\E\n - \Q'Quux::Role' requires the method 'meth2' to be implemented by 'Quux::Class4' but the method is only an attribute accessor/x, + qr/'Quux::Role' requires the methods 'meth3' and 'meth4' to be implemented by 'Quux::Class4'/, 'exception mentions all the require methods that are accessors at once, as well as missing methods, but not the one that exists'; } diff --git a/t/030_roles/007_roles_and_req_method_edge_cases.t b/t/030_roles/007_roles_and_req_method_edge_cases.t index 9017f85..df24b95 100644 --- a/t/030_roles/007_roles_and_req_method_edge_cases.t +++ b/t/030_roles/007_roles_and_req_method_edge_cases.t @@ -173,9 +173,9 @@ method modifier. has 'foo' => (is => 'ro'); - ::dies_ok { + ::lives_ok { with 'Role::RequireFoo'; - } '... the required "foo" method exists, but it is a before (and we will die)'; + } '... the required "foo" method exists, and is an accessor'; } # ... @@ -211,7 +211,7 @@ method modifier. use Moose; extends 'Foo::Class::Base'; - ::dies_ok { + ::lives_ok { with 'Foo::Role'; } '... our role combined successfully'; } diff --git a/t/030_roles/011_overriding.t b/t/030_roles/011_overriding.t index 1ee53e1..75afe7a 100644 --- a/t/030_roles/011_overriding.t +++ b/t/030_roles/011_overriding.t @@ -174,6 +174,7 @@ ok(Role::D::And::E::Conflict->meta->requires_method('bar'), '... Role::D::And::E } "define role Role::I"; sub zot { 'Role::I::zot' } + sub zzy { 'Role::I::zzy' } package Class::C; use Moose; @@ -205,26 +206,21 @@ is( Class::E->new->xxy, "Role::J::xxy", "... got the right &xxy method" ); ok(Role::I->meta->requires_method('foo'), '... Role::I still have the &foo requirement'); { - # fix these later ... - TODO: { - local $TODO = "add support for attribute methods fufilling reqs"; + lives_ok { + package Class::D; + use Moose; - lives_ok { - package Class::D; - use Moose; + has foo => ( default => __PACKAGE__ . "::foo", is => "rw" ); - has foo => ( default => __PACKAGE__ . "::foo", is => "rw" ); + sub zot { 'Class::D::zot' } - sub zot { 'Class::D::zot' } + with qw(Role::I); - with qw(Role::I); - - } "resolved with attr"; + } "resolved with attr"; - can_ok( Class::D->new, qw(foo bar xxy zot) ); - is( eval { Class::D->new->bar }, "Role::H::bar", "bar" ); - is( eval { Class::D->new->xxy }, "Role::I::xxy", "xxy" ); - } + can_ok( Class::D->new, qw(foo bar xxy zot) ); + is( eval { Class::D->new->bar }, "Role::H::bar", "bar" ); + is( eval { Class::D->new->zzy }, "Role::I::zzy", "zzy" ); is( eval { Class::D->new->foo }, "Class::D::foo", "foo" ); is( eval { Class::D->new->zot }, "Class::D::zot", "zot" ); diff --git a/t/600_todo_tests/002_various_role_shit.t b/t/600_todo_tests/002_various_role_shit.t index 6fe05db..f1fbc06 100644 --- a/t/600_todo_tests/002_various_role_shit.t +++ b/t/600_todo_tests/002_various_role_shit.t @@ -83,7 +83,6 @@ sub req_or_has ($$) { has twist => ( is => "rw" ); { - local our $TODO = "accessors don't satisfy role requires"; ::lives_ok { with qw(Dancer) }; }