confound++

diff --git a/Changes b/Changes
index 7644888..b092c61 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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';
+}