четверг, 10 декабря 2009 г.

Постепенное применение стандартов кодирования с помощью Test::Perl::Critic::Progressive

Введение

Применение стандартов кодирования (Perl Best Practices, если быть более точным) к существующему коду - задача не из простых. По началу кажется, что для того чтобы код прошёл проверку Perl::Critic-ом необходимо переписать практически всё. Однако в большинстве случаев такой подход является либо нерациональным, либо просто невозможным. Именно для таких ситуаций Jeffrey Thalhammer и разработал модуль Test::Perl::Critic::Progressive. Как следует из названия - это тестовый модуль. Он прогоняет код через Perl::Critic практически так же как это делает модуль Test::Perl::Critic, но с одним исключением: Test::Perl::Critic::Progressive не ругается на уже существующие нарушения в коде, но в то же время препятствует появлению новых. Кроме того, замена одних нарушений другими также пресекается. Это как раз то, что нужно для поддержки уже существующих программных проектов.

Принцип работы модуля достаточно прост: во время первого запуска тестов Test::Perl::Critic::Progressive сканирует код и сохраняет все обнаруженные в нём нарушения в так-называемом history-файле. Фактически, это просто текстовый файл, в котором записано какие нарушения и в каком количестве были обнаружены. Во время последующих запусков Test::Perl::Critic::Progressive сверяет данные об обнаруженных нарушениях с тем, что сохранено в history-файле: если нарушений хотя бы одного типа стало больше - то тест будет провален.

Пример

