Pull to refresh

Perl::Critic + Subversion = внедрение единых практик кодирования в команде

Reading time 13 min
Views 2.8K
Язык Perl хорошо известен той степенью свободы (a.k.a. TIMTOWTDI), которую он даёт программисту в выборе способа решения той или иной задачи. У этой медали, к сожалению, есть и оборотная сторона, которая может проявиться при командной разработке крупных проектов. Если в команде нет единых практик кодирования и каждый из разработчиков придерживается принципа TIMTOWTDI, то новичку в таком коллективе не позавидуешь.

В 2005 году активный участник Perl-сообщества Дамиан Конвей (Damian Conway) опубликовал книгу Perl Best Practices, в которой собрал и структурировал 256 рекомендаций по написанию понятного, надёжного и поддерживаемого Perl-кода. Краткую шпаргалку с выжимкой из книги можно скачать отсюда.

Годом позже, Jeffrey Thalhammer и группа товарищей выпустила Perl::Critic — гибкий и расширяемый фреймворк, позволяющий автоматизировать проверку Perl-кода на предмет его соответствия большей части рекомендаций из книги Конвея, а также многих других полезных практик.

Perl::Critic подаётся под разными соусами: во-первых, в комплекте с модулем поставляется одноимённая утилита — perlcritic, во-вторых, проверку кода можно оформить в виде тестов с помощью Test::Perl::Critic либо Test::Perl::Critic::Progressive, в-третьих, критик легко интегрируется в VIM и Emacs.

В этом рецепте я расскажу о том как проверять Perl-код на лету при коммите в Subversion-репозиторий. Bon Appétit!



Механизм работы триггеров в Subversion


Subversion, как и многие другие системы контроля версий, позволяет выполнять произвольные пользовательские команды в заданные фазы обработки коммита. В частности, в Subversion имеется триггер (hook) pre-commit, который запускается непосредственно перед тем как коммит окончательно попадёт в репозиторий. По сути, это просто одноимённый shell-скрипт который Subversion рассчитывает найти в директории hooks/ репозитория. В качестве одного из своих аргументов данный скрипт получает идентификатор текущей транзакции. По этому идентификатору скрипт может полностью проанализировать текущий коммит: какие файлы или директории были добавлены, удалены, изменены, получить новый контент файлов, diff изменений и т.д. Для этой цели можно воспользоваться утилитой svnlook либо модулем SVN::Look.

Если pre-commit hook вернёт 0, то коммит будет благополучно сохранён, а если нет — то вся транзакция будет отвергнута (т.е. все файлы, директории, изменения свойств). При этом вывод скрипта на STDERR будет перенаправлен svn-клиенту. Таким образом, вызывая Perl::Critic внутри pre-commit hook-а можно преградить коду с нарушениями дорогу в репозиторий.

От теории к практике


1. Загружаем и устанавливаем скрипт perlcritic-checker.pl

wget http://perlcritic-checker.googlecode.com/files/perlcritic-checker-1.2.6.tar.gz
tar -xzf perlcritic-checker-1.2.6.tar.gz
cd perlcritic-checker-1.2.6
perl ./Makefile.PL
make
make test
sudo make install

По-умолчанию, скрипт и документация будут установлены в /usr/local.

2. Создаём тестовый SVN-репозиторий

export SVN_REPO=/tmp/svn_repo
svnadmin create $SVN_REPO

3. Устанавливаем конфигурационные файлы

Во-первых, установим конфиг perlcritic-checker'а:

