| Subject: | New policy: Modules::ProhibitInheritingExporter |
As a follow-up to bug #75300 I wrote a policy to report modules that use
Exporter by inheriting from it (use base 'Exporter') instead of just
importing its 'import' sub (use Exporter 'import').
The name of the policy is Modules::ProhibitInheritingExporter.
Patch attached.
Comments welcome. This is my first policy and english is not my native
language. Also I'm not sure about the naming, the severity and the themes.
--
Olivier Mengué - http://perlresume.org/DOLMEN
| Subject: | Modules-ProhibitInheritingExporter.patch |
Index: lib/Perl/Critic/Policy/Modules/ProhibitInheritingExporter.pm
===================================================================
--- lib/Perl/Critic/Policy/Modules/ProhibitInheritingExporter.pm (revision 0)
+++ lib/Perl/Critic/Policy/Modules/ProhibitInheritingExporter.pm (revision 0)
@@ -0,0 +1,118 @@
+##############################################################################
+# $URL$
+# $Date$
+# $Author$
+# $Revision$
+##############################################################################
+
+package Perl::Critic::Policy::Modules::ProhibitInheritingExporter;
+
+use 5.006001;
+use strict;
+use warnings;
+use Readonly;
+
+use Perl::Critic::Utils qw{ :severities };
+use base 'Perl::Critic::Policy';
+
+our $VERSION = '1.117';
+
+#-----------------------------------------------------------------------------
+
+Readonly::Scalar my $DESC => q{Module inherits from Exporter};
+Readonly::Scalar my $EXPL => q{Use "use Exporter 'import'" instead}; ## no critic (RequireInterpolation)
+
+#-----------------------------------------------------------------------------
+
+sub supported_parameters { return () }
+sub default_severity { return $SEVERITY_MEDIUM }
+sub default_themes { return qw( core maintenance performance ) }
+sub applies_to { return 'PPI::Document' }
+
+#-----------------------------------------------------------------------------
+
+sub violates {
+ my ( $self, $elem, $doc ) = @_;
+
+ my $includes_ref = $doc->find('PPI::Statement::Include');
+ return if not $includes_ref; #ok
+ # This covers both C<use Exporter;> and C<use base 'Exporter';>
+ my @inherit_exporter =
+ grep {
+ m/
+ use \s+ (?: base|parent ) \b .* (?<! [:\w]) Exporter (?: :: ) ? (?! [:\w] )
+ /xms
+ } @{ $includes_ref };
+
+ if ( @inherit_exporter ) {
+ return $self->violation( $DESC, $EXPL, $inherit_exporter[0] );
+ }
+ return; #ok
+}
+
+1;
+
+__END__
+
+#-----------------------------------------------------------------------------
+
+=pod
+
+=head1 NAME
+
+Perl::Critic::Policy::Modules::ProhibitInheritingExporter - Use Exporter via C<use Exporter 'import'> instead of C<use base 'Exporter'>.
+
+=head1 AFFILIATION
+
+This Policy is part of the core L<Perl::Critic|Perl::Critic>
+distribution.
+
+
+=head1 DESCRIPTION
+
+Old version of L<Exporter> recommended to use it with C<our @ISA = 'Exporter'>
+as the only way (C<use base 'Exporter'> and C<use parent 'Exporter'> are
+alternative way or writing this, but the same under). This is an easy way to
+make the 'import' method of Exporter available in the importing package.
+However the C<Exporter> package contains also other functions that will
+consequently be also available as methodes of your package without you
+explicitely knowing it. This can lead to problems hard to debug.
+
+The best practice is to import only the C<Exporter::import> sub into your
+package.
+
+ package Foo;
+
+ use base 'Exporter'; # not ok
+ use parent 'Exporter'; # not ok
+ use Exporter 'import'; # ok
+
+
+=head1 CONFIGURATION
+
+This Policy is not configurable except for the standard options.
+
+
+=head1 AUTHOR
+
+Olivier MenguE<eacute> <dolmen@cpan.org>
+
+
+=head1 COPYRIGHT
+
+Copyright (c) 2012 Olivier MenguE<eacute>. All rights reserved.
+
+This program is free software; you can redistribute it and/or modify
+it under the same terms as Perl itself. The full text of this license
+can be found in the LICENSE file included with this module.
+
+=cut
+
+# Local Variables:
+# mode: cperl
+# cperl-indent-level: 4
+# fill-column: 78
+# indent-tabs-mode: nil
+# c-indentation-style: bsd
+# End:
+# ex: set ts=8 sts=4 sw=4 tw=78 ft=perl expandtab shiftround :
Property changes on: lib/Perl/Critic/Policy/Modules/ProhibitInheritingExporter.pm
___________________________________________________________________
Added: svn:keywords
+ URL Date Author Revision
Index: t/Modules/ProhibitInheritingExporter.run
===================================================================
--- t/Modules/ProhibitInheritingExporter.run (revision 0)
+++ t/Modules/ProhibitInheritingExporter.run (revision 0)
@@ -0,0 +1,134 @@
+## name Basic failure, "use base 'Exporter';"
+## failures 1
+## cut
+
+use base 'Exporter';
+@EXPORT = qw(foo bar);
+
+#-----------------------------------------------------------------------------
+
+## name Basic failure, "use parent 'Exporter';"
+## failures 1
+## cut
+
+use parent 'Exporter';
+@EXPORT = qw(foo bar);
+
+#-----------------------------------------------------------------------------
+
+## name Basic failure, 'use base "Exporter";'
+## failures 1
+## cut
+
+use base "Exporter";
+@EXPORT = qw(foo bar);
+
+#-----------------------------------------------------------------------------
+
+## name Basic failure, 'use base Exporter;' (bareword)
+## failures 1
+## cut
+
+use base Exporter;
+@EXPORT = qw(foo bar);
+
+#-----------------------------------------------------------------------------
+
+## name Basic failure, 'use base Exporter::;' (bareword)
+## failures 1
+## cut
+
+use base Exporter::;
+@EXPORT = qw(foo bar);
+
+#-----------------------------------------------------------------------------
+
+## name Basic failure, "use base qq<Exporter>;"
+## failures 1
+## cut
+
+use base qw<Exporter>;
+@EXPORT = qw(foo bar);
+
+#-----------------------------------------------------------------------------
+
+## name Basic failure, "use base qw< Exporter >;"
+## failures 1
+## cut
+
+use base qw< Exporter >;
+@EXPORT = qw(foo bar);
+
+#-----------------------------------------------------------------------------
+## name Basic failure, "use base qw< Exporter >;"
+## failures 1
+## cut
+
+use base qw< Exporter:: >;
+@EXPORT = qw(foo bar);
+
+#-----------------------------------------------------------------------------
+
+## name Basic failure, "use parent 0.001 'Exporter';"
+## failures 1
+## cut
+
+use parent 0.001 'Exporter';
+@EXPORT = qw(foo bar);
+
+#-----------------------------------------------------------------------------
+
+## name Basic pass, "use Exporter 'import';"
+## failures 0
+## cut
+
+use Exporter 'import';
+our @EXPORT = qw(foo bar);
+
+#-----------------------------------------------------------------------------
+
+## name Pass, 'use base 'Exporter::Heavy';' (not bare Exporter)
+## failures 0
+## cut
+
+use base 'Exporter::Heavy';
+@EXPORT = qw(foo bar);
+
+#-----------------------------------------------------------------------------
+## name Pass, 'use base Exporter::Heavy;' (bareword, not bare Exporter)
+## failures 0
+## cut
+
+use base Exporter::Heavy;
+@EXPORT = qw(foo bar);
+
+#-----------------------------------------------------------------------------
+## name Pass, 'use base Moose::Exporter;' (bareword, not bare Exporter)
+## failures 0
+## cut
+
+use Moose ();
+use base Moose::Exporter;
+
+#-----------------------------------------------------------------------------
+## name No 'use' at all
+## failures 0
+## cut
+
+print 123; # no use at all; for test coverage
+
+##############################################################################
+# $URL$
+# $Date$
+# $Author$
+# $Revision$
+##############################################################################
+
+# Local Variables:
+# mode: cperl
+# cperl-indent-level: 4
+# fill-column: 78
+# indent-tabs-mode: nil
+# c-indentation-style: bsd
+# End:
+# ex: set ts=8 sts=4 sw=4 tw=78 ft=perl expandtab shiftround :
Property changes on: t/Modules/ProhibitInheritingExporter.run
___________________________________________________________________
Added: svn:keywords
+ URL Date Author Revision