Avoid "Due to a method name conflict in roles..." errors

When composing into a role that chooses to define a
_get_default_configfile method, we would conflict with the existing
method already defined here.  Simply not defining one, and checking
if one exists with ->can, is nicer than forcing users to say
{ -excludes => '_get_default_configfile' }.

diff --git a/Changes b/Changes
index 0d5e5ad..0bc5f94 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,6 +1,7 @@
 Revision history for {{$dist->name}}
 
 {{$NEXT}}
+          - allow clean composition into a role, without requiring an -exclude
 
 0.09      2013-02-11 09:35:50 PST-0800 (Karen Etheridge)
           - removed prereqs which are only used for optional tests

diff --git a/dist.ini b/dist.ini
index 6edb886..1fcb4ad 100644 (file)
--- a/dist.ini
+++ b/dist.ini
@@ -28,7 +28,7 @@ version_regexp = ^v([\d._]+)(-TRIAL)?$
 
 [AutoPrereqs]
 skip = ^A$
-skip = ^Generic$
+skip = ^Generic
 skip = ^MooseX::SimpleConfig$
 skip = ^MooseX::Getopt$
 

diff --git a/lib/MooseX/ConfigFromFile.pm b/lib/MooseX/ConfigFromFile.pm
index 6214934..5fa8cac 100644 (file)
--- a/lib/MooseX/ConfigFromFile.pm
+++ b/lib/MooseX/ConfigFromFile.pm
@@ -9,11 +9,6 @@ use namespace::autoclean;
 
 requires 'get_config_from_file';
 
