Fix for xslate#88

Making sure that applying a role in runtime to an instance properly checks for the existence of the role's required attributes in that instance.
sergeykolychev · Jul 29, 2018 · 6dc4c93 · 6dc4c93
1 parent 25ebc09
commit 6dc4c93
Show file tree

Hide file tree

Showing 5 changed files with 70 additions and 14 deletions.
diff --git a/lib/Mouse/Meta/Method/Constructor.pm b/lib/Mouse/Meta/Method/Constructor.pm
@@ -90,9 +90,9 @@ sub _generate_initialize_object {
 
         # build cde for an attribute
         if (defined $init_arg) {
-            my $value = "\$args->{q{$init_arg}}";
+            my $value = "\$is_applying_role ? \$args->{q{$key}} : \$args->{q{$init_arg}}";
 
-            $code .= "if (exists $value) {\n";
+            $code .= "if (exists \$args->{q{$init_arg}} or (\$is_applying_role and exists \$args->{q{$key}})) {\n";
 
             if($need_coercion){
                 $value = "$constraint_var->coerce($value)";
@@ -142,7 +142,7 @@ sub _generate_initialize_object {
         }
         elsif ($attr->is_required) {
             $code .= "\$meta->throw_error('Attribute ($key) is required')";
-            $code .= "    unless \$is_cloning;\n";
+            $code .= "    if (not \$is_cloning or \$is_applying_role);\n";
         }
 
         $code .= "}\n" if defined $init_arg;
@@ -173,7 +173,7 @@ sub _generate_initialize_object {
 #line 1 "%s"
     package %s;
     sub {
-        my($meta, $instance, $args, $is_cloning) = @_;
+        my($meta, $instance, $args, $is_cloning, $is_applying_role) = @_;
         %s;
         return $instance;
     }

diff --git a/lib/Mouse/Meta/Role/Application.pm b/lib/Mouse/Meta/Role/Application.pm
@@ -81,7 +81,7 @@ sub apply {
     if(defined $instance){ # Application::ToInstance
         # rebless instance
         bless $instance, $consumer->name;
-        $consumer->_initialize_object($instance, $instance, 1);
+        $consumer->_initialize_object($instance, $instance, 1, 1);
     }
 
     return;

diff --git a/lib/Mouse/PurePerl.pm b/lib/Mouse/PurePerl.pm
@@ -314,12 +314,12 @@ sub clone_object {
         || $class->throw_error("You must pass an instance of the metaclass (" . $class->name . "), not ($object)");
 
     my $cloned = bless { %$object }, ref $object;
-    $class->_initialize_object($cloned, $args, 1);
+    $class->_initialize_object($cloned, $args, 1, 0);
     return $cloned;
 }
 
 sub _initialize_object{
-    my($self, $object, $args, $is_cloning) = @_;
+    my($self, $object, $args, $is_cloning, $is_applying_role) = @_;
     # The initializer, which is used everywhere, must be clear
     # when an attribute is added. See Mouse::Meta::Class::add_attribute.
     my $initializer = $self->{_mouse_cache}{_initialize_object} ||=

diff --git a/t/900_mouse_bugs/021_issue88.t b/t/900_mouse_bugs/021_issue88.t
@@ -0,0 +1,53 @@
+#!perl
+use strict;
+use Test::More tests => 4;
+use Test::Exception;
+
+{
+    package Role;
+    use Mouse::Role;
+    has 'name' =>
+        (isa => 'Str', is => 'ro', required => 1);
+
+}
+
+{
+    package RoleInitArg;
+    use Mouse::Role;
+    has 'name' =>
+        (isa => 'Str', init_arg => 'Name', is => 'ro', required => 1);
+
+}
+
+{
+    package User;
+    use Mouse;
+    has 'name' =>
+        (isa => 'Str', is => 'ro');
+
+    sub BUILD
+    {
+        my $self = shift;
+        Mouse::Util::apply_all_roles($self, 'Role')
+    }
+}
+
+{
+    package UserInitArg;
+    use Mouse;
+    has 'name' =>
+        (isa => 'Str', init_arg => 'Name', is => 'ro');
+
+    sub BUILD
+    {
+        my $self = shift;
+        Mouse::Util::apply_all_roles($self, 'RoleInitArg')
+    }
+}
+
+package main;
+
+lives_ok { User->new(name => 'Tim') }, 'lives with plain attribute';
+lives_ok { UserInitArg->new(Name => 'Tim') }, 'lives with init_arg';
+dies_ok  { User->new() }, 'dies without plain attribute';
+dies_ok  { UserInitArg->new() }, 'dies without init_arg';
diff --git a/xs-src/Mouse.xs b/xs-src/Mouse.xs
@@ -282,7 +282,7 @@ mouse_report_unknown_args(pTHX_ SV* const meta, AV* const attrs, HV* const args)
 
 
 static void
-mouse_class_initialize_object(pTHX_ SV* const meta, SV* const object, HV* const args, bool const is_cloning) {
+mouse_class_initialize_object(pTHX_ SV* const meta, SV* const object, HV* const args, bool const is_cloning, bool const is_applying_role) {
     AV* const xc    = mouse_get_xc(aTHX_ meta);
     AV* const attrs = MOUSE_xc_attrall(xc);
     I32 const len   = AvFILLp(attrs) + 1;
@@ -308,7 +308,7 @@ mouse_class_initialize_object(pTHX_ SV* const meta, SV* const object, HV* const
         SV* const init_arg = MOUSE_xa_init_arg(xa);
         HE* he;
 
-        if(SvOK(init_arg) && ( he = hv_fetch_ent(args, init_arg, FALSE, 0U) ) ){
+        if(SvOK(init_arg) && ((he = hv_fetch_ent(args, init_arg, FALSE, 0U)) || (is_applying_role && (he = hv_fetch_ent(args, slot, FALSE, 0U)))) ){
             SV* value = HeVAL(he);
             if(flags & MOUSEf_ATTR_HAS_TC){
                 value = mouse_xa_apply_type_constraint(aTHX_ xa, value, flags);
@@ -340,6 +340,9 @@ mouse_class_initialize_object(pTHX_ SV* const meta, SV* const object, HV* const
                 if(flags & MOUSEf_ATTR_IS_WEAK_REF){
                     weaken_slot(object, slot);
                 }
+                if(is_applying_role && flags & MOUSEf_ATTR_IS_REQUIRED){
+                    mouse_throw_error(attr, NULL, "Attribute (%"SVf") is required", slot);
+                }
             }
             /* don't check "required" while cloning (or reblesseing) */
             else if(flags & MOUSEf_ATTR_IS_REQUIRED) {
@@ -589,7 +592,7 @@ CODE:
     SV* object;
 
     object = mouse_instance_create(aTHX_ MOUSE_xc_stash(xc));
-    mouse_class_initialize_object(aTHX_ meta, object, args, FALSE);
+    mouse_class_initialize_object(aTHX_ meta, object, args, FALSE, FALSE);
     mouse_buildall(aTHX_ xc, object, sv_2mortal(newRV_inc((SV*)args)));
     ST(0) = object; /* because object is mortal, we should return it as is */
     XSRETURN(1);
@@ -610,16 +613,16 @@ CODE:
     }
 
     proto = mouse_instance_clone(aTHX_ object);
-    mouse_class_initialize_object(aTHX_ meta, proto, args, TRUE);
+    mouse_class_initialize_object(aTHX_ meta, proto, args, TRUE, FALSE);
     ST(0) = proto; /* because object is mortal, we should return it as is */
     XSRETURN(1);
 }
 
 void
-_initialize_object(SV* meta, SV* object, HV* args, bool is_cloning = FALSE)
+_initialize_object(SV* meta, SV* object, HV* args, bool is_cloning = FALSE, bool is_applying_role = FALSE)
 CODE:
 {
-    mouse_class_initialize_object(aTHX_ meta, object, args, is_cloning);
+    mouse_class_initialize_object(aTHX_ meta, object, args, is_cloning, is_applying_role);
 }
 
 void
@@ -730,7 +733,7 @@ CODE:
 
     /* new_object */
     object = mouse_instance_create(aTHX_ MOUSE_xc_stash(xc));
-    mouse_class_initialize_object(aTHX_ meta, object, (HV*)SvRV(args), FALSE);
+    mouse_class_initialize_object(aTHX_ meta, object, (HV*)SvRV(args), FALSE, FALSE);
     /* BUILDALL */
     mouse_buildall(aTHX_ xc, object, args);
     ST(0) = object; /* because object is mortal, we should return it as is */