Silence some perl critic warnings.

* Variables::RequireNegativeIndice * Variables::ProhibitUnusedVariables * Variables::ProhibitAugmentedAssignmentInDeclaration * Modules::ProhibitConditionalUseStatements
pgbackrest · Apr 10, 2017 · 21a000d · 21a000d
1 parent 8546571
commit 21a000d
Show file tree

Hide file tree

Showing 10 changed files with 21 additions and 29 deletions.
diff --git a/doc/lib/BackRestDoc/Common/DocExecute.pm b/doc/lib/BackRestDoc/Common/DocExecute.pm
@@ -318,7 +318,7 @@ sub execute
                         }
                         else
                         {
-                            $strOutput .= (defined($strOutput) ? "\n" : '') . $stryOutput[@stryOutput - 1];
+                            $strOutput .= (defined($strOutput) ? "\n" : '') . $stryOutput[-1];
                         }
                     }
                 }

diff --git a/doc/lib/BackRestDoc/Common/DocRender.pm b/doc/lib/BackRestDoc/Common/DocRender.pm
@@ -644,7 +644,7 @@ sub processTag
 
     if (!defined($strTag))
     {
-        use Data::Dumper;
+        require Data::Dumper;
         confess Dumper($oTag);
     }
 

diff --git a/doc/lib/BackRestDoc/Html/DocHtmlPage.pm b/doc/lib/BackRestDoc/Html/DocHtmlPage.pm
@@ -275,7 +275,6 @@ sub sectionProcess
     my $oSectionBodyElement = $oSectionElement->addNew(HTML_DIV, "section-body");
 
     # Process each child
-    my $oSectionBodyExe;
     my $iSectionNo = 1;
 
     foreach my $oChild ($oSection->nodeList())

diff --git a/doc/xml/release.xml b/doc/xml/release.xml
@@ -203,6 +203,14 @@
                     <release-item>
                         <p>Refactor <code>backupLabel()</code> and add unit tests.</p>
                     </release-item>
+
+                    <release-item>
+                        <release-item-contributor-list>
+                            <release-item-contributor id="shang.cynthia"/>
+                        </release-item-contributor-list>
+
+                        <p>Silence some perl critic warnings.</p>
+                    </release-item>
                 </release-refactor-list>
             </release-core-list>
 

diff --git a/lib/pgBackRest/Common/Log.pm b/lib/pgBackRest/Common/Log.pm
@@ -355,7 +355,7 @@ sub logDebugProcess
                 $$oParamHash{$strParamName}{default} = true;
             }
 