Допустим имеется программный проект следующей структуры:
legacy-project/
|-- lib
| `-- SomeModule.pm
`-- t
|-- perl-critic-progressive.t
`-- perlcritic-profile

2 directories, 3 files

SomeModule.pm:
package SomeModule;

#===============================================================================
# REVISION: $Id:$
# AUTHORS: Alexander Simakov, <xdr (dot) box (at) Google Mail>
# CREATED: 2009-12-10 20:48:19
# DESCRIPTION: This module worked for us for many years!
#===============================================================================

use strict;

# Oops! we don't use warnings
# Oops! we have no our $VERSION!

sub do_something($$) { # Oops! We use prototypes
my $name = shift;

# Oops! We use "" for literal string instead of ''
my $result = "Hello " . $name;

return $result;
}

1;


В этом модуле, по мнению Perl::Critic-а, имеется ряд нарушений. Для удобства все они отмечены комментариями "Oops!". Предположим, что это и есть те самые застарелые проблемы (которых в реальной ситуации будет конечно больше), которые в настоящий момент устранять не хочется.

perl-critic-progressive.t:
#!/usr/bin/perl

#===============================================================================
# REVISION: $Id:$
# AUTHORS: Alexander Simakov, <xdr (dot) box (at) Google Mail>
# CREATED: 2009-12-10 20:59:28
# DESCRIPTION: Test everything in lib/ using Test::Perl::Critic::Progressive
#===============================================================================

use strict;
use warnings;

use Test::Perl::Critic::Progressive qw(:all);

# For building relative paths
use FindBin qw($Bin);
FindBin::again();

use Readonly; # Instead of "use constant" pragma

Readonly my $ROOT => "$Bin/../lib";
Readonly my $PROFILE => "$Bin/perlcritic-profile";
Readonly my $HISTORY => "$Bin/perlcritic-history";

{
# Set file for storing history data
set_history_file($HISTORY);

# Setup Perl::Critic parameters
set_critic_args( -profile => $PROFILE );

# Critique files starting from $ROOT
progressive_critic_ok($ROOT);
}


Этот тест прогоняет весь Perl-код начиная с указанной директории через Perl::Critic. Конфигурация самого Perl::Critic-а задается в отдельном профиле (см. ниже). Также следует обратить внимание на history-файл, о котором шла речь выше. Если файла ещё нет, то он будет создан при первом запуске. Наличие же данного файла является сигналом Test::Perl::Critic::Progressive-у заступить на охрану вашего кода от нарушений.

perlcritic-profile:
severity = brutal
verbose = 8
profile-strictness = fatal

Итак, всё готово для запуска тестов:
prove t/

t/perl-critic-progressive....ok
All tests successful.
Files=1, Tests=1, 2 wallclock secs ( 1.85 cusr + 0.09 csys = 1.94 CPU)

Как видите, тест прошёл успешно, в время как perlcritic высказал бы своё недовольство по поводу данного кода:

perlcritic -profile t/perlcritic-profile lib/SomeModule.pm

[Modules::RequireVersionVar] No "VERSION" variable found at line 1, column 1. (Severity: 2)
[Subroutines::ProhibitSubroutinePrototypes] Subroutine prototypes used at line 15, column 1. (Severity: 5)
[TestingAndDebugging::RequireUseWarnings] Code before warnings are enabled at line 15, column 1. (Severity: 4)
[ValuesAndExpressions::ProhibitInterpolationOfLiterals] Useless interpolation of literal string at line 19, column 18. (Severity: 1)

Вот как выглядит history-файл (для наглядности большая часть нулевых счётчиков не показа):
$VAR1 = [
{
'Perl::Critic::Policy::ValuesAndExpressions::RequireUpperCaseHeredocTerminator' => 0,
'Perl::Critic::Policy::References::ProhibitDoubleSigils' => 0,
...
'Perl::Critic::Policy::Modules::RequireVersionVar' => 1,
'Perl::Critic::Policy::TestingAndDebugging::RequireUseWarnings' => 1,
...
'Perl::Critic::Policy::Subroutines::ProhibitSubroutinePrototypes' => 1,
...
'Perl::Critic::Policy::ValuesAndExpressions::ProhibitInterpolationOfLiterals' => 1,
...
'Perl::Critic::Policy::BuiltinFunctions::ProhibitVoidGrep' => 0,
'Perl::Critic::Policy::InputOutput::RequireBriefOpen' => 0
}
];

Теперь, предположим что файл SomeModule.pm претерпел изменения и теперь он выглядит следующим образом:

package SomeModule;

#===============================================================================
# REVISION: $Id:$
# AUTHORS: Alexander Simakov, <xdr (dot) box (at) Google Mail>
# CREATED: 2009-12-10 20:48:19
# DESCRIPTION: This module worked for us for many years!
#===============================================================================

use strict;
use warnings; # Yes! we use warnings now: -1 violation

# Oops! we have no our $VERSION!

sub do_something($$) { # Oops! We use prototypes
my $name = shift;

# Oops! We use "" for literal string instead of ''
my $result = "Hello " . $name;

return $result;
}

sub camelStyle($) { # Camel style, prototypes: +2 violations
my $x = shift;

return $x**2;
}

1;

Обратите внимание, что нарушений одного типа стало меньше, а других типов - больше. Но даже если бы общая сумма нарушений и уменьшилась, увеличение хотя бы одного из счётчиков не позволило бы тесту пройти успешно:

prove t/

t/perl-critic-progressive....# NamingConventions::ProhibitMixedCaseSubs: Got 1 violation(s). Expected no more than 0.
# Subroutines::ProhibitSubroutinePrototypes: Got 2 violation(s). Expected no more than 1.
# Too many Perl::Critic violations...
# Got a total of 5. Expected no more than 4.
t/perl-critic-progressive....NOK 1/1
# Failed test 'Test::Perl::Critic::Progressive'
# at t/perl-critic-progressive.t line 33.
# Looks like you failed 1 test of 1.
t/perl-critic-progressive....dubious
Test returned status 1 (wstat 256, 0x100)
DIED. FAILED test 1
Failed 1/1 tests, 0.00% okay
Failed Test Stat Wstat Total Fail List of Failed
-------------------------------------------------------------------------------
t/perl-critic-progressive.t 1 256 1 1 1
Failed 1/1 test scripts. 1/1 subtests failed.
Files=1, Tests=1, 3 wallclock secs ( 2.08 cusr + 0.09 csys = 2.17 CPU)
Failed 1/1 test programs. 1/1 subtests failed.


Недостатки

Как можно было понять из вывода команды prove и содержимого history-файла - Test::Perl::Critic::Progressive не запоминает в каких файлах и в каком месте были обнаружены нарушения. Фиксируется только факт нарушения и соответствующий счётчик. Если информация о файлах необходима, то соответствующую логику придётся реализовывать самостоятельно.

Ссылки

четверг, 3 декабря 2009 г.

Using Perl::Critic in Subversion hook-script

Overview

A few weeks ago I wrote subversion hook-script that checks Perl code using Perl::Critic module. The script is called perlcritic-checker.pl If the code has violations then the entire commit will be rejected. The appropriate error message (colored and sorted by severity level) will be sent back to the client. This way you can employ consistent coding rules in your team.

The script can be configured to apply different Perl::Critic policy profiles for different paths in the repository. For example, you can be very strict with brand new projects, make an indulgence for legacy projects and completely disable Perl::Critic for auto-generated and third-party code.

The code and configuration file examples can be downloaded from here.

Discussion thread on PerlMonks

Who is who

Damian Conway - the author of "Perl Best Practices". The book on which Perl::Critic is mostly based. Jeffrey Thalhammer - the author of Perl::Critic module.

See also