[Perl5-syntax] Adding Moose type checking to Method::Signatures

[Michael G Schwern]

On 2011.2.25 12:34 PM, Buddy Burden wrote:
>> But type syntax was easy to add, so I added it.
> 
> Ah, okay cool.  You didn't have to do all that, you know ... I was
> perfectly willing to send you patches for that stuff.  But, since you
> did ... thank you! :D

It put a bug up my ass and I'm glad to have someone else looking at the guts
of MS.


>> I'm wary of inject_extra() because it seems to encourage hacking on the code
>> MS has generated rather than just adding their own.  This is dangerous.
> 
> Well, potentially, yes.  I was just imagining that there might be some
> cases where you might want to replace code instead of adding to it ...
> for instance, one might want to replace the whole kit and kaboodle
> with a call to MXPV's validated_list or somesuch.  So I thought it
> might be nice to allow for that.  But if you don't like it, that's
> cool by me.  It won't impact anything I personally want to do,
> certainly.

I'd like to hold off on that until we have a real use case.  Because it seems
dangerous and because lacking a catch all it will drive us to do things better.

For example, I would like to see the joining of the code separated out.  And
the code generators for various pieces split out like inject_for_type().


>> Type checking on the invocant is what made MXMS the speed demon it is today. :P
> 
> Heh.  Yeah, I noticed your comment in the MXMS bug, but, really,
> there's got to be _something_ else going on, doesn't there?  The
> performance is _so_ much worse that I just can't conceive it's only
> because of that.

I think I did some profiling on it at some point.

Anyhow, runtime performance is important.


> But I personally agree with you _mostly_ that you usually don't want
> to type check the invocant.  The only thing it's nice for is making
> sure class methods aren't called as object methods, and vice-versa.
> So I typically declare my class methods as:
> 
>     method do_the_thing_with_the_thing (Str $class: Int :$other_param,
> Str :$etc)

Object methods being called as class methods: agreed, bad.

But I don't have a beef with class methods being called as object methods.
Class->class_method and $obj->class_method are functionally equivalent.  I
don't think denying that will catch real bugs compared to the effort.


>> You can type check it if you really want to.
>>
>>    sub inject_for_signature {
>>        my $self = shift;
>>        my $signature = shift;
>>        my $code = $self->SUPER::inject_for_signature($signature);
>>
>>        my $class = ref $self;
>>        return "${class}::type_check($signature->{invocant}, __PACKAGE__); ".$code;
>>    }
> 
> Ah, but, see, that doesn't work for my example above, because you
> haven't parsed out the type from the signature.

IMO the type of the invocant would always be inferred.  You should not be
allowed to explicitly set the type.  The type of the invocant of a class
method is not a string, but a class or subclass of the package in which the
method was declared.  That $invocant is a string and $invocant->isa(__PACKAGE__).

What's missing is the ability to differentiate between class and object methods.


> Also, forgive me if
> I'm misreading the code, but wouldn't that try to type check the
> invocant before it was declared?

Could be.  Extracting inject_for_invocant() would give you the control you need.


>> You can find that in the repo.  Let me know how it works out.  It's pretty
>> thinly tested.
>> https://github.com/schwern/method-signatures/commits/
> 
> It looks great to me, although I haven't had a chance to pull it down
> and start really hacking on it yet.  The only thing I would want to
> add is the parse_extra type of hook (if you have a better name, of
> course, I'm all for that), JIC I decide I want to try to implement
> does, or type coercions.  Which I would definitely not want to do
> right away, because I don't have any code that needs that, but I can
> see wanting it down the road.  So I could submit a patch for that if
> you're amenable.

I think what I'd like instead of parse_extra (same issues as inject_extra) is
this...

I agree with most of the MXMS syntax, just not with the implementation.  And
I'd like to bring the syntax of the various signature modules together.  So if
you want does and coercions and all that, patch them into MS.  I would
particularly like constraints (ie. "where").  This would improve MS for
everybody and MS doesn't have to worry about supporting 3rd party parser hacks.

I was considering also making MS more configurable about its features.  For
example, a lot of people don't like that it declares the methods at compile
time like a normal sub.  That could be turned off by passing a flag to
import() which would pass that along to the object.  I was thinking about
using it also for syntax... but I don't think I want to get into optional
syntax, just optional behaviors.

Another example is a flag to control how types are checked.  For example, MS
could ship with a purely class based type check on by default (fast and no
dependencies) with the option to turn on a Mouse/Moose based one.


> Thanx again for taking the time to do this for me.  I know you work on
> a lot of modules, so I'm sure your schedule is crazy busy.  I'm sure
> people don't say it enough, so allow me to: you're one of those folks
> without whom CPAN would be a much impoverished place.  So thanx for
> all you do. :-)

Thank you!  And yes, I'm crazy busy, so I'm happy to see you hacking.

If you tell me your Github ID I can add you as a collaborator.


-- 
s7ank: i want to be one of those guys that types "s/j&jd//.^$ueu*///djsls/sm."
       and it's a perl script that turns dog crap into gold.