cat > $SVN_REPO/hooks/perlcritic-checker.conf<<'EOF'
#
# perlcritic-checker's config example
#
{
    # Progressive mode: {0|1}. In progressive mode perlcritic-checker
    # doesn't complain about existing violations but prevents
    # introducing new ones. Nice feature for applying Perl::Critic
    # to the existing projects gradually.
    progressive_mode => 1,

    # Emergency commits: {0|1}. There are situations when you *do* need
    # to commit changes bypassing all checks (e.g. emergency bug fixes).
    # This featue allows you bypass Perl::Critic using "magic" prefix in
    # comment message, e.g.: svn ci -m "NO CRITIC: I am in hurry" FooBar.pm
    allow_emergency_commits => 1,

    # Magic prefix described above can be customized:
    emergency_comment_prefix => 'NO CRITIC',

    # Limit maximal number of reported violations. This parameter works
    # differently in strict and progressive modes. In strict mode it
    # will truncate long list of violations: only N most severe violations
    # will be shown. In progressive mode such behaviour has no sense,
    # that's why user will be asked to run perlcritic locally.
    #
    # In fact, this parameter is a workaround for a subtle bug in generic
    # svn-client that happens when svn hook (i.e. perlcritic-checker.pl)
    # outputs too much data: svn-client just reports "Connection closed
    # unexpectedly". In order to reproduce this bug several additional
    # conditions should be met:
    # - repository access scheme: 'svn://' (svnserve daemon)
    # - client and server on different machines
    # - svn-client and -server are running on linux
    #
    # If you face the same problem, try to use the option below.
    #max_violations => 50,

    # SVN repository path -- to -- Perl::Critic's profile mapping.
    #
    # This feature allows you to apply different Perl::Critic's
    # policies for different paths in the repository. For example,
    # you can be very strict with brand-new projects, make an
    # indulgence for some existing project and completely disable
    # checking of auto-generated or third-party code.
    #
    # Each modified (added, updated, copied, moved) file name in the
    # repository is matched against a sequence of patterns below.
    # Keep in mind, *last* matching rule - wins.
    #
    # Profile paths can be either absolute or relative. In the later
    # case they will be mapped under $REPOS/hooks/perlcritic.d directory.
    profiles => [

        # Apply very strict profile by default
        {   pattern => qr{[.](pm|pl|t)$},
            profile => 'perlcritic-brutal.conf',
        },
    ],
}
EOF

В этой конфигурации сказано, что настройки Perl::Critic'a для всех *.pm, *.pl и *.t файлов необходимо брать из файла perlcritic-brutal.conf. Также мы включаем прогрессивный режим (читай ниже) и разрешаем экстренные коммиты, т.е. коммиты проверка которых не производится (NO CRITIC).

Следующим шагом устанавливаем профиль Perl::Critic'а (perlcritic-brutal.conf), на который мы сослались в предыдущем файле:

mkdir $SVN_REPO/hooks/perlcritic.d
cat > $SVN_REPO/hooks/perlcritic.d/perlcritic-brutal.conf<<'EOF'
#
# Perl::Critic profile example
#

# Make perlcritic very exacting
severity = brutal

# You can choose any level from 1 to 11, but 8 is recommended
verbose = 8

# Colorize violations depending on their severity level
color = 1

# Halt if this file contains errors
profile-strictness = fatal

# Enable only two groups of policies: Core and Perl Best Practices
theme = core || pbp

# Explicitly set full path to Perl::Tidy's config
[CodeLayout::RequireTidyCode]
perltidyrc = /etc/perltidyrc
EOF

В этом профиле включается самый строгий режим работы критика (severity = brutal), в котором он будет придираться даже к мелочам (однако мы то знаем, что в авиации мелочей не бывает). Директива theme указывает, что из всего множества доступных Policy, а их около полутора сотен, проверять необходимо только те, которые числятся в группах Core или Perl Best Practices. Также мы явно указываем путь к настройкам программы perltidy — утилиты для автоматического форматирования Perl-кода. Нарушение форматирования — это одна из причин, по которым коммит может не пройти.

Отмечу, что этот профиль содержит только базовые настройки, однако это хорошая отправная точка. Более подробно о возможностях Perl::Critic'а можно почитать в документации: perldoc Perl::Critic.

Далее создаём конфиг для perltidy (с привилегиями root-а):

sudo su -
cat > /etc/perltidyrc<<'EOF'
--perl-best-practices
EOF
exit

Этот файл крайне прост: он инструктирует perltidy форматировать код согласно рекомендациям из уже упомянутой книги Perl Best Practices. Если вы привыкли к другому стилю, то выбрать нужные опции форматирования поможет страница руководства perltidy(1p).

Имя и путь файла выбраны не случайно. По умолчанию, perltidy сперва проверяет пользовательский конфиг $HOME/.perltidyrc, а затем уже глобальный — /etc/perltidyrc. Таким образом, поместив конфиг в /etc мы сделаем его одновременно доступным как для perlcritic-checker'а (работающего обычно под пользователем svn) так и для самой программы perltidy, какой бы пользователь её не запустил.

Наконец, нам осталось проинструктировать Subversion вызывать скрипт perlcritic-checker.pl для проверки коммитов. Как нетрудно догадаться, для этого нужно отредактировать pre-commit hook:

cat > $SVN_REPO/hooks/pre-commit<<'EOF'
#!/bin/sh

