Re: [HACKERS] perlcritic

Hi Peter,

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

> I posted this about 18 months ago but then ran out of steam. [ ] Here
> is an updated patch.  The testing instructions below still apply.
> Especially welcome would be ideas on how to address some of the places
> I have marked with ## no critic.

Attached is a patch on top of yours that addresses all the ## no critic
annotations except RequireFilenameMatchesPackage, which can't be fixed
without more drastic reworking of the plperl build process.

Tested on perl 5.8.1 and 5.24.0 by configuring with --with-perl and
--enable-tap-tests followed by make check-world, and running pgindent
--build.

-- 
"A disappointingly low fraction of the human race is,
 at any given time, on fire." - Stig Sandbeck Mathisen

From cdf3ca19cbbf03111243f9b39eb6f402f25b4502 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Wed, 1 Mar 2017 15:32:45 +0000
Subject: [PATCH] Fix most perlcritic exceptions

The RequireFilenameMatchesPackage ones would require reworking the
plperl build process more drastically.
---
 src/pl/plperl/plc_perlboot.pl | 9 +++------
 src/tools/msvc/gendef.pl      | 2 +-
 src/tools/pgindent/pgindent   | 6 +++---
 3 files changed, 7 insertions(+), 10 deletions(-)

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;
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;
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);
@@ -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();
-- 
2.11.0


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers