From: Shawn M Moore Date: Tue, 21 Apr 2009 07:15:13 +0000 (-0400) Subject: Moose now warns about class implicitly overriding methods from local X-Git-Tag: 0.75_01~9 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=d443cad0cbcc03efe89556a96ff231822b59125d;p=gitmo%2FMoose.git Moose now warns about class implicitly overriding methods from local roles --- diff --git a/lib/Moose/Meta/Role/Application/ToClass.pm b/lib/Moose/Meta/Role/Application/ToClass.pm index 8b67325..5036d80 100644 --- a/lib/Moose/Meta/Role/Application/ToClass.pm +++ b/lib/Moose/Meta/Role/Application/ToClass.pm @@ -98,13 +98,15 @@ sub apply_attributes { sub apply_methods { my ($self, $role, $class) = @_; + my @implicitly_overridden; + foreach my $method_name ($role->get_method_list) { - unless ($self->is_method_excluded($method_name)) { # it if it has one already if ($class->has_method($method_name) && # and if they are not the same thing ... $class->get_method($method_name)->body != $role->get_method($method_name)->body) { + push @implicitly_overridden, $method_name; next; } else { @@ -130,6 +132,14 @@ sub apply_methods { ); } } + + if (@implicitly_overridden) { + my $plural = @implicitly_overridden > 1 ? "s" : ""; + # we use \n because we have no hope of guessing the right stack frame, + # it's almost certainly never going to be the one above us + warn "The " . $class->name . " class has implicitly overridden the method$plural (" . join(', ', @implicitly_overridden) . ") from role " . $role->name . ". If this is intentional, please exclude the method$plural from composition to silence this warning (see Moose::Cookbook::Roles::Recipe2)\n"; + } + # we must reset the cache here since # we are just aliasing methods, otherwise # the modifiers go wonky. diff --git a/t/030_roles/003_apply_role.t b/t/030_roles/003_apply_role.t index 1ccff67..fded07f 100644 --- a/t/030_roles/003_apply_role.t +++ b/t/030_roles/003_apply_role.t @@ -3,8 +3,9 @@ use strict; use warnings; -use Test::More tests => 86; +use Test::More tests => 87; use Test::Exception; +use Test::Output; { package FooRole; @@ -43,7 +44,10 @@ use Test::Exception; use Moose; extends 'BarClass'; - with 'FooRole'; + + ::stderr_like { + with 'FooRole'; + } qr/The FooClass class has implicitly overridden the method \(goo\) from role FooRole\. If this is intentional, please exclude the method from composition to silence this warning \(see Moose::Cookbook::Roles::Recipe2\)/; sub blau {'FooClass::blau'} # << the role wraps this ... diff --git a/t/030_roles/005_role_conflict_detection.t b/t/030_roles/005_role_conflict_detection.t index b99b90f..a9e4f87 100644 --- a/t/030_roles/005_role_conflict_detection.t +++ b/t/030_roles/005_role_conflict_detection.t @@ -3,8 +3,9 @@ use strict; use warnings; -use Test::More tests => 87; # it's really 124 with kolibrie's tests; +use Test::More tests => 89; use Test::Exception; +use Test::Output; =pod @@ -107,7 +108,11 @@ Role method conflicts ::lives_ok { with 'Role::Bling'; - with 'Role::Bling::Bling'; + + ::stderr_like { + with 'Role::Bling::Bling'; + } qr/The My::Test4 class has implicitly overridden the method \(bling\) from role Role::Bling::Bling\./; + } '... role methods didnt conflict when manually combined'; package My::Test5; @@ -115,7 +120,10 @@ Role method conflicts ::lives_ok { with 'Role::Bling::Bling'; - with 'Role::Bling'; + + ::stderr_like { + with 'Role::Bling'; + } qr/The My::Test5 class has implicitly overridden the method \(bling\) from role Role::Bling\./; } '... role methods didnt conflict when manually combined (in opposite order)'; package My::Test6; diff --git a/t/030_roles/011_overriding.t b/t/030_roles/011_overriding.t index 75afe7a..d4c24eb 100644 --- a/t/030_roles/011_overriding.t +++ b/t/030_roles/011_overriding.t @@ -3,8 +3,9 @@ use strict; use warnings; -use Test::More tests => 39; +use Test::More tests => 44; use Test::Exception; +use Test::Output; @@ -34,7 +35,9 @@ use Test::Exception; use Moose; ::lives_ok { - with qw(Role::C); + ::stderr_like { + with qw(Role::C); + } qr/The Class::A class has implicitly overridden the method \(zot\) from role Role::C\./; } "define class A"; sub zot { 'Class::A::zot' } @@ -70,7 +73,9 @@ is( Class::A->new->xxy, "Role::B::xxy", "... got the right xxy method" ); use Moose; ::lives_ok { - with 'Role::A::Conflict'; + ::stderr_like { + with 'Role::A::Conflict'; + } qr/The Class::A::Resolved class has implicitly overridden the method \(bar\) from role Role::A::Conflict\./; } '... did fufill the requirement of &bar method'; sub bar { 'Class::A::Resolved::bar' } @@ -112,7 +117,9 @@ is( Class::A::Resolved->new->bar, 'Class::A::Resolved::bar', "... got the right use Moose; ::lives_ok { - with qw(Role::F); + ::stderr_like { + with qw(Role::F); + } qr/The Class::B class has implicitly overridden the method \(zot\) from role Role::F\./; } "define class Class::B"; sub zot { 'Class::B::zot' } @@ -189,7 +196,9 @@ ok(Role::D::And::E::Conflict->meta->requires_method('bar'), '... Role::D::And::E use Moose; ::lives_ok { - with qw(Role::I); + ::stderr_like { + with qw(Role::I); + } qr/The Class::E class has implicitly overridden the method \(zot\) from role Role::I\./; } "resolved with method"; sub foo { 'Class::E::foo' } @@ -214,7 +223,9 @@ ok(Role::I->meta->requires_method('foo'), '... Role::I still have the &foo requi sub zot { 'Class::D::zot' } - with qw(Role::I); + ::stderr_like { + with qw(Role::I); + } qr/The Class::D class has implicitly overridden the method \(zot\) from role Role::I\./; } "resolved with attr"; diff --git a/t/030_roles/019_build.t b/t/030_roles/019_build.t index fc4bb0e..ce0bce5 100644 --- a/t/030_roles/019_build.t +++ b/t/030_roles/019_build.t @@ -1,7 +1,7 @@ #!/usr/bin/env perl use strict; use warnings; -use Test::More tests => 6; +use Test::More tests => 8; # this test script ensures that my idiom of: # role: sub BUILD, after BUILD @@ -28,6 +28,14 @@ do { }; do { + package ExplicitClassWithBUILD; + use Moose; + with 'TestRole' => { excludes => 'BUILD' }; + + sub BUILD { push @CALLS, 'ExplicitClassWithBUILD::BUILD' } +}; + +do { package ClassWithoutBUILD; use Moose; with 'TestRole'; @@ -44,6 +52,14 @@ do { 'TestRole::BUILD:after', ]); + ExplicitClassWithBUILD->new; + + is_deeply([splice @CALLS], [ + 'TestRole::BUILD:before', + 'ExplicitClassWithBUILD::BUILD', + 'TestRole::BUILD:after', + ]); + ClassWithoutBUILD->new; is_deeply([splice @CALLS], [ @@ -54,6 +70,7 @@ do { if (ClassWithBUILD->meta->is_mutable) { ClassWithBUILD->meta->make_immutable; + ExplicitClassWithBUILD->meta->make_immutable; ClassWithoutBUILD->meta->make_immutable; redo; } diff --git a/t/200_examples/001_example.t b/t/200_examples/001_example.t index 473da12..3488cf0 100644 --- a/t/200_examples/001_example.t +++ b/t/200_examples/001_example.t @@ -3,8 +3,9 @@ use strict; use warnings; -use Test::More tests => 20; +use Test::More tests => 22; use Test::Exception; +use Test::Output; @@ -52,7 +53,9 @@ use Test::Exception; package Constraint::AtLeast; use Moose; - with 'Constraint'; + ::stderr_is { + with 'Constraint' => { excludes => 'error_message' }; + } ""; sub validate { my ($self, $field) = @_; @@ -64,7 +67,9 @@ use Test::Exception; package Constraint::NoMoreThan; use Moose; - with 'Constraint'; + ::stderr_is { + with 'Constraint' => { excludes => 'error_message' }; + } ''; sub validate { my ($self, $field) = @_; diff --git a/t/600_todo_tests/002_various_role_shit.t b/t/600_todo_tests/002_various_role_shit.t index 7a44274..96719ab 100644 --- a/t/600_todo_tests/002_various_role_shit.t +++ b/t/600_todo_tests/002_various_role_shit.t @@ -3,8 +3,9 @@ use strict; use warnings; -use Test::More tests => 39; +use Test::More tests => 40; use Test::Exception; +use Test::Output; sub req_or_has ($$) { my ( $role, $method ) = @_; @@ -99,7 +100,9 @@ sub req_or_has ($$) { package Foo; use Moose; - with qw(Bar); + ::stderr_like { + with qw(Bar); + } qr/The Foo class has implicitly overridden the method \(xxy\) from role Bar\./; has oink => ( is => "rw", @@ -135,6 +138,7 @@ sub req_or_has ($$) { { local our $TODO = "attrs and methods from a role should clash"; + local $SIG{__WARN__} = sub { 'Ignore!' }; ::dies_ok { with qw(Tree Dog) } } }