From: Dave Rolsky Date: Thu, 7 Aug 2008 16:27:33 +0000 (+0000) Subject: It turns out namespace::clean's different semantics break some code X-Git-Tag: 0_55_01~43^2~14 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=2c9c87972e8fc59e67cdf519795408a309a85ba3;p=gitmo%2FMoose.git It turns out namespace::clean's different semantics break some code (like MX::Singleton::Meta::Class) which expect "no Moose" to remove the keywords _right now_, as opposed to at the end of file scope. I adjusted the tests to account for this, and reverted back to the old manual method of removing keywords. --- diff --git a/Makefile.PL b/Makefile.PL index 740bc53..b54831a 100644 --- a/Makefile.PL +++ b/Makefile.PL @@ -13,7 +13,7 @@ my $win32 = !! ( $^O eq 'Win32' or $^O eq 'cygwin' ); requires 'Scalar::Util' => $win32 ? '1.17' : '1.18'; requires 'Carp'; requires 'Class::MOP' => '0.64'; -requires 'namespace::clean' => '0.08'; +requires 'List::MoreUtils'; requires 'Sub::Exporter' => '0.972'; # only used by oose.pm, not Moose.pm :P diff --git a/lib/Moose/Exporter.pm b/lib/Moose/Exporter.pm index d33a8bc..193d815 100644 --- a/lib/Moose/Exporter.pm +++ b/lib/Moose/Exporter.pm @@ -4,7 +4,6 @@ use strict; use warnings; use Class::MOP; -use namespace::clean 0.08 (); use List::MoreUtils qw( uniq ); use Sub::Exporter; @@ -206,17 +205,36 @@ sub _make_unimport_sub { my $class = shift; my $exported = shift; - # [12:24] yes. that's horrible. I know. but it should work. - # - # This will hopefully be replaced in the future once - # namespace::clean has an API for it. return sub { - @_ = ( 'namespace::clean', @{$exported} ); - - goto &namespace::clean::import; + my $caller = scalar caller(); + Moose::Exporter->_remove_keywords( $caller, $exported ); }; } +sub _remove_keywords { + shift; + my $package = shift; + my $keywords = shift; + + no strict 'refs'; + + # loop through the keywords ... + foreach my $name ( @{$keywords} ) { + + # if we find one ... + if ( defined &{ $package . '::' . $name } ) { + my $keyword = \&{ $package . '::' . $name }; + + # make sure it is from us + my ($pkg_name) = Class::MOP::get_code_info($keyword); + next if $pkg_name eq $package; + + # and if it is from us, then undef the slot + delete ${ $package . '::' }{$name}; + } + } +} + 1; __END__ diff --git a/t/050_metaclasses/012_moose_exporter.t b/t/050_metaclasses/012_moose_exporter.t index dc17b52..45c0770 100644 --- a/t/050_metaclasses/012_moose_exporter.t +++ b/t/050_metaclasses/012_moose_exporter.t @@ -6,33 +6,34 @@ use warnings; use Test::More tests => 38; use Test::Exception; -# All the BEGIN blocks are necessary to emulate the behavior of -# loading modules via use and the similar compile-time effect of "no -# ..." + { package MooseX::Empty; use Moose (); - BEGIN { Moose::Exporter->build_import_methods( also => 'Moose' ); } + Moose::Exporter->build_import_methods( also => 'Moose' ); } { package WantsMoose; - BEGIN { MooseX::Empty->import(); } + MooseX::Empty->import(); sub foo { 1 } - BEGIN { - ::can_ok( 'WantsMoose', 'has' ); - ::can_ok( 'WantsMoose', 'with' ); - ::can_ok( 'WantsMoose', 'foo' ); - } + ::can_ok( 'WantsMoose', 'has' ); + ::can_ok( 'WantsMoose', 'with' ); + ::can_ok( 'WantsMoose', 'foo' ); - BEGIN{ MooseX::Empty->unimport();} + MooseX::Empty->unimport(); } { + # Note: it's important that these methods be out of scope _now_, + # after unimport was called. We tried a + # namespace::clean(0.08)-based solution, but had to abandon it + # because it cleans the namespace _later_ (when the file scope + # ends). ok( ! WantsMoose->can('has'), 'WantsMoose::has() has been cleaned' ); ok( ! WantsMoose->can('with'), 'WantsMoose::with() has been cleaned' ); can_ok( 'WantsMoose', 'foo' ); @@ -53,31 +54,27 @@ use Test::Exception; return $caller . ' called wrapped1'; } - BEGIN { - Moose::Exporter->build_import_methods( - with_caller => ['wrapped1'], - also => 'Moose', - ); - } + Moose::Exporter->build_import_methods( + with_caller => ['wrapped1'], + also => 'Moose', + ); } { package WantsSugar; - BEGIN { MooseX::Sugar->import() } + MooseX::Sugar->import(); sub foo { 1 } - BEGIN { - ::can_ok( 'WantsSugar', 'has' ); - ::can_ok( 'WantsSugar', 'with' ); - ::can_ok( 'WantsSugar', 'wrapped1' ); - ::can_ok( 'WantsSugar', 'foo' ); - ::is( wrapped1(), 'WantsSugar called wrapped1', - 'wrapped1 identifies the caller correctly' ); - } + ::can_ok( 'WantsSugar', 'has' ); + ::can_ok( 'WantsSugar', 'with' ); + ::can_ok( 'WantsSugar', 'wrapped1' ); + ::can_ok( 'WantsSugar', 'foo' ); + ::is( wrapped1(), 'WantsSugar called wrapped1', + 'wrapped1 identifies the caller correctly' ); - BEGIN{ MooseX::Sugar->unimport();} + MooseX::Sugar->unimport(); } { @@ -101,38 +98,34 @@ use Test::Exception; return 'as_is1'; } - BEGIN { - Moose::Exporter->build_import_methods( - with_caller => ['wrapped2'], - as_is => ['as_is1'], - also => 'MooseX::Sugar', - ); - } + Moose::Exporter->build_import_methods( + with_caller => ['wrapped2'], + as_is => ['as_is1'], + also => 'MooseX::Sugar', + ); } { package WantsMoreSugar; - BEGIN { MooseX::MoreSugar->import() } + MooseX::MoreSugar->import(); sub foo { 1 } - BEGIN { - ::can_ok( 'WantsMoreSugar', 'has' ); - ::can_ok( 'WantsMoreSugar', 'with' ); - ::can_ok( 'WantsMoreSugar', 'wrapped1' ); - ::can_ok( 'WantsMoreSugar', 'wrapped2' ); - ::can_ok( 'WantsMoreSugar', 'as_is1' ); - ::can_ok( 'WantsMoreSugar', 'foo' ); - ::is( wrapped1(), 'WantsMoreSugar called wrapped1', - 'wrapped1 identifies the caller correctly' ); - ::is( wrapped2(), 'WantsMoreSugar called wrapped2', - 'wrapped2 identifies the caller correctly' ); - ::is( as_is1(), 'as_is1', - 'as_is1 works as expected' ); - } - - BEGIN{ MooseX::MoreSugar->unimport();} + ::can_ok( 'WantsMoreSugar', 'has' ); + ::can_ok( 'WantsMoreSugar', 'with' ); + ::can_ok( 'WantsMoreSugar', 'wrapped1' ); + ::can_ok( 'WantsMoreSugar', 'wrapped2' ); + ::can_ok( 'WantsMoreSugar', 'as_is1' ); + ::can_ok( 'WantsMoreSugar', 'foo' ); + ::is( wrapped1(), 'WantsMoreSugar called wrapped1', + 'wrapped1 identifies the caller correctly' ); + ::is( wrapped2(), 'WantsMoreSugar called wrapped2', + 'wrapped2 identifies the caller correctly' ); + ::is( as_is1(), 'as_is1', + 'as_is1 works as expected' ); + + MooseX::MoreSugar->unimport(); } { @@ -165,13 +158,13 @@ use Test::Exception; ); } - BEGIN { Moose::Exporter->build_import_methods( also => 'Moose' ); } + Moose::Exporter->build_import_methods( also => 'Moose' ); } { package NewMeta; - BEGIN { HasInitMeta->import() } + HasInitMeta->import(); } {