is_script() now tries to check if the filename ends in ".PL".

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.

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
 

diff --git a/lib/Perl/Critic/Utils.pm b/lib/Perl/Critic/Utils.pm
index 61ce292..4dbe050 100644 (file)
--- a/lib/Perl/Critic/Utils.pm
+++ b/lib/Perl/Critic/Utils.pm
@@ -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()>.
 
 

diff --git a/t/05_utils.t b/t/05_utils.t
index 5e2137b..04b258a 100644 (file)
--- a/t/05_utils.t
+++ b/t/05_utils.t
@@ -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' );