Re: perlcritic

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