Login
is_script() now tries to check if the filename ends in ".PL".
authorJeffrey Ryan Thalhammer <jeff@imaginative-software.com>
Wed, 9 Jul 2008 19:37:30 +0000 (19:37 +0000)
committerJeffrey Ryan Thalhammer <jeff@imaginative-software.com>
Wed, 9 Jul 2008 19:37:30 +0000 (19:37 +0000)
This prevents it from misclassifying Makefile.PL and Build.PL
and other ".PL" scripts as modules.  But if the document is
coming from a pipe (rather than a file) then the filename
won't be available and is_script() will still misclassify
the file.  Another approach may be to look for the absence
of a "package" statement, but I'm not sure that is very
reliable.  I suspect that many newbie developers and those
with very old legacy systems may have lots of libraries that
don't have any package statements.

Changes
lib/Perl/Critic/Utils.pm
t/05_utils.t

diff --git a/Changes b/Changes
index b6ebe34..35ed1c8 100644 (file)
--- a/Changes
+++ b/Changes
@@ -14,6 +14,9 @@
       multiple captures used in a substitution, e.g. s/(.)(.)/$2$1/.
     * Subroutines::ProhibitAmpersandSigils no longer complains about
       "sort &foo(...)".
+    * Makefile.PL, Build.PL and other ".PL" scripts which typically do not
+      have a shebang are no longer mistaken as modules.  This prevents
+      spurrious warnings from Modules::RequireEndWithOne. RT #20481.
 
 [1.088] Released on 2008-07-04
 
index 61ce292..4dbe050 100644 (file)
@@ -758,7 +758,19 @@ sub is_function_call {
 sub is_script {
     my $doc = shift;
 
-    return shebang_line($doc) ? 1 : 0;
+    return 1 if shebang_line($doc);
+    return 1 if _is_PL_file($doc);
+    return 0;
+}
+
+#-----------------------------------------------------------------------------
+
+sub _is_PL_file {
+    my ($doc) = @_;
+    return if not $doc->can('filename');
+    my $filename = $doc->filename() || return;
+    return 1 if $filename =~ m/[.] PL \z/smx;
+    return 0;
 }
 
 #-----------------------------------------------------------------------------
@@ -1415,6 +1427,11 @@ passed the nodes that represent the interior of a list, like:
 
 Given a L<PPI::Document|PPI::Document>, test if it starts with
 C</#!.*/>.  If so, it is judged to be a script instead of a module.
+Also, if the filename of the document ends in ".PL" then it is
+also judged to be a script.  However, this only works if the
+document is a L<PPI::Document::File|PPI::Document::File>.  If it
+isn't, then the filename is not available and it has no bearing on
+how the document is judged.
 See C<shebang_line()>.
 
 
index 5e2137b..04b258a 100644 (file)
@@ -15,11 +15,12 @@ use English qw< -no_match_vars >;
 use Carp qw< confess >;
 
 use PPI::Document;
+use PPI::Document::File;
 
 use Perl::Critic::PolicyFactory;
 use Perl::Critic::TestUtils qw(bundled_policy_names);
 
-use Test::More tests => 115;
+use Test::More tests => 116;
 
 #-----------------------------------------------------------------------------
 
@@ -38,6 +39,7 @@ test_export();
 test_find_keywords();
 test_is_hash_key();
 test_is_script();
+test_is_script_with_PL_files();
 test_is_perl_builtin();
 test_is_perl_global();
 test_precedence_of();
@@ -168,6 +170,24 @@ sub test_is_script {
 
 #-----------------------------------------------------------------------------
 
+sub test_is_script_with_PL_files {
+
+    # Testing for .PL files (e.g. Makefile.PL, Build.PL)
+    # See http://rt.cpan.org/Ticket/Display.html?id=20481
+    my $temp_file = File::Temp->new(SUFFIX => '.PL');
+
+    # The file must have content, or PPI will barf...
+    print {$temp_file} "some code\n";
+    close $temp_file; # Just to flush the buffer.
+
+    my $doc = PPI::Document::File->new($temp_file->filename());
+    $doc->index_locations();
+    ok(is_script($doc), 'is_script, false for .PL files');
+
+}
+
+#-----------------------------------------------------------------------------
+
 sub test_is_perl_builtin {
     is(   is_perl_builtin('print'),  1, 'Is perl builtin function'     );
     isnt( is_perl_builtin('foobar'), 1, 'Is not perl builtin function' );