Skip Menu |

This queue is for tickets about the Moose CPAN distribution.

Report information
The Basics
Id: 70322
Status: resolved
Priority: 0/
Queue: Moose

People
Owner: Nobody in particular
Requestors: rjsen [...] me.com
Cc:
AdminCc:

Bug Information
Severity: Important
Broken in:
  • 1.09
  • 2.0007
  • 2.0202
Fixed in: 2.0300-TRIAL



Subject: Subtypes of union types have invalid behavior with is_a_type_of and is_subtype_of
There's a bug in Moose::Meta::TypeConstraint that causes is_a_type_of and is_subtype_of to return false negatives for subtypes of union types. For example, if I create a subtype 'Ints', as 'Int|ArrayRef[Int]'; then an attribute has 'foo' => (is => 'ro', isa => 'Ints'); if I do a $self->meta->get_attrribute('foo')->type_constraint->is_a_type_of('Ref'); it returns false when it should return true. I'm fairly certain the problem is that is_subtype_of is doing a $parent->equals when it should be using $parent->is_a_type_of. I've attached a script that illustrates the problem as well as a proposed patch. I've seen this issue in Moose 1.09, 2.0007, and 2.0202 running on perl 5.10.0 on both Debian 5 and MacOS X 10.6.
Subject: TypeConstraint.patch
diff --git a/lib/Moose/Meta/TypeConstraint.pm b/lib/Moose/Meta/TypeConstraint.pm index f66888d..5e2e10b 100644 --- a/lib/Moose/Meta/TypeConstraint.pm +++ b/lib/Moose/Meta/TypeConstraint.pm @@ -288,7 +288,7 @@ sub is_subtype_of { my $current = $self; while (my $parent = $current->parent) { - return 1 if $parent->equals($type); + return 1 if $parent->is_a_type_of($type); $current = $parent; }
Subject: moosebug.pl
#!/usr/bin/perl # declare a custom subtype of a union package MooseBug::Type; use Moose::Util::TypeConstraints; subtype 'Ints', as 'Int|ArrayRef[Int]'; ################################################## # simple class package MooseBug::Class; use Moose; has 'foo' => (is => 'ro', isa => 'Int|ArrayRef[Int]', default => 1); has 'bar' => (is => 'ro', isa => 'Ints', default => 1); ################################################## package main; my $mb = MooseBug::Class->new(); my $meta = $mb->meta; foreach my $attr ($meta->get_all_attributes) { my $tc = $attr->type_constraint; # both foo and bar should be a type of Ref if ($tc->is_a_type_of('Ref')) { print $attr->name . " is a ref type\n"; } else { print $attr->name . " is NOT a ref type\n"; } if ($tc->has_parent) { print $attr->name . " has parent\n"; if ($tc->parent->is_a_type_of('Ref')) { print $attr->name . "'s PARENT IS a ref type\n"; } } }
CC: undisclosed-recipients: ;
Subject: Re: [rt.cpan.org #70322] Subtypes of union types have invalid behavior with is_a_type_of and is_subtype_of
Date: Wed, 17 Aug 2011 12:32:08 -0500
To: Ramesh Sen via RT <bug-Moose [...] rt.cpan.org>
From: Jesse Luehrs <doy [...] tozt.net>
On Wed, Aug 17, 2011 at 01:29:20PM -0400, Ramesh Sen via RT wrote: Show quoted text
> There's a bug in Moose::Meta::TypeConstraint that causes is_a_type_of and is_subtype_of to > return false negatives for subtypes of union types. For example, if I create a > subtype 'Ints', as 'Int|ArrayRef[Int]'; > then an attribute > has 'foo' => (is => 'ro', isa => 'Ints'); > if I do a > $self->meta->get_attrribute('foo')->type_constraint->is_a_type_of('Ref'); > it returns false when it should return true. > > I'm fairly certain the problem is that is_subtype_of is doing a $parent->equals when it should be > using $parent->is_a_type_of. I've attached a script that illustrates the problem as well as a > proposed patch.
Is this the same as #67731? -doy
From: rjsen [...] me.com
On Wed Aug 17 13:32:17 2011, doy@tozt.net wrote: Show quoted text
> On Wed, Aug 17, 2011 at 01:29:20PM -0400, Ramesh Sen via RT wrote:
> > There's a bug in Moose::Meta::TypeConstraint that causes
> is_a_type_of and is_subtype_of to
> > return false negatives for subtypes of union types. For example, if
> I create a
> > subtype 'Ints', as 'Int|ArrayRef[Int]'; > > then an attribute > > has 'foo' => (is => 'ro', isa => 'Ints'); > > if I do a > > $self->meta->get_attrribute('foo')->type_constraint- > >is_a_type_of('Ref'); > > it returns false when it should return true. > > > > I'm fairly certain the problem is that is_subtype_of is doing a
> $parent->equals when it should be
> > using $parent->is_a_type_of. I've attached a script that illustrates
> the problem as well as a
> > proposed patch.
> > Is this the same as #67731? > > -doy
I think that ticket is reporting the opposite issue - that union types report themselves as being types of things they shouldn't, while what I'm seeing is that union types aren't reporting themselves as being types of things they should. It's entirely possible they're related, though.
On Wed Aug 17 13:29:18 2011, rjsen wrote: Show quoted text
> There's a bug in Moose::Meta::TypeConstraint that causes is_a_type_of > and is_subtype_of to > return false negatives for subtypes of union types. For example, if I > create a > subtype 'Ints', as 'Int|ArrayRef[Int]'; > then an attribute > has 'foo' => (is => 'ro', isa => 'Ints'); > if I do a > $self->meta->get_attrribute('foo')->type_constraint-
> >is_a_type_of('Ref');
> it returns false when it should return true.
Actually, it _should_ return false, and the fact that it didn't was a bug. See RT 67731. I think the issue about using ->equals instead of ->is_subtype_of may be a bug too. I'll investigate further.
From: rjsen [...] me.com
On Fri Sep 16 09:53:33 2011, DROLSKY wrote: Show quoted text
> On Wed Aug 17 13:29:18 2011, rjsen wrote:
> > There's a bug in Moose::Meta::TypeConstraint that causes is_a_type_of > > and is_subtype_of to > > return false negatives for subtypes of union types. For example, if I > > create a > > subtype 'Ints', as 'Int|ArrayRef[Int]'; > > then an attribute > > has 'foo' => (is => 'ro', isa => 'Ints'); > > if I do a > > $self->meta->get_attrribute('foo')->type_constraint-
> > >is_a_type_of('Ref');
> > it returns false when it should return true.
> > Actually, it _should_ return false, and the fact that it didn't was a > bug. See RT 67731. > > I think the issue about using ->equals instead of ->is_subtype_of may be > a bug too. I'll investigate further.
Hmm, that seems to contradict the documentation in Moose::Meta::TypeConstraint::Union: Show quoted text
> $constraint->is_a_type_of($type_name_or_object) > This returns true if any of the member type constraints return true for the is_a_type_of
method. Since one of the member types (ArrayRef[Int]) correctly returns true for is_a_type_of('Ref'), a union with ArrayRef[Int] as a member should as well. Unless the documentation is wrong/out of date, of course.
Subject: Re: [rt.cpan.org #70322] Subtypes of union types have invalid behavior with is_a_type_of and is_subtype_of
Date: Fri, 16 Sep 2011 12:36:33 -0500 (CDT)
To: Ramesh Sen via RT <bug-Moose [...] rt.cpan.org>
From: Dave Rolsky <autarch [...] urth.org>
On Fri, 16 Sep 2011, Ramesh Sen via RT wrote: Show quoted text
>> Actually, it _should_ return false, and the fact that it didn't was a >> bug. See RT 67731. >> >> I think the issue about using ->equals instead of ->is_subtype_of may be >> a bug too. I'll investigate further.
> > Hmm, that seems to contradict the documentation in Moose::Meta::TypeConstraint::Union: >
>> $constraint->is_a_type_of($type_name_or_object) >> This returns true if any of the member type constraints return true for the is_a_type_of
> method. > > Since one of the member types (ArrayRef[Int]) correctly returns true for is_a_type_of('Ref'), a > union with ArrayRef[Int] as a member should as well. Unless the documentation is wrong/out > of date, of course.
Yes, the docs were documenting something that made no sense, so it's being changed in the next release ;) -dave /*============================================================ http://VegGuide.org http://blog.urth.org Your guide to all that's veg House Absolute(ly Pointless) ============================================================*/
From: rjsen [...] me.com
On Fri Sep 16 13:36:42 2011, autarch@urth.org wrote: Show quoted text
> On Fri, 16 Sep 2011, Ramesh Sen via RT wrote: >
> >> Actually, it _should_ return false, and the fact that it didn't was
> a
> >> bug. See RT 67731. > >> > >> I think the issue about using ->equals instead of ->is_subtype_of
> may be
> >> a bug too. I'll investigate further.
> > > > Hmm, that seems to contradict the documentation in
> Moose::Meta::TypeConstraint::Union:
> >
> >> $constraint->is_a_type_of($type_name_or_object) > >> This returns true if any of the member type constraints return true
> for the is_a_type_of
> > method. > > > > Since one of the member types (ArrayRef[Int]) correctly returns true
> for is_a_type_of('Ref'), a
> > union with ArrayRef[Int] as a member should as well. Unless the
> documentation is wrong/out
> > of date, of course.
> > Yes, the docs were documenting something that made no sense, so it's > being > changed in the next release ;) > > > -dave > >
/*======================================================== ==== Show quoted text
> http://VegGuide.org http://blog.urth.org > Your guide to all that's veg House Absolute(ly Pointless) >
========================================================= ===*/ OK, that makes sense. Is there any chance you could make both options available? E.g., have is_a_type_of follow the behavior described in RT 67731, but also have a has_member_with_type_of to support this behavior.
On Fri Sep 16 13:57:13 2011, rjsen wrote: Show quoted text
> On Fri Sep 16 13:36:42 2011, autarch@urth.org wrote:
> > On Fri, 16 Sep 2011, Ramesh Sen via RT wrote: > >
> > >> Actually, it _should_ return false, and the fact that it didn't
> was
> > a
> > >> bug. See RT 67731. > > >> > > >> I think the issue about using ->equals instead of ->is_subtype_of
> > may be
> > >> a bug too. I'll investigate further.
> > > > > > Hmm, that seems to contradict the documentation in
> > Moose::Meta::TypeConstraint::Union:
> > >
> > >> $constraint->is_a_type_of($type_name_or_object) > > >> This returns true if any of the member type constraints return
> true
> > for the is_a_type_of
> > > method. > > > > > > Since one of the member types (ArrayRef[Int]) correctly returns
> true
> > for is_a_type_of('Ref'), a
> > > union with ArrayRef[Int] as a member should as well. Unless the
> > documentation is wrong/out
> > > of date, of course.
> > > > Yes, the docs were documenting something that made no sense, so it's > > being > > changed in the next release ;) > > > > > > -dave > > > >
> /*======================================================== > ====
> > http://VegGuide.org http://blog.urth.org > > Your guide to all that's veg House Absolute(ly Pointless) > >
> ========================================================= > ===*/ > > > OK, that makes sense. Is there any chance you could make both options > available? E.g., have > is_a_type_of follow the behavior described in RT 67731, but also have > a > has_member_with_type_of to support this behavior.
Does this really needs its own method? You can look at the component type constraints and call ->is_a_type_of on them individually.