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)

History Show all quoted text
  Mon Dec 22 09:51:46 2008 EDAVIS [...] cpan.org - Ticket created
Subject: InputOutput::ProhibitReadlineInForLoop mistakes glob for readline
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.
  Mon Dec 22 22:19:53 2008 perl [...] galumph.com - Correspondence added
Subject: Re: [rt.cpan.org #41871] InputOutput::ProhibitReadlineInForLoop mistakes glob for readline
Date: Mon, 22 Dec 2008 21:19:39 -0600
To: bug-Perl-Critic [...] rt.cpan.org
From: Elliot Shank <perl [...] galumph.com>
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 { ... }
  Mon Dec 22 22:19:53 2008 The RT System itself - Status changed from 'new' to 'open'
  Tue Dec 23 08:02:39 2008 EDAVIS [...] cpan.org - Correspondence added
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?
  Tue Dec 23 13:12:44 2008 jeff [...] imaginative-software.com - Correspondence added
Subject: Re: [rt.cpan.org #41871] InputOutput::ProhibitReadlineInForLoop mistakes glob for readline
Date: Tue, 23 Dec 2008 10:12:25 -0800
To: bug-Perl-Critic [...] rt.cpan.org
From: Jeffrey Thalhammer <jeff [...] imaginative-software.com>
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
  Sat Mar 07 17:06:27 2009 wyant [...] cpan.org - Correspondence added
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
  Tue Nov 09 10:15:24 2010 wyant [...] cpan.org - Correspondence added
Patch committed to PPI wishlist item #43933. Whether it is accepted, and when the next PPI release is, is another matter.
  Tue Nov 09 10:15:25 2010 wyant [...] cpan.org - Status changed from 'open' to 'patched'
  Thu Jan 13 23:22:33 2011 wyant [...] cpan.org - Correspondence added
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.
  Thu Jan 13 23:22:34 2011 The RT System itself - Status changed from 'patched' to 'open'
  Thu Jan 13 23:22:34 2011 wyant [...] cpan.org - Status changed from 'open' to 'stalled'
  Tue Jan 31 13:31:31 2012 EDAVIS [...] cpan.org - Correspondence added
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.
  Tue Jan 31 13:31:33 2012 The RT System itself - Status changed from 'stalled' to 'open'