#
# SVN pre-commit hook script example
#

REPOS="$1"
TXN="$2"
PREFIX="/usr/local"

# Make sure that Perl code comply to coding standards
$PREFIX/bin/perlcritic-checker.pl                        \
    --repository  "$REPOS"                               \
    --config      "$REPOS/hooks/perlcritic-checker.conf" \
    --transaction "$TXN" || exit 1

# All checks have passed, so allow the commit
exit 0
EOF
chmod 755 $SVN_REPO/hooks/pre-commit

На этом этап настройки завершён. Всё готово для первого эксперимента.

Хороший, плохой, прогрессивный


Итак, создадим в репозитории папку для тестового проекта и сделаем checkout рабочей копии:

export WORK_COPY="/tmp/test_project"
svn mkdir file://$SVN_REPO/test_project -m ""
svn checkout file://$SVN_REPO/test_project $WORK_COPY

Для эксперимента нам понадобится «хороший» файл (файл без нарушений):

cat > $WORK_COPY/good_file.pl<<'EOF'
#!/usr/bin/perl

#===============================================================================
#     REVISION:  $Id$
#  DESCRIPTION:  File without violations
#===============================================================================

use strict;
use warnings;

our $VERSION = '1.0';

sub main {
    return;
}

main();
EOF
chmod 755 $WORK_COPY/good_file.pl

и «плохой» файл (файл с нарушениями):

cat > $WORK_COPY/bad_file.pl<<'EOF'
#!/usr/bin/perl

#===============================================================================
#     REVISION:  $Id$
#  DESCRIPTION:  File with violations
#===============================================================================

use strict;

#use warnings;

our $VERSION = "1" . "." . "0";

sub main() {
    return undef;
}

main();
EOF
chmod 755 $WORK_COPY/bad_file.pl

Коммит хорошего файла должен пройти без проблем:

cd $WORK_COPY
svn add good_file.pl
svn commit -m "This commit should be OK" good_file.pl

А вот коммит плохого файла должен вызвать недовольство критика:

cd $WORK_COPY
svn add bad_file.pl
svn commit -m "This commit should fail" bad_file.pl


Adding         bad_file.pl
Transmitting file data .svn: Commit failed (details follow):
svn: Commit blocked by pre-commit hook (exit code 1) with output:
test_project/bad_file.pl: [Subroutines::ProhibitExplicitReturnUndef] "return" statement with explicit "undef" at line 15, column 5.  (Severity: 5)
test_project/bad_file.pl: [Subroutines::ProhibitSubroutinePrototypes] Subroutine prototypes used at line 14, column 1.  (Severity: 5)
test_project/bad_file.pl: [TestingAndDebugging::RequireUseWarnings] Code before warnings are enabled at line 12, column 1.  (Severity: 4)
test_project/bad_file.pl: [ValuesAndExpressions::ProhibitNoisyQuotes] Quotes used with a noisy string at line 12, column 22.  (Severity: 2)
test_project/bad_file.pl: [ValuesAndExpressions::ProhibitInterpolationOfLiterals] Useless interpolation of literal string at line 12, column 16.  (Severity: 1)
test_project/bad_file.pl: [ValuesAndExpressions::ProhibitInterpolationOfLiterals] Useless interpolation of literal string at line 12, column 22.  (Severity: 1)
test_project/bad_file.pl: [ValuesAndExpressions::ProhibitInterpolationOfLiterals] Useless interpolation of literal string at line 12, column 28.  (Severity: 1)
---
You can bypass all checks by placing 'NO CRITIC' in the begining of the comment message,
e.g.: svn ci -m "NO CRITIC: emergency hotfix" FooBar.pm

image

Видно, что нарушения отсортированы по убыванию степени серьёзности (Severity), а также раскрашены в различные цвета. Если нарушение относится к конкретной строке файла, а таких большинство, то указывается номер строки и номер позиции на этой строке. Если по названию и краткому описанию Policy не понятна его суть, то получить расширенную справку можно так: perlcritic --doc TestingAndDebugging::RequireUseWarnings | less

Отмечу, что в зависимости от версии модуля Perl::Critic и списка установленных Policy, вывод может несколько отличаться (чтобы уменьшить вероятность этого расхождения мы специально сузили список активных Policy при помощи директивы theme в файле perlcritic-brutal.conf).

