The attached patch allows a clean run from the following script adapted
from pgperltidy:

    {
        find . -type f -a \( -name '*.pl' -o -name '*.pm' \) -print
        find . -type f -perm -100 -exec file {} \; -print \
               | egrep -i ':.*perl[0-9]*\>' \
               | cut -d: -f1
    } \
    | sort -u  | xargs perlcritic --exclude ProhibitLeadingZeros

The changes are

  * disable perlcritic on Gen_dummy_probes.pl, since it's generated code
    from s2p
  * protect a couple of package declarations in plperl code from
    perltidy since it splits the lines and renders the 'no critic'
    directives there useless
  * mark a string eval in Catalog.pm with 'no critic', since it's
    clearly necessary.

We should probably set up a policy file for perlcritic that turns off or
at least lowers the severity of the ProhibitLeadingZeros policy. Making
it severity 5 seems a bit odd.

w.r.t. perltidy, I note that our policy has these two lines:

    --vertical-tightness=2
    --vertical-tightness-closing=2

I've been looking at syncing the buildfarm client with our core code
perltidy settings. However, I don't actually like these two and I've
decided to exercise some editorial discretion and not use them.

Note that the perltidy man page does suggest that these can make things
less readable, and it also states unequivocally "You must also use the
-lp flag when you use the -vt flag". That is the --line-up-parentheses
flag and it's something we don't use. Enabling it would generate about
12k lines of diff.


cheers


andrew

-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

diff --git a/src/backend/catalog/Catalog.pm b/src/backend/catalog/Catalog.pm
index 7497d9c..f387c86 100644
--- a/src/backend/catalog/Catalog.pm
+++ b/src/backend/catalog/Catalog.pm
@@ -250,7 +250,10 @@ sub ParseData
 
 			if ($lcnt == $rcnt)
 			{
-				eval '$hash_ref = ' . $_;
+				# We're treating the input line as a piece of Perl, so we
+				# need to use string eval here. Tell perlcritic we know what
+				# we're doing.
+				eval '$hash_ref = ' . $_; ## no critic (ProhibitStringyEval)
 				if (!ref $hash_ref)
 				{
 					die "$input_file: error parsing line $.:\n$_\n";
diff --git a/src/backend/utils/Gen_dummy_probes.pl b/src/backend/utils/Gen_dummy_probes.pl
index a38fea3..91d7968 100644
--- a/src/backend/utils/Gen_dummy_probes.pl
+++ b/src/backend/utils/Gen_dummy_probes.pl
@@ -14,6 +14,9 @@
 #
 #-------------------------------------------------------------------------
 
+# turn off perlcritic for autogened code
+## no critic
+
 $0 =~ s/^.*?(\w+)[\.\w+]*$/$1/;
 
 use strict;
diff --git a/src/pl/plperl/plc_perlboot.pl b/src/pl/plperl/plc_perlboot.pl
index ff05964..05334a6 100644
--- a/src/pl/plperl/plc_perlboot.pl
+++ b/src/pl/plperl/plc_perlboot.pl
@@ -51,9 +51,9 @@ sub ::encode_array_constructor
 }
 
 {
-
-	package PostgreSQL::InServer
-	  ;    ## no critic (RequireFilenameMatchesPackage);
+#<<< protect next line from perltidy so perlcritic annotation works
+	package PostgreSQL::InServer;  ## no critic (RequireFilenameMatchesPackage)
+#>>>
 	use strict;
 	use warnings;
 
diff --git a/src/pl/plperl/plc_trusted.pl b/src/pl/plperl/plc_trusted.pl
index 7b11a3f..dea3727 100644
--- a/src/pl/plperl/plc_trusted.pl
+++ b/src/pl/plperl/plc_trusted.pl
@@ -1,7 +1,8 @@
 #  src/pl/plperl/plc_trusted.pl
 
-package PostgreSQL::InServer::safe
-  ;    ## no critic (RequireFilenameMatchesPackage);
+#<<< protect next line from perltidy so perlcritic annotation works
+package PostgreSQL::InServer::safe; ## no critic (RequireFilenameMatchesPackage)
+#>>>
 
 # Load widely useful pragmas into plperl to make them available.
 #

Reply via email to