Musing on Perl::Critic config

I've been exploring the Perl::Critic configuration file somewhat, and something struck me as unusual.

It seems fine for using for the usecase of "inherit some predefined set of rules and apply them selectively", which works fine if you just want to adopt some other standard that is pre-existing, and just make minor adjustments.

However, if you're wanting to create policy sets for others to use, it seems poorly suited.

1. Existing Systems

1.1. Severity level

This feature seems confusing.

It looks simple at first, but its problematic for anyone to utilise this to create a large, custom policy set.

The gist I get of reading it is you specify a severity level in your configuration, and policies that are included with a larger severity number than that are used. ie: Specify severity = 2 and items with severity levels 2,3,4,5, will be applied, while rules with severity level 1 are ignored.

The documentations suggest it might be a good idea to manually override the severity level for a plugin, ie:

[BuiltinFunctions::ProhibitBooleanGrep]
severity = 3

This might be good if you're just tweaking the odd plugin or 2, so that you can adjust your severity setting for that plugin, but if that's what you're doing, why not just use the

include = 
exclude = 

Fields instead?

And its also confusing, because in places its worded in such a way you'd think:

Maybe this severity level when specified on the plugin, indicates how strongly it reacts to a failure, not simply the plugins inclusion

I don't seriously think thats the case, but I did ask myself that question at least once.

1.2. Thematic Selectors

An alternative way of possibly selecting large groups of policies is using the thematic method.

theme = ( theme_i_want + other_theme_i_want ) - theme_i_dont_want

This looks reasonable too, until you realise the complexity of creating such a named theme.

[BuiltinFunctions::ProhibitBooleanGrep]
add_themes = kbase

And doing this just to make a theme ( or set of themes ) that has been well reviewed seems sort-of excessive in the effort department

1.3. Include/Exclude List

One can of course simply ask, Why not just use the simple include/exclude list?. My reason for not wanting to use that is mostly a maintenance one. But there is an annoying thing in the configuration format that means you have to place all includes on one line and all excludes on another. If you use more than one line, well, it doesn't like you.

include = Foo Bar
include = Baz       ; Only Baz gets included

And this quickly gets out of hand.

exclude = RequireTidyCode RequirePodSections ProhibitPostfixControls RequireRcsKeywords RequireExplicitPackage
include = Moose::ProhibitMultipleWiths Moose::ProhibitNewMethod Moose::RequireCleanNamespace Moose::RequireMakeImmutable

And thats's just a start.

2. Observations about the Existing Design Ethic

It seems to me, that the existing system provides 3 different ways1 to achieve the same outcome. Sure, you can do cool tricks like

  • Define multiple themes in the one file and then change your theme expression to suit the project
  • Have a "Standard" primary theme you use in all your .perlcriticrc files ( which you copy into varying projects ) and just adjust the severity/theme expression to match the project, and make minor adjustments

But outside that, the amount of configuration re-use seems minimal, and the entire design seems to be based on the idea that "Absolutely everything not defined in the official Perl::Critic distro must be explicitly defined, and defined in this file".

If what you wanted to do however was write a policy set for uploading to CPAN for multiple projects to use as a reference standard,

ie: You work at $COMPANY and there's an agreed upon set of policies that all code must meet for check-in, and these policy rules must be used on multiple projects in multiple repositories from multiple machines and users ( and the policy is periodically revised and updated ), requiring all of the above to adopt changes.

In this usage scenario, not only do you have the fun of distribution issues, but the very task of authoring and maintaining such a policy set appears to be a very overwhelming and complicated task due to the substantial amount of redundancy in the definition syntax.

1: Well, there's actually at least 4 ways of selecting policies, you can reject a policy entirely using the - prefix in the section name in the ini.


3. What would be nice


It would be nice if there was some way to create a standalone policy, perhaps, implemented as a module, that defined a policy class of some kind, perhaps Perl::Critic::PolicyGroup or something. Maybe something exists for this at present?, If so, I sure as hell can't find it on CPAN.



.perlcritic.rc
policygroup = Author::KENTNL


package Perl::Critic::PolicyGroup::Author::KENTNL;

use Perl::Critic::PolicyGroup::Builder;

inherit_policygroup 'ModernPerl';
include_policy 'BuiltinFunctions::ProhibitBooleanGrep'; include_policy 'CodeLayout::ProhibitHardTabs' => { allow_leading_tabs => 1, };
exclude_policy 'ControlStructures::ProhibitPostfixControls';

I'd write something myself, but:

  1. I have no idea where to start really
  2. I feel like something this obviously missing should already exist "somewhere" and I'm just not seeing it

I'm pretty sure there are a few really good use cases for it too, consider policy groups such as "ModernPerl", which could be a more modern policy set than the standard and venerable 'pbp' set , and could be distributed on CPAN seperate to Perl::Critic itself as Perl::Critic::PolicyGroup::ModernPerl, which strikes me as a "Good Idea ™"


( p.s. Sorry if this seems a bit messed up, I started writing it in October ... and then somehow got distracted, lost my train of thought, ... and then only just finished it up with my ideas part today )

2 Comments

Yeah, the Perl::Critic configuration file was not really designed to be concise. I never even thought about "inheriting" configuration files until I saw Dist::Zilla's plugin bundles.

Even though I love Dist::Zilla, I never use the plugin bundles anyway. To me, they make the configuration too opaque -- it is hard to see what is going on and make small (usually temporary) changes. But I understand how they make it really easy to distribute a configuration.

Doing something similar for Perl::Critic would be a pretty big change. However, you could easily teach Perl::Critic to understand a URL for a configuration file. That might help solve some of your distribution problems. Feel free to request a commit bit from http://perlcritic.tigris.org

One last thing: the --profile-proto option on perlcritic[1] is very useful for generating a complete configuration file for every Policy that you have installed. You still have to make your own decisions about themes, severities, etc. but it saves you from having to do a lot of typing and gives you a nice snapshot of all the possible configuration options.

A good start would be to add an include/overlay directive to allow overlaying one policy file over another.

I agree the configuration choices are dizzying. I want to say I've never had a problem with them, but I think I have Stockholm Syndrome.

There is an upside to the "everything in one file" perlcritic config and that is it is all in one place. For any sort of distributed project, most CPAN modules, you want to make sure that the complete configuration is inside the project, not sitting in the project leader's home directory. This makes perlcritic available to anyone and everyone who is working on the project, very democratic.

On the flip side, I do maintain my own ~/.perlcritic with my own general tweaks across all my projects. This gets copied into each project and then each project gets its own tweaks. That makes it difficult to update each project from my general one. It would be nice if I could have a project .perlcriticrc which could include a .perlcriticrc.common or something (also located in the project directory) similar to /etc/rc.local vs /etc/rc.common. Corporate infrastructure would likely want something similar.

I think an include and/or overlay directive would solve a lot of problems.

Leave a comment

About KENTNL

user-pic Surprise!