3

We've adopted use strict in our legacy Perl codebase over the last few years. I've been tasked with adding it to the remaining modules, while ensuring of course that it doesn't break anything.

Now for use strict 'vars' and use strict 'subs' this is easy, because these are compile-time errors that a simple perl -c catches. But is there a systematic way for checking for runtime errors triggered by use strict 'refs'?

Of course, I could provoke runtime errors by calling all functions in all possible circumstances (i.e. with full coverage), but that is cumbersome esp. because this code lacks unit tests. If you have any better idea, I'd be very grateful.

Stefan Majewsky
  • 5,427
  • 2
  • 28
  • 51
  • Right now I'm grepping the source code for `/[@%$]\s*[${]/`, which finds all de-referencing syntaxes that I know of. Then I check every occurrence manually. – Stefan Majewsky Apr 17 '14 at 07:16
  • 1
    that's how I'd do it. but look for `/->\s*[[{]/` too – ysth Apr 17 '14 at 08:00
  • 2
    though I'd reject the task and say unit tests have to come first – ysth Apr 17 '14 at 08:01
  • (Full coverage isn't sufficient. e.g. You can get full test coverage of `$r = $x ? [] : "abc"; return if !$y; push @$r, "def";` with runs `$x=0;$y=0;` and `$x=1;$y=1;`, but that won't discover the problem with `$x=0;$y=1;`.) – ikegami Apr 17 '14 at 19:29

1 Answers1

4

Because Perl is untyped (a variable can potentially hold either a reference or non-reference), this can only be checked at run time. Consider:

use strict;

# Usage: foo(HASHREF)
sub foo {
   print $_[0]{name}, "\n" if int(rand(2));
}

my $var = int(rand(2))
   ? { name => "Hello" }
   : "World";  # oopsie!

foo($var);

Which will fail about 25% of the time. Static analysis of an untyped language can't catch that kind of thing — it can only be caught by your test suite, so your priority should be to improve that.

That said, there are ways to improve the code that could help you out. For example, we can prove that the foo sub itself is correctly written if we do a type check on incoming parameters:

sub foo {
   my $href = $_[0];
   ref($href) eq 'HASH' or die("Expected hashref!");
   print $href->{name}, "\n" if int(rand(2));
}

This pushes the burden of ensuring the correctness of foo's behaviour out of foo, and onto foo's caller. Within foo you now have a little ring-fenced section of code where you can be confident of each variable's data types. Progressively adding assertions like this allows you to keep widening that "zone of confidence".

Note that the small change above, means that our code will now fail 50% of the time, not 25% of the time. Because foo now fails more consistently, this will help us catch the real source of the error:

my $var = int(rand(2))
   ? { name => "Hello" }
   : "World";  # oopsie!

foo($var);

Partly because I wrote it, and partly because it's damn fast, I'd recommend you take a look at Type::Params for checking sub parameters. Here's an example of how to use it:

use feature qw(state);
use Types::Standard qw( HashRef );
use Type::Params qw( compile );

sub foo {
   state $signature = compile( HashRef );
   my ($href) = $signature->(@_);
   print $href->{name}, "\n" if int(rand(2));
}

Expanding that to a sub which takes multiple parameters...

use feature qw(state);
use Types::Standard qw( -types );
use Type::Params qw( compile );

sub query_get_rows {
   state $signature = compile( InstanceOf['DBI::db'], Str, Optional[Int] );
   my ($dbh, $query, $limit) = $signature->(@_);

   # do stuff here
}
tobyink
  • 13,478
  • 1
  • 23
  • 35
  • 1
    “*Static analysis can't catch that kind of thing*” – well, it kind of could, if Perl had a static type system. It maybe can't prove that there will be a problem, but if it can't prove the absence of any problems, that's warning enough. Here, `foo` has the type `Dict[name => Str] -> Bool`. The type of `$var` is a sum type (which is illegal when used implicitly in most statically typed languages) of `Dict[name => Str] | Str`. We can see that this *may* be legal as an argument to `foo`, but there are instances of this type where this isn't the case – any sane compiler would reject this. – amon Apr 17 '14 at 09:40
  • It was my intention that "Static analysis can't catch that kind of thing" was qualified by "Because Perl is untyped" (in the previous paragraph). I'll clarify. – tobyink Apr 17 '14 at 16:20