📜 ⬆️ ⬇️

Perl :: Critic + Subversion = implementation of common coding practices in the team

The Perl language is well known to the degree of freedom (aka TIMTOWTDI ), which it gives the programmer in choosing the way to solve a particular task. This medal, unfortunately, has a downside, which can manifest itself in the team development of large projects. If there is no uniform coding practices in the team and each of the developers adheres to the principle of TIMTOWTDI, then you will not envy the newcomer to such a team.

In 2005, an active member of the Perl community, Damian Conway, published the Perl Best Practices book, in which he collected and structured 256 recommendations for writing understandable, reliable, and supported Perl code. A brief cheat sheet with a squeeze from the book can be downloaded from here .

A year later, Jeffrey Thalhammer and a group of comrades released Perl :: Critic , a flexible and extensible framework that allows you to automate the checking of Perl code for its compliance with most of the recommendations from Conway’s book, as well as many other useful practices.
')
Perl :: Critic is served under different sauces: firstly, the utility of the same name - perlcritic is supplied with the module, secondly, the verification of the code can be made into tests using Test :: Perl :: Critic or Test :: Perl :: Critic :: Progressive ; thirdly, the critic is easily integrated into VIM and Emacs .

In this recipe, I’ll talk about how to check Perl code on the fly when committing to a Subversion repository. Bon Appétit!



The trigger mechanism in Subversion


Subversion, like many other version control systems, allows you to execute arbitrary user commands at specified phases of commit processing. In particular, Subversion has a pre-commit trigger (hook) that runs just before the commit gets into the repository. In fact, this is just a shell script of the same name that Subversion expects to find in the hooks/ repository directory. As one of its arguments, this script gets the ID of the current transaction. Using this identifier, the script can fully analyze the current commit: which files or directories have been added, deleted, changed, get new content of files, diff changes, etc. For this purpose, you can use the svnlook utility or the SVN :: Look module.

If the pre-commit hook returns 0, then the commit will be safely saved, and if not, the entire transaction will be rejected (i.e., all files, directories, property changes). In this case, the output of the script to STDERR will be redirected to the svn-client. Thus, by calling Perl::Critic inside a pre-commit hook, you can block the code with violations of the way to the repository.

From theory to practice


1. Download and install the perlcritic-checker.pl script

 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

By default, the script and documentation will be installed in /usr/local .

2. Create a test SVN repository

 export SVN_REPO = / tmp / svn_repo
 svnadmin create $ SVN_REPO

3. Install the configuration files

First, install the perlcritic-checker 'config:

 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
     # 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 * need *
     # to commit changes bypassing all checks (eg emergency bug fixes).
     # This feat allows you to bypass Perl :: Critic using "magic" prefix in
     # comment message, eg: 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
     # most severe violations
     # will be shown.  In progressive mode such behavior has no sense,
     # that's why the user will be asked to run perlcritic locally.
     #
     # In fact, this parameter is a workaround for a subtle bug in generic
     # svn-client thats when svn hook (ie 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

In this configuration, it is stated that the Perl::Critic 'a settings for all *.pm , *.pl and *.t files must be taken from the perlcritic-brutal.conf . We also enable the progressive mode (read below) and allow emergency commits, i.e. commits are not checked ( NO CRITIC ).

The next step is to install the Perl::Critic 'a profile ( perlcritic-brutal.conf ), which we referred to in the previous file:

 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 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

This profile includes the strictest mode of the critic's work ( severity = brutal ), in which he will find fault even with trifles (however, we know that there are no trifles in aviation). The theme directive indicates that of the entire set of available Policy, and there are about a hundred and fifty, you need to check only those listed in Core or Perl Best Practices groups. We also explicitly indicate the path to the settings of the perltidy program, a utility for automatically formatting Perl-code. Formatting violation is one of the reasons why a commit may not pass.

I note that this profile contains only basic settings, but this is a good starting point. For more information about the features of Perl::Critic you can read the documentation: perldoc Perl::Critic .

Next, create a config for perltidy (with root privileges):

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

This file is extremely simple: it instructs perltidy format the code according to recommendations from the already mentioned Perl Best Practices book. If you're used to a different style, then the perltidy manual page (1p) will help you choose the formatting options you need.

The name and path of the file was not chosen randomly. By default, perltidy first checks the user config $HOME/.perltidyrc , and then the global one /etc/perltidyrc . Thus, by placing the config in /etc we will make it simultaneously accessible both to perlcritic-checker 'a (usually working under the svn user) and to the perltidy program perltidy , whatever the user perltidy it.

Finally, it remains for us to instruct Subversion to call the perlcritic-checker.pl script to verify commits. As you might guess, for this you need to edit the 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

This completes the setup phase. Everything is ready for the first experiment.

Good, bad, progressive


So, create a folder for the test project in the repository and check out the working copy:

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

For the experiment, we need a “good” file (file without violations):

 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

and “bad” file (file with violations):

 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

A good file commit should pass without any problems:

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

But a bad file commit should cause critic criticism:

 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 at noisy string at line 12, column 22. (Severity: 2)
 test_project / bad_file.pl: [ValuesAndExpressions :: ProhibitInterpolationOfLiterals] Useless interpolation literal string at line 12, column 16. (Severity: 1)
 test_project / bad_file.pl: [ValuesAndExpressions :: ProhibitInterpolationOfLiterals] Useless interpolation literal string at line 12, column 22. (Severity: 1)
 test_project / bad_file.pl: [ValuesAndExpressions :: ProhibitInterpolationOfLiterals] Useless interpolation literal string at line 12, column 28. (Severity: 1)
 ---
 You can bypass all checks by saying "NO CRITIC" in,
 eg: svn ci -m "NO CRITIC: emergency hotfix" FooBar.pm

image

It can be seen that the violations are sorted by decreasing degree of severity (Severity), as well as painted in different colors. If the violation relates to a specific file line, and such a majority, then the line number and the position number on this line are indicated. If the name and brief description of the Policy does not understand its essence, then you can get extended help like this: perlcritic --doc TestingAndDebugging::RequireUseWarnings | less perlcritic --doc TestingAndDebugging::RequireUseWarnings | less

I note that depending on the version of the Perl::Critic module and the list of established Policy, the output may differ slightly (to reduce the likelihood of this discrepancy, we specifically narrowed the list of active Policy using the theme directive in the perlcritic-brutal.conf ).

But back to our bad file. Having received a refusal from Subversion to accept our commit, you can go two ways. The first way (correct) is to eliminate the violations and try again. The second way (forced) is to mark the commit as emergency and thereby bypass all checks. Here's what it looks like:

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

In this form, the commit will not be criticized and will fall into the repository bypassing the department of censorship.

I note that emergency commits are an extreme measure and in practice they are used very rarely. In addition to directly eliminating the violation, there are other ways to get tested. One of them is a special comment for Perl::Critic

 ## no critic (Subroutines :: ProhibitExplicitReturnUndef)

placed at the end of the line where the violation was detected (attention - ##). In brackets, separated by commas, you can list several Policy at once. If this comment is placed on a separate line, then its effect will be distributed until the end of the current block. As a result, a comment placed outside the blocks will be valid until the end of the file. In order to turn on the criticism ahead of time (i.e. before the completion of the block), it suffices to say on a separate line

 ## use critic

I want to once again draw attention to the two characters '#' and to the fact that the Policy is turned off by name, and all are turned on at once.

Well, and finally, if you realize that you are turning off the same Policy in the code over and over again, consider whether it is better to disable it once in the Perl::Critic profile. Then, some policies can be configured. Often in the Policy configuration, you can override the threshold values ​​of its operation, set an exception list, change the operation mode, etc.

So we got to the most interesting - the perlcritic-checker 's progressive mode of operation. Without this capability, it would be impossible to implement this system in the framework of large existing Perl projects. As practice has shown, the number of violations in previously uncritical code (even at a non- severity level) is simply off scale. Modifying the lion's share of the code, and working code, only for the sake of Perl::Critic 'is impractical, and the abuse of emergency commits ( NO CRITIC ) will reduce the value of this system to zero.

So, how does perlcritic-checker 's progressive mode perlcritic-checker ? In the progressive mode, perlcritic-checker for each modified file compares the list of violations to the TO and the list of violations NOW. Both lists of violations are grouped by the name Policy and are supplied with a counter of occurrence. So, if at least one violation was added to the file in the current commit before, then the entire commit will be rejected, and a message will be sent to the user, indicating what type of violations were added and in what quantity. Thus, the perlcritic-checker closes its eyes to existing violations, but prohibits the introduction of new ones.

I note that this scheme does not apply to newly added files. At the first commit, the file is criticized in the usual strict mode, which in general is logical: there was obviously no violation in the previously non-existent file.

Let us demonstrate by example. As we remember, in spite of the fact that the “bad” file contained violations, we insisted on its commit using NO CRITIC . Fill in this file with new violations and fix some of the existing ones (will the perlcritic-checker forgive the lack of use strict in exchange for the included 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 file 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 at a noisy string at line 11, column 22. (Severity: 2)
 test_project / bad_file.pl: [ValuesAndExpressions :: ProhibitInterpolationOfLiterals] Useless interpolation literal string at line 11, column 16. (Severity: 1)
 test_project / bad_file.pl: [ValuesAndExpressions :: ProhibitInterpolationOfLiterals] Useless interpolation literal string at line 11, column 22. (Severity: 1)
 test_project / bad_file.pl: [ValuesAndExpressions :: ProhibitInterpolationOfLiterals] Useless interpolation 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 must fix at least 1 violation (s) of this type (was: 0, now: 1) (Severity: 4)
 ---
 You can bypass all checks by saying "NO CRITIC" in,
 eg: svn ci -m "NO CRITIC: emergency hotfix" FooBar.pm

image

As expected, the commit failed. Pay attention to the new section of the report, highlighted by a contrasting background. In this section, the perlcritic-checker showed a kind of diff violations. In particular, it says that we added the second violation of the type [Subroutines::ProhibitSubroutinePrototypes] and made two new violations that were previously not found in the file at all: [TestingAndDebugging::RequireUseStrict] and [Subroutines::RequireFinalReturn] . As follows from the report, the violation [TestingAndDebugging::RequireUseWarnings] , which we have corrected, does not give any additional concessions, which was to be expected.

Here the reader may have a question: why do we need the first part of the report which simply lists all the violations, even those already existing at the time of the commit? Let me remind you that the comparison of the lists of violations before and now is done by the name Policy and the counter of occurrence. In other words, the perlcritic-checker does not distinguish the violation of the type [Subroutines::ProhibitSubroutinePrototypes] on line 13 and line 17. In order for this item to disappear from the “progressive” part of the report, the user can eliminate any of them. Thus, the upper part of the report serves as a kind of reference book in which, by the name of the Policy, you can find the line numbers in which this Policy was violated.

In conclusion, the NO CRITIC works in the progressive mode too, but abuse of this feature is highly discouraged.

Tips for further tuning


  1. Customize your editor's integration with the critic: VIM , Emacs .
  2. Customize the integration of your editor with perltidy : VIM , Emacs .
  3. Keep your money in the savings bank Perl::Critic 'profiles under version control and update the working copy on the server at each commit. So all members of your team will work locally with a single profile, and any changes in it perlcritic-checker will pick up automatically.
  4. Start with the strictest profile ( severity = brutal , all Policy included) and add exceptions and corrections to it as necessary.
  5. Modify the templates for the *.pm , *.pl and *.t files in your editor so that they are initially correct from the point of view of the critic.
  6. Take the time to read the Perl Best Practices book.


Conclusion


When introducing Perl::Critic 'into your production process, it is important to understand that this is just a convenient tool for restoring order, and not a dogma, all the rules (Policy) of which you must blindly follow. Approaching from this position, your team will eventually come to the configuration that perfectly fits your requirements, your project and your habits.

Links


Source: https://habr.com/ru/post/115395/


All Articles