Check a perl script for repeat "my" declarations

[graff]

In response to this SoPW node, I felt compelled to invent something simple and quick that would scan a perl script and report any cases where "my" was used to declare the same variable name more than once.

This approach is not subtle or discerning -- just check all easily-identifyable uses of "my" with variable names, and keep these (with line numbers) in a hash. When done scanning the script, report the cases with more than one line number. (There is also the "--listall" option, which does what you would expect...)

Caveats (at least, the ones I know of):

• The line numbers being reported are based on the output of B::Deparse, where all commentary has been stripped, and spacing and punctuation have been normalized. This means you may need to work a little harder to relate this diagnosis to your original script, but doing it this way helps me make the search for "my" a lot less speculative (and I do save the Deparsed version of the script in a separate file, so you can actually use the line-number info that is being given).
• There are bound to be other ways that "my" shows up in a "deparsed" script besides the ones being searched for in this version, so there may be some "misses".
• Someone is bound to try this on a script containing a quoted literal string that looks like a "my" declaration, so there may be some false alarms as well.
• This doesn't pay any attention to scope. If you use anything like  foreach my $arg ( @list ) in multiple places in your script, and these are all sensible and proper, this diagnosis will cite them all as "repeats" anyway -- better to be stupid and safe, I think.

That said, here's the code (tested on a few scripts):

#!/usr/bin/perl

# Program:      scan-perl-vars.perl
# Written by:   dave graff
# Purpose:      look for and summarize the declarations and calls
#                of lexically-scoped variables in a perl script

use strict;
my $Usage = "$0 [--listall] my_script.perl\n".
    " will list 'my' vars that have been declared more than once in a 
+script\n".
    " (with --listall, will list all lexical variables, with line numb
+ers)\n";

my $listyp = ' more than once';
if ( @ARGV > 1 and $ARGV[0] eq '--listall' ) {
    $listyp = ''; 
    shift;
}
die $Usage unless ( @ARGV == 1 && -r $ARGV[0] );

my $erlog = "/tmp/scn-src.$$.errlog";

my $file = shift;
warn "Deparsing $file...\n";
my @lines = `perl -MO=Deparse,-p $file 2> $erlog`;
die "Nothing deparsed from $file -- sorry.\n"
    unless ( @lines );

my $log = `cat $erlog`;
die "Syntax errors in $file -- come back when you're ready.\n"
    unless ( $log =~ /^$file syntax OK/ );

# Store an array of subroutine defs in a hash element
# keyed by file name:

my %vardecs;  # HoA keyed by var name, stores line #(s) of declaration
+(s)

my $lineno = 0;
open( DP, ">$file.deparse" ) or warn "couldn't write deparsed code to 
+$file.deparse:$!\n";

foreach (@lines) {
    $lineno++;
    print DP "$lineno\t$_";
    while (( my $nxt = index( $_, 'my' )) >= 0 ) {
        if ( /\bmy\(([^\)]+)/ ) {
            foreach my $name (split(/,/,$1)) {
                push @{$vardecs{$name}}, $lineno;
            }
        }
        elsif ( /\bmy\s+([\%\$\*\@]\S+)/ ) {
            push @{$vardecs{$1}}, $lineno;
            my $nxt = 0;
        }
        $_ = substr( $_, $nxt+3 );
    }
}
close DP;

# Now do some reporting

die "No lexically-scoped variables in $file\n" unless ( keys %vardecs 
+);

my $nlisted = 0;
foreach (sort keys %vardecs) {
    next unless ( scalar @{$vardecs{$_}} > 1 or $listyp eq '');
    print join( ' ', $_, @{$vardecs{$_}}, "\n" );
    $nlisted++;
}
print "\nTotal of $nlisted 'my' variables declared$listyp in $file\n";
print "(see $file.deparse for correct line-number references)\n"
    if ( $nlisted );

[robartes]

A slight, though still naieve, improvement would be to parse the output of B::Concise for padsv pad ops. Something like this:

use strict;

my $script=shift;
my @output=qx/perl -MO=Concise,-exec $script/;
my $line=1;
my %lexicals;


foreach (@output) {
  if (/nextstate\([\w:]+ \d+ [\w.]+:(\d+)\)/) { $line=$1;next };
  if (/pad[ahs]v\[(.[\w_]+):(\d+),\d+\]/) {
    unless ( exists ( $lexicals{$1}->{$2} ) ) {
      $lexicals{$1}->{$2}=[];
    }
    push @{$lexicals{$1}->{$2}}, $line;
  }
}

foreach (keys %lexicals) {
  if ( scalar (keys %{$lexicals{$_}}) > 1 ) {
    print "Same lexical variable name in different scopes detected:\n"
+;
    print " Variable $_ is used in:\n";
    my $entry=$lexicals{$_};
    print "  Scope $_, line(s) ". join(',',@{$entry->{$_}})."\n" for k
+eys       %$entry;
    print "\n";
  }
}

When run on a script containing my $camel;{my $camel;my $frog='my $camel';}; this produces:

mymy.pl syntax OK
Same lexical variable name in different scopes detected:
 Variable $camel is used in:
  Scope 1, line(s) 5
  Scope 2, line(s) 7

As seen, only real lexical declarations are detected, not string literals containing my. I have opted to print all lines containing the variable in the various scopes it appears in, but an easy change would be to just report the first line number it appears in, which should be the guilty declaration.

The code is tested, but it's definitely for fun only :)

Update: Changed regexp to also detect hash and array ops.

CU
Robartes-

[Aristotle]

I was going to propose a very similar angle, only I'd use B::Xref as it does 90% of what you need for this job. Its output is also more consistent and easier to parse for as this task.

Makeshifts last the longest.

[awkmonk]

Yes it's Cool.

It does occur to me that if you defined the setting of the my clauses in a function (with it's own lexically defined HoA) you could - via recurrsion - quite easily make it handle the lexical scope of the defined fields...or something like that ;0)

Be careful though - you start out just finding my variables and soon you'll find yourself parsing Perl into COBOL or some other silly language.