Skip Menu |
 

Preferred bug tracker

Please visit the preferred bug tracker to report your issue.

This queue is for tickets about the Perl-Critic CPAN distribution.

Report information
The Basics
Id: 41871
Status: open
Priority: 0/
Queue: Perl-Critic

People
Owner: Nobody in particular
Requestors: EDAVIS [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: 1.092
Fixed in: (no value)



Subject: InputOutput::ProhibitReadlineInForLoop mistakes glob for readline
MIME-Version: 1.0
X-Mailer: MIME-tools 5.427 (Entity 5.427)
Content-Type: text/plain
Charset: utf8
Content-Disposition: inline
Content-Transfer-Encoding: binary
X-RT-Original-Encoding: utf-8
Content-Length: 776
Download (untitled) / with headers
text/plain 776b
This test program: #!/usr/bin/perl # $Id: $ use warnings; use strict; use 5.010; foreach (<*>) {} saved as 'test' and checked with: % perlcritic -1 --verbose "%f:%l:%c:%m [%p] (%e)\n" test produces two warnings: test:6:10:Glob written as <...> [BuiltinFunctions::RequireGlobFunction] (See page 167 of PBP) test:6:10:Readline inside "for" loop [InputOutput::ProhibitReadlineInForLoop] (See page 211 of PBP) The first one is correct. It is indeed a glob written as <...>. But the second warning is incorrect. I suggest changing InputOutput::ProhibitReadlineInForLoop to stop it flagging these false positives. Whatever code is currently in BuiltinFunctions::RequireGlobFunction to recognize a glob could be used in the other rule to distinguish globs from filehandles.
MIME-Version: 1.0
X-Spam-Status: No, hits=-0.0 required=8.0 tests=SPF_PASS
In-Reply-To: <rt-3.6.HEAD-19694-1229957506-939.41871-4-0 [...] rt.cpan.org>
References: <RT-Ticket-41871 [...] rt.cpan.org> <rt-3.6.HEAD-19694-1229957506-939.41871-4-0 [...] rt.cpan.org>
Message-ID: <495058CB.70105 [...] galumph.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
X-RT-Original-Encoding: utf-8
Received: from la.mx.develooper.com (x1.develooper.com [63.251.223.170]) by diesel.bestpractical.com (Postfix) with SMTP id 653F04D8040 for <bug-Perl-Critic [...] rt.cpan.org>; Mon, 22 Dec 2008 22:19:47 -0500 (EST)
Received: (qmail 15636 invoked by uid 103); 23 Dec 2008 03:19:46 -0000
Received: from x16.dev (10.0.100.26) by x1.dev with QMQP; 23 Dec 2008 03:19:46 -0000
Received: from pendrell.textdrive.com (HELO pendrell.textdrive.com) (207.7.108.149) by 16.mx.develooper.com (qpsmtpd/0.43rc1) with ESMTP; Mon, 22 Dec 2008 19:19:44 -0800
Received: from quaquaversal.local (unknown [76.237.187.200]) by pendrell.textdrive.com (Postfix) with ESMTP id 1859CBB30B for <bug-Perl-Critic [...] rt.cpan.org>; Tue, 23 Dec 2008 03:19:40 +0000 (GMT)
Delivered-To: cpan-bug+Perl-Critic [...] diesel.bestpractical.com
Subject: Re: [rt.cpan.org #41871] InputOutput::ProhibitReadlineInForLoop mistakes glob for readline
User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.18) Gecko/20081105 Thunderbird/2.0.0.18 Mnenhy/0.7.5.0
Return-Path: <perl [...] galumph.com>
X-Spam-Check-BY: 16.mx.develooper.com
X-Original-To: bug-Perl-Critic [...] rt.cpan.org
Date: Mon, 22 Dec 2008 21:19:39 -0600
X-Spam-Level: *
To: bug-Perl-Critic [...] rt.cpan.org
Content-Transfer-Encoding: 7bit
From: Elliot Shank <perl [...] galumph.com>
RT-Message-ID: <rt-3.6.HEAD-19694-1230002393-1076.41871-0-0 [...] rt.cpan.org>
Content-Length: 938
Download (untitled) / with headers
text/plain 938b
Show quoted text
> foreach (<*>) {} > > saved as 'test' and checked with: > > % perlcritic -1 --verbose "%f:%l:%c:%m [%p] (%e)\n" test > > produces two warnings:
[chop] Show quoted text
> The first one is correct. It is indeed a glob written as <...>. But > the second warning is incorrect.
[chop] Show quoted text
> I suggest changing InputOutput::ProhibitReadlineInForLoop to stop it > flagging these false positives. Whatever code is currently in > BuiltinFunctions::RequireGlobFunction to recognize a glob could be used > in the other rule to distinguish globs from filehandles.
This is a bug in PPI and not Perl::Critic: PPI::Document PPI::Statement::Compound [ 1, 1, 1 ] PPI::Token::Word 'foreach' PPI::Structure::ForLoop ( ... ) PPI::Statement [ 1, 10, 10 ] PPI::Token::QuoteLike::Readline '<*>' PPI::Structure::Block { ... }
MIME-Version: 1.0
In-Reply-To: <rt-3.6.HEAD-19694-1230002393-1076.41871-0-0 [...] rt.cpan.org>
X-Mailer: MIME-tools 5.427 (Entity 5.427)
Content-Disposition: inline
Charset: utf8
References: <RT-Ticket-41871 [...] rt.cpan.org> <rt-3.6.HEAD-19694-1229957506-939.41871-4-0 [...] rt.cpan.org> <495058CB.70105 [...] galumph.com> <rt-3.6.HEAD-19694-1230002393-1076.41871-0-0 [...] rt.cpan.org>
Content-Type: text/plain
Message-ID: <rt-3.6.HEAD-19694-1230037359-1901.41871-0-0 [...] rt.cpan.org>
Content-Transfer-Encoding: binary
X-RT-Original-Encoding: utf-8
Content-Length: 115
Download (untitled) / with headers
text/plain 115b
Show quoted text
>This is a bug in PPI and not Perl::Critic
Will you pass the bug report upstream or would you like me to do that?
MIME-Version: 1.0 (Apple Message framework v930.3)
X-Spam-Status: No, hits=0.0 required=8.0 tests=
In-Reply-To: <rt-3.6.HEAD-19694-1230037359-1901.41871-5-0 [...] rt.cpan.org>
X-Mailer: Apple Mail (2.930.3)
References: <RT-Ticket-41871 [...] rt.cpan.org> <rt-3.6.HEAD-19694-1229957506-939.41871-4-0 [...] rt.cpan.org> <495058CB.70105 [...] galumph.com> <rt-3.6.HEAD-19694-1230002393-1076.41871-5-0 [...] rt.cpan.org> <rt-3.6.HEAD-19694-1230037359-1901.41871-5-0 [...] rt.cpan.org>
Message-ID: <A9574DED-600F-49E0-9111-FD7D3BEC86DB [...] imaginative-software.com>
Content-Type: text/plain; charset="utf-8"; delsp="yes"; format="flowed"
X-RT-Original-Encoding: utf-8
Received: from la.mx.develooper.com (x1.develooper.com [63.251.223.170]) by diesel.bestpractical.com (Postfix) with SMTP id 4402119B8098 for <bug-Perl-Critic [...] rt.cpan.org>; Tue, 23 Dec 2008 13:12:39 -0500 (EST)
Received: (qmail 25963 invoked by uid 103); 23 Dec 2008 18:12:38 -0000
Received: from x16.dev (10.0.100.26) by x1.dev with QMQP; 23 Dec 2008 18:12:38 -0000
Received: from wf-out-1314.google.com (HELO wf-out-1314.google.com) (209.85.200.169) by 16.mx.develooper.com (qpsmtpd/0.43rc1) with ESMTP; Tue, 23 Dec 2008 10:12:35 -0800
Received: by wf-out-1314.google.com with SMTP id 28so2641347wfa.27 for <bug-Perl-Critic [...] rt.cpan.org>; Tue, 23 Dec 2008 10:12:31 -0800 (PST)
Received: by 10.143.32.7 with SMTP id k7mr3258632wfj.162.1230055951242; Tue, 23 Dec 2008 10:12:31 -0800 (PST)
Received: from ?10.0.1.196? (adsl-75-61-92-48.dsl.pltn13.sbcglobal.net [75.61.92.48]) by mx.google.com with ESMTPS id 29sm21238238wfg.26.2008.12.23.10.12.28 (version=TLSv1/SSLv3 cipher=RC4-MD5); Tue, 23 Dec 2008 10:12:30 -0800 (PST)
Delivered-To: cpan-bug+Perl-Critic [...] diesel.bestpractical.com
Subject: Re: [rt.cpan.org #41871] InputOutput::ProhibitReadlineInForLoop mistakes glob for readline
Return-Path: <jeff [...] imaginative-software.com>
X-Spam-Check-BY: 16.mx.develooper.com
X-Original-To: bug-Perl-Critic [...] rt.cpan.org
Date: Tue, 23 Dec 2008 10:12:25 -0800
X-Spam-Level: *
To: bug-Perl-Critic [...] rt.cpan.org
Content-Transfer-Encoding: 7bit
From: Jeffrey Thalhammer <jeff [...] imaginative-software.com>
RT-Message-ID: <rt-3.6.HEAD-19694-1230055964-1233.41871-0-0 [...] rt.cpan.org>
Content-Length: 526
Download (untitled) / with headers
text/plain 526b
On Dec 23, 2008, at 5:02 AM, EDAVIS via RT wrote: Show quoted text
> Queue: Perl-Critic > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=41871 > >
>> This is a bug in PPI and not Perl::Critic
> > Will you pass the bug report upstream or would you like me to do that?
There may already be one, but if not, I'd really appreciate if you could post it. My hands are rather full today. Thanks Ed. Happy holidays! Jeffrey Thalhammer Imaginative Software Systems vcard: http://www.imaginative-software.com/contact/jeff.vcf
MIME-Version: 1.0
In-Reply-To: <rt-3.6.HEAD-19694-1230055964-1233.41871-0-0 [...] rt.cpan.org>
X-Mailer: MIME-tools 5.427 (Entity 5.427)
Content-Disposition: inline
Charset: utf8
References: <RT-Ticket-41871 [...] rt.cpan.org> <rt-3.6.HEAD-19694-1229957506-939.41871-4-0 [...] rt.cpan.org> <495058CB.70105 [...] galumph.com> <rt-3.6.HEAD-19694-1230002393-1076.41871-5-0 [...] rt.cpan.org> <rt-3.6.HEAD-19694-1230037359-1901.41871-5-0 [...] rt.cpan.org> <A9574DED-600F-49E0-9111-FD7D3BEC86DB [...] imaginative-software.com> <rt-3.6.HEAD-19694-1230055964-1233.41871-0-0 [...] rt.cpan.org>
Content-Type: text/plain
Message-ID: <rt-3.6.HEAD-2264-1236463587-573.41871-0-0 [...] rt.cpan.org>
Content-Transfer-Encoding: binary
X-RT-Original-Encoding: utf-8
Content-Length: 1088
On Tue Dec 23 13:12:44 2008, jeff@imaginative-software.com wrote: Show quoted text
> On Dec 23, 2008, at 5:02 AM, EDAVIS via RT wrote: >
> > Queue: Perl-Critic > > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=41871 > > >
> >> This is a bug in PPI and not Perl::Critic
> > > > Will you pass the bug report upstream or would you like me to do that?
> > > There may already be one, but if not, I'd really appreciate if you > could post it. My hands are rather full today. Thanks Ed. > > Happy holidays! > > Jeffrey Thalhammer > Imaginative Software Systems > vcard: http://www.imaginative-software.com/contact/jeff.vcf > >
I didn't see anything I recognized as the PPI RT entry for this, so I filed my own as a wishlist item: #43933: Recognize <*> as being a glob operation, not a readline. My apologies to all if I have duplicated. Whether it will be accepted is another question. One of the tests requires '<$up../*.v>' to be parsed as a readline. On the other hand, you never know. And if it's rejected, that might make us more likely to do something on our own. Tom Wyant
MIME-Version: 1.0
In-Reply-To: <rt-3.6.HEAD-2264-1236463587-573.41871-0-0 [...] rt.cpan.org>
X-Mailer: MIME-tools 5.427 (Entity 5.427)
Content-Disposition: inline
References: <RT-Ticket-41871 [...] rt.cpan.org> <rt-3.6.HEAD-19694-1229957506-939.41871-4-0 [...] rt.cpan.org> <495058CB.70105 [...] galumph.com> <rt-3.6.HEAD-19694-1230002393-1076.41871-5-0 [...] rt.cpan.org> <rt-3.6.HEAD-19694-1230037359-1901.41871-5-0 [...] rt.cpan.org> <A9574DED-600F-49E0-9111-FD7D3BEC86DB [...] imaginative-software.com> <rt-3.6.HEAD-19694-1230055964-1233.41871-0-0 [...] rt.cpan.org> <rt-3.6.HEAD-2264-1236463587-573.41871-0-0 [...] rt.cpan.org>
Content-Type: text/plain; charset="UTF-8"
Message-ID: <rt-3.8.HEAD-2360-1289315724-1266.41871-0-0 [...] rt.cpan.org>
Content-Transfer-Encoding: binary
X-RT-Original-Encoding: utf-8
Content-Length: 121
Download (untitled) / with headers
text/plain 121b
Patch committed to PPI wishlist item #43933. Whether it is accepted, and when the next PPI release is, is another matter.
MIME-Version: 1.0
In-Reply-To: <rt-3.8.HEAD-2360-1289315724-1266.41871-0-0 [...] rt.cpan.org>
X-Mailer: MIME-tools 5.427 (Entity 5.427)
Content-Disposition: inline
References: <RT-Ticket-41871 [...] rt.cpan.org> <rt-3.6.HEAD-19694-1229957506-939.41871-4-0 [...] rt.cpan.org> <495058CB.70105 [...] galumph.com> <rt-3.6.HEAD-19694-1230002393-1076.41871-5-0 [...] rt.cpan.org> <rt-3.6.HEAD-19694-1230037359-1901.41871-5-0 [...] rt.cpan.org> <A9574DED-600F-49E0-9111-FD7D3BEC86DB [...] imaginative-software.com> <rt-3.6.HEAD-19694-1230055964-1233.41871-0-0 [...] rt.cpan.org> <rt-3.6.HEAD-2264-1236463587-573.41871-0-0 [...] rt.cpan.org> <rt-3.8.HEAD-2360-1289315724-1266.41871-0-0 [...] rt.cpan.org>
Content-Type: text/plain; charset="UTF-8"
Message-ID: <rt-3.8.HEAD-17550-1294978953-1469.41871-0-0 [...] rt.cpan.org>
Content-Transfer-Encoding: binary
X-RT-Original-Encoding: utf-8
Content-Length: 673
Download (untitled) / with headers
text/plain 673b
The PPI patch to deal with this issue was backed out. The basic issue is that a glob can be _anything_, and code to capture a greater percentage of globs _as_ globs was making too much other stuff into globs. For example, <input type="image" name="zoom1" src="/buttons/zoom/green2.gif" alt="show 2000 bp" border="-title" show 2000 bp> is perfectly valid Perl (and actually appears in CPAN), because it gets parsed as a glob. But convincing PPI to accept this without finding a glob in '$foo < 0 && $bar->[0] == 42' appears to be beyond me, at least for the moment. I am going to mark this ticket "stalled", until someone comes along smart enough to figure out this mess.
MIME-Version: 1.0
In-Reply-To: <rt-3.8.HEAD-17550-1294978953-1469.41871-0-0 [...] rt.cpan.org>
X-Mailer: MIME-tools 5.427 (Entity 5.427)
Content-Disposition: inline
References: <RT-Ticket-41871 [...] rt.cpan.org> <rt-3.6.HEAD-19694-1229957506-939.41871-4-0 [...] rt.cpan.org> <495058CB.70105 [...] galumph.com> <rt-3.6.HEAD-19694-1230002393-1076.41871-5-0 [...] rt.cpan.org> <rt-3.6.HEAD-19694-1230037359-1901.41871-5-0 [...] rt.cpan.org> <A9574DED-600F-49E0-9111-FD7D3BEC86DB [...] imaginative-software.com> <rt-3.6.HEAD-19694-1230055964-1233.41871-0-0 [...] rt.cpan.org> <rt-3.6.HEAD-2264-1236463587-573.41871-0-0 [...] rt.cpan.org> <rt-3.8.HEAD-2360-1289315724-1266.41871-0-0 [...] rt.cpan.org> <rt-3.8.HEAD-17550-1294978953-1469.41871-0-0 [...] rt.cpan.org>
Content-Type: text/plain; charset="UTF-8"
Message-ID: <rt-3.8.HEAD-17367-1328034691-687.41871-0-0 [...] rt.cpan.org>
Content-Transfer-Encoding: binary
X-RT-Original-Encoding: utf-8
Content-Length: 603
Download (untitled) / with headers
text/plain 603b
Thanks for looking into this. I suggest the key thing is to avoid false positives. If the 'Glob written as <...>' policy fires and is working properly, then the 'Readline inside "for" loop' policy cannot correctly fire for the same code. I suggest that if some code matches whatever test is used by 'Glob written as <...>' to see if it looks like a glob, then suppress the 'Readline inside "for" loop' check. The other way round would also be possible - if it looks like a readline, don't check it to see if it is a glob - but it looks as though the glob test is slightly more reliable on real code.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.