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: 36123
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: (no value)
Fixed in: (no value)



Subject: InputOutput::RequireBracedFileHandleWithPrint confused by here document
Download (untitled) / with headers
text/plain 422b
This test program: #!/usr/bin/perl use warnings; use strict; use 5.010; sub f { uc $_[0] } print f <<END Some text. END ; run through 'perlcritic -single-policy InputOutput::RequireBracedFileHandleWithPrint' produces the message File handle for "print" is not braced at line 7, column 1. See page 217 of PBP. (Severity: 1) But the print statement does not give a filehandle.
Subject: Re: [rt.cpan.org #36123] InputOutput::RequireBracedFileHandleWithPrint confused by here document
Date: Fri, 23 May 2008 07:32:55 -0500
To: bug-Perl-Critic [...] rt.cpan.org
From: Elliot Shank <perl [...] galumph.com>
Download (untitled) / with headers
text/plain 579b
EDAVIS via RT wrote: Show quoted text
> This test program: > > #!/usr/bin/perl > use warnings; > use strict; > use 5.010; > sub f { uc $_[0] } > print f <<END > Some text. > END > ; > > run through 'perlcritic -single-policy > InputOutput::RequireBracedFileHandleWithPrint' produces the message > > File handle for "print" is not braced at line 7, column 1. See page 217 > of PBP. (Severity: 1) > > But the print statement does not give a filehandle.
I don't think this is reasonably fixable. Without the sub declaration, the problem is correct.
Download (untitled) / with headers
text/plain 360b
Show quoted text
>>File handle for "print" is not braced at line 7, column 1.
Show quoted text
>I don't think this is reasonably fixable.
One heuristic is that while print UPPERCASE_BAREWORD 1, 2, 3; is likely to be a bareword filehandle, print lowercase_bareword 1, 2, 3; is likely to be a function call. And of course print $scalar_variable 1, 2, 3; must be a filehandle.
Subject: Re: [rt.cpan.org #36123] InputOutput::RequireBracedFileHandleWithPrint confused by here document
Date: Fri, 23 May 2008 22:42:12 -0500
To: bug-Perl-Critic [...] rt.cpan.org
From: Chris Dolan <chris [...] chrisdolan.net>
On May 23, 2008, at 8:25 AM, EDAVIS via RT wrote: Show quoted text
> > Queue: Perl-Critic > Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=36123 > >
>>> File handle for "print" is not braced at line 7, column 1.
>
>> I don't think this is reasonably fixable.
> > One heuristic is that while > > print UPPERCASE_BAREWORD 1, 2, 3; > > is likely to be a bareword filehandle, > > print lowercase_bareword 1, 2, 3; > > is likely to be a function call. And of course > > print $scalar_variable 1, 2, 3; > > must be a filehandle.
That logic makes sense, but personally I don't like it. I consider myself a Perl expert, but even I can't remember when I look at some code what conditions print() considers its first arg a filehandle or not. I prefer the P::C approach that barewords as the first arg to print() is always wrong -- either should you wrap it in curlies or you should use parens on the function call. The key point, I believe, is to make it easier for a *human* to interpret the intention of the code -- in that case, the above doesn't help unless the human already knows the rule. Chris
From: EDAVIS [...] cpan.org
Download (untitled) / with headers
text/plain 490b
Hmm, in that case the warning text should say something like The first argument to 'print' may be a filehandle. If it is, put it in braces (see PBP page xxx). If it is not a filehandle but a subroutine call, use parentheses for the call (see PBP page yyy). Myself, I don't find any difficulty with the convention that all caps are used for bareword filehandles and all lowercase for function names - all the perl5 standard documentation and tutorials follow this - but I see that YMMV.
Download (untitled) / with headers
text/plain 939b
On Mon May 26 10:23:37 2008, EDAVIS wrote: Show quoted text
> Hmm, in that case the warning text should say something like > > The first argument to 'print' may be a filehandle. If it is, put it > in braces (see PBP page xxx). If it is not a filehandle but a > subroutine call, use parentheses for the call (see PBP page yyy). > > Myself, I don't find any difficulty with the convention that all caps > are used for bareword filehandles and all lowercase for function > names - all the perl5 standard documentation and tutorials follow > this - but I see that YMMV.
False positives really suck, but it looks like this will be hard to fix. I agree with Chris -- we should avoid conflating multiple rules together (e.g. requiring uppercase/lowercase for handles/subs). Do you think the world ready for RequireParensForUserFunctionCalls? Maybe it should go into P::C::More. That might reduce the likelihood of people stumbling into this bug. -Jeff
Download (untitled) / with headers
text/plain 618b
Show quoted text
>I agree with Chris -- we should avoid conflating multiple rules >together (e.g. requiring uppercase/lowercase for handles/subs).
Agreed, I don't think this policy should enforce any rule for naming subroutines, but it's surely okay to use that as a heuristic to decide what's likely to be a filehandle and what isn't. Doesn't perlcritic (and perl itself) already apply heuristics based on whitespace and other things? If you change the policy to not warn when the 'filehandle' begins with a lowercase letter, that will make it a weaker policy generating fewer warnings. That can't be an intrusion, by any measure.
Subject: Re: [rt.cpan.org #36123] InputOutput::RequireBracedFileHandleWithPrint confused by here document
Date: Mon, 14 Jul 2008 11:11:56 -0500
To: bug-Perl-Critic [...] rt.cpan.org
From: Elliot Shank <perl [...] galumph.com>
Download (untitled) / with headers
text/plain 271b
EDAVIS via RT wrote: Show quoted text
> If you change the policy to not warn when the 'filehandle' begins with a > lowercase letter, that will make it a weaker policy generating fewer > warnings. That can't be an intrusion, by any measure.
It can't? It certainly looks like one to me.
Subject: Re: [rt.cpan.org #36123] InputOutput::RequireBracedFileHandleWithPrint confused by here document
Date: Mon, 14 Jul 2008 11:11:56 -0500
To: bug-Perl-Critic [...] rt.cpan.org
From: Elliot Shank <perl [...] galumph.com>
Download (untitled) / with headers
text/plain 271b
EDAVIS via RT wrote: Show quoted text
> If you change the policy to not warn when the 'filehandle' begins with a > lowercase letter, that will make it a weaker policy generating fewer > warnings. That can't be an intrusion, by any measure.
It can't? It certainly looks like one to me.
Download (untitled) / with headers
text/plain 637b
I mean that if policy A generates warnings for the set of programs P(A), and policy B for P(B), then if P(B) is a subset of P(A), you can say that policy B is weaker than A. And if B is overly moralistic or flags certain false positives, by construction A is just as bad and perhaps worse. In this case, changing the policy by adding a heuristic 'do not warn if thing after print statement is lowercase' would make it a weaker policy. It would warn in strictly fewer cases. The resulting weakened policy might still have annoying cases where it warns when it should not, but those cases exist just as much in the policy as it stands.


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.