Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Succinct MooseX::Declare method signature validation errors

Tags:

perl

moose

I've been a proponent of adopting Moose (and MooseX::Declare) at work for several months. The style it encourages will really help the maintainability of our codebase, but not without some initial cost of learning new syntax, and especially in learning how to parse type validation errors.

I've seen discussion online of this problem, and thought I'd post a query to this community for:

a) known solutions

b) discussion of what validation error messages should look like

c) propose a proof of concept that implements some ideas

I'll also contact the authors, but I've seen some good discussion this forum too, so I thought I'd post something public.

#!/usr/bin/perl

use MooseX::Declare;
class Foo {

    has 'x' => (isa => 'Int', is => 'ro');

    method doit( Int $id, Str :$z, Str :$y ) {
        print "doit called with id = " . $id . "\n";
        print "z = " . $z . "\n";
        print "y = " . $y . "\n";
    }

    method bar( ) {
        $self->doit(); # 2, z => 'hello', y => 'there' );
    }
}

my $foo = Foo->new( x => 4 );
$foo->bar();

Note the mismatch in the call to Foo::doit with the method's signature.

The error message that results is:

Validation failed for 'MooseX::Types::Structured::Tuple[MooseX::Types::Structured::Tuple[Object,Int],MooseX::Types::Structured::Dict[z,MooseX::Types::Structured::Optional[Str],y,MooseX::Types::Structured::Optional[Str]]]' failed with value [ [ Foo=HASH(0x2e02dd0) ], {  } ], Internal Validation Error is: Validation failed for 'MooseX::Types::Structured::Tuple[Object,Int]' failed with value [ Foo{ x: 4 } ] at /usr/local/share/perl/5.10.0/MooseX/Method/Signatures/Meta/Method.pm line 441
 MooseX::Method::Signatures::Meta::Method::validate('MooseX::Method::Signatures::Meta::Method=HASH(0x2ed9dd0)', 'ARRAY(0x2eb8b28)') called at /usr/local/share/perl/5.10.0/MooseX/Method/Signatures/Meta/Method.pm line 145
    Foo::doit('Foo=HASH(0x2e02dd0)') called at ./type_mismatch.pl line 15
    Foo::bar('Foo=HASH(0x2e02dd0)') called at ./type_mismatch.pl line 20

I think that most agree that this is not as direct as it could be. I've implemented a hack in my local copy of MooseX::Method::Signatures::Meta::Method that yields this output for the same program:

Validation failed for

   '[[Object,Int],Dict[z,Optional[Str],y,Optional[Str]]]' failed with value [ [ Foo=HASH(0x1c97d48) ], {  } ]

Internal Validation Error:

   '[Object,Int]' failed with value [ Foo{ x: 4 } ]

Caller: ./type_mismatch.pl line 15 (package Foo, subroutine Foo::doit)

The super-hacky code that does this is

    if (defined (my $msg = $self->type_constraint->validate($args, \$coerced))) {
        if( $msg =~ /MooseX::Types::Structured::/ ) {
            $msg =~ s/MooseX::Types::Structured:://g;
            $msg =~ s/,.Internal/\n\nInternal/;
            $msg =~ s/failed.for./failed for\n\n   /g;
            $msg =~ s/Tuple//g;
            $msg =~ s/ is: Validation failed for/:/;
        }
        my ($pkg, $filename, $lineno, $subroutine) = caller(1);
        $msg .= "\n\nCaller: $filename line $lineno (package $pkg, subroutine $subroutine)\n";
        die $msg;
    }

[Note: With a few more minutes of crawling the code, it looks like MooseX::Meta::TypeConstraint::Structured::validate is a little closer to the code that should be changed. In any case, the question about the ideal error message, and whether anyone is actively working on or thinking about similar changes stands.]

Which accomplishes 3 things:

1) Less verbose, more whitespace (I debated including s/Tuple//, but am sticking with it for now)

2) Including calling file/line (with brittle use of caller(1))

3) die instead of confess -- since as I see it the main advantage of confess was finding the user's entry point into the typechecking anyway, which we can achieve in less verbose ways

Of course I don't actually want to support this patch. My question is: What is the best way of balancing completeness and succinctness of these error messages, and are there any current plans to put something like this in place?

like image 231
Adam Pingel Avatar asked Dec 02 '10 23:12

Adam Pingel


2 Answers

I'm glad you like MooseX::Declare. However, the method signature validation errors you're talking about aren't really from there, but from MooseX::Method::Signatures, which in turn uses MooseX::Types::Structured for its validation needs. Every validation error you currently see comes unmodified from MooseX::Types::Structured.

I'm also going to ignore the stack-trace part of the error message. I happen to find them incredibly useful, and so does the rest of Moose cabal. I'm not going to removed them by default.

If you want a way to turn them off, Moose needs to be changed to throw exception objects instead of strings for type-constraint validation errors and possibly other things. Those could always capture a backtrace, but the decision on whether or not to display it, or how exactly to format it when displaying, could be made elsewhere, and the user would be free to modify the default behaviour - globally, locally, lexically, whatever.

What I'm going to address is building the actual validation error messages for method signatures.

As pointed out, MooseX::Types::Structured does the actual validation work. When something fails to validate, it's its job to raise an exception. This exception currently happens to be a string, so it's not all that useful when wanting to build beautiful errors, so that needs to change, similar to the issue with stack traces above.

Once MooseX::Types::Structured throws structured exception objects, which might look somewhat like

bless({
    type => Tuple[Tuple[Object,Int],Dict[z,Optional[Str],y,Optional[Str]]],
    err  => [
        0 => bless({
            type => Tuple[Object,Int],
            err  => [
                0 => undef,
                1 => bless({
                    type => Int,
                    err  => bless({}, 'ValidationError::MissingValue'),
                }, 'ValidationError'),
            ],
        }, 'ValidationError::Tuple'),
        1 => undef,
    ],
}, 'ValidationError::Tuple')

we would have enough information available to actually correlate individual inner validation errors with parts of the signature in MooseX::Method::Signatures. In the above example, and given your (Int $id, Str :$z, Str :$y) signature, it'd be easy enough to know that the very inner Validation::MissingValue for the second element of the tuple for positional parameters was supposed to provide a value for $id, but couldn't.

Given that, it'll be easy to generate errors such as

http://files.perldition.org/err1.png

or

http://files.perldition.org/err2.png

which is kind of what I'm going for, instead of just formatting the horrible messages we have right now more nicely. However, if one wanted to do that, it'd still be easy enough once we have structured validation exceptions instead of plain strings.

None of this is actually hard - it just needs doing. If anyone feels like helping out with this, come talk to us in #moose on irc.perl.org.

like image 154
rafl Avatar answered Oct 26 '22 08:10

rafl


Method::Signatures::Modifiers is a package which hopes to fix some of the problems of MooseX::Method::Signatures. Simply use it to overload.

use MooseX::Declare;
use Method::Signatures::Modifiers;

class Foo
{
    method bar (Int $thing) {
        # this method is declared with Method::Signatures instead of MooseX::Method::Signatures
    }
}
like image 44
Joel Berger Avatar answered Oct 26 '22 09:10

Joel Berger