Re: [HACKERS] perlcritic

ilmari@ilmari.org (Dagfinn Ilmari Mannsåker) writes:

> 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.

Attached is an updated version of the patch, in which
src/tools/msvc/gendef.pl actually compiles.  If someone on Windows could
test it, that would be great.

-- 
"The surreality of the universe tends towards a maximum" -- Skud's Law
"Never formulate a law or axiom that you're not prepared to live with
 the consequences of."                              -- Skud's Meta-Law

From 2bbdd768bdbabe10e0af6b95d2d09d29095d3a8b 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..598699e6ea 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