better diagnostics when bad parameters given to has

diff --git a/Changes b/Changes
index 4a754ef..1d8d87a 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,3 +1,5 @@
+  - better diagnostics when bad parameters given to has
+
 1.003000 - 2013-07-15
   - fix composing roles that require methods provided by the other (RT#82711)
   - document optional use of Class::XSAccessor with caveats

diff --git a/lib/Moo.pm b/lib/Moo.pm
index 271a793..675998b 100644 (file)
--- a/lib/Moo.pm
+++ b/lib/Moo.pm
@@ -37,12 +37,18 @@ sub import {
     $class->_maybe_reset_handlemoose($target);
   };
   _install_tracked $target => has => sub {
-    my ($name_proto, %spec) = @_;
-    my $name_isref = ref $name_proto eq 'ARRAY';
-    foreach my $name ($name_isref ? @$name_proto : $name_proto) {
-      # Note that when $name_proto is an arrayref, each attribute
+    my $name_proto = shift;
+    my @name_proto = ref $name_proto eq 'ARRAY' ? @$name_proto : $name_proto;
+    if (@_ % 2 != 0) {
+      require Carp;
+      Carp::croak("Invalid options for " . join(', ', map "'$_'", @name_proto)
+        . " attribute(s): even number of arguments expected, got " . scalar @_)
+    }
+    my %spec = @_;
+    foreach my $name (@name_proto) {
+      # Note that when multiple attributes specified, each attribute
       # needs a separate \%specs hashref
-      my $spec_ref = $name_isref ? +{%spec} : \%spec;
+      my $spec_ref = @name_proto > 1 ? +{%spec} : \%spec;
       $class->_constructor_maker_for($target)
             ->register_attribute_specs($name, $spec_ref);
       $class->_accessor_maker_for($target)

diff --git a/lib/Moo/Role.pm b/lib/Moo/Role.pm
index 7b10f05..596f19b 100644 (file)
--- a/lib/Moo/Role.pm
+++ b/lib/Moo/Role.pm
@@ -29,10 +29,16 @@ sub import {
   # get symbol table reference
   my $stash = do { no strict 'refs'; \%{"${target}::"} };
   _install_tracked $target => has => sub {
-    my ($name_proto, %spec) = @_;
-    my $name_isref = ref $name_proto eq 'ARRAY';
-    foreach my $name ($name_isref ? @$name_proto : $name_proto) {
-      my $spec_ref = $name_isref ? +{%spec} : \%spec;
+    my $name_proto = shift;
+    my @name_proto = ref $name_proto eq 'ARRAY' ? @$name_proto : $name_proto;
+    if (@_ % 2 != 0) {
+      require Carp;
+      Carp::croak("Invalid options for " . join(', ', map "'$_'", @name_proto)
+        . " attribute(s): even number of arguments expected, got " . scalar @_)
+    }
+    my %spec = @_;
+    foreach my $name (@name_proto) {
+      my $spec_ref = @name_proto > 1 ? +{%spec} : \%spec;
       ($INFO{$target}{accessor_maker} ||= do {
         require Method::Generate::Accessor;
         Method::Generate::Accessor->new

diff --git a/t/has-array.t b/t/has-array.t
index 277c86e..173ca6b 100644 (file)
--- a/t/has-array.t
+++ b/t/has-array.t
@@ -27,4 +27,18 @@ can_ok(
   qw( attr1 attr2 attr3 attr4 ),
 );
 
+like(exception {
+  package Local::Test::Role2;
+  use Moo::Role;
+  has [qw/ attr1 attr2 /] => (is => 'ro', 'isa');
+}, qr/^Invalid options for 'attr1', 'attr2' attribute\(s\): even number of arguments expected, got 3/,
+  'correct exception when has given bad parameters in role');
+
+like(exception {
+  package Local::Test::Class2;
+  use Moo;
+  has [qw/ attr3 attr4 /] => (is => 'ro', 'isa');
+}, qr/^Invalid options for 'attr3', 'attr4' attribute\(s\): even number of arguments expected, got 3/,
+  'correct exception when has given bad parameters in class');
+
 done_testing;