Bug #121472 for Perl-Critic-More: None of these policies should include themes used in the core

Sun Apr 30 17:58:17 2017 DROLSKY [...] cpan.org - Ticket created

Subject:

None of these policies should include themes used in the core

The ErrorHandling::RequireUseOfExceptions policy includes "maintenance" in its list of themes. That means if I am using the maintenance theme (from core) and then install this distro, I suddenly get a new and unexpected policy being applied. In general, it seems to me that installing a non-core set of critic policies should should never lead to those policies being applied to existing profiles. They should always require an opt-in.

Thu May 18 01:49:24 2017 thaljef [...] cpan.org - Correspondence added

RT-Send-CC:

andy [...] petdance.com

The only theme that should be solely owned by Perl::Critic is "core". Third-party distributions are free to provide policies with "maintenance" or "security" or "bug" or any other theme they choose. If you add "&& core" to your --theme, it will restrict the policy set to those that ship with Perl::Critic. But cherry picking policies from other distributions might get icky. For better or worse, I chose a zero-conf approach to policies: If the policy is installed, then you have implicitly opted to apply it.

Thu May 18 01:49:25 2017 The RT System itself - Status changed from 'new' to 'open'

Thu May 18 02:00:30 2017 DROLSKY [...] cpan.org - Correspondence added

On 2017-05-18 00:49:24, THALJEF wrote: Show quoted text

> The only theme that should be solely owned by Perl::Critic is "core". > Third-party distributions are free to provide policies with > "maintenance" or "security" or "bug" or any other theme they choose. > > If you add "&& core" to your --theme, it will restrict the policy set > to those that ship with Perl::Critic. But cherry picking policies from > other distributions might get icky. > > For better or worse, I chose a zero-conf approach to policies: If the > policy is installed, then you have implicitly opted to apply it.

So how would I update this: theme = core + pbp + bugs + maintenance + cosmetic + complexity + security + tests + moose do I do this? theme = core + (pbp && core) + (bugs && core) + (maintenance && core) + (cosmetic && core) + (complexity && core) + (security && core) + (tests && core) + (moose && core)

Thu May 18 02:41:11 2017 thaljef [...] cpan.org - Correspondence added

RT-Send-CC:

andy [...] petdance.com

Something like this: theme = (core && (pbp || bugs || maintenance || cosmetic || complexity || security || tests)) || moose As far as I know, there are no moose policies in the core, so that's why it's not &&-ed with "core". This is what I mean by "icky" when it comes to selecting third-party policies by theme. Gory details: each theme name is converted to a one if the policy is in the theme, or a zero if it is not. Then the whole expression is eval'd. If the result is true, then the policy is applied, unless the --exclude or --severity options say otherwise.

Thu May 18 02:55:36 2017 thaljef [...] cpan.org - Correspondence added

RT-Send-CC:

andy [...] petdance.com

FYI: We still support the algebraic operators, but at some point I think we decided the logic operators were easier to understand. So they are the only documented ones. https://metacpan.org/source/THALJEF/Perl-Critic-1.126/lib/Perl/Critic/Theme.pm#L109-126

Sat Dec 09 03:52:21 2017 SREZIC [...] cpan.org - Correspondence added

On 2017-05-18 01:49:24, THALJEF wrote: Show quoted text

> For better or worse, I chose a zero-conf approach to policies: If the > policy is installed, then you have implicitly opted to apply it.

I am not a Perl::Critic user, but I think this approach is quite problematic. CPAN modules may be installed for various reasons, it could be just a dependency the author is not aware of. Regards, Slaven