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)

History Show all quoted text
  Fri May 23 04:18:16 2008 EDAVIS [...] cpan.org - Ticket created
Subject: InputOutput::RequireBracedFileHandleWithPrint confused by here document
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.
  Fri May 23 08:36:33 2008 perl [...] galumph.com - Correspondence added
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>
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.
  Fri May 23 08:36:35 2008 The RT System itself - Status changed from 'new' to 'open'
  Fri May 23 09:25:23 2008 EDAVIS [...] cpan.org - Correspondence added
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.
  Fri May 23 23:44:29 2008 chris [...] chrisdolan.net - Correspondence added
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
  Mon May 26 10:23:37 2008 EDAVIS [...] cpan.org - Correspondence added
From: EDAVIS [...] cpan.org
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.
  Mon Jul 14 01:41:48 2008 thaljef [...] cpan.org - Correspondence added
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
  Mon Jul 14 01:41:50 2008 thaljef [...] cpan.org - Status changed from 'open' to 'stalled'
  Mon Jul 14 11:54:52 2008 EDAVIS [...] cpan.org - Correspondence added
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.
  Mon Jul 14 11:54:54 2008 The RT System itself - Status changed from 'stalled' to 'open'
  Mon Jul 14 13:27:58 2008 perl [...] galumph.com - Correspondence added
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>
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.
  Mon Jul 14 13:34:39 2008 perl [...] galumph.com - Correspondence added
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>
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.
  Mon Jul 14 17:10:01 2008 EDAVIS [...] cpan.org - Correspondence added
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.