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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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;