From: gfx <gfuji@cpan.org>
Date: Mon, 22 Feb 2010 08:28:25 +0000 (+0900)
Subject: Resolve a todo: if you set both 'isa' and 'does' on an attribute, the 'isa' must... 
X-Git-Tag: 0.50_03~2
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=d503a4f3aa9eda772309f6c99ccd4dfcdfed059d;p=gitmo%2FMouse.git

Resolve a todo: if you set both 'isa' and 'does' on an attribute, the 'isa' must do 'does'
---

diff --git a/Changes b/Changes
index 48a9853..343a4cc 100644
--- a/Changes
+++ b/Changes
@@ -1,6 +1,11 @@
 Revision history for Mouse
 
 0.50_03
+    * Mouse::Meta::Attribute
+        - Catch up about Moose 0.84 about warnings (gfx)
+          - If an attribute generates no accessors, it will be warned
+          - If both 'isa' and 'does' are specified and 'isa' does not do
+            'does', then it will be warned
     * Mouse::Object
         - Fix a possible segv which is caused by destructors (gfx)
     * Mouse::Util::TypeConstraints
diff --git a/lib/Mouse/Meta/Attribute.pm b/lib/Mouse/Meta/Attribute.pm
index b4511d0..eb3613f 100644
--- a/lib/Mouse/Meta/Attribute.pm
+++ b/lib/Mouse/Meta/Attribute.pm
@@ -335,6 +335,12 @@ sub install_accessors{
         $attribute->create($metaclass, $attribute->name, %{$attribute});
     }
 
+    if(!$attribute->{associated_methods} && ($attribute->{is} || '') ne 'bare'){
+        Carp::cluck(
+            'Attribute (' . $attribute->name . ') of class ' . $metaclass->name
+            . ' has no associated methods (did you mean to provide an "is" argument?)');
+    }
+
     return;
 }
 
