Convention for noting that a dist is intentionally not using strict and warnings

[neilb]

It would be handy if I could include a comment like the following my my code

# CPANTS use strict
# CPANTS use warnings

Or non CPANTS-specific:

# not using strict;
# not using warnings;

This would flag that there's a reason why I'm not using strict and warnings, so don't fail me on that please :-)

See the discussion on questhub for a quest to get all one's dists "CPANTS clean". Both @book and I have modules (Devel::TraceUse and Devel::Dependencies respectively) where we don't want to use strict and warnings because it will affect the behaviour of the modules.

@tobyink suggested the following:

my $cpants = q/
use strict;
use warnings;
#/;

Which I did, but now CPANTS is complaining because my declared dependencies don't match my code :-)

Which means I have to do one of:

• lie about my dependencies, which I don't like
• accept that I fail this quest (never!)
• delete the dist from CPAN (which seems drastic)
• come up with some hack like deleting the relevant entries from %INC and document that the module must be use'd first. Hacky kludge at best.

[charsbar]

Good point. How about adding something like x_cpants_ignore (name to be determined) in META files to tell CPANTS which metrics should be ignored?

[neilb]

yeah, that sounds good.

[book]

The "special comments" approach has the benefit of working with any intentionnaly ignored module, for any checking tool. It has the big drawback of cluttering the code, which becomes even more annoying if the distribution contains several modules.

The META option can be used by other tools than CPANTS, and does not clutter the code. Given that intentionnaly not using some module seems to be a distribution-level choice for an author, going the META route makes more sense.

What about x_no_use with a list of modules? That way, it's useful for more than just CPANTS, and no connection to the CPANTS metrics is needed.

[neilb]

The "special comments" approach has the benefit of working with any intentionnaly ignored module, for any checking tool. It has the big drawback of cluttering the code, which becomes even more annoying if the distribution contains several modules.

The comments approach also has the advantage that it's self documenting, so someone looking at the code thinking "ha, the bozo hasn't even used strict or warnings", gets to see a comment why.

That said, I prefer the metadata approach. There should just be a comment as well :-)

[charsbar]

What about x_no_use with a list of modules? That way, it's useful for more than just CPANTS, and no connection to the CPANTS metrics is needed.

It's ok for me to support something like x_no_use to modify the behavior of some specific metric, but I'm not sure how many people want that level of finer control. Also, if x_no_use helps other tools, it's probably more appropriate to discuss that elsewhere to attract more attention (maybe a topic for QAH 2014?).

I'd rather stick to x_cpants_* (or maybe x_cpants => { ignore => [qw/some metrics/] } or x_cpants => [qw/-some -metrics/]). It's more explicit, and Test::Kwalitee and its friends can reuse the information easily.

Comments to explain why you don't use some modules would always be nice, but comments to trick CPANTS / Module::ExtractUse may not work in the future, especially when someone makes a faster and more reliable module detector than M::EU with something like Compiler::Lexer / Perl::Lexer.

[tobyink]

come up with some hack like deleting the relevant entries from %INC and document that the module must be use'd first.

That can end up producing some nasty warnings:

$ perl -wE'use strict; BEGIN{ delete $INC{q/strict.pm/} }; use strict;'
Subroutine bits redefined at /home/tai/perl5/perlbrew/perls/perl-5.16.1/lib/5.16.1/strict.pm line 23.
Subroutine import redefined at /home/tai/perl5/perlbrew/perls/perl-5.16.1/lib/5.16.1/strict.pm line 42.
Subroutine unimport redefined at /home/tai/perl5/perlbrew/perls/perl-5.16.1/lib/5.16.1/strict.pm line 47.
$

You need to go further and clean all the subs defined by strict.pm out of the symbol table:

$ perl -wE'use strict; BEGIN{ delete $INC{q/strict.pm/}; undef %strict:: }; use strict'
$

This may cause problems if somebody has taken a reference to something that lived in %strict::. Once strict.pm has reloaded, the reference will no longer be pointing to the new things in %strict::.

[book]

I've added the topic for discussion to the Perl QA Hackathon project page.

[karenetheridge]

package Foo;
no strict;
no warnings;

It's pretty clear from that what the author's intention is, and no sugar in comments is necessary.

I sort of like Karen's self-documenting suggestion, but no warnings won't work on Perls older than 5.6 and it's conceivable (?!?) that someone might want to do that. And asking people to clutter their .pm files with stuff they don't actually need seems like a bad idea.

I prefer @charsbar's suggestion of a top-level x_cpants key in META

x_cpants => { ignore => [ qw/some metrics/ ] }

That keeps the top-level of the META pretty clean and allows for future hints to x_cpants other than 'ignore'.

[karenetheridge]

x_cpants++

[maddingue]

Hmm, maybe I'm arriving a bit late in the discussion, but I remember have a similar one a few years ago, when domm first added the "use strict" metric, and I made the same remark, because I maintain the CPAN version of XSLoader, which deliberately does not use strict. The discussion was to add a commented statement (#use strict;) but I can't remember if the handling was implemented in CPANTS.
https://metacpan.org/source/SAPER/XSLoader-0.16/XSLoader.pm

@dagolden warnings::compat provides a warnings module for older Perls

@maddingue yes, but then I would have to specify a dependency on warnings::compat, simply to be able to say no warnings::compat in order to tell CPANTS not to give me a demerit. This way lies madness. 😄

[maddingue]

@dagolden call me mad :3
https://metacpan.org/source/SAPER/Sys-Syslog-0.33/Makefile.PL#L60

@maddingue but you use warnings. I don't think that's crazy at all. What I think is crazy is to make warnings::compat a prereq only to say no warnings.

"You must have this thing installed so I can tell you that I'm not using it!" :-)

[tobyink]

In the interests of bikeshedding, I'd like to propose:

x_cpants_ignore => {
   no_warnings => "I'm not using warnings because I want to support Perl 5.5 and 5.4.",
   no_strict   => "I'm not using strict because I'm a bloody fool",
},

This way people don't just get a free pass to ignore a metric - they actually need to provide an excuse. The excuses for ignored metrics could be displayed as part of the distribution overview on the CPANTS website.