Item15323: fixed multiple errors found by perlcrit

foswiki · May 21, 2024 · 67e910d · 67e910d
1 parent 2ebff34
commit 67e910d
Show file tree

Hide file tree

Showing 13 changed files with 71 additions and 59 deletions.
diff --git a/core/lib/Foswiki/Access/TopicACLAccess.pm b/core/lib/Foswiki/Access/TopicACLAccess.pm
@@ -240,8 +240,9 @@ sub _getACL {
 
     # Dump the users web specifier if userweb
     my @list = grep { /\S/ } map {
-        s/^($Foswiki::cfg{UsersWebName}|%USERSWEB%|%MAINWEB%)\.//;
-        $_
+        my $tmp = $_;
+        $tmp =~ s/^($Foswiki::cfg{UsersWebName}|%USERSWEB%|%MAINWEB%)\.//;
+        $tmp
     } split( /[,\s]+/, $text );
 
     #print STDERR "getACL($mode): ".join(', ', @list)."\n" if MONITOR;

diff --git a/core/lib/Foswiki/Access/TopicACLReadOnlyAccess.pm b/core/lib/Foswiki/Access/TopicACLReadOnlyAccess.pm
@@ -10,12 +10,13 @@ DENY any access except VIEW access - Admin permitted anythingeverything
 
 package Foswiki::Access::TopicACLReadOnlyAccess;
 
+use strict;
+use warnings;
+
 use Foswiki::Access::TopicACLAccess;
 @ISA = qw(Foswiki::Access::TopicACLAccess);
 use constant MONITOR => 0;
 
-use strict;
-use warnings;
 use Assert;
 
 BEGIN {

diff --git a/core/lib/Foswiki/Logger/PlainFile.pm b/core/lib/Foswiki/Logger/PlainFile.pm
@@ -1,30 +1,4 @@
 # See bottom of file for license and copyright information
-package Foswiki::Logger::PlainFile::EventIterator;
-use strict;
-use warnings;
-use Assert;
-
-use Fcntl qw(:flock);
-
-BEGIN {
-    if ( $Foswiki::cfg{UseLocale} ) {
-        require locale;
-        import locale();
-    }
-}
-
-# Internal class for Logfile iterators.
-# So we don't break encapsulation of file handles.  Open / Close in same file.
-our @ISA = qw/Foswiki::Iterator::EventIterator/;
-
-# # Object destruction
-# # Release locks and file
-sub DESTROY {
-    my $this = shift;
-    flock( $this->{handle}, LOCK_UN )
-      if ( defined $this->{logLocked} );
-    close( delete $this->{handle} ) if ( defined $this->{handle} );
-}
 
 package Foswiki::Logger::PlainFile;
 
@@ -132,7 +106,9 @@ sub log {
     # OK too.
     unshift( @fields, "$time $level" );
     my $message =
-      '| ' . join( ' | ', map { s/\|/&vbar;/g; $_ } @fields ) . ' |';
+        '| '
+      . join( ' | ', map { my $tmp = $_; $tmp =~ s/\|/&vbar;/g; $tmp } @fields )
+      . ' |';
 
     my $file;
     my $mode = '>>';
@@ -350,6 +326,33 @@ sub _rotate {
 
 }
 
+package Foswiki::Logger::PlainFile::EventIterator;
+use strict;
+use warnings;
+use Assert;
+
+use Fcntl qw(:flock);
+
+BEGIN {
+    if ( $Foswiki::cfg{UseLocale} ) {
+        require locale;
+        import locale();
+    }
+}
+
+# Internal class for Logfile iterators.
+# So we don't break encapsulation of file handles.  Open / Close in same file.
+our @ISA = qw/Foswiki::Iterator::EventIterator/;
+
+# # Object destruction
+# # Release locks and file
+sub DESTROY {
+    my $this = shift;
+    flock( $this->{handle}, LOCK_UN )
+      if ( defined $this->{logLocked} );
+    close( delete $this->{handle} ) if ( defined $this->{handle} );
+}
+
 1;
 __END__
 Module of Foswiki - The Free and Open Source Wiki, http://foswiki.org/

diff --git a/core/lib/Foswiki/Macros/FORMFIELD.pm b/core/lib/Foswiki/Macros/FORMFIELD.pm
@@ -42,7 +42,7 @@ sub FORMFIELD {
     my $fdef = $topicObject->get( 'FIELD', $formField );
     if ( $fdef && ( !defined $fdef->{value} || $fdef->{value} eq '' ) ) {
 
-        # SMELL: weird; ignores format=
+        # ignores format=
         $text = $args->{default} // '';
         $text =~ s/!($Foswiki::regex{wikiWordRegex})/<nop>$1/gs;
         return $text;

diff --git a/core/lib/Foswiki/Macros/PERLDEPENDENCYREPORT.pm b/core/lib/Foswiki/Macros/PERLDEPENDENCYREPORT.pm
@@ -1,5 +1,5 @@
 # See bottom of file for license and copyright information
-package Foswiki;
+package Foswiki;    ## no critic
 
 # Detailed core and plugin dependency report - see
 # System.VarPERLDEPENDENCYREPORT and System.PerlDependencyReport

diff --git a/core/lib/Foswiki/Plugins.pm b/core/lib/Foswiki/Plugins.pm
@@ -284,10 +284,12 @@ Initialisation that is done after the user is known.
 =cut
 
 sub enable {
-    my $this     = shift;
-    my $prefs    = $this->{session}->{prefs};
-    my $dissed   = $prefs->getPreference('DISABLEDPLUGINS') || '';
-    my %disabled = map { s/^\s+//; s/\s+$//; $_ => 1 } split( /,/, $dissed );
+    my $this   = shift;
+    my $prefs  = $this->{session}->{prefs};
+    my $dissed = $prefs->getPreference('DISABLEDPLUGINS') || '';
+    my %disabled =
+      map { my $tmp = $_; $tmp =~ s/^\s+//; $tmp =~ s/\s+$//; $tmp => 1 }
+      split( /,/, $dissed );
 
     # Set the session for this call stack
     local $Foswiki::Plugins::SESSION = $this->{session};
@@ -319,7 +321,7 @@ sub enable {
     foreach my $plugin ( @{ $this->{plugins} } ) {
         my $sub = $plugin->{module} . "::lateInitPlugin";
         if ( defined &$sub ) {
-            no strict 'refs';
+            no strict 'refs';    ## no critic
             &$sub();
             use strict 'refs';
         }
@@ -387,7 +389,7 @@ sub dispatch {
         ASSERT( $Foswiki::Plugins::SESSION->isa('Foswiki') ) if DEBUG;
 
         # apply handler on the remaining list of args
-        no strict 'refs';
+        no strict 'refs';    ## no critic
         my $status = $plugin->invoke( $handlerName, @_ );
         use strict 'refs';
         if ( $status && $onlyOnceHandlers{$handlerName} ) {

diff --git a/core/lib/Foswiki/Render.pm b/core/lib/Foswiki/Render.pm
@@ -451,7 +451,7 @@ qr/<[Tt][Ee][Xx][Tt][Aa][Rr][Ee][Aa]\b.*?<\/[Tt][Ee][Xx][Tt][Aa][Rr][Ee][Aa]>/s,
 
                 # Indent pseudo-list
                 $line .= '&nbsp;'
-                  if ( length($line) eq 28 )
+                  if ( length($line) == 28 )
                   ;    # empty divs are not rendered, so make it non-empty.
                 _addListItem( $this, \@result, '', 'div', 'foswikiIndent', $1 );
                 $isList = 1;
@@ -1098,7 +1098,7 @@ sub getAnchorNames {
     my $id = $topicObject->getPath();
     my $a  = $this->{_anchorNames}{$id};
     unless ($a) {
-        $a = new Foswiki::Render::Anchors();
+        $a = Foswiki::Render::Anchors->new();
         $this->{_anchorNames}{$id} = $a;
     }
     return $a;
@@ -1723,12 +1723,6 @@ sub _adjustH {
 
     return $text unless scalar(@blocks) > 1;
 
-    sub _cap {
-        return 1 if ( $_[0] < 1 );
-        return 6 if ( $_[0] > 6 );
-        return $_[0];
-    }
-
     my $off = 0;
     my $out = '';
     while ( scalar(@blocks) ) {
@@ -1745,6 +1739,12 @@ sub _adjustH {
     return $out;
 }
 
+sub _cap {
+    return 1 if ( $_[0] < 1 );
+    return 6 if ( $_[0] > 6 );
+    return $_[0];
+}
+
 # _takeOutProtected( \$text, $re, $id, \%map ) -> $text
 #
 #   * =$text= - Text to process

diff --git a/core/lib/Foswiki/Request/Cache.pm b/core/lib/Foswiki/Request/Cache.pm
@@ -71,7 +71,7 @@ sub save {
     # passthrough file is only written to once, so if it already exists,
     # suspect a security hack (O_EXCL)
     my $F;
-    sysopen( $F, $this->_cacheFile($uid), O_RDWR | O_EXCL | O_CREAT, 0600 )
+    sysopen( $F, $this->_cacheFile($uid), O_RDWR | O_EXCL | O_CREAT, oct(600) )
       || die 'Unable to open '
       . $this->_cacheFile($uid)
       . ' for write; check the setting of {WorkingDir} in configure,'
@@ -255,7 +255,7 @@ sub _loadUpload {
     my $data = <$F>;
     $data = Foswiki::Sandbox::untaintUnchecked($data);
     my $info = undef;
-    eval $data;
+    eval $data;    ## no critic
     $F->close();
     unlink($ifn);
 

diff --git a/core/lib/Foswiki/UI/Manage.pm b/core/lib/Foswiki/UI/Manage.pm
@@ -49,7 +49,7 @@ sub manage {
 
         if ( defined &$method ) {
             $session->logger->log( { level => 'info', action => $action } );
-            no strict 'refs';
+            no strict 'refs';    ## no critic
             &$method($session);
         }
         else {

diff --git a/core/lib/Foswiki/UI/Register.pm b/core/lib/Foswiki/UI/Register.pm
@@ -72,7 +72,7 @@ sub register_cgi {
         $session->enterContext('absolute_urls');
 
         try {
-            no strict 'refs';
+            no strict 'refs';    ## no critic
             &$handler($session);
             use strict 'refs';
         }
@@ -1953,12 +1953,13 @@ sub _validateRegistration {
     }
     catch Error with {
         my $e = shift;
+        $e =~ s/ at .*//ms;
         throw Foswiki::OopsException(
             'register',
             web    => $data->{webName},
             topic  => $session->{topicName},
             def    => 'registration_invalid',
-            params => [ $e->stringify ]
+            params => [$e]
         );
 
     };
@@ -2073,10 +2074,10 @@ sub _clearPendingRegistrationsForUser {
 
     # Remove the integer code to leave just the wikiname
     $file =~ s/\.\d+$//;
-    foreach my $f (<$file.*>) {
+    foreach my $f ( glob("$file.*") ) {
 
         # Read from disc, implictly validated
-        unlink( Foswiki::Sandbox::untaintUnchecked($f) );
+        unlink Foswiki::Sandbox::untaintUnchecked($f);
     }
 }
 

diff --git a/core/lib/Foswiki/UI/Rest.pm b/core/lib/Foswiki/UI/Rest.pm
@@ -291,7 +291,7 @@ sub rest {
     my $error = 0;
 
     try {
-        no strict 'refs';
+        no strict 'refs';    ## no critic
         $result = &$function( $session, $subject, $verb, $session->{response} );
         use strict 'refs';
     }

diff --git a/core/lib/Foswiki/UI/Save.pm b/core/lib/Foswiki/UI/Save.pm
@@ -639,7 +639,7 @@ WARN
     }
 
     if ($attachments) {
-        foreach $a ( @{$attachments} ) {
+        foreach my $a ( @{$attachments} ) {
             try {
                 $a->{tom}->copyAttachment( $a->{name}, $topicObject );
             }

diff --git a/core/lib/Foswiki/UI/Statistics.pm b/core/lib/Foswiki/UI/Statistics.pm
@@ -576,11 +576,15 @@ sub _getTopList {
     if (@list) {
 
         # Strip initial spaces
-        @list = map { s/^\s*//; $_ } @list;
+        @list = map { my $tmp = $_; $tmp =~ s/^\s*//; $tmp } @list;
 
         @list =    # Prepend spaces depending on no. of digits
-          map { s/^([0-9][0-9][^0-9])/\&nbsp\;$1/;    $_ }
-          map { s/^([0-9][^0-9])/\&nbsp\;\&nbsp\;$1/; $_ }
+          map { my $tmp = $_; $tmp =~ s/^([0-9][0-9][^0-9])/\&nbsp\;$1/; $tmp }
+          map {
+            my $tmp = $_;
+            $tmp =~ s/^([0-9][^0-9])/\&nbsp\;\&nbsp\;$1/;
+            $tmp
+          }
 
           # Sort numerically, descending order
           sort { ( split / /, $b )[0] <=> ( split / /, $a )[0] } @list;