Re: perlcritic - Mailing list pgsql-hackers

On 3/1/17 11:21, Dagfinn Ilmari Mannsåker wrote:
> diff --git a/src/pl/plperl/plc_perlboot.pl b/src/pl/plperl/plc_perlboot.pl
> index 292c9101c9..b4212f5ab2 100644
> --- a/src/pl/plperl/plc_perlboot.pl
> +++ b/src/pl/plperl/plc_perlboot.pl
> @@ -81,18 +81,15 @@ sub ::encode_array_constructor
>          } sort keys %$imports;
>          $BEGIN &&= "BEGIN { $BEGIN }";
>  
> -        return qq[ package main; sub { $BEGIN $prolog $src } ];
> +        # default no strict and no warnings
> +        return qq[ package main; sub { no strict; no warnings; $BEGIN $prolog $src } ];
>      }
>  
>      sub mkfunc
>      {
> -        ## no critic (ProhibitNoStrict, ProhibitStringyEval);
> -        no strict;      # default to no strict for the eval
> -        no warnings;    # default to no warnings for the eval
> -        my $ret = eval(mkfuncsrc(@_));
> +        my $ret = eval(mkfuncsrc(@_)); ## no critic (ProhibitStringyEval);
>          $@ =~ s/\(eval \d+\) //g if $@;
>          return $ret;
> -        ## use critic
>      }
>  
>      1;

I have no idea what this code does or how to test it, so I didn't touch it.

> diff --git a/src/tools/msvc/gendef.pl b/src/tools/msvc/gendef.pl
> index 64227c2dce..e2653f11d8 100644
> --- a/src/tools/msvc/gendef.pl
> +++ b/src/tools/msvc/gendef.pl
> @@ -174,7 +174,7 @@ sub usage
>  
>  my %def = ();
>  
> -while (<$ARGV[0]/*.obj>)  ## no critic (RequireGlobFunction);
> +while (glob($ARGV[0]/*.obj))
>  {
>      my $objfile = $_;
>      my $symfile = $objfile;

I think what this code is meant to do might be better written as a
foreach loop.  Again, can't test it.

> diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent
> index a6b24b5348..51d6a28953 100755
> --- a/src/tools/pgindent/pgindent
> +++ b/src/tools/pgindent/pgindent
> @@ -159,8 +159,7 @@ sub process_exclude
>          while (my $line = <$eh>)
>          {
>              chomp $line;
> -            my $rgx;
> -            eval " \$rgx = qr!$line!;";  ## no critic (ProhibitStringyEval);
> +            my $rgx = eval { qr!$line! };
>              @files = grep { $_ !~ /$rgx/ } @files if $rgx;
>          }
>          close($eh);

After further thinking, I changed this to just
   my $rgx = qr!$line!;

which works just fine.

> @@ -435,7 +434,8 @@ sub diff
>  
>  sub run_build
>  {
> -    eval "use LWP::Simple;";  ## no critic (ProhibitStringyEval);
> +    require LWP::Simple;
> +    LWP::Simple->import(qw(getstore is_success));
>  
>      my $code_base = shift || '.';
>      my $save_dir = getcwd();

I think this is mean to not fail compilation if you don't have that
module, so I left it as is.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services