Но вернёмся обратно к нашему плохому файлу. Получив отказ от Subversion'а принять наш коммит, можно пойти двумя путями. Первый путь (правильный) — устранить нарушения и попробовать ещё раз. Второй путь (вынужденный) — отметить коммит как экстренный и обойти тем самым все проверки. Вот как это выглядит:

cd $WORK_COPY
svn commit -m "NO CRITIC: I am in hurry and I dont care" bad_file.pl

В таком виде коммит не будет подвергнут критике и попадёт в репозиторий минуя отдел цензуры.

Отмечу, что экстренные коммиты это крайняя мера и на практике они используются очень редко. Помимо непосредственного устранения нарушения есть и другие способы пройти проверку. Один из них — специальный комментарий для Perl::Critic'а вида

## no critic (Subroutines::ProhibitExplicitReturnUndef)

помещённый в конце строки, где было обнаружено нарушение (внимание — ##). В скобках через запятую можно перечислять и несколько Policy сразу. Если же этот комментарий помещён на отдельной строке, то его действие будет распространятся вплоть до завершения текущего блока. Как следствие, комментарий, помещённый вне блоков, будет действовать вплоть до конца файла. Для того чтобы включить критика досрочно (т.е. до завершения блока), достаточно на отдельной строке сказать:

## use critic

Хочу ещё раз обратить внимание на два символа '#' и на тот факт, что выключаются Policy поимённо, а включаются все сразу.

Ну и, наконец, если вы поймаете себя на том, что раз за разом отключаете в коде одну и ту же Policy, подумайте, не лучше ли отключить её один раз в профиле Perl::Critic'а. Потом, некоторые Policy можно сконфигурировать. Зачастую в конфигурации Policy можно переопределить пороговые значения её срабатывания, задать список исключений, поменять режим работы и т.д.

Вот мы и подобрались к самому интересному — прогрессивному режиму работы perlcritic-checker'а. Без этой возможности, было бы невозможно внедрить данную систему в рамках крупных уже существующих Perl-проектов. Как показала практика, количество нарушений в некритикованном ранее коде (даже на нестрогом уровне severity) просто зашкаливает. Модифицировать львиную долю кода, причём работающего кода, только ради Perl::Critic'а нецелесообразно, а злоупотребление экстренными коммитами (NO CRITIC) сведёт ценность данной системы к нулю.

Итак, как же работает прогрессивный режим perlcritic-checker'а? В прогрессивном режиме perlcritic-checker для каждого изменённого файла сравнивает список нарушений ДО и список нарушений СЕЙЧАС. Оба списка нарушений группируются по названию Policy и снабжаются счётчиком встречаемости. Так вот, если в текущем коммите в файл будет добавлено хоть одно нарушение которого небыло раньше, то весь коммит будет отвергнут, а пользователю будет отправлен отечет, в котором будет указано нарушений какого типа было добавлено и в каком количестве. Таким образом, perlcritic-checker закрывает глаза на уже имеющиеся нарушения, но запрещает вносить новые.

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

Продемонстрируем на примере. Как мы помним, не смотря на то, что «плохой» файл содержал нарушения, мы настояли на его коммите при помощи NO CRITIC. Внесём в этот файл новых нарушений и исправим что-нибудь из уже имеющихся (а простит ли perlcritic-checker отсутствие use strict в обмен на включённый use warnings?):

cat > $WORK_COPY/bad_file.pl<<'EOF'
#!/usr/bin/perl

#===============================================================================
#     REVISION:  $Id$
#  DESCRIPTION:  File with violations (version 2)
#===============================================================================

#use strict;
use warnings;

our $VERSION = "1" . "." . "0";

sub foo() {
    1 + 1;
}

sub main() {
    return undef;
}

main();
EOF
cd $WORK_COPY
svn commit -m "Add more violations to file" bad_file.pl


Sending        bad_file.pl
Transmitting file data .svn: Commit failed (details follow):
svn: Commit blocked by pre-commit hook (exit code 1) with output:
test_project/bad_file.pl: [Subroutines::ProhibitExplicitReturnUndef] "return" statement with explicit "undef" at line 18, column 5.  (Severity: 5)
test_project/bad_file.pl: [Subroutines::ProhibitSubroutinePrototypes] Subroutine prototypes used at line 13, column 1.  (Severity: 5)
test_project/bad_file.pl: [Subroutines::ProhibitSubroutinePrototypes] Subroutine prototypes used at line 17, column 1.  (Severity: 5)
test_project/bad_file.pl: [TestingAndDebugging::RequireUseStrict] Code before strictures are enabled at line 11, column 1.  (Severity: 5)
test_project/bad_file.pl: [Subroutines::RequireFinalReturn] Subroutine does not end with "return" at line 13, column 1.  (Severity: 4)
test_project/bad_file.pl: [ValuesAndExpressions::ProhibitNoisyQuotes] Quotes used with a noisy string at line 11, column 22.  (Severity: 2)
test_project/bad_file.pl: [ValuesAndExpressions::ProhibitInterpolationOfLiterals] Useless interpolation of literal string at line 11, column 16.  (Severity: 1)
test_project/bad_file.pl: [ValuesAndExpressions::ProhibitInterpolationOfLiterals] Useless interpolation of literal string at line 11, column 22.  (Severity: 1)
test_project/bad_file.pl: [ValuesAndExpressions::ProhibitInterpolationOfLiterals] Useless interpolation of literal string at line 11, column 28.  (Severity: 1)
test_project/bad_file.pl: [Subroutines::ProhibitSubroutinePrototypes] You should fix at least 1 violation(s) of this type (was: 1, now: 2) (Severity: 5)
test_project/bad_file.pl: [TestingAndDebugging::RequireUseStrict] You should fix at least 1 violation(s) of this type (was: 0, now: 1) (Severity: 5)
test_project/bad_file.pl: [Subroutines::RequireFinalReturn] You should fix at least 1 violation(s) of this type (was: 0, now: 1) (Severity: 4)
---
You can bypass all checks by placing 'NO CRITIC' in the begining of the comment message,
e.g.: svn ci -m "NO CRITIC: emergency hotfix" FooBar.pm

image

Как и ожидалось, коммит не прошёл. Обратите внимание на новую секцию отчёта, выделенную контрастным фоном. В этой секции perlcritic-checker показал своего рода diff нарушений. В частности, там сказано, что мы добавили второе нарушение типа [Subroutines::ProhibitSubroutinePrototypes] и внесли два новых нарушения которых раньше в файле вообще небыло: [TestingAndDebugging::RequireUseStrict] и [Subroutines::RequireFinalReturn]. Как вытекает из отчёта, исправленное нами нарушение [TestingAndDebugging::RequireUseWarnings] дополнительных поблажек не даёт, чего и следовало ожидать.

Тут у читателя может возникнуть вопрос: зачем нужна первая часть отчёта в которой просто перечисляются всё нарушения, даже уже имеющиеся на момент коммита? Напомню, что сравнение списков нарушений ДО и СЕЙЧАС производится по названию Policy и счётчику встречаемости. Другими словами, perlcritic-checker не различает нарушение типа [Subroutines::ProhibitSubroutinePrototypes] на строке 13 и строке 17. Для того чтобы этот пункт изчез из «прогрессивной» части отчёта, пользователь может устранить любое из них. Таким образом, верхняя часть отчёта служит своего рода справочником, в котором по названию Policy можно найти номера строк в которых это Policy было нарушено.

В заключение скажу, что заклинание NO CRITIC работает и в прогрессивном режиме тоже, однако злоупотребление этой возможностью крайне не рекомендуется.

Советы по дальнейшему тюнингу


  1. Настройте интеграцию своего редактора с критиком: VIM, Emacs.
  2. Настройте интеграцию своего редактора c perltidy: VIM, Emacs.
  3. Храните ваши деньги в сберегательной кассе профили Perl::Critic'а под контролем версий и обновляйте рабочую копию на сервере при каждом коммите. Так все члены вашей команды будут локально работать с единым профилем, а любые изменения в нем perlcritic-checker будет подхватывать автоматически.
  4. Начните с самого строгого профиля (severity = brutal, все Policy включены) и добавляйте в него исключения и корректировки по мере необходимости.
  5. Модифицируйте шаблоны для *.pm, *.pl и *.t файлов в своём редакторе таким образом, чтобы они изначально были корректны с точки зрения критика.
  6. Не поленитесь прочитать книгу Perl Best Practices.


Заключение


При внедрении Perl::Critic'а в свой производственный процесс важно понимать, что это всего лишь удобный инструмент для наведения порядка, а не догма, всем правилам (Policy) которой необходимо слепо следовать. Подходя с этой позиции, ваша команда в итоге придёт к той конфигурации, которая идеально соответствует вашим требованиям, вашему проекту и вашим привычкам.

Ссылки


Tags:
Hubs:
+19
Comments 9
Comments Comments 9

Articles