andreas1234567 has asked for the wisdom of the Perl Monks concerning the following question:

In my first attempt to use Params::Validate I wrote callback instead of callbacks:
use Params::Validate qw(:all); use Test::More; plan tests => 4; sub howareyoufeelingtoday { validate_pos( @_, { callback => { 'valids' => sub { shift =~ qr/^(happy|sad|mental)$/ } } } ); return 1; } ok(howareyoufeelingtoday(q{happy}), q{happy}); ok(howareyoufeelingtoday(q{sad}), q{sad}); ok(howareyoufeelingtoday(q{mental}), q{mental}); ok(howareyoufeelingtoday(q{unhappy}), q{unhappy}); # Should not pass
Running it shows that the fourth test unexpectedly passes (because the checks are not executed):
$ perl pv.pl 1..4 ok 1 - happy ok 2 - sad ok 3 - mental ok 4 - unhappy
I went along to try to add a safety net to catch future typing errors, and naively tried to make changes to Validate.xs but it seems to break the other tests in all kinds of ways.
diff --git a/lib/Params/Validate.xs b/lib/Params/Validate.xs index 396a369..3f3f0f4 100644 --- a/lib/Params/Validate.xs +++ b/lib/Params/Validate.xs @@ -508,10 +508,12 @@ static IV validate_one_param(SV* value, SV* params, HV* spec, SV* id, HV* optio +ns, IV* untaint) { SV** temp; IV i; + IV found = 0; /* check type */ if ((temp = hv_fetch(spec, "type", 4, 0))) { IV type; + found = 1; if ( ! ( SvOK(*temp) && looks_like_number(*temp) @@ -557,6 +559,7 @@ validate_one_param(SV* value, SV* params, HV* spec +, SV* id, HV* options, IV* unt /* check isa */ if ((temp = hv_fetch(spec, "isa", 3, 0))) { + found = 1; SvGETMAGIC(*temp); if (SvROK(*temp) && SvTYPE(SvRV(*temp)) == SVt_PVAV) { @@ -581,6 +584,7 @@ validate_one_param(SV* value, SV* params, HV* spec +, SV* id, HV* options, IV* unt /* check can */ if ((temp = hv_fetch(spec, "can", 3, 0))) { + found = 1; SvGETMAGIC(*temp); if (SvROK(*temp) && SvTYPE(SvRV(*temp)) == SVt_PVAV) { AV* array = (AV*) SvRV(*temp); @@ -605,6 +609,7 @@ validate_one_param(SV* value, SV* params, HV* spec +, SV* id, HV* options, IV* unt /* let callbacks to do their tests */ if ((temp = hv_fetch(spec, "callbacks", 9, 0))) { + found = 1; SvGETMAGIC(*temp); if (SvROK(*temp) && SvTYPE(SvRV(*temp)) == SVt_PVHV) { HE* he; @@ -681,6 +686,7 @@ validate_one_param(SV* value, SV* params, HV* spec +, SV* id, HV* options, IV* unt IV has_regex = 0; IV ok; + found = 1; SvGETMAGIC(*temp); if (SvPOK(*temp)) { @@ -732,6 +738,15 @@ validate_one_param(SV* value, SV* params, HV* spe +c, SV* id, HV* options, IV* unt } } + /* Catch unknown options not in (callbacks,can,isa,regex,type) + */ + if (!found) { + SV* buffer; + + buffer = sv_2mortal(newSVsv(id)); + sv_catpv(buffer, "unknown option error\n"); + FAIL(buffer, options); + } + if ((temp = hv_fetch(spec, "untaint", 7, 0))) { if (SvTRUE(*temp)) { *untaint = 1;
I would appreciate any enlightenment on how to make Params::Validate catch my typing errors.

Update: Bug rt://57831 has been filed.

--
No matter how great and destructive your problems may seem now, remember, you've probably only seen the tip of them. [1]

Replies are listed 'Best First'.
Re: How to make Params::Validate catch my typing errors
by almut (Canon) on May 25, 2010 at 18:00 UTC

    Another problem could be (in addition to the issues you have apparently encountered) that you're essentially assuming that things are ok as long as one of the options (like "callbacks", "type", etc.) is correct, instead of testing if any of them is wrong. In other words, a correct "type" option could potentially mask an incorrect "callback" option, because the existence of the former could set found = 1 in case both occur in the same spec...

    Anyhow, something like this - right at the beginning of validate_one_param() - should work:

    static char* valid_keys[] = { "callbacks", "can", "default", "depends", "isa", "optional", " +regex", "type" }; HE* he; hv_iterinit(spec); while (he = hv_iternext(spec)) { STRLEN len; char* key = HePV(he, len); int ok = 0; int j; for (j=0; j < sizeof(valid_keys)/sizeof(valid_keys[0]); j++) { if (strcmp(key, valid_keys[j]) == 0) { ok = 1; break; } } if (!ok) { SV* buffer = sv_2mortal(newSVpv("'",0)); sv_catpv(buffer, key); sv_catpv(buffer, "': unknown option error\n"); FAIL(buffer, options); } }

    At least it does pass all the tests that come with the module. And with your typo-ed 'callback', it croaks with

    $ ./841578.pl 1..4 'callback': unknown option error at ./841578.pl line 14 main::howareyoufeelingtoday('happy') called at ./841578.pl lin +e 18 # Looks like your test exited with 9 before it could output anything.

    (Of course, you could also implement the strcmp loop as a hash lookup, if you like)