p5-type-tiny icon indicating copy to clipboard operation
p5-type-tiny copied to clipboard

Type::Params: unexpected error when die'ing in goto_next in a multiple signature

Open djerius opened this issue 1 year ago • 2 comments

Type::Tiny 2.004000

I'm trying to handle a legacy API issue by using multiple signatures. The old API is

foo( $hashref, %options )

i.e., first is positional, rest are named.

The new API is

foo( head => $hashref, %options );

i.e., all are named.

In order to transparently convert from the first to the second, I'm doing this:

sub goto_next {
    $_[1]{head} = $_[0];
    return $_[1];
}

signature_for foo => (
    multiple => [ {
            # legacy
            goto_next => \&goto_next,
            head      => [Str],
            named     => [ uri => Optional [Any], head => Optional [Any] ],
        },
        {
            # new
            named => [ head => Str, uri => Optional [Any] ]
        },
    ],
);
sub foo { p @_ }

I need to provide the head parameter to the named options for the legacy API otherwise the constructed parameter class doesn't have a head attribute.

This however leaves open the possibility that this call:

foo( $head_value, head => $another_head_value );

could arise if someone tweaked %options to include head and forgot to fix the actual call.

So I figured I could catch this via:

sub goto_next {
    die "legacy API: do not supply a named 'head' parameter"
      if $_[1]->has_head;
    $_[1]{head} = $_[0];
    return $_[1];
}

But this call

foo( 'foo', head => undef );

results in a rather unexpected error:

$ perl boom.pl
Use of uninitialized value in pattern match (m//) at ../5.36/lib/perl5/site_perl/5.36.1/Error/TypeTiny.pm line 61.
Use of uninitialized value in pattern match (m//) at ../5.36/lib/perl5/site_perl/5.36.1/Error/TypeTiny.pm line 61.
Parameter validation failed at file? line NaN.

While debugging, I reverted to a similar approach with a single signature, i.e.

signature_for foo => (
    goto_next => \&goto_next
    head  => [Str],
    named => [ uri => Optional [Any], head => Optional [Any] ],
);

With the result that the call

foo( 'foo', head => undef );

results in the expected outcome:

$ perl tst.pl
legacy API: do not supply a named 'head' parameter at tst.pl line 9.

djerius avatar Jan 05 '24 21:01 djerius

BTW, this is rather low priority, as there's a simple (rather less convoluted) workaround:

sub foo (@args){
    state $signature = signature( named => [ head => Str, uri => Optional [Any] ] );

    # support legacy API: foo( $head, %options )
    if ( @args %2 ) {
        my $head = shift @args;
        my %arg = @args;
        die "legacy API: do not supply a named 'head' parameter"
          if exists $arg{head};
        $arg{head} = $head;
        @args = %arg;
    }
    my $opt = $signature->( @args );

    p $opt;
}

djerius avatar Jan 05 '24 22:01 djerius

If the old API is this:

foo( $hashref, %options )

Then this part seems wrong:

        {
            # legacy
            goto_next => \&goto_next,
            head      => [Str],   # <-- Why `Str`? Shouldn't it be `HashRef`?
            named     => [ uri => Optional [Any], head => Optional [Any] ],
        },

If changed to HashRef everything seems fine. I agree the error messages are an issue though.

# This works

package ABC;

use strict;
use warnings;
use Data::Dumper;
use Types::Common qw( signature_for signature -types );

signature_for foo => (
   multiple => [
		{
			# legacy
			goto_next => sub { $_[1]{head} = $_[0]; return $_[1]; },
			head      => [ HashRef ],
			named     => [ uri => Optional[Any], head => Optional[Any] ],
		},
		{
			# new
			named => [ head => HashRef, uri => Optional[Any] ]
		},
	],
);

sub foo {
	print Dumper @_;
}

package main;

ABC::foo( { xyz => 1 }, uri => 'about:blank' );
ABC::foo( head => { xyz => 1 }, uri => 'about:blank' );

tobyink avatar Sep 06 '24 21:09 tobyink