diff --git a/lib/Mouse/PurePerl.pm b/lib/Mouse/PurePerl.pm
index a09d606..2aa712e 100644
--- a/lib/Mouse/PurePerl.pm
+++ b/lib/Mouse/PurePerl.pm
@@ -447,12 +447,23 @@ sub _process_options{
 
     my $tc;
     if(exists $args->{isa}){
-        $args->{type_constraint} = Mouse::Util::TypeConstraints::find_or_create_isa_type_constraint($args->{isa});
+        $tc = $args->{type_constraint} = Mouse::Util::TypeConstraints::find_or_create_isa_type_constraint($args->{isa});
     }
-    elsif(exists $args->{does}){
-        $args->{type_constraint} = Mouse::Util::TypeConstraints::find_or_create_does_type_constraint($args->{does});
+
+    if(exists $args->{does}){
+        if(defined $tc){ # both isa and does supplied
+            my $does_ok = do{
+                local $@;
+                eval{ "$tc"->does($args) };
+            };
+            if(!$does_ok){
+                $class->throw_error("Cannot have both an isa option and a does option because '$tc' does not do '$args->{does}' on attribute ($name)");
+            }
+        }
+        else {
+            $tc = $args->{type_constraint} = Mouse::Util::TypeConstraints::find_or_create_does_type_constraint($args->{does});
+        }
     }
-    $tc = $args->{type_constraint};
 
     if($args->{coerce}){
         defined($tc)
diff --git a/t/020_attributes/005_attribute_does.t b/t/020_attributes/005_attribute_does.t
index a895bdb..267f98d 100644
--- a/t/020_attributes/005_attribute_does.t
+++ b/t/020_attributes/005_attribute_does.t
@@ -28,7 +28,7 @@ use Test::Exception;
     # if isa and does appear together, then see if Class->does(Role)
     # if it does work... then the does() check is actually not needed
     # since the isa() check will imply the does() check
-    has 'foo' => (is => 'rw', isa => 'Foo::Class', does => 'Foo::Role');
+    has 'foo' => (is => 'rw', isa => 'Foo::Class');
 
     package Foo::Class;
     use Mouse;
@@ -77,12 +77,10 @@ lives_ok {
     use Test::More;
     use Mouse;
 
-    local $TODO = 'setting both isa and does';
-
     # if isa and does appear together, then see if Class->does(Role)
     # if it does not,.. we have a conflict... so we die loudly
     ::dies_ok {
-        has 'foo' => (isa => 'Foo::Class', does => 'Bar::Class');
+        has 'foo' => (is => 'rw', isa => 'Foo::Class', does => 'Bar::Class');
     } '... cannot have a does() which is not done by the isa()';
 }
 
@@ -97,12 +95,10 @@ lives_ok {
     use Test::More;
     use Mouse;
 
-    local $TODO = 'setting both isa and does';
-
     # if isa and does appear together, then see if Class->does(Role)
     # if it does not,.. we have a conflict... so we die loudly
     ::dies_ok {
-        has 'foo' => (isa => 'Bling', does => 'Bar::Class');
+        has 'foo' => (is => 'rw', isa => 'Bling', does => 'Bar::Class');
     } '... cannot have a isa() which is cannot does()';
 }
 
diff --git a/xs-src/MouseAttribute.xs b/xs-src/MouseAttribute.xs
index 7b25881..f8012da 100644
--- a/xs-src/MouseAttribute.xs
+++ b/xs-src/MouseAttribute.xs
@@ -184,6 +184,43 @@ mouse_xa_set_default(pTHX_ AV* const xa, SV* const object) {
     return value;
 }
 
+static void
+mouse_check_isa_does_does(pTHX_ SV* const klass, SV* const name, SV* const isa, SV* const does){
+    STRLEN len;
+    const char* const pv = SvPV_const(isa, len); /* need strigify */
+    bool does_ok;
+    dSP;
+
+    ENTER;
+    SAVETMPS;
+
+    SAVESPTR(ERRSV);
+    ERRSV = sv_newmortal();
+
+    PUSHMARK(SP);
+    EXTEND(SP, 2);
+    mPUSHp(pv, len);
+    PUSHs(does);
+    PUTBACK;
+
+    call_method("does", G_EVAL | G_SCALAR);
+
+    SPAGAIN;
+    does_ok = sv_true(POPs);
+    PUTBACK;
+
+    FREETMPS;
+    LEAVE;
+
+    if(!does_ok){
+        mouse_throw_error(klass, NULL,
+            "Cannot have both an isa option and a does option"
+            "because '%"SVf"' does not do '%"SVf"' on attribute (%"SVf")",
+            isa, does, name
+        );
+    }
+}
+
 MODULE = Mouse::Meta::Attribute  PACKAGE = Mouse::Meta::Attribute
 
 PROTOTYPES: DISABLE
@@ -327,17 +364,25 @@ CODE:
         tc = newSVsv(POPs);
         PUTBACK;
     }
-    else if((svp = hv_fetchs(args, "does", FALSE))){
-        SPAGAIN;
-        PUSHMARK(SP);
-        XPUSHs(*svp);
-        PUTBACK;
 
-        call_pv("Mouse::Util::TypeConstraints::find_or_create_does_type_constraint",
-            G_SCALAR);
-        SPAGAIN;
-        tc = newSVsv(POPs);
-        PUTBACK;
+    if((svp = hv_fetchs(args, "does", FALSE))){
+        /* check 'isa' does 'does' */
+        if(tc){
+            mouse_check_isa_does_does(aTHX_ klass, name, tc, *svp);
+            /* nothing to do */
+        }
+        else{
+            SPAGAIN;
+            PUSHMARK(SP);
+            XPUSHs(*svp);
+            PUTBACK;
+
+            call_pv("Mouse::Util::TypeConstraints::find_or_create_does_type_constraint",
+                G_SCALAR);
+            SPAGAIN;
+            tc = newSVsv(POPs);
+            PUTBACK;
+        }
     }
     if(tc){
         (void)hv_stores(args, "type_constraint", tc);