-# overridable in consuming class or role to provide a default value
-# This is called before instantiation, so it must be a class method,
-# and not depend on any other attributes
-sub _get_default_configfile { }
-
 has configfile => (
     is => 'ro',
     isa => Path|Undef,
@@ -23,7 +18,10 @@ has configfile => (
     lazy => 1,
     # it sucks that we have to do this rather than using a builder, but some old code
     # simply swaps in a new default sub into the attr definition
-    default => sub { shift->_get_default_configfile },
+    default => sub {
+        my $class = shift;
+        $class->_get_default_configfile if $class->can('_get_default_configfile');
+    },
 );
 
 sub new_with_config {
@@ -170,8 +168,8 @@ a hashref of arguments to pass to C<new()> which are sourced from the configfile
 
 =head2 _get_default_configfile
 
-This class method returns nothing by default, but can and should be redefined
-in a consuming class to return the default value of the configfile (if not
+This class method is not implemented in this role, but can and should be defined
+in a consuming class or role to return the default value of the configfile (if not
 passed into the constructor explicitly).
 
 =head1 COPYRIGHT

diff --git a/t/05_default_sub.t b/t/05_default_sub.t
index f5a44f9..b19be3d 100644 (file)
--- a/t/05_default_sub.t
+++ b/t/05_default_sub.t
@@ -1,7 +1,7 @@
 use strict;
 use warnings FATAL => 'all';
 
-use Test::More tests => 33;
+use Test::More tests => 49;
 use Test::Fatal;
 use Test::Deep '!blessed';
 use Test::NoWarnings 1.04 ':early';
@@ -51,6 +51,50 @@ is(
     'no exceptions',
 );
 
+{
+    package Base;
+    use Moose;
+}
+{
+    package GenericRole;
+    use Moose::Role;
+    with 'MooseX::ConfigFromFile';
+    sub get_config_from_file
+    {
+        my ($class, $file) = @_;
+        $loaded_file{$file}++;
+        +{}
+    }
+    around BUILDARGS => sub {
+        my ($orig, $class) = (shift, shift);
+        my $args = $class->$orig(@_);
+        $constructor_args{$class} = $args;
+    };
+    sub __my_configfile
+    {
+        my $class = blessed($_[0]) || $_[0];
+        $configfile_sub{$class}++;
+        $class . ' file'
+    }
+}
+
+is(
+    exception {
+        my $obj = Moose::Meta::Class->create_anon_class(
+            superclasses => ['Base'],
+            roles => ['GenericRole'],
+        )->name->new_with_config;
+        is($obj->configfile, undef, 'no configfile set');
+        cmp_deeply(\%loaded_file, {}, 'no files loaded');
+        cmp_deeply(
+            $constructor_args{blessed($obj)},
+            { },
+            'correct constructor args passed',
+        );
+    },
+    undef,
+    'no exceptions',
+);
 
 # this is a classic legacy usecase from old documentation that we must
 # continue to support
@@ -140,6 +184,28 @@ is(
     'no exceptions',
 );
 
+{
+    package OverriddenMethodAsRole;
+    use Moose::Role;
+    with 'GenericRole';
+    around configfile => sub { my $orig = shift; shift->__my_configfile };
+}
+
+is(
+    exception {
+        my $obj = Moose::Meta::Class->create_anon_class(
+            superclasses => ['Base'],
+            roles => ['OverriddenMethodAsRole'],
+        )->name->new_with_config;
+        is($obj->configfile, blessed($obj) . ' file', 'configfile set via overridden sub');
+        # this is not fixable - the reader method has been shadowed
+        # is($configfile_sub{blessed($obj)}, 1, 'configfile was calculated just once');
+        is($loaded_file{blessed($obj) . ' file'}, 1, 'correct file was loaded from');
+    },
+    undef,
+    'no exceptions',
+);
+
 
 # overridable method for configfile default, and configfile init_arg is changed
 {
@@ -169,6 +235,51 @@ is(
     'no exceptions',
 );
 
+{
+    package OverriddenMethodAndChangedNameAsRole;
+    use Moose::Role;
+    with 'GenericRole';
+    use MooseX::Types::Path::Tiny 'Path';
+    use MooseX::Types::Moose 'Undef';
+    use Try::Tiny;
+    has configfile => (
+        is => 'ro',
+        isa => Path|Undef,
+        coerce => 1,
+        predicate => 'has_configfile',
+        do { try { require MooseX::Getopt; (traits => ['Getopt']) } },
+        lazy => 1,
+        # it sucks that we have to do this rather than using a builder, but some old code
+        # simply swaps in a new default sub into the attr definition
+        default => sub { shift->_get_default_configfile },
+
+        # this is the overridden bit
+        init_arg => 'my_configfile',
+    );
+    around configfile => sub { my $orig = shift; shift->__my_configfile };
+}
+
+is(
+    exception {
+        my $obj = Moose::Meta::Class->create_anon_class(
+            superclasses => ['Base'],
+            roles => ['OverriddenMethodAndChangedNameAsRole'],
+        )->name->new_with_config;
+        is($obj->configfile, blessed($obj) . ' file', 'configfile set via overridden sub');
+        cmp_deeply(
+            $constructor_args{blessed($obj)},
+            { my_configfile => blessed($obj) . ' file' },
+            'correct constructor args passed',
+        );
+        # this is not fixable - the reader method has been shadowed
+        # is($configfile_sub{blessed($obj)}, 1, 'configfile was calculated just once');
+        is($loaded_file{blessed($obj) . ' file'}, 1, 'correct file was loaded from');
+    },
+    undef,
+    'no exceptions',
+);
+
+
 # newly-supported overridable method for configfile default
 {
     package NewSub;
@@ -193,6 +304,33 @@ is(
     'no exceptions',
 );
 
+{
+    package NewSubAsRole;
+    use Moose::Role;
+    with 'GenericRole';
+    sub _get_default_configfile { shift->__my_configfile }
+}
+
+is(
+    exception {
+        my $obj = Moose::Meta::Class->create_anon_class(
+            superclasses => ['Base'],
+            roles => ['NewSubAsRole'],
+        )->name->new_with_config;
+        is($obj->configfile, blessed($obj) . ' file', 'configfile set via new sub');
+        cmp_deeply(
+            $constructor_args{blessed($obj)},
+            {  configfile => blessed($obj) . ' file' },
+            'correct constructor args passed',
+        );
+        is($configfile_sub{blessed($obj)}, 1, 'configfile was calculated just once');
+        is($loaded_file{blessed($obj) . ' file'}, 1, 'correct file was loaded from');
+    },
+    undef,
+    'no exceptions',
+);
+
+
 # newly-supported overridable method for configfile default, and configfile
 # init_arg has been changed
 {