http://www.perlmonks.org?node_id=1055158

boftx has asked for the wisdom of the Perl Monks concerning the following question:

In an earlier thread, it was suggested that I go ahead and write a policy that does what I want. Good advice, and I have undertaken the task, but I seem to have run into a fundamental flaw that has to do with how policies can be disabled in the config file.

My goal is to sub-class Subroutines::RequireArgUnpack and modify sub violates so that it will recognize a string similar to my $self = shift; as a valid way to unpack @_. I have a (very) crude first approximation working as I desire. But...

Both my sub-class and the original module are loaded, and executed by perlcritic. The obvious solution is to exclude Subroutines::RequireArgUnpack in .perlcriticrc, but this also prevents my new sub-class from executing, too. I also tried forcing mine to be loaded anyway with an include statement, but that does not work.

The only approach that I have found that will do what I want to do is to take a full cut and paste of Subroutines::RequireArgUnpack to my new policy without trying to subclass it, make my changes, and then exclude the original.

This is not desirable. In a perfect world I could make a sub-class, override a single method, and then have my sub-classed version pulled in even when the parent class has been excluded. There is no way I am going to put a module on CPAN that is a cut-and-paste with only (relatively) minor changes, and I have seen a few discussions elsewhere complaining about issue I raised to make this a worthy contribution otherwise. Obviously I can do what is needed for our private install at work, but it is always better to give back to CPAN when possible.

Does anyone have any thoughts on how to get past how Perl::Critic excludes policies? I suspect the behavior I am seeing is related to the use of Module::Pluggable.

Update

Added "Solved" to title. See post below for solution.

On time, cheap, compliant with final specs. Pick two.

Replies are listed 'Best First'.
Re: Problems with overriding a Perl::Critic policy
by toolic (Bishop) on Sep 22, 2013 at 00:14 UTC
    Post the code that you have, how you are using it, what you expect to get and what you are actually getting. I think people will be able to offer specific help if they see the code.
Re: Problems with overriding a Perl::Critic policy - SOLVED
by boftx (Deacon) on Sep 22, 2013 at 21:57 UTC

    Thanks to a nudge from the post above (thanks!) I tried delegating to Subroutines::RequireArgUnpacking instead of sub-classing and now I can exclude that one but have my policy still execute.

    Here is the gist of the technique I used. Please keep in mind that this is strictly proof of concept at this point. I will post the final code to Meditations when it is done as an RFC. I know already that I will want further suggestions on how to fine tune the regex expressions being used as well as how to go about integrating the existing P::C test harness for regression tests.

    package Perl::Critic::Policy::Local::RequireArgUnpackOrShift; use strict; use warnings; use Readonly; use Perl::Critic::Utils qw< :booleans :characters :severities words_from_string >; use Perl::Critic::Policy::Subroutines::RequireArgUnpacking; use base 'Perl::Critic::Policy'; Readonly::Scalar my $AT_ARG => q{@_}; ## no critic (InterpolationOfMet +achars) Readonly::Scalar my $DESC => qq{Always unpack $AT_ARG first}; Readonly::Scalar my $EXPL => [178]; my $rau = Perl::Critic::Policy::Subroutines::RequireArgUnpacking->new; my @delegations = ( qw( supported_parameters default_severity default_themes applies_to _is_size_check _is_postfix_foreach _is_cast_of_array _is_cast_of_scalar _get_arg_symbols _magic_finder ) ); my @has_self = ( qw( _is_unpack _is_delegation ) ); { no strict 'refs'; for (@delegations) { my $attribute = $_; *{ __PACKAGE__ . '::' . $attribute } = sub { my $fq_sname = join('::',ref($rau),$attribute); return &{$fq_sname}(@_); }; } for (@has_self) { my $attribute = $_; *{ __PACKAGE__ . '::' . $attribute } = sub { my $self = shift; return $rau->$attribute(@_); }; } } sub violates { my ( $self, $elem, undef ) = @_; warn "In OrShift violates\n"; # do some nifty new stuff here, delegate all methods and subroutin +es # to the original policy except for my tests. return; } 1;

    Update

    Corrected items in the arrays at the top.

    Before someone asks, yes, I could have done the delegation much easier with Moose. I decided against that because Perl::Critic does not use Moose and is 5.6 compliant. I wanted to keep anything I added the same if it is eventually released to CPAN.

    Second, I did not use AUTOLOAD for two reasons: a) anything that depends upon using a "obj->can()" test would fail, and b) I had to have two different types of delegation based on whether or not the call would be to a simple sub-routine or an object method call (passing in the object as the first element.)

    On time, cheap, compliant with final specs. Pick two.
Re: Problems with overriding a Perl::Critic policy
by Anonymous Monk on Sep 22, 2013 at 04:37 UTC

    Untested. Just an idea:

    require Subroutines::RequireArgUnpack; # Add a fake parent. unshift @Subroutines::RequireArgUnpack::ISA = 'Subroutines::RequireArg +Unpack::Fake'; # Put method you want to override in fake parent. *Subroutines::RequireArgUnpack::Fake = \&Subroutines::RequireArgUnpack +::violates; # Override method in original class. *Subroutines::RequireArgUnpack::violates = sub { my ($self, @args) = @_; # Maybe call overriden method. $self->SUPER::violates(@args); };
      I don’t understand what you’re trying to do. Care to explain how this idea would allow one to exclude Subroutines::RequireArgUnpack, but to include the subclass? What’s the purpose of the fake parent?