test: Add new perl critic test case

Mark false positives in the perl code so that perlcritic ignores them,
and so that they are documented in-place.

Makefile.am

@@ -131,9 +131,11 @@ endif
 
 test_cases = \
 	test/000_pod.t \
+	test/100_critic.t \
 	$(nil)
 
 test_data = \
+	test/100_critic/perlcriticrc \
 	$(nil)
 
 include $(top_srcdir)/Makecheck.am

README

@@ -78,6 +78,10 @@ To run the test suite («make check»):
   TimeDate perl module
   IO-String perl module
 
+  Define the environment variable DPKG_DEVEL_MODE to run the test suite
+  in development mode, to include tests that might not be pertinent during
+  normal release builds.
+
 To enable additional developer's documentation («make doc») this software
 will be needed:
 

dselect/mkcurkeys.pl

@@ -85,12 +85,14 @@ for (my $i = 33; $i <= 126; $i++) {
     p();
 }
 
+## no critic (BuiltinFunctions::ProhibitReverseSortBlock)
 for $k (sort {
     looks_like_number($a) ?
         looks_like_number($b) ? $a <=> $b : -1
             : looks_like_number($b) ? 1 :
                 $a cmp $b
                 } keys %base) {
+    ## use critic
     $v= $base{$k};
     $v= $name{$k} if defined($name{$k});
     $v= $over{$k} if defined($over{$k});

scripts/Dpkg/Source/Package.pm

@@ -71,7 +71,7 @@ our $diff_ignore_default_regexp = '
 # Take out comments and newlines
 $diff_ignore_default_regexp =~ s/^#.*$//mg;
 $diff_ignore_default_regexp =~ s/\n//sg;
-no warnings 'qw';
+no warnings 'qw'; ## no critic (TestingAndDebugging::ProhibitNoWarnings)
 our @tar_ignore_default_pattern = qw(
 *.a
 *.la

scripts/t/300_Dpkg_BuildOptions.t

@@ -22,7 +22,7 @@ use warnings;
 use_ok('Dpkg::BuildOptions');
 
 {
-    no warnings;
+    no warnings; ## no critic (TestingAndDebugging::ProhibitNoWarnings)
     # Disable warnings related to invalid values fed during
     # the tests
     report_options(quiet_warnings => 1);

scripts/t/600_Dpkg_Changelog.t

@@ -166,10 +166,12 @@ foreach my $file ("$datadir/countme", "$datadir/shadow", "$datadir/fields",
 		       { to => '1:2.0~rc2-1sarge2' }, 3,
 		       '1:2.0~rc2-1sarge2/1:2.0~rc2-1sarge1/1.5-1',
 		       'to => "1:2.0~rc2-1sarge2"' );
+	## no critic (ControlStructures::ProhibitUntilBlocks)
 	check_options( $changes, \@data,
 		       { until => '1:2.0~rc2-1sarge2' }, 2,
 		       '1:2.0~rc2-1sarge1/1.5-1',
 		       'until => "1:2.0~rc2-1sarge2"' );
+	## use critic
 	#TODO: test combinations
     }
     if ($file eq "$datadir/fields") {

test/100_critic.t

@@ -0,0 +1,91 @@
+#!/usr/bin/perl
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+use strict;
+use warnings;
+
+use Test::More;
+
+unless (defined $ENV{DPKG_DEVEL_MODE}) {
+    plan skip_all => 'not running in development mode';
+}
+
+if (defined $ENV{srcdir}) {
+    chdir $ENV{srcdir} or die "cannot chdir to source directory: $!";
+}
+
+if (not eval { require Test::Perl::Critic }) {
+    plan skip_all => 'Test::Perl::Critic required to criticize code';
+}
+if (not eval { require Perl::Critic::Utils }) {
+    plan skik_all => 'Perl::Critic::Utils required to criticize code';
+}
+
+my @policies = qw(
+    BuiltinFunctions::ProhibitLvalueSubstr
+    BuiltinFunctions::ProhibitReverseSortBlock
+    BuiltinFunctions::ProhibitSleepViaSelect
+    BuiltinFunctions::ProhibitUniversalCan
+    BuiltinFunctions::ProhibitUniversalIsa
+    BuiltinFunctions::RequireSimpleSortBlock
+    ClassHierarchies::ProhibitAutoloading
+    ClassHierarchies::ProhibitExplicitISA
+    ClassHierarchies::ProhibitOneArgBless
+    CodeLayout::RequireConsistentNewlines
+    ControlStructures::ProhibitLabelsWithSpecialBlockNames
+    ControlStructures::ProhibitUntilBlocks
+    Documentation::RequirePackageMatchesPodName
+    InputOutput::ProhibitInteractiveTest
+    InputOutput::ProhibitOneArgSelect
+    InputOutput::RequireEncodingWithUTF8Layer
+    Miscellanea::ProhibitFormats
+    Miscellanea::ProhibitUnrestrictedNoCritic
+    Miscellanea::ProhibitUselessNoCritic
+    Modules::ProhibitEvilModules
+    Modules::RequireBarewordIncludes
+    Modules::RequireEndWithOne
+    Modules::RequireExplicitPackage
+    Modules::RequireFilenameMatchesPackage
+    TestingAndDebugging::ProhibitNoStrict
+    TestingAndDebugging::ProhibitNoWarnings
+    ValuesAndExpressions::ProhibitComplexVersion
+    ValuesAndExpressions::ProhibitLongChainsOfMethodCalls
+    ValuesAndExpressions::ProhibitQuotesAsQuotelikeOperatorDelimiters
+    ValuesAndExpressions::ProhibitSpecialLiteralHeredocTerminator
+    ValuesAndExpressions::ProhibitVersionStrings
+    ValuesAndExpressions::RequireConstantVersion
+    ValuesAndExpressions::RequireUpperCaseHeredocTerminator
+    Variables::ProhibitAugmentedAssignmentInDeclaration
+    Variables::ProhibitPerl4PackageNames
+    Variables::ProtectPrivateVars
+    Variables::RequireNegativeIndices
+);
+
+Test::Perl::Critic->import(
+    -profile => 'test/100_critic/perlcriticrc',
+    -verbose => 8,
+    -include => \@policies,
+    -only => 1,
+);
+
+my @dirs = qw(test src/t utils/t scripts/t dselect scripts/Dpkg);
+my @files = glob 'scripts/Dpkg.pm scripts/*.pl scripts/changelog/*.pl';
+push @files, Perl::Critic::Utils::all_perl_files(@dirs);
+
+plan tests => scalar @files;
+
+for my $file (@files) {
+    critic_ok($file);
+}

test/100_critic/perlcriticrc

@@ -0,0 +1,69 @@
+# Perl Critic configuration file
+severity = 1
+verbose = %f %l:%c (Severity: %s)\n  %P (%s)\n  near '%r'\n%d\n
+
+#[RegularExpressions::RequireExtendedFormatting]
+#minimum_regex_length_to_complain_about = 60
+
+# Complex is not always bad.
+[-BuiltinFunctions::ProhibitComplexMappings]
+# Needed when generating code.
+[-BuiltinFunctions::ProhibitStringyEval]
+# FIXME: Bogus check.
+[-Documentation::PodSpelling]
+# While this might be good for performance, it is bad for keeping docs updated.
+[-Documentation::RequirePodAtEnd]
+# FIXME: Bogus check.
+[-Documentation::RequirePodLinksIncludeText]
+# TODO: Standardize sections, but the ones from this are not usable.
+[-Documentation::RequirePodSections]
+# Too many false positives.
+[-CodeLayout::RequireTidyCode]
+# Forcing this just turns into noise (depending on the context, it makes sense).
+[-CodeLayout::RequireTrailingCommas]
+# These are fine.
+[-ControlStructures::ProhibitCascadingIfElse]
+# These are fine, too many in the code base anyway.
+[-ControlStructures::ProhibitPostfixControls]
+# These are fine, usually as long as they are not double negations.
+[-ControlStructures::ProhibitUnlessBlocks]
+# FIXME: Too many false positives.
+[-ControlStructures::ProhibitUnreachableCode]
+# TODO: Check it out, add new Dpkg::Program module?
+[-InputOutput::ProhibitBacktickOperators]
+# Needed, using <>/<@ARGV> is not correct, Prompt is not a core module.
+[-InputOutput::ProhibitExplicitStdin]
+# TODO: Maybe, some of these are part of the public/current API.
+[-Modules::ProhibitAutomaticExportation]
+# Complex is not always bad.
+[-Modules::ProhibitExcessMainComplexity]
+# FIXME: Too many false positives; non-modules all trigger.
+[-Modules::RequireVersionVar]
+# These are fine.
+[-NamingConventions::ProhibitAmbiguousNames]
+# When . is used in the code it means what it does.
+[-RegularExpressions::RequireDotMatchAnything]
+# When ^ or $ are used in the code they mean what they do.
+[-RegularExpressions::RequireLineBoundaryMatching]
+# TODO: While valid, these are part of the public/current API.
+[-Subroutines::ProhibitBuiltinHomonyms]
+# Needed.
+[-Subroutines::ProhibitSubroutinePrototypes]
+# Adding these seems like more noise.
+[-Subroutines::RequireFinalReturn]
+# Readers need to know perl, English module is worse.
+[-Variables::ProhibitPunctuationVars]
+# Readers need to know perl.
+[-Variables::RequireInitializationForLocalVars]
+# FIXME: Too many false positives; on ::main and for $ENV, $SIG, $?, $a, $b.
+[-Variables::RequireLocalizedPunctuationVars]
+# Readonly is not a core module.
+[-ValuesAndExpressions::ProhibitConstantPragma]
+# TODO: Check it out, using other quotes might be less readable and uniform.
+[-ValuesAndExpressions::ProhibitEmptyQuotes]
+# Used for help output.
+[-ValuesAndExpressions::ProhibitImplicitNewlines]
+# Octals are fine.
+[-ValuesAndExpressions::ProhibitLeadingZeros]
+# TODO: Check it out, some magic numbers are fine, octals for example.
+[-ValuesAndExpressions::ProhibitMagicNumbers]