Skip Menu |
 

This queue is for tickets about the Moose CPAN distribution.

Report information
The Basics
Id: 123299
Status: open
Priority: 0/
Queue: Moose

People
Owner: Nobody in particular
Requestors: jon.rubin [...] grantstreet.com
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)



Subject: Inline Type Constraints don't accept stringifiable messages
Date: Mon, 16 Oct 2017 12:31:43 -0400
To: bug-Moose [...] rt.cpan.org
From: Jon Rubin <jon.rubin [...] grantstreet.com>
Download (untitled) / with headers
text/plain 3.2k
Inlining time constraints causes Moose to throw the wrong error when the *message* returns an object with a stringification overload. This was not happening on Moose 2.0402, and has been happening at least for Moose 2.1604 and the current version (2.2006). I am using perl 5.22.2 on a Centos 6 box: Linux pexdev002-dev3.grantstreet.com 2.6.32-696.10.3.el6.x86_64 #1 SMP Tue Sep 26 18:14:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Proposed fix: Simply allow things with stringification overloads to count as strings. package Moose::Exception::ValidationFailedForInlineTypeConstraint; ... # Not sure of the correct way to get Str in Moose world +use MooseX::Types::Moose qw(Str); has 'type_constraint_message' => ( is => 'ro', - isa => 'Str', + isa => Str | Moose::Util::TypeConstraints->duck_type([qw< ("" >]), required => 1 ); Alternative Proposed fix: This ensures that you have a string in the end, so may be safer for backwards-compatibility. But it obscures what the original object was in reporting errors ('successful exception' instead of '123' in the reproduction case below), and encourages coercing to the Str builtin, which you discourage. package Moose::Exception::ValidationFailedForInlineTypeConstraint; ... has 'type_constraint_message' => ( is => 'ro', isa => 'Str', + coerce => 1, required => 1 ); Reproduction Case: # Old Moose (2.0402) # $ perl /var/tmp/moose_and_magic_strings.pl # Attribute (validation_fail) does not pass the type constraint because: successful exception at constructor Bad::Moose::new (defined at /var/tmp/moose_and_magic_strings.pl line 28) line 31. # Bad::Moose::new('Bad::Moose', 'validation_fail', 123) called at /var/tmp/moose_and_magic_strings.pl line 32 # # New Moose (2.1604) # $ perl /var/tmp/moose_and_magic_strings.pl # Attribute (type_constraint_message) does not pass the type constraint because: Validation failed for 'Str' with value String::Object=HASH(0x2e2da48) at /usr/local/perl/active/lib/perl5/x86_64-linux/Moose/Object.pm line 24 # Moose::Object::new('Moose::Exception::ValidationFailedForInlineTypeConstraint', 'type_constraint_message', 'String::Object=HASH(0x2e2da48)', 'class_name', 'Bad::Moose', 'attribute_name', 'validation_fail', 'value', 123) called at constructor Bad::Moose::new (defined at /var/tmp/moose_and_magic_strings.pl line 28) line 31 # Bad::Moose::new('Bad::Moose', 'validation_fail', 123) called at /var/tmp/moose_and_magic_strings.pl line 32 package String::Object; use overload '""' => \&as_string, fallback => 1; sub new { my $self = bless {}, __PACKAGE__; } sub as_string { 'successful exception' } package Bad::Moose::Classes; use Moose; use MooseX::Types::Moose qw( Str ); use MooseX::Types -declare => [ 'AlwaysFail', 'StringObject' ]; subtype AlwaysFail, as Str, where { 0 }, message { String::Object->new }; class_type StringObject, { class => 'String::Object' }; coerce Str, from StringObject, via { 'coerced' }; package Bad::Moose; use Moose; has validation_fail => ( is => 'ro', isa => 'AlwaysFail', ); __PACKAGE__->meta->make_immutable; # This is the key line package main; Bad::Moose->new( validation_fail => '123' ); -- Jon Rubin Grant Street Group Ph: (412) 391-5555, Ext. 1323
Download (untitled) / with headers
text/plain 108b
This definitely looks like a real bug. If you wanted to submit a PR with a test case that'd be very helpful.
Download (untitled) / with headers
text/plain 297b
I'd also note that this is a consequence of Moose's types not accepting objects which overload the operation of the type (stringification, numification, etc.). I think this is a mistake in the Moose type system, and it is fairly easy to fix (we can copy the code out of Specio that handles this).
Download (untitled) / with headers
text/plain 484b
On 2017-11-26 20:28:53, DROLSKY wrote: Show quoted text
> I'd also note that this is a consequence of Moose's types not > accepting objects which overload the operation of the type > (stringification, numification, etc.). > > I think this is a mistake in the Moose type system, and it is fairly > easy to fix (we can copy the code out of Specio that handles this).
Code will break if we change this behaviour. Otherwise, we would have cored the constraints from MooseX::Stypes::Stringable long ago.
Subject: Re: [rt.cpan.org #123299] Inline Type Constraints don't accept stringifiable messages
Date: Tue, 23 Jan 2018 14:50:50 -0500
To: bug-Moose [...] rt.cpan.org
From: Jon Rubin <jon.rubin [...] grantstreet.com>
Download (untitled) / with headers
text/plain 1.1k
Show quoted text
> This definitely looks like a real bug. If you wanted to submit a PR with
a test case that'd be very helpful. I'm not sure what the solution *should* be, and I don't think I know enough about Moose's history to make a well-informed decision. As alluded to above, it appears that duck-typing overloaded strings might break things - but it's not clear to me if allowing them for errors' type_constraint_message would be a problem. On Mon, Nov 27, 2017 at 1:24 AM, Karen Etheridge via RT < bug-Moose@rt.cpan.org> wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=123299 > > > On 2017-11-26 20:28:53, DROLSKY wrote:
> > I'd also note that this is a consequence of Moose's types not > > accepting objects which overload the operation of the type > > (stringification, numification, etc.). > > > > I think this is a mistake in the Moose type system, and it is fairly > > easy to fix (we can copy the code out of Specio that handles this).
> > > Code will break if we change this behaviour. Otherwise, we would have > cored the constraints from MooseX::Stypes::Stringable long ago. >
-- Jon Rubin Grant Street Group Ph: (412) 391-5555, Ext. 1323
Download (untitled) / with headers
text/plain 704b
On 2018-01-23 13:59:40, jon.rubin@grantstreet.com wrote: Show quoted text
> > This definitely looks like a real bug. If you wanted to submit a PR with
> a test case that'd be very helpful. > > I'm not sure what the solution *should* be, and I don't think I know enough > about Moose's history to make a well-informed decision. As alluded to > above, it appears that duck-typing overloaded strings might break things - > but it's not clear to me if allowing them for errors' > type_constraint_message would be a problem.
Well, we could accept stringifiable objects and then turn them into strings immediately instead of storing the objects. That seems like it would partially solve the problem without causing breakage.
Subject: Re: [rt.cpan.org #123299] Inline Type Constraints don't accept stringifiable messages
Date: Tue, 23 Jan 2018 15:40:02 -0500
To: bug-Moose [...] rt.cpan.org
From: Jon Rubin <jon.rubin [...] grantstreet.com>
Okay, I can do that. Is there any particular PR format you're looking for (eg. directly in git?) On Tue, Jan 23, 2018 at 3:33 PM, Dave Rolsky via RT <bug-Moose@rt.cpan.org> wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=123299 > > > On 2018-01-23 13:59:40, jon.rubin@grantstreet.com wrote:
> > > This definitely looks like a real bug. If you wanted to submit a PR
> with
> > a test case that'd be very helpful. > > > > I'm not sure what the solution *should* be, and I don't think I know
> enough
> > about Moose's history to make a well-informed decision. As alluded to > > above, it appears that duck-typing overloaded strings might break things
> -
> > but it's not clear to me if allowing them for errors' > > type_constraint_message would be a problem.
> > Well, we could accept stringifiable objects and then turn them into > strings immediately instead of storing the objects. > > That seems like it would partially solve the problem without causing > breakage. >
-- Jon Rubin Grant Street Group Ph: (412) 391-5555, Ext. 1323
Download (untitled) / with headers
text/plain 344b
On 2018-01-23 14:42:29, jon.rubin@grantstreet.com wrote: Show quoted text
> Okay, I can do that. Is there any particular PR format you're looking for > (eg. directly in git?)
Yes, git PRs are preferred, but I'd like to wait for ether to weigh in on my suggestion first. There's no point working on this if there's some obvious problem with it that she notices.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.