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
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.
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>
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 { ... }
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?
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>
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
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
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.
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.
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.