-            $oValue = $oyResult[@oyResult - 1];
+            $oValue = $oyResult[-1];
 
             if (!defined($oValue) && $bParamRequired)
             {

diff --git a/lib/pgBackRest/File.pm b/lib/pgBackRest/File.pm
@@ -1245,7 +1245,6 @@ sub copy
     if ($bSourceRemote || $bDestinationRemote)
     {
         # Build the command and open the local file
-        my $hFile;
         my $hIn,
         my $hOut;
         my $strRemote;
@@ -1312,8 +1311,6 @@ sub copy
         if ($bController)
         {
             # Test for an error when reading output
-            my $strOutput;
-
             eval
             {
                 ($bResult, my $strResultChecksum, my $iResultFileSize, my $rResultExtra) =

diff --git a/lib/pgBackRest/Info.pm b/lib/pgBackRest/Info.pm
@@ -264,7 +264,7 @@ sub formatTextBackup
             {name => 'oBackupInfo', trace => true},
         );
 
-    my $strOutput .=
+    my $strOutput =
         '    ' . $$oBackupInfo{&INFO_KEY_TYPE} . ' backup: ' . $$oBackupInfo{&INFO_KEY_LABEL} . "\n" .
 
         '        timestamp start/stop: ' .

diff --git a/test/lib/pgBackRestTest/Common/LogTest.pm b/test/lib/pgBackRestTest/Common/LogTest.pm
@@ -151,7 +151,7 @@ sub supplementalAdd
     open(my $hFile, '<', $strFileName)
         or confess &log(ERROR, "unable to open ${strFileName} for appending to test log");
 
-    my $strHeader .= "+ supplemental file: " . $self->regExpReplaceAll($strFileName);
+    my $strHeader = "+ supplemental file: " . $self->regExpReplaceAll($strFileName);
 
     if (defined($strComment))
     {

diff --git a/test/lib/pgBackRestTest/Expire/ExpireEnvTest.pm b/test/lib/pgBackRestTest/Expire/ExpireEnvTest.pm
@@ -256,7 +256,7 @@ sub backupCreate
                           defined($oLastManifest) ? $oLastManifest->get(MANIFEST_SECTION_BACKUP, MANIFEST_KEY_LABEL) : undef,
                           $lTimestamp);
 
-    my $strBackupClusterSetPath .= "$$oStanza{strBackupClusterPath}/${strBackupLabel}";
+    my $strBackupClusterSetPath = "$$oStanza{strBackupClusterPath}/${strBackupLabel}";
     filePathCreate($strBackupClusterSetPath);
 
     &log(INFO, "create backup ${strBackupLabel}");

diff --git a/test/lint/perlcritic.policy b/test/lint/perlcritic.policy
@@ -55,45 +55,33 @@ severity = 5
 # Natural ordering here indicates the order in which they should be addressed.
 #-----------------------------------------------------------------------------------------------------------------------------------
 
-# S4 - Requires array offset from end to use negative syntax.
-[-Variables::RequireNegativeIndices]
-
-# S3 - Requires all variables to be used.
-[-Variables::ProhibitUnusedVariables]
-
-# S2 - Requires all long numbers to have thousand separators.
+# S2 - Requires all long numbers to have thousand separators. Probably a good idea bit need to change a fair amount of code.
 [-ValuesAndExpressions::RequireNumberSeparators]
 
-# S4 - Requires simple assignments in declarations. (TEST ONLY)
-[-Variables::ProhibitAugmentedAssignmentInDeclaration]
-
-# S4 - Requires parans when logical and bitwise booleans are mixed.
+# S4 - Requires parens when logical and bitwise booleans are mixed.
 [-ValuesAndExpressions::ProhibitMixedBooleanOperators]
 
 # S4 - Requires that sub names not overlap with built-ins - a bummer for object members.
 [-Subroutines::ProhibitBuiltinHomonyms]
 
-# S4 - Requires block form of grep for readability.
+# S4 - Requires block form of grep for readability. Needs to be fixed in about 15 places.
 [-BuiltinFunctions::RequireBlockGrep]
 
-# S4 - Requires modification of certain vars (e.g. $SIG) to have local scope.
+# S4 - Requires modification of certain vars (e.g. $SIG) to have local scope. Needs to be fixed in about 20 places.
 [-Variables::RequireLocalizedPunctuationVars]
 
 # S4 - Requires close() to be called soon after open but seems arbitrary.
 [-InputOutput::RequireBriefOpen]
 
-# S1 - Requires reverse keyword for reverse sorts instead of block.
+# S1 - Requires reverse keyword for reverse sorts instead of block. May not be able to since $a $b are passed as a parameter.
 [-BuiltinFunctions::ProhibitReverseSortBlock]
 
-# S3 - Requires use of Carp instead of die or warn.
+# S3 - Requires use of Carp instead of die or warn. Doesn't seem useful.
 [-ErrorHandling::RequireCarping]
 
-# S3 - Requires use of local vars in packages.
+# S3 - Requires use of local vars in packages. Can't use as it prohibits use of $DBI::errstr.
 [-Variables::ProhibitPackageVars]
 
-# S3 - Require conditional use statements to use dynamic load logic
-[-Modules::ProhibitConditionalUseStatements]
-
 # S3 - Requires that certain operators not be mixed.
 [-ValuesAndExpressions::ProhibitMismatchedOperators]