in reply to
Request for Comments on Perl::Critic
First off, great job on Perl::Critic. The first thing I thought after
reading PBP is "Hey, I wonder if there's a tool that would check my code
for this stuff". And, as usual, it was on CPAN.
Here are my thoughts on some possible additions and modifications that might be useful:
- A HOWTO document on writing custom policies. This would make it much
easier for people who don't yet grok PPI (like myself) to just dive in and write something,
whether it's implementing the remaining things from PBP that can be submitted back to
you, or a set of policy modules to match their own internal coding standards.
- Package configuration files for better editor integration.
Being able to just type:
within Vim is very nice, and is easier to configure than the :grep hack I
suggested earlier. I think I've already sent you my perlcritic.vim
with instructions on setting it up, but if you didn't get it, let me know.
- An extension to the ## no critic syntax that would allow (or
optionally require?) an explanation of the reason for overriding the
Perl::Critic check in that case. Maybe something like:
I could see this being possibly implemented as a
Perl::Critic::Policy::Pragma::NoCriticRequiresComments, except that
the 'no critic' blocks get stripped out before the policies are applied. Not sure if there's an easy way around that.
## no critic: All the bad stuff below is necessary because [...]
- A way to allow sets of policies to reference a URL, or page numbers in
a different book. This would give you a mechanism to refer to the Safari
version of PBP, or for new policies, allow them to refer to completely
different documents altogether (ie: internal coding standards on company
website, other books like Code Complete or AntiPatterns, or even just a 2nd
edition of PBP with different page numbers).
You could do it with a Perl::Critic::Reference class that would provide a
lookup table and remove page references entirely from the Policy modules.
You could look up the appropriate reference with something like:
Data files containing the explanation reference could be subclasses
of Perl::Critic::Reference (with the parent delegating the ->lookup()
to each of the load()'ed subclasses in order until a hit is found), or they
could just be YAML files merged into a single hash for lookup.
my $ref = Perl::Critic::Reference->new();
# Load reference data for policies from three different files
$ref->load( qw( PBP FooCorpCodingStyle CodeComplete ) );
# and later...
my $explanation = $ref->lookup($violation->policy());
Hmm... that Reference idea sounds useful, and not too difficult. If you'd
take the patch, I think I might try coding it up.
One last comment before I head off to sleep. I'm not sure I agree with the
idea that Perl::Critic could or should be enhanced to correct the code. I
use Perl::Critic to warn me when I'm doing something silly, so that I can
fix it, and by fixing it, learn to not do it again. If it tried to correct
those things for me, there's less of an opportunity to learn. Plus, some
of the things detected by the policies would be downright nasty, if not
impossible, to auto-correct without potentially breaking the code.