Cpanel::JSON::XS::true and Moose don't play well together

choroba has asked for the wisdom of the Perl Monks concerning the following question:

At work, we've been bitten by a weird bug. It could be reduced to the following short code:

#!/usr/bin/perl
use warnings;
use strict;
use Cpanel::JSON::XS;

{
    package My::Package;
    use Moose;
    has attr => ( is => 'ro', isa => 'Bool' );

    __PACKAGE__->meta->make_immutable;
}

my $obj = 'My::Package'->new(attr => Cpanel::JSON::XS::true());
[download]

which fails with

Attribute (attr) does not pass the type constraint because: Validation
+ failed for 'Bool' with value JSON::PP::Boolean=SCALAR(0x1dc1558) at 
+constructor My::Package::new (defined at 1.pl line 15) line 31
[download]

(The false() function works correctly.)

The problem seems to be reported as the #61 on Cpanel::JSON::XS's GitHub. But I'm not sure the real problem lies in the JSON module. Does Moose play right?

If we look at the relevant code, we can see (in Moose/Util/TypeConstraints/Builtins.pm):

    subtype 'Bool'
        => as 'Item'
        => where { !defined($_) || $_ eq "" || "$_" eq '1' || "$_" eq 
+'0' }
        => inline_as {
            '('
                . '!defined(' . $_[1] . ') '
                . '|| ' . $_[1] . ' eq "" '
                . '|| (' . $_[1] . '."") eq "1" '
                . '|| (' . $_[1] . '."") eq "0"'
            . ')'
        };
[download]

The problem is that the JSON booleans overload both stringification and string equality (from Cpanel/JSON/XS.pm):

  &overload::import( 'overload', # workaround 5.6 reserved keyword war
+ning
    "0+"     => sub { ${$_[0]} },
    "++"     => sub { $_[0] = ${$_[0]} + 1 },
    "--"     => sub { $_[0] = ${$_[0]} - 1 },
    '""'     => sub { ${$_[0]} == 1 ? 'true' : '0' }, # GH 29
    'eq'     => sub {
      my ($obj, $op) = ref ($_[0]) ? ($_[0], $_[1]) : ($_[1], $_[0]);
      if ($op eq 'true' or $op eq 'false') {
        return "$obj" eq 'true' ? 'true' eq $op : 'false' eq $op;
      }
      else {
        return $obj ? 1 == $op : 0 == $op;
      }
    },
    fallback => 1);
[download]

But Moose first stringifies the object before calling eq, i.e. it bypasses any eq overloading on a Bool object. The json true stringifies to "true" which isn't equal to "1", but

Cpanel::JSON::XS::true() eq '1';
[download]

returns true because of the overloaded eq.

So, whom should I report the bug to?

Update: Yes, I know I can coerce the Moose boolean from the JSON boolean:

{
    use Moose::Util::TypeConstraints;
    my $class = ref Cpanel::JSON::XS::true();
    class_type($class);
    coerce Bool => from $class => via { !! $_ };
}

#...

    has attr => ( is => 'ro', isa => 'Bool', coerce => 1 );
[download]

But wasn't the whole point of the overloading in the JSON library to avoid such things and make it possible to use it directly with the correct behaviour based on the context?

Update 2: Similar problem was reported to Moose as a bug, but was rejected.

($q=q:Sq=~/;[c](.)(.)/;chr(-||-|5+lengthSq)`"S|oS2"`map{chr |+ord
}map{substrSq`S_+|`|}3E|-|`7**2-3:)=~y+S|`+$1,++print+eval$q,q,a,
[download]

Comment on Cpanel::JSON::XS::true and Moose don't play well together Select or Download Code

Replies are listed 'Best First'.
Re: Cpanel::JSON::XS::true and Moose don't play well together by tobyink (Canon) on Jul 07, 2017 at 15:40 UTC
None of the Moose built-in types treat overloaded objects as the non-object types they overload. So for example, an object overloading `""` will not pass the `Str` type constraint. And objects overloading `0+` will not pass `Num` or `Int`. Whether you think this is a good design decision or not, it's the way Moose works, and changing it now would likely break a lot of stuff that relies on the existing behaviour. Luckily, you don't need to use the built-in Moose `Bool` type. You can define your own type constraints. Moose::Util::TypeConstraints makes this possible. MooseX::Types makes it a bit easier. But (and I'm biased on this!), I'd recommend using Types::Standard which is a type constraint library that works with Moose, Mouse, and Moo, but doesn't require any of them. Here's one way you could achieve your aims with it: `{ package My::Package; use Moose; use Types::Standard qw( Bool Overload ); has attr => ( is => 'ro', isa => Bool \| Overload["bool"], ); __PACKAGE__->meta->make_immutable; }` [download] Or even: `{ package My::Package; use Moose; use Types::Standard qw( Bool Overload ); has attr => ( is => 'ro', isa => Bool->plus_coercions( Overload["bool"], sub { !!$_ +} ), coerce => 1, ); __PACKAGE__->meta->make_immutable; }` [download]	[reply] [d/l] [select]

Replies are listed 'Best First'.

Re: Cpanel::JSON::XS::true and Moose don't play well together
by tobyink (Canon) on Jul 07, 2017 at 15:40 UTC

None of the Moose built-in types treat overloaded objects as the non-object types they overload.

So for example, an object overloading "" will not pass the Str type constraint. And objects overloading 0+ will not pass Num or Int.

Whether you think this is a good design decision or not, it's the way Moose works, and changing it now would likely break a lot of stuff that relies on the existing behaviour.

Luckily, you don't need to use the built-in Moose Bool type. You can define your own type constraints.

Moose::Util::TypeConstraints makes this possible. MooseX::Types makes it a bit easier. But (and I'm biased on this!), I'd recommend using Types::Standard which is a type constraint library that works with Moose, Mouse, and Moo, but doesn't require any of them. Here's one way you could achieve your aims with it:

{
    package My::Package;
    use Moose;
    use Types::Standard qw( Bool Overload );

    has attr => (
        is     => 'ro',
        isa    => Bool | Overload["bool"],
    );

    __PACKAGE__->meta->make_immutable;
}
[download]

Or even:

{
    package My::Package;
    use Moose;
    use Types::Standard qw( Bool Overload );

    has attr => (
        is     => 'ro',
        isa    => Bool->plus_coercions( Overload["bool"], sub { !!$_  
+} ),
        coerce => 1,
    );

    __PACKAGE__->meta->make_immutable;
}
[download]

[reply]
[d/l]
[select]