perlcritic script

Started by Andrew Dunstanover 7 years ago5 messages
#1Andrew Dunstan
andrew.dunstan@2ndquadrant.com
1 attachment(s)

Here's a small patch to add a script to call perlcritic, in the same way
that we have a script to call perltidy. Is also includes a perlcriticrc
file containing a policy to allow octal constants with leading zeros.
That's the only core severity 5 policy we are currently no in compliance
with.

We should probably look at being rather more aggressive with perlcritic.
I've made the buildfarm client code compliant with some exceptions down
to severity level 3. Here are the profile exceptions:

[-Variables::RequireLocalizedPunctuationVars]
[TestingAndDebugging::ProhibitNoWarnings]
allow = once
[-InputOutput::RequireBriefOpen]
[-Subroutines::RequireArgUnpacking]
[-RegularExpressions::RequireExtendedFormatting]
[-Variables::ProhibitPackageVars]
[-ErrorHandling::RequireCarping]
[-ValuesAndExpressions::ProhibitComplexVersion]
[InputOutput::ProhibitBacktickOperators]
only_in_void_context = 1
[-Modules::ProhibitExcessMainComplexity]
[-Subroutines::ProhibitExcessComplexity]
[-ValuesAndExpressions::ProhibitImplicitNewlines]
[-ControlStructures::ProhibitCascadingIfElse]
[-ControlStructures::ProhibitNegativeExpressionsInUnlessAndUntilConditions]
[-ErrorHandling::RequireCheckingReturnValueOfEval ]
[-BuiltinFunctions::ProhibitComplexMappings]

There are also 21 places in the code with "no critic" markings at
severity 3 and above.

cheers

andrew

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

Attachments:

0001-Add-a-script-and-a-config-file-to-run-perlcritic.patchtext/x-patch; name=0001-Add-a-script-and-a-config-file-to-run-perlcritic.patchDownload
From c63ed411fb28d7422fc5a34ddae6d8c6fbf2666f Mon Sep 17 00:00:00 2001
From: Andrew Dunstan <andrew@dunslane.net>
Date: Mon, 7 May 2018 15:55:25 -0400
Subject: [PATCH] Add a script and a config file to run perlcritic

---
 src/tools/pgperlcritic/perlcriticrc | 12 ++++++++++++
 src/tools/pgperlcritic/pgperlcritic | 26 ++++++++++++++++++++++++++
 2 files changed, 38 insertions(+)
 create mode 100644 src/tools/pgperlcritic/perlcriticrc
 create mode 100755 src/tools/pgperlcritic/pgperlcritic

diff --git a/src/tools/pgperlcritic/perlcriticrc b/src/tools/pgperlcritic/perlcriticrc
new file mode 100644
index 0000000..5ff92cb
--- /dev/null
+++ b/src/tools/pgperlcritic/perlcriticrc
@@ -0,0 +1,12 @@
+######################################################################
+#
+# src/tools/pgperlcritic/perlcriticrc
+#
+# config  file for perlcritic for Postgres project
+#
+#####################################################################
+
+severity = 5
+
+# allow octal constants with leading zeros
+[-ValuesAndExpressions::ProhibitLeadingZeros]
diff --git a/src/tools/pgperlcritic/pgperlcritic b/src/tools/pgperlcritic/pgperlcritic
new file mode 100755
index 0000000..0449871
--- /dev/null
+++ b/src/tools/pgperlcritic/pgperlcritic
@@ -0,0 +1,26 @@
+#!/bin/sh
+
+# src/tools/pgperlcritic/pgperlcritic
+
+test -f src/tools/pgperlcritic/perlcriticrc || {
+	echo could not find src/tools/pgperlcritic/perlcriticrc
+	exit 1
+	}
+
+set -e
+
+# set this to override default perlcritic program:
+PERLCRITIC=${PERLCRITIC:-perlcritic}
+
+# locate all Perl files in the tree
+{
+	# take all .pl and .pm files
+	find . -type f -a \( -name '*.pl' -o -name '*.pm' \) -print
+	# take executable files that file(1) thinks are perl files
+	find . -type f -perm -100 -exec file {} \; -print |
+	egrep -i ':.*perl[0-9]*\>' |
+	cut -d: -f1
+} |
+sort -u |
+xargs $PERLCRITIC --quiet --profile=src/tools/pgperlcritic/perlcriticrc
+
-- 
2.9.5

#2Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Andrew Dunstan (#1)
Re: perlcritic script

On 5/8/18 13:57, Andrew Dunstan wrote:

+	# take executable files that file(1) thinks are perl files
+	find . -type f -perm -100 -exec file {} \; -print |
+	egrep -i ':.*perl[0-9]*\>' |

How portable is that?

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

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#2)
Re: perlcritic script

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

On 5/8/18 13:57, Andrew Dunstan wrote:

+	# take executable files that file(1) thinks are perl files
+	find . -type f -perm -100 -exec file {} \; -print |
+	egrep -i ':.*perl[0-9]*\>' |

How portable is that?

Well, it's the same code that's in pgperltidy ... but I agree that
it's making a lot of assumptions about the behavior of file(1).

regards, tom lane

#4Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Tom Lane (#3)
Re: perlcritic script

On 5/8/18 16:51, Tom Lane wrote:

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

On 5/8/18 13:57, Andrew Dunstan wrote:

+	# take executable files that file(1) thinks are perl files
+	find . -type f -perm -100 -exec file {} \; -print |
+	egrep -i ':.*perl[0-9]*\>' |

How portable is that?

Well, it's the same code that's in pgperltidy ... but I agree that
it's making a lot of assumptions about the behavior of file(1).

OK, but then it's not a problem for this thread.

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

#5Tels
nospam-pg-abuse@bloodgate.com
In reply to: Peter Eisentraut (#4)
Re: perlcritic script

Moin,

On Tue, May 8, 2018 5:03 pm, Peter Eisentraut wrote:

On 5/8/18 16:51, Tom Lane wrote:

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

On 5/8/18 13:57, Andrew Dunstan wrote:

+	# take executable files that file(1) thinks are perl files
+	find . -type f -perm -100 -exec file {} \; -print |
+	egrep -i ':.*perl[0-9]*\>' |

How portable is that?

Well, it's the same code that's in pgperltidy ... but I agree that
it's making a lot of assumptions about the behavior of file(1).

OK, but then it's not a problem for this thread.

If I'm not mistaken, the first line in the "find" code could be more
compact like so:

find . -type f -iname '*.p[lm]'

(-print is default, and the -name argument is a regexp, anyway. And IMHO
it could be "-iname" so we catch "test.PM", too?).

Also, "-print" does not handle filenames with newlines well, so "-print0"
should be used, however, this can be tricky when the next step isn't xarg,
but sort. Looking at the man page, on my system this would be:

find . -type f -name '*.p[lm]' -print0 | sort -u -z | xargs -0 ...

Not sure if that is more, or less, portable then the original -print
variant, tho.

Best regards,

Tels