From: Dave Rolsky Date: Thu, 19 Jun 2008 14:39:17 +0000 (+0000) Subject: This change gets Recipe 11 working. Here are the details ... X-Git-Tag: 0_55~106 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=db9476b157548c0e0d0f5d8305b9b42414cec6c4;p=gitmo%2FMoose.git This change gets Recipe 11 working. Here are the details ... When a role (RoleA) does some other role (RoleB) and explicitly aliases some method(s) from RoleB, those methods were always added to the required method list for any consumer of RoleA. However, if RoleA provides an implementation of those methods, they should not be required by a consumer. This fixes the recipe 11 tests. I also had to change some existing role application tests, which explicitly tested for the old behavior. Perigrin and I both agree that the old behavior makes no sense, since what else is the purpose of aliasing a method in a role? I also added some additional tests to make sure that when the aliasing role _does not_ implement the method, it _does_ get added to the required method list. --- diff --git a/Changes b/Changes index d9cf1ba..8c787a6 100644 --- a/Changes +++ b/Changes @@ -3,6 +3,15 @@ Revision history for Perl extension Moose 0.51 * Moose::Role - add unimport so "no Moose::Role" actually does something + * Moose::Meta::Role::Application::ToRole + - when RoleA did RoleB, and RoleA aliased a method from RoleB in + order to provide its own implementation, that method still got + added to the list of required methods for consumers of + RoleB. Now an aliased method is only added to the list of + required methods if the role doing the aliasing does not + provide its own implementation. See Recipe 11 for an example + of all this. (thanks Dave Rolsky) + - added tests for this 0.50 Thurs. Jun 11, 2008 - Fixed a version number issue by bumping all modules diff --git a/lib/Moose/Meta/Role/Application/ToRole.pm b/lib/Moose/Meta/Role/Application/ToRole.pm index 815d80e..c3e90d1 100644 --- a/lib/Moose/Meta/Role/Application/ToRole.pm +++ b/lib/Moose/Meta/Role/Application/ToRole.pm @@ -70,6 +70,27 @@ sub apply_methods { foreach my $method_name ($role1->get_method_list) { next if $self->is_method_excluded($method_name); + + if ($self->is_method_aliased($method_name)) { + my $aliased_method_name = $self->get_method_aliases->{$method_name}; + # it if it has one already + if ($role2->has_method($aliased_method_name) && + # and if they are not the same thing ... + $role2->get_method($aliased_method_name)->body != $role1->get_method($method_name)->body) { + confess "Cannot create a method alias if a local method of the same name exists"; + } + + $role2->alias_method( + $aliased_method_name, + $role1->get_method($method_name) + ); + + if (!$role2->has_method($method_name)) { + $role2->add_required_methods($method_name); + } + + next; + } # it if it has one already if ($role2->has_method($method_name) && @@ -88,19 +109,6 @@ sub apply_methods { } - if ($self->is_method_aliased($method_name)) { - my $aliased_method_name = $self->get_method_aliases->{$method_name}; - # it if it has one already - if ($role2->has_method($aliased_method_name) && - # and if they are not the same thing ... - $role2->get_method($aliased_method_name)->body != $role1->get_method($method_name)->body) { - confess "Cannot create a method alias if a local method of the same name exists"; - } - $role2->alias_method( - $aliased_method_name, - $role1->get_method($method_name) - ); - } } } diff --git a/t/000_recipes/011_advanced_role_composition.t b/t/000_recipes/011_advanced_role_composition.t index d207117..e2e4723 100644 --- a/t/000_recipes/011_advanced_role_composition.t +++ b/t/000_recipes/011_advanced_role_composition.t @@ -1,16 +1,16 @@ use strict; use warnings; -use Test::More skip_all => 'not working yet'; +use Test::More tests => 5; use Class::MOP; -# follow is original code +# This is copied directly from recipe 11 { package Restartable; use Moose::Role; has 'is_paused' => ( is => 'rw', - isa => 'Boo', + isa => 'Bool', default => 0, ); @@ -64,8 +64,8 @@ use Class::MOP; } } -# follow is test -do { +# This is the actual tests +{ my $unreliable = Moose::Meta::Class->create_anon_class( superclasses => [], roles => [qw/Restartable::ButUnreliable/], @@ -75,11 +75,11 @@ do { 'load_state' => sub { }, }, )->new_object(); - ok $unreliable, 'Restartable::ButUnreliable based class'; - can_ok $unreliable, qw/start stop/, '... can call start and stop'; -}; + ok $unreliable, 'made anon class with Restartable::ButUnreliable role'; + can_ok $unreliable, qw/start stop/; +} -do { +{ my $cnt = 0; my $broken = Moose::Meta::Class->create_anon_class( superclasses => [], @@ -90,9 +90,9 @@ do { 'load_state' => sub { }, }, )->new_object(); - ok $broken, 'Restartable::ButBroken based class'; + ok $broken, 'made anon class with Restartable::ButBroken role'; $broken->start(); - is $cnt, 1, '... start is exploded'; + is $cnt, 1, '... start called explode'; $broken->stop(); - is $cnt, 2, '... stop is also exploeded'; -}; + is $cnt, 2, '... stop also called explode'; +} diff --git a/t/030_roles/013_method_aliasing_in_composition.t b/t/030_roles/013_method_aliasing_in_composition.t index 42d3400..0405e13 100644 --- a/t/030_roles/013_method_aliasing_in_composition.t +++ b/t/030_roles/013_method_aliasing_in_composition.t @@ -3,7 +3,7 @@ use strict; use warnings; -use Test::More tests => 31; +use Test::More tests => 36; use Test::Exception; BEGIN { @@ -60,10 +60,22 @@ ok(My::Class->meta->has_method($_), "we have a $_ method") for qw(foo baz bar ro } ok(My::OtherRole->meta->has_method($_), "we have a $_ method") for qw(foo baz role_bar); -ok(My::OtherRole->meta->requires_method('bar'), '... and the &bar method is required'); +ok(!My::OtherRole->meta->requires_method('bar'), '... and the &bar method is not required'); ok(!My::OtherRole->meta->requires_method('role_bar'), '... and the &role_bar method is not required'); { + package My::AliasingRole; + use Moose::Role; + + ::lives_ok { + with 'My::Role' => { alias => { bar => 'role_bar' } }; + } '... this succeeds'; +} + +ok(My::AliasingRole->meta->has_method($_), "we have a $_ method") for qw(foo baz role_bar); +ok(My::AliasingRole->meta->requires_method('bar'), '... and the &bar method is required'); + +{ package Foo::Role; use Moose::Role;