Актуализирован список perlcritic policies, исправлены минорные замечания critic-а

.perlcriticrc

@@ -1,3 +1,19 @@
+#!/usr/bin/env perlcritic
+
+# Задействованные модули критика:
+#
+# Perl::Critic::More
+# Perl::Critic::Pulp
+# Perl::Critic::TooMuchCode
+# Perl::Critic::Lax
+# Perl::Critic::CognitiveComplexity
+# Perl::Critic::Policy::CompileTime
+# Perl::Critic::Policy::ProhibitImplicitImport
+# Perl::Critic::Policy::Dynamic::NoIndirect
+# Perl::Critic::StricterSubs
+# Perl::Critic::Policy::ProhibitImplicitImport
+# Perl::Critic::Policy::CodeLayout::RequireUseUTF8
+
 [-ControlStructures::ProhibitUnlessBlocks]
 # oh. c'mon. just a little bit
 
@@ -52,3 +68,25 @@
 
 [-Subroutines::ProhibitSubroutinePrototypes]
 # i like it!
+
+[-TooMuchCode::ProhibitUnnecessaryUTF8Pragma]
+
+[-CodeLayout::RequireUseUTF8]
+
+[-Miscellanea::ProhibitUnrestrictedNoCritic]
+
+[-Editor::RequireEmacsFileVariables]
+
+[-CodeLayout::RequireASCII]
+
+[-TooMuchCode::ProhibitDuplicateLiteral]
+
+[-ErrorHandling::RequireUseOfExceptions]
+
+[-ValuesAndExpressions::RequireConstantOnLeftSideOfEquality]
+
+[-Lax::RequireConstantOnLeftSideOfEquality::ExceptEq]
+
+[-Lax::ProhibitEmptyQuotes::ExceptAsFallback]
+
+[-ValuesAndExpressions::RestrictLongStrings]

README

@@ -9,7 +9,7 @@ aleesa-phrases
 Механика работы
 -----------------------------------------------------------------------------------------------------------------------
 Сервис подписан на канал phrases. В этот канал пишет другой демоночек - aleesa-misc. А ещё aleesa-cron (чтобы фортунки
-по утрам рассылать)
+по утрам рассылать).
 
 Ответ направляется в redis-канал, указанный в поле plugin входящего сообщения.
 
@@ -32,6 +32,7 @@ aleesa-phrases
 
 В каталоге contrib находится скрипт для alpine linux, системы инициализации openrc. Этот скрипт достаточно положить В
 /etc/init.d/aleesa-misc сконфигурить сервис по инструкции выше и после этого сервис запускается командой:
-service aleesa-misc start
-в автозагрузку его можно прописать через команду
-rc-update add aleesa-misc default
+service aleesa-phrases start
+
+в автозагрузку его можно прописать через команду:
+rc-update add aleesa-phrases default

bin/aleesa-phrases

@@ -1,6 +1,6 @@
 #!/usr/bin/perl
 
-use 5.018;
+use 5.018; ## no critic (ProhibitImplicitImport)
 use strict;
 use warnings;
 use utf8;
@@ -29,9 +29,10 @@ no Cwd;
 use Cwd::utf8 qw (chdir abs_path);
 use File::Basename qw (basename dirname);
 use File::Path qw (make_path);
+use File::Spec qw ();
 use Log::Any qw ($log);
-use Log::Any::Adapter;
-use Log::Any::For::Std;
+use Log::Any::Adapter ();
+use Log::Any::For::Std ();
 use POSIX qw (setgid setuid setsid);
 
 use Conf qw (LoadConf);
@@ -70,8 +71,8 @@ if ($UID == 0) {
 		make_path (
 			$piddir, {
 				uid   => $uid,
-				group => $gid
-			}
+				group => $gid,
+			},
 		);
 	}
 
@@ -82,8 +83,8 @@ if ($UID == 0) {
 			make_path (
 					$logdir, {
 							uid   => $uid,
-							group => $gid
-					}
+							group => $gid,
+					},
 			) or die "Unable to create $logdir: $OS_ERROR\n";
 		}
 	}
@@ -98,7 +99,7 @@ if ($UID == 0) {
 if (defined $logfile) {
 	Log::Any::Adapter->set ('File', $logfile, log_level => $loglevel);
 } else {
-	Log::Any::Adapter->set ('File', '/dev/null');
+	Log::Any::Adapter->set ('File', File::Spec->devnull);
 }
 
 # По-умолчанию, демонизируемся, если явно не указано, что этого делать не надо
@@ -114,7 +115,7 @@ my $apath = abs_path $PROGRAM_NAME;
 unless (defined $apath) {
 	$log->fatal ("[FATAL] Unable to get abs_path: $OS_ERROR\n");
 	exit 1;
-};
+}
 
 my $pidfile = sprintf '%s/%s.pid', $piddir, basename $apath;
 $log->info ("[INFO] Creating pidfile at $pidfile");
@@ -142,7 +143,7 @@ $PROGRAM_NAME = abs_path $PROGRAM_NAME;              ## no critic (Variables::Re
 # И запустим уже наш сервис
 RunPhrases ();
 
-if (-f $pidfile) {
+if (-e $pidfile) {
 	unlink $pidfile;
 }
 
@@ -150,11 +151,11 @@ exit 0;
 
 sub daemonize {
 	__fork ();
-	POSIX::setsid ();
-	open STDIN, '<', '/dev/null';                    ## no critic (InputOutput::RequireCheckedSyscalls, InputOutput::RequireCheckedOpen)
+	setsid ();
+	open STDIN, '<', File::Spec->devnull;                    ## no critic (InputOutput::RequireCheckedSyscalls, InputOutput::RequireCheckedOpen)
 	# we handle STDOUT && STDERR via Log::Any::For::Std, so do not close them explicitly
-	#open STDOUT, '>', '/dev/null';                   ## no critic (InputOutput::RequireCheckedSyscalls, InputOutput::RequireCheckedOpen)
-	#open STDERR, '>', '/dev/null';                   ## no critic (InputOutput::RequireCheckedSyscalls, InputOutput::RequireCheckedOpen)
+	#open STDOUT, '>', File::Spec->devnull;                   ## no critic (InputOutput::RequireCheckedSyscalls, InputOutput::RequireCheckedOpen)
+	#open STDERR, '>', File::Spec->devnull;                   ## no critic (InputOutput::RequireCheckedSyscalls, InputOutput::RequireCheckedOpen)
 	__fork ();
 	umask 0;
 	return;
@@ -178,7 +179,7 @@ sub __fork {
 sub __signal_handler {
 	$log->info ("[INFO] Caught a signal $OS_ERROR");
 
-	if (-f $pidfile) {
+	if (-e $pidfile) {
 		unlink $pidfile;
 	}
 

bin/fortune_seed.pl

@@ -1,6 +1,6 @@
 #!/usr/bin/perl
 
-use 5.018;
+use 5.018; ## no critic (ProhibitImplicitImport)
 use strict;
 use warnings;
 use utf8;

bin/friday_seed.pl

@@ -1,6 +1,6 @@
 #!/usr/bin/perl
 
-use 5.018;
+use 5.018; ## no critic (ProhibitImplicitImport)
 use strict;
 use warnings;
 use utf8;

bin/proverb_seed.pl

@@ -1,6 +1,6 @@
 #!/usr/bin/perl
 
-use 5.018;
+use 5.018; ## no critic (ProhibitImplicitImport)
 use strict;
 use warnings;
 use utf8;

cpanfile

@@ -1,14 +1,17 @@
+## no critic
 requires 'Carp',                    '==1.50';
 requires 'Clone',                   '==0.45';
 requires 'CHI',                     '==0.61';
-requires 'CHI::Driver::BerkeleyDB', '==0.05' ;
+requires 'CHI::Driver::BerkeleyDB', '==0.05';
+requires 'Cwd',                     '==3.75';
 requires 'Cwd::utf8',               '==0.011';
 requires 'Data::Dumper',            '==2.183';
 requires 'Encode',                  '==3.16';
 requires 'English',                 '==1.11';
 requires 'EV',                      '==4.33';
-requires 'File::Basename',          '0';
+requires 'File::Basename',          '==2.85';
 requires 'File::Path',              '==2.18';
+requires 'File::Spec',              '==3.75';
 requires 'JSON',                    '==4.03';
 requires 'JSON::XS',                '==4.03';
 requires 'local::lib',              '==2.000028';
@@ -21,9 +24,9 @@ requires 'Mojo::IOLoop',            '0';
 requires 'Mojo::IOLoop::Signal',    '==0.003';
 requires 'Mojo::Redis',             '==3.27';
 requires 'Mojo::Util',              '0';
-requires 'open',                    '0';
+requires 'open',                    '==1.12';
 requires 'POSIX',                   '0';
 requires 'Protocol::Redis::XS',     '0';
 requires 'SQLite_File',             '==0.1006';
 requires 'utf8',                    '0';
-requires 'version',                 '0';
+requires 'version',                 '==0.9929';

lib/Conf.pm

@@ -1,14 +1,13 @@
 package Conf;
-# Загружает конфиг
+# Загружает конфиг файл
 
-use 5.018;
+use 5.018; ## no critic (ProhibitImplicitImport)
 use strict;
 use warnings;
 use utf8;
 use open qw (:std :utf8);
 use English qw ( -no_match_vars );
-use Encode qw (encode decode);
-use JSON::XS;
+use JSON::XS ();
 
 use version; our $VERSION = qw (1.0);
 use Exporter qw (import);
@@ -16,17 +15,26 @@ our @EXPORT_OK = qw (LoadConf);
 
 sub LoadConf {
 	my $c = 'data/config.json';
-	open my $CH, '<', $c or die "[FATAL] No conf at $c: $OS_ERROR\n";
-	my $json;
+	my $CH;
+
+	unless (open $CH, '<', $c) {
+		die "[FATAL] No conf at $c: $OS_ERROR\n";
+	}
+
 	my $len = (stat $c) [7];
+	my $json;
 	my $readlen = read $CH, $json, $len;
-	$json = encode 'UTF-8', $json;
 
 	unless ($readlen) {
 		close $CH;                                   ## no critic (InputOutput::RequireCheckedSyscalls
 		die "[FATAL] Unable to read $c: $OS_ERROR\n";
 	}
 
+	if ($readlen != $len) {
+		close $CH;                                   ## no critic (InputOutput::RequireCheckedSyscalls
+		die "[FATAL] File $c is $len bytes on disk, but we read only $readlen bytes\n";
+	}
+
 	close $CH;                                       ## no critic (InputOutput::RequireCheckedSyscalls
 	my $j = JSON::XS->new->utf8->relaxed;
 	return $j->decode ($json);

lib/Fortune.pm

@@ -1,17 +1,18 @@
 package Fortune;
 # Прокатят любые источники из fortune_mod. Текстовой файл с разделителем "\n%\n" мнжду фразами.
 
-use 5.018;
+use 5.018; ## no critic (ProhibitImplicitImport)
 use strict;
 use warnings;
 use utf8;
 use open qw (:std :utf8);
+use Encode qw (decode);
 use English qw ( -no_match_vars );
-use Carp qw (cluck croak);
+use Carp qw (croak);
 use File::Path qw (make_path);
 use Log::Any qw ($log);
 use Math::Random::Secure qw (irand);
-use SQLite_File;
+use SQLite_File ();
 
 use Conf qw (LoadConf);
 
@@ -30,7 +31,7 @@ sub Seed () {
 
 	my $backingfile = sprintf '%s/fortune.sqlite', $dir;
 
-	if (-f $backingfile) {
+	if (-e $backingfile) {
 		unlink $backingfile   ||  croak "Unable to remove $backingfile: $OS_ERROR";
 	}
 
@@ -41,7 +42,7 @@ sub Seed () {
 	while (my $fortunefile = readdir $srcdirhandle) {
 		my $srcfile = sprintf '%s/%s', $srcdir, $fortunefile;
 
-		unless (-f $srcfile) {
+		unless (-e $srcfile) {
 			next;
 		}
 
@@ -77,7 +78,7 @@ sub Fortune () {
 
 	my $phrase = $array[irand ($#array + 1)];
 	# decode?
-	utf8::decode $phrase;
+	$phrase = decode 'UTF-8', $phrase;
 	untie @array;
 	return $phrase;
 }

lib/Friday.pm

@@ -1,17 +1,18 @@
 package Friday;
 # Данные модуля спёрты с https://raw.githubusercontent.com/isida/vi/master/data/friday.txt
 
-use 5.018;
+use 5.018; ## no critic (ProhibitImplicitImport)
 use strict;
 use warnings;
 use utf8;
 use open qw (:std :utf8);
+use Encode qw (decode);
 use English qw ( -no_match_vars );
-use Carp qw (cluck croak);
+use Carp qw (croak);
 use File::Path qw (make_path);
 use Log::Any qw ($log);
 use Math::Random::Secure qw (irand);
-use SQLite_File;
+use SQLite_File ();
 
 use Conf qw (LoadConf);
 
@@ -34,7 +35,7 @@ sub Seed () {
 	for (0..$#dow) {
 		my $backingfile = sprintf '%s/%s.sqlite', $dir, $dow[$_];
 
-		if (-f $backingfile) {
+		if (-e $backingfile) {
 			unlink $backingfile   ||  croak "Unable to remove $backingfile: $OS_ERROR\n";
 		}
 
@@ -87,7 +88,7 @@ sub Friday () {
 	};
 
 	my $phrase = $array[irand ($#array + 1)];
-	utf8::decode $phrase;
+	$phrase = decode 'UTF-8', $phrase;
 	untie @array;
 	return $phrase;
 }

lib/Karma.pm

@@ -1,15 +1,15 @@
 package Karma;
+# Хранит и выдаёт карму фразы
 
-use 5.018;
+use 5.018; ## no critic (ProhibitImplicitImport)
 use strict;
 use warnings;
 use utf8;
 use open qw (:std :utf8);
 
 use English qw ( -no_match_vars );
-use Carp qw (cluck);
-use CHI;
-use CHI::Driver::BerkeleyDB;
+use CHI ();
+use CHI::Driver::BerkeleyDB ();
 use Log::Any qw ($log);
 use Mojo::Util qw (trim);
 
@@ -23,22 +23,26 @@ my $c = LoadConf ();
 my $cachedir = $c->{cachedir};
 my $max = 5;
 
-# swallow phrase and return answer
 sub KarmaSet (@) {
 	my $chatid = shift;
 	my $phrase = shift;
 	my $action = shift;
 
+	unless (defined $phrase) {
+		$phrase = '';
+	}
+
 	$phrase = trim ($phrase);
 
-	if ((! defined $phrase) || $phrase eq '') {
+	# Костылёк для кармы пустоты
+	if ($phrase eq '') {
 		$phrase = ' ';
 	}
 
 	my $cache = CHI->new (
 		driver => 'BerkeleyDB',
 		root_dir => $cachedir,
-		namespace => __PACKAGE__ . '_' . $chatid
+		namespace => __PACKAGE__ . '_' . $chatid,
 	);
 
 	my $score = $cache->get ($phrase);
@@ -81,16 +85,21 @@ sub KarmaGet (@) {
 	my $chatid = shift;
 	my $phrase = shift;
 
+	unless (defined $phrase) {
+		$phrase = '';
+	}
+
 	$phrase = trim ($phrase);
 
-	if ((! defined $phrase) || $phrase eq '') {
+	# Костылёк для кармы пустоты
+	if ($phrase eq '') {
 		$phrase = ' ';
 	}
 
 	my $cache = CHI->new (
 		driver => 'BerkeleyDB',
 		root_dir => $cachedir,
-		namespace => __PACKAGE__ . '_' . $chatid
+		namespace => __PACKAGE__ . '_' . $chatid,
 	);
 
 	my $score = $cache->get ($phrase);

lib/Phrases.pm

@@ -1,7 +1,7 @@
 package Phrases;
 
 # Общие модули - синтаксис, кодировки итд
-use 5.018;
+use 5.018; ## no critic (ProhibitImplicitImport)
 use strict;
 use warnings;
 use utf8;
@@ -12,12 +12,12 @@ use Clone qw (clone);
 use Log::Any qw ($log);
 use Math::Random::Secure qw (irand);
 # Чтобы "уж точно" использовать hiredis-биндинги, загрузим этот модуль перед Mojo::Redis
-use Protocol::Redis::XS;
-use Mojo::Redis;
-use Mojo::IOLoop;
-use Mojo::IOLoop::Signal;
+use Protocol::Redis::XS ();
+use Mojo::Redis ();
+use Mojo::IOLoop ();
+use Mojo::IOLoop::Signal ();
 use Mojo::Util qw (trim);
-use Data::Dumper;
+use Data::Dumper qw (Dumper);
 
 use Conf qw (LoadConf);
 use Fortune qw (Fortune);
@@ -67,11 +67,14 @@ my $parse_message = sub {
 		$answer->{misc}->{msg_format} = 0;
 	}
 
+	$answer->{userid} = 0 + $m->{userid};
+	$answer->{chatid} = 0 + $m->{chatid};
+
 	$log->debug ('[DEBUG] Incoming message ' . Dumper ($m));
 
 	# Нормальная команда
-	if (substr ($m->{message}, 0, 1) eq $answer->{misc}->{csign}) {
-		my $cmd = substr ($m->{message}, 1);
+	if (substr ($m->{message}, 0, length ($answer->{misc}->{csign})) eq $answer->{misc}->{csign}) {
+		my $cmd = substr $m->{message}, length ($answer->{misc}->{csign});
 
 		if ($cmd eq 'friday'  ||  $cmd eq 'пятница') {
 			$reply = Friday ();
@@ -94,36 +97,60 @@ my $parse_message = sub {
 			} else {
 				$reply = trim (Fortune ());
 			}
-		} elsif ($cmd =~ /^(karma|карма)\s(.+)/su) {
-			# TODO: избавиться от регулярки в пользу манипуляци со строкой через substr
-			$reply = KarmaGet ($m->{chatid}, $2);
+		} elsif (length ($cmd) >= length ('karma ')  &&  substr ($cmd, 0, length ('karma ')) eq 'karma ') {
+			# По-умолчанию достаём карму пустоты
+			my $text = undef;
+
+			if (length ($cmd) > length ('karma ')) {
+				$text = substr $cmd, length ('karma ');
+			}
+
+			$reply = KarmaGet ($m->{chatid}, $text);
+		} elsif (length ($cmd) >= length ('карма ')  &&  substr ($cmd, 0, length ('карма ')) eq 'карма ') {
+			# По-умолчанию достаём карму пустоты
+			my $text = undef;
+
+			if (length ($cmd) > length ('карма ')) {
+				$text = substr $cmd, length ('карма ');
+			}
+
+			$reply = KarmaGet ($m->{chatid}, $text);
+		# Достаём карму пустоты :)
+		} elsif ($cmd eq 'karma') {
+			$reply = KarmaGet ($m->{chatid}, undef);
+		# Достаём карму пустоты :)
+		}elsif ($cmd eq 'карма') {
+			$reply = KarmaGet ($m->{chatid}, undef);
 		}
 	# Скорее всего изменение кармы, но это не точно
-	} elsif (substr ($m->{message}, -2) eq '++'  ||  substr ($m->{message}, -2) eq '--') {
-		my @arr = split /\n/s, $m->{message};
+	} elsif (length ($m->{message}) > length ('++')) {
+		if (substr ($m->{message}, 0 - length ('++')) eq '++'  ||  substr ($m->{message}, 0 - length ('--')) eq '--') {
+			my @arr = split /\n/s, $m->{message};
 
-		# Предполагается, что фраза - это только одна строка
-		if ($#arr < 1) {
-			$reply = KarmaSet (
-				$m->{chatid},
-				trim (substr $m->{message}, 0, -2),
-				substr $m->{message}, -2
-			);
-		# Если у нас более одной строки - пусть с этим разбирается craniac, у него есть *мозги*
-		} else {
-			$send_to = 'craniac';
-			$reply = $m->{message};
+			# Предполагается, что фраза - это только одна строка
+			if ($#arr < 1) {
+				$reply = KarmaSet (
+					$m->{chatid},
+					trim (substr $m->{message}, 0, 0 - length ('++')),
+					substr $m->{message}, 0 - length ('++'),
+				);
+			# Если у нас более одной строки - пусть с этим разбирается craniac, у него есть *мозги*
+			} else {
+				$send_to = 'craniac';
+				$reply = $m->{message};
+			}
 		}
 	}
 
-	$log->debug ("[DEBUG] Sending message to channel $send_to " . Dumper ($answer));
-
 	if (defined $reply && $answer->{misc}->{answer}) {
 		$answer->{message} = $reply;
+		$log->debug ("[DEBUG] Sending message to channel $send_to " . Dumper ($answer));
 
 		$self->json ($send_to)->notify (
-			$send_to => $answer
+			$send_to => $answer,
 		);
+	} else {
+		$log->debug ('[DEBUG] No reply generated, skip sending message.');
 	}
 
 	return;
@@ -133,7 +160,7 @@ my $__signal_handler = sub {
 	my ($self, $name) = @_;
 	$log->info ("[INFO] Caught a signal $name");
 
-	if (defined $main::pidfile && -f $main::pidfile) {
+	if (defined $main::pidfile && -e $main::pidfile) {
 		unlink $main::pidfile;
 	}
 
@@ -146,7 +173,7 @@ sub RunPhrases {
 	$log->info ("[INFO] Connecting to $c->{server}, $c->{port}");
 
 	my $redis = Mojo::Redis->new (
-		sprintf 'redis://%s:%s/1', $c->{server}, $c->{port}
+		sprintf 'redis://%s:%s/1', $c->{server}, $c->{port},
 	);
 
 	$log->info ('[INFO] Registering connection-event callback');
@@ -163,11 +190,11 @@ sub RunPhrases {
 					my ($conn, $error) = @_;
 					$log->error ("[ERROR] Redis connection error: $error");
 					return;
-				}
+				},
 			);
 
 			return;
-		}
+		},
 	);
 
 	my $pubsub = $redis->pubsub;
@@ -178,13 +205,13 @@ sub RunPhrases {
 		$log->debug ("[DEBUG] Subscribing to $channel");
 
 		$sub->{$channel} = $pubsub->json ($channel)->listen (
-			$channel => sub { $parse_message->(@_); }
+			$channel => sub { $parse_message->(@_); },
 		);
 	}
 
 	Mojo::IOLoop::Signal->on (
 		TERM => $__signal_handler,
-		INT  => $__signal_handler
+		INT  => $__signal_handler,
 	);
 
 	do { Mojo::IOLoop->start } until Mojo::IOLoop->is_running;

lib/Proverb.pm

@@ -1,18 +1,19 @@
 package Proverb;
 # Пословицы и поговорки
 
-use 5.018;
+use 5.018; ## no critic (ProhibitImplicitImport)
 use strict;
 use warnings;
 use utf8;
 use open qw (:std :utf8);
+use Encode qw (decode);
 use English qw ( -no_match_vars );
-use Carp qw (cluck croak);
+use Carp qw (croak);
 use File::Path qw (make_path);
 use Log::Any qw ($log);
 use Math::Random::Secure qw (irand);
 use Mojo::Util qw (trim);
-use SQLite_File;
+use SQLite_File ();
 
 use Conf qw (LoadConf);
 
@@ -31,7 +32,7 @@ sub Seed () {
 
 	my $backingfile = sprintf '%s/proverb.sqlite', $dir;
 
-	if (-f $backingfile) {
+	if (-e $backingfile) {
 		unlink $backingfile   ||  croak "Unable to remove $backingfile: $OS_ERROR";
 	}
 
@@ -41,7 +42,7 @@ sub Seed () {
 	while (my $proverbfile = readdir $srcdirhandle) {
 		my $srcfile = sprintf '%s/%s', $srcdir, $proverbfile;
 
-		unless (-f $srcfile) {
+		unless (-e $srcfile) {
 			next;
 		}
 
@@ -79,7 +80,7 @@ sub Proverb () {
 
 	my $phrase = $array[irand ($#array + 1)];
 	# decode?
-	utf8::decode $phrase;
+	$phrase = decode 'UTF-8', $phrase;
 	untie @array;
 	return $phrase;
 }