Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
djerius opened this issue Jan 5, 2024 · 3 comments
Labels

Comments

@djerius
Copy link
Contributor

djerius commented Jan 5, 2024

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
Copy link
Contributor Author

djerius commented Jan 5, 2024

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;
}

@tobyink
Copy link
Owner

tobyink commented Sep 6, 2024

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
Copy link
Owner

tobyink commented Mar 14, 2025

In the next version, you'll be able to do:

package ABC;

use strict;
use warnings;
use experimental qw( builtin signatures );
use Data::Dumper;
use Types::Common qw( signature_for -types );

signature_for get_page => (
  named => [
    headers  => ArrayRef,
    uri      => Str,
    username => Optional[Str],
    password => Optional[Str],
  ],
  list_to_named => builtin::true,
);

sub get_page ( $args ) {
	print Dumper( $args );
}

package main;

ABC::get_page( headers => [ 'User-Agent' => 'Mozilla/1.0' ], uri => 'about:blank', username => 'bob', password => 'p4ssw0rd' );
ABC::get_page( [ 'User-Agent' => 'Mozilla/1.0' ], uri => 'about:blank', username => 'bob', password => 'p4ssw0rd' );
ABC::get_page( [ 'User-Agent' => 'Mozilla/1.0' ], 'about:blank', username => 'bob', password => 'p4ssw0rd' );
ABC::get_page( [ 'User-Agent' => 'Mozilla/1.0' ], 'about:blank' );
ABC::get_page( 'about:blank', [ 'User-Agent' => 'Mozilla/1.0' ] );
ABC::get_page( 'about:blank', [ 'User-Agent' => 'Mozilla/1.0' ], { username => 'bob', password => 'p4ssw0rd' } );

And they should all "just work". Basically when extracting named arguments from @_, any leading arguments which don't look like named arguments will be treated as arguments missing their name, and Type::Params will attempt to guess which argument the caller meant.

(Only for required parameters, though you can opt in to using the feature for optional parameters on a per-parameter basis.)

@tobyink tobyink added the bug label Apr 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants