Bug #74834 for MooseX-Types: Combined type libraries do not support import({into => ...})

MooseX::Types::Combined is inconsistent with the individual type libraries in that it does not support `->import({into => $package}, ...)`. Use case: I was building a `use Custom::Moose` module for a big project at $work. At first I had $_->import({into => $caller}, ":all") for @types; which worked. Later I added a few custom types and decided to use MooseX::Types::Combine to make them all available from one spot. That module worked fine, but when I changed the Custom::Moose import call to Custom::Moose::Types->import({into => $caller}, ":all"); my tests for Custom::Moose stopped compiling and it took me a lot of head-scratching and source diving to figure out why. I don't think this patch breaks anything because if you pass a hash ref to import now you get an error like: MultiExporter::Into asked for a type (HASH(0x2c9aab0)) which is not found in any of the type libraries (TestLibrary TestLibrary2) combined by Combined. Patch is also available as a git branch if desired.

diff --git a/lib/MooseX/Types/Combine.pm b/lib/MooseX/Types/Combine.pm index 43aa79f..9862870 100644 --- a/lib/MooseX/Types/Combine.pm +++ b/lib/MooseX/Types/Combine.pm @@ -31,10 +31,18 @@ sub import { my ($class, @types) = @_; my $caller = caller; + # Allow {into => ...} to be specified by the caller. + # Below we pass references to shallow clones to avoid side-effects. + my %options = (@types and (ref($types[0]) eq 'HASH')) + ? %{ shift(@types) } : (); + + # ('-into' ||= caller) because there's no reason to import into __PACKAGE__ + $options{ -into } ||= $options{into} || $caller; + my %types = $class->_provided_types; if ( grep { $_ eq ':all' } @types ) { - $_->import( { -into => $caller }, q{:all} ) + $_->import( { %options }, q{:all} ) for $class->provide_types_from; return; } @@ -52,7 +60,7 @@ sub import { push @{ $from{ $types{$type} } }, $type; } - $_->import({ -into => $caller }, @{ $from{ $_ } }) + $_->import({ %options }, @{ $from{ $_ } }) for keys %from; } diff --git a/t/18_combined_libs.t b/t/18_combined_libs.t index 976d924..322e2e9 100644 --- a/t/18_combined_libs.t +++ b/t/18_combined_libs.t @@ -38,4 +38,59 @@ qr/\Qmain asked for a type (NonExistentType) which is not found in any of the ty is exception { 'Combined'->import(':all') }, undef, ':all syntax works'; +# Test that passing {into => $package} works +# like it does when importing individual type libs. +# Explicitly test both 'into' and '-into' to confirm that the aliasing +# in MooseX::Types::Base doesn't get swallowed by Sub::Exporter +# (which happened during development). + +{ + package MultiExporter::Into; + sub import { + Combined->import({ into => scalar caller}, @_[1 .. $#_]); + } +} + +{ + package MultiExporter::DashInto; + sub import { + Combined->import({-into => scalar caller}, @_[1 .. $#_]); + } +} + +# use string evals around barewords to protect from compilation failure +{ + package NoImport; + + ::like ::exception { eval 'Foo2Alias'; die $@ if $@ }, + qr/Bareword.+\bFoo2Alias\b.+not allowed/, + 'sanity check - bareword not imported'; +} + +foreach my $into ( qw( Into DashInto ) ){ + eval <<IMPORTINTO; + { + package ImportOne::${into}; + MultiExporter::${into}->import('Foo2Alias'); + ::ok eval 'Foo2Alias', 'imported by name'; + } + + { + package ImportAll::${into}; + MultiExporter::${into}->import(':all'); + ::ok eval 'Foo2Alias', 'all imported from TestLibrary with $into'; + ::ok eval 'MTFNPY', 'all imported from TestLibrary2 with $into'; + } +IMPORTINTO + die $@ if $@; +} + +# test that passing a hashref w/o 'into' still imports to the correct package +{ + package ImportAll::NoInto; + Combined->import({}, ':all'); + ::ok eval 'Foo2Alias', 'all imported from TestLibrary with no into'; + ::ok eval 'MTFNPY', 'all imported from TestLibrary2 with no into'; +} + done_testing();