Stringify type constraint objects more consistently.

[tobyink]

MooseX::Storage::Engine needs to store type constraints as hash keys. In some places it does this using:

$hash{ $constraint }

And in others it uses:

$hash{ $constraint->name }

With a Moose::Meta::TypeConstraint object, the two have the same effect. However, for Type::Tiny and Specio, they can return different things.

In the case of Type::Tiny, anonymous type constraints are a lot more common than they are in Moose, so name often returns "ANON" even if the stringified type constraint ("" overload) is a more helpful string.

In the case of Specio::Constraint::Simple, there is no "" overload, so the stringification is something like "Specio::Constraint::Simple=HASH(0x12345678)", even if the name method returns something more helpful.

This pull request ensures that stringification is performed more consistently, always using object stringification to generate the hash keys, and not the name method (though the name method is still used in some other places, such as error messages). It adds test cases using MooseX::Storage with Specio and with Type::Tiny, though these are skipped (Test::Requires) if the relevant modules are not available.

Ultimately it might be a good idea to switch from using stringified type names as hash keys, and move to using a fieldhash, but this seems like a reasonable interim solution.

I've also bundled another fix for Specio, avoiding calling the is_subtype_of method on Specio type constraints (which don't have any such method), and use is_a_type_of instead.

[shadowcat-mst]

->name is expected to be unique, hence why MX::T type proxies stringify to that.

Type::Tiny should pick a different name for the method that provides a non-unique name if it's intended to be Moose compatible.

[rjbs]

I agree with @shadowcat-mst

[tobyink]

It's perfectly easy to create two different type constraints in Moose which return the same name via the name method.

use v5.14;
use Test::More;
use Moose::Util::TypeConstraints;

my $type1 = subtype as 'Str';
my $type2 = subtype as 'Int';

coerce $type1, from 'HashRef', via { join "|", %$_ };

note "Type constraints are different";
ok($type1->has_coercion);
ok(not $type2->has_coercion);

note "Names are the same";
is($type1->name, $type2->name);

done_testing;

The return value of the name method cannot be relied upon to be unique.

[perigrin]

Seems to me that is both a bug and undefined behavior as calling name on an anonymous type is ... silly.

-Chris

On Sun, Sep 29, 2013 at 8:13 PM, Toby Inkster [email protected]
wrote:

It's perfectly easy to create two different type constraints in Moose which return the same name via the name method.

use v5.14;
use Test::More;
use Moose::Util::TypeConstraints;
my $type1 = subtype as 'Str';
my $type2 = subtype as 'Str';
coerce $type1, from 'HashRef', via { join "|", %$_ };
note "Type constraints are different";
ok($type1->has_coercion);
ok(not $type2->has_coercion);
note "Names are the same";
is($type1->name, $type2->name);
done_testing;

The return value of the name method cannot be relied upon to be unique.

Reply to this email directly or view it on GitHub:
#1 (comment)

[tobyink]

It's not undefined. It's quite clearly documented in Moose::Meta::TypeConstraint, and has been since Moose-0.72_01.

[karenetheridge]

I think we're all in agreement that despite the M::M::TypeConstraint documentation saying that we use ANON for all names when one is not provided, we should use something unique instead (just as we do when creating anonymous classes).

However, I don't see how it's possible to properly serialize an anonymous type constraint. It still won't work, whether or not the name is unique, as the underlying coderef that actually defines the type's behaviour will be lost.

[tobyink]

This pull request has nothing to do with serializing type constraints or coderefs.

[karenetheridge]

As stated on irc -- IMO whether anon class names are unique or not is a red herring here (but we should fix it) -- what is more relevant is whether it's more correct to call ->name or to use the type's string serialization to perform the lookup in %TYPE. I think we should use ->name, and expect all type classes to support that.

[dagolden]

I'd go further and say that types should have a unique identifier, whether that is name or some new attribute.

For serialization, saying "do this when it has this type" we really mean this particular type not any type with the same name.

[tobyink]

Type::Tiny does have a unique identifier for each type (though it may differ between different runs of the same program), though it's not exposed by the public API.

[tobyink]

No movement on this PR for a while, so I just thought I'd bring it onto everybody's radars again.

I wrote this PR to solve a real issue that IIRC @dagolden was having with Meerkat. What can I do to move it along?

[karenetheridge]

What can I do to move it along?

I've lost track of what was discussed earlier that we wanted to have changed, and what things were just misunderstandings that have now been clarified.

So perhaps we should start with a rebased, squashed, and force-pushed update of the changes in this branch, and go back to see if there are still concerns/conflicts?