From bc2187b03389edec18072d47c17363b32efea898 Mon Sep 17 00:00:00 2001
From: Brian Moses Hall <moseshll@umich.edu>
Date: Tue, 5 Nov 2024 16:07:57 -0500
Subject: [PATCH] Addresses #170 Clear up `Perl::Critic` gripes

---
 bin/criticize.pl | 44 ++++++++++++++++++++------------------------
 bin/newyear.pl   |  2 +-
 cgi/CRMS.pm      | 12 ++++++------
 cgi/Metadata.pm  | 10 +++++-----
 4 files changed, 32 insertions(+), 36 deletions(-)

diff --git a/bin/criticize.pl b/bin/criticize.pl
index 5c339e0e..8c381c3a 100755
--- a/bin/criticize.pl
+++ b/bin/criticize.pl
@@ -21,9 +21,8 @@ BEGIN
 my $usage = <<END;
 USAGE: $0
 
-Runs Perl::Critic on all .pm, .pl, and extensionless files in cgi/ and bin/.
-This is offered as a sort of sanity check and is not currently part of the
-test suite.
+Runs Perl::Critic on all .pm, .pl, and extensionless files in bin/, cgi/, lib/, and t/.
+This is not currently part of the test suite.
 
 -h       Print this help message.
 -v       Emit verbose debugging information. May be repeated.
@@ -39,39 +38,36 @@ END
 if ($help) { print $usage. "\n"; exit(0); }
 print "Verbosity $verbose\n" if $verbose;
 
-
 my $critic = Perl::Critic->new();
 
-my @dirs = ($ENV{'SDRROOT'}. '/crms/cgi', $ENV{'SDRROOT'}. '/crms/bin');
+my $exit_status = 0;
+my @dirs = map { $ENV{SDRROOT} . $_; } ('/crms/bin', '/crms/cgi', '/crms/lib', '/crms/t');
 my %seen;
-while (my $pwd = shift @dirs)
-{
-  opendir(DIR, "$pwd") or die "Can't open $pwd\n";
+while (my $dir = shift @dirs) {
+  opendir(DIR, "$dir") or die "Can't open $dir\n";
   my @files = readdir(DIR);
   closedir(DIR);
-  foreach my $file (sort @files)
-  {
+  foreach my $file (sort @files) {
     next if $file =~ /^\.\.?$/;
-    next if $file eq 'legacy';
-    my $path = "$pwd/$file";
-    if (-d $path)
-    {
+    my $path = "$dir/$file";
+    if (-d $path) {
       next if $seen{$path};
       $seen{$path} = 1;
       push @dirs, $path;
     }
     my $desc = `file $path`;
     chomp $desc;
-    next unless $desc =~ m/perl/i;
-    my @violations = $critic->critique($path);
-    if (scalar @violations)
-    {
-      print RED "$path\n";
-      print BOLD "  $_" for @violations;
-    }
-    else
-    {
-      print GREEN "$path\n";
+    if ($desc =~ m/perl/i || $path =~ m/\.pm$/ || $path =~ m/\.t$/) {
+      my @violations = $critic->critique($path);
+      if (scalar @violations) {
+        print RED "$path\n";
+        print BOLD "  $_" for @violations;
+        $exit_status = 1;
+      }
+      else {
+        print GREEN "$path\n" if $verbose;
+      }
     }
   }
 }
+exit($exit_status);
diff --git a/bin/newyear.pl b/bin/newyear.pl
index eeda7331..a9439840 100755
--- a/bin/newyear.pl
+++ b/bin/newyear.pl
@@ -438,7 +438,7 @@ sub SubmitNewYearReview
       $record->title || '', $record->publication_date->text || '',
       $record->country || '', "$acurr/$rcurr", $extracted_data,
       $predictions, "$new_attr/$new_reason", $msg);
-    @values = map { local $_ = $_; $_ =~ s/\s+/ /g; $_; } @values;
+    @values = map { s/\s+/ /gr } @values;
     printf $tsv_fh "%s\n", join("\t", @values);
   }
 }
diff --git a/cgi/CRMS.pm b/cgi/CRMS.pm
index 48d30516..417062bb 100755
--- a/cgi/CRMS.pm
+++ b/cgi/CRMS.pm
@@ -1,5 +1,9 @@
 package CRMS;
 
+use strict;
+use warnings;
+use utf8;
+
 ## ----------------------------------------------------------
 ## Object of shared code for the CRMS DB CGI and BIN scripts.
 ## ----------------------------------------------------------
@@ -10,10 +14,6 @@ BEGIN {
   use lib $ENV{'SDRROOT'} . '/crms/lib';
 }
 
-use strict;
-use warnings;
-use utf8;
-
 use CGI;
 use Data::Dumper;
 use Date::Calc qw(:all);
@@ -1105,7 +1105,7 @@ sub WriteRightsFile
   }
   my $temp = $perm . '.tmp';
   if (-f $temp) { die "file already exists: $temp\n"; }
-  open (my $fh, '>:utf8', $temp) || die "failed to open rights file ($temp): $!\n";
+  open (my $fh, '>:encoding(UTF-8)', $temp) || die "failed to open rights file ($temp): $!\n";
   print $fh $rights;
   close $fh;
   rename $temp, $perm;
@@ -1273,7 +1273,7 @@ sub CheckAndLoadItemIntoCandidates
       $self->RemoveFromCandidates($id);
     }
   }
-  return undef;
+  return;
 }
 
 sub AddItemToCandidates
diff --git a/cgi/Metadata.pm b/cgi/Metadata.pm
index 4635232d..f8e14ef3 100644
--- a/cgi/Metadata.pm
+++ b/cgi/Metadata.pm
@@ -1,5 +1,9 @@
 package Metadata;
 
+use strict;
+use warnings;
+use utf8;
+
 BEGIN {
   die "SDRROOT environment variable not set" unless defined $ENV{'SDRROOT'};
   use lib $ENV{'SDRROOT'} . '/crms/lib';
@@ -16,10 +20,6 @@ BEGIN {
 
 use vars qw(@ISA @EXPORT @EXPORT_OK);
 
-use strict;
-use warnings;
-use utf8;
-
 use Carp;
 use DB_File;
 use File::Slurp;
@@ -111,7 +111,7 @@ sub US_Cities {
   if (!defined $US_CITIES) {
     my %us_cities;
     my $us_cities_db = $ENV{'SDRROOT'} . '/crms/post_zephir_processing/data/us_cities.db';
-    tie %us_cities, 'DB_File', $us_cities_db, O_RDONLY, 0644, $DB_BTREE or die "can't open db file $us_cities_db: $!";
+    tie %us_cities, 'DB_File', $us_cities_db, O_RDONLY, oct(644), $DB_BTREE or die "can't open db file $us_cities_db: $!";
     $US_CITIES = \%us_cities;
   }
   return $US_CITIES;