From: Stevan Little Date: Fri, 12 Oct 2007 04:01:11 +0000 (+0000) Subject: confound++ X-Git-Tag: 0_27~25 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=e7f8d0c27375484d94d340573aeb1fb41a1a1216;p=gitmo%2FMoose.git confound++ --- diff --git a/Changes b/Changes index 7644888..b092c61 100644 --- a/Changes +++ b/Changes @@ -3,6 +3,12 @@ Revision history for Perl extension Moose 0.27 * fixing some misc. bits in the docs that got mentioned on CPAN Forum + + * Moose::Meta::Role + - fixed how required methods are handled + when they encounter overriden or modified + methods from a class (thanks to confound). + - added tests for this 0.26 Thurs. Sept. 27, 2007 == New Features == diff --git a/lib/Moose.pm b/lib/Moose.pm index 5b577f7..2994965 100644 --- a/lib/Moose.pm +++ b/lib/Moose.pm @@ -860,6 +860,8 @@ Nathan (kolibre) Gray Christian (chansen) Hansen +Hans Dieter (confound) Pearcey + Eric (ewilhelm) Wilhelm Guillermo (groditi) Roditi diff --git a/lib/Moose/Meta/Role.pm b/lib/Moose/Meta/Role.pm index 317689c..4506c90 100644 --- a/lib/Moose/Meta/Role.pm +++ b/lib/Moose/Meta/Role.pm @@ -9,7 +9,7 @@ use Carp 'confess'; use Scalar::Util 'blessed'; use B 'svref_2object'; -our $VERSION = '0.09'; +our $VERSION = '0.10'; our $AUTHORITY = 'cpan:STEVAN'; use Moose::Meta::Class; @@ -411,20 +411,23 @@ sub _check_required_methods { # not a method modifier, because those do # not satisfy the requirements ... my $method = $other->find_method_by_name($required_method_name); - # check if it is an override or a generated accessor .. - ($method->isa('Class::MOP::Method::Accessor')) - && confess "'" . $self->name . "' requires the method '$required_method_name' " . - "to be implemented by '" . $other->name . "', the method is only an attribute"; - # before/after/around methods are a little trickier - # since we wrap the original local method (if applicable) - # so we need to check if the original wrapped method is - # from the same package, and not a wrap of the super method - if ($method->isa('Class::MOP::Method::Wrapped') || - $method->isa('Moose::Meta::Method::Overriden')) { - ($other->name->isa($method->get_original_method->package_name)) - || confess "'" . $self->name . "' requires the method '$required_method_name' " . - "to be implemented by '" . $other->name . "', the method is only a method modifier"; - } + + # check if it is a generated accessor ... + (!$method->isa('Class::MOP::Method::Accessor')) + || confess "'" . $self->name . "' requires the method '$required_method_name' " . + "to be implemented by '" . $other->name . "', the method is only an attribute 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 } } } diff --git a/t/030_roles/007_roles_and_required_method_edge_cases.t b/t/030_roles/007_roles_and_required_method_edge_cases.t index 258e390..fa63489 100644 --- a/t/030_roles/007_roles_and_required_method_edge_cases.t +++ b/t/030_roles/007_roles_and_required_method_edge_cases.t @@ -3,7 +3,7 @@ use strict; use warnings; -use Test::More tests => 19; +use Test::More tests => 17; use Test::Exception; =pod @@ -89,9 +89,9 @@ second class citizens. override 'foo' => sub { 'Class::ProvideFoo::foo' }; - ::dies_ok { + ::lives_ok { with 'Role::RequireFoo'; - } '... the required "foo" method exists, but it is an override (and we will die)'; + } '... the required "foo" method exists, although it is overriden locally'; } @@ -121,22 +121,22 @@ method modifier. before 'foo' => sub { 'Class::ProvideFoo::foo:before' }; - ::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, although it is a before modifier locally'; package Class::ProvideFoo::Before3; use Moose; extends 'Class::ProvideFoo::Base'; - ::lives_ok { - with 'Role::RequireFoo'; - } '... the required "foo" method will not exist yet (and we will die)'; - sub foo { 'Class::ProvideFoo::foo' } before 'foo' => sub { 'Class::ProvideFoo::foo:before' }; + ::lives_ok { + with 'Role::RequireFoo'; + } '... the required "foo" method exists locally, and it is modified locally'; + package Class::ProvideFoo::Before4; use Moose; @@ -152,21 +152,7 @@ method modifier. ::lives_ok { with 'Role::RequireFoo'; } '... the required "foo" method exists in the symbol table (and we will live)'; - - package Class::ProvideFoo::Before5; - use Moose; - - extends 'Class::ProvideFoo::Base'; - - before 'foo' => sub { 'Class::ProvideFoo::foo:before' }; - - ::isa_ok(__PACKAGE__->meta->get_method('foo'), 'Class::MOP::Method::Wrapped'); - ::isnt(__PACKAGE__->meta->get_method('foo')->get_original_method->package_name, __PACKAGE__, - '... but the original method is not from our package'); - - ::dies_ok { - with 'Role::RequireFoo'; - } '... the required "foo" method exists, but it is a before wrapping the super (and we will die)'; + } =pod @@ -270,3 +256,31 @@ method modifier. with 'Bar::Role'; } 'required method exists in superclass as non-modifier, so we live'; } + +{ + package Bar2::Class::Base; + use Moose; + + sub bar { "hello!" } +} +{ + package Bar2::Role; + use Moose::Role; + requires 'bar'; +} +{ + package Bar2::Class::Child; + use Moose; + extends 'Bar2::Class::Base'; + override bar => sub { "o noes" }; + # technically we could run lives_ok here, too, but putting it into a + # grandchild class makes it more obvious why this matters. +} +{ + package Bar2::Class::Grandchild; + use Moose; + extends 'Bar2::Class::Child'; + ::lives_ok { + with 'Bar2::Role'; + } 'required method exists in superclass as non-modifier, so we live'; +}