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: 37632
Status: resolved
Worked: 45 min
Priority: 0/
Queue: Perl-Critic
People
Owner: thaljef [...] cpan.org
Requestors: mhasch-cpanbugs [...] cozap.com
Cc:
AdminCc:
Bug Information
Severity: Normal
Broken in: 1.088
Fixed in: (no value)

History Show all quoted text
  Sat Jul 12 02:04:57 2008 hasch-cpan [...] cozap.com - Ticket created
Subject: Method call sub() confuses parser
Expressions involving the call of a method named 'sub' appear to confuse the parser when analyzing the structure of subroutines. See sample code.
Subject: method_sub_confuses_parser.pl
#!/usr/local/bin/perl use strict; use warnings; sub foo { my ( $d1, $d2 ) = @_; while ($d2) { ( $d1, $d2 ) = ( $d2->sub( bar() ), $d1 ); } return $d1; } __END__ Subroutine does not end with "return" at line 6, column 1. See page 197 of PBP. (Severity: 4) Comma used to separate statements at line 8, column 18. See pages 68,71 of PBP. (Severity: 4) Unreachable code at line 12, column 1. Consider removing it. (Severity: 4)
  Sat Jul 12 03:05:22 2008 perl [...] galumph.com - Correspondence added
Subject: Re: [rt.cpan.org #37632] Method call sub() confuses parser
Date: Sat, 12 Jul 2008 02:05:06 -0500
To: bug-Perl-Critic [...] rt.cpan.org
From: Elliot Shank <perl [...] galumph.com>
Martin Becker via RT wrote: Show quoted text
> Expressions involving the call of a method named 'sub' appear to confuse > the parser when analyzing the structure of subroutines. See sample code.
Yup. It thinks there's a subroutine prototype. Minimal example: $x->sub(bar()) PPI::Document PPI::Statement PPI::Token::Symbol '$x' PPI::Token::Operator '->' PPI::Token::Word 'sub' PPI::Token::Prototype '(bar()' PPI::Statement::UnmatchedBrace PPI::Token::Structure ')'
  Sat Jul 12 03:05:24 2008 The RT System itself - Status changed from 'new' to 'open'
  Sat Jul 12 14:48:33 2008 chris [...] chrisdolan.net - Correspondence added
Subject: Re: [rt.cpan.org #37632] Method call sub() confuses parser
Date: Sat, 12 Jul 2008 13:48:13 -0500
To: bug-Perl-Critic [...] rt.cpan.org
From: Chris Dolan <chris [...] chrisdolan.net>
Hmm, I'm surprised that's legal at all. We should make a policy to forbid subroutines named "sub" or "package" or "use", etc. Chris On Jul 12, 2008, at 2:05 AM, Elliot Shank via RT wrote: Show quoted text
> Queue: Perl-Critic > Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=37632 > > > Martin Becker via RT wrote:
>> Expressions involving the call of a method named 'sub' appear to >> confuse >> the parser when analyzing the structure of subroutines. See >> sample code.
> > Yup. It thinks there's a subroutine prototype. > > Minimal example: $x->sub(bar()) > > > PPI::Document > PPI::Statement > PPI::Token::Symbol '$x' > PPI::Token::Operator '->' > PPI::Token::Word 'sub' > PPI::Token::Prototype '(bar()' > PPI::Statement::UnmatchedBrace > PPI::Token::Structure ')' >
  Sat Jul 12 15:02:20 2008 jeff [...] imaginative-software.com - Correspondence added
Subject: Re: [rt.cpan.org #37632] Method call sub() confuses parser
Date: Sat, 12 Jul 2008 12:01:43 -0700
To: bug-Perl-Critic [...] rt.cpan.org
From: "Jeffrey Thalhammer" <jeff [...] imaginative-software.com>
On Sat, Jul 12, 2008 at 11:48 AM, Chris Dolan via RT < bug-Perl-Critic@rt.cpan.org> wrote: Show quoted text
> Queue: Perl-Critic > Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=37632 > > > Hmm, I'm surprised that's legal at all. We should make a policy to > forbid subroutines named "sub" or "package" or "use", etc. > Chris >
At the moment, Subroutines::ProhibitBuiltinHomonyms only covers user functions that have the same name as a builtin function. But we could easily extend that to include user functions that have the same name as a perl keyword. Or do you think that should be a separate Policy? -Jeff
  Sat Jul 12 15:42:21 2008 perl [...] galumph.com - Correspondence added
Subject: Re: [rt.cpan.org #37632] Method call sub() confuses parser
Date: Sat, 12 Jul 2008 14:42:06 -0500
To: bug-Perl-Critic [...] rt.cpan.org
From: Elliot Shank <perl [...] galumph.com>
Jeffrey Thalhammer via RT wrote: Show quoted text
> At the moment, Subroutines::ProhibitBuiltinHomonyms only covers user > functions that have the same name as a builtin function. But we could > easily extend that to include user functions that have the same name as a > perl keyword. Or do you think that should be a separate Policy?
I'd keep it a single policy. And I don't think there needs to be separation of keywords vs. builtins. Just dump them into a single bucket.
  Mon Jul 14 01:12:22 2008 thaljef [...] cpan.org - Correspondence added 45 min
As of rev 2589, I've expanded Subroutines::ProhibitBuiltinHomonyms to forbid subroutines that have the same name as a perl keyword (like sub, foreach, while). But this bug is really about a misparse by PPI. So I've filed a new bug here: http://rt.cpan.org/Ticket/Display.html?id=37664
  Mon Jul 14 01:12:25 2008 thaljef [...] cpan.org - Given to THALJEF
  Mon Jul 14 19:15:46 2008 hasch-cpan [...] cozap.com - Correspondence added
On Sat Jul 12 15:02:20 2008, jeff@imaginative-software.com wrote: Show quoted text
> On Sat, Jul 12, 2008 at 11:48 AM, Chris Dolan via RT < > bug-Perl-Critic@rt.cpan.org> wrote: >
> > Queue: Perl-Critic > > Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=37632 > > > > > Hmm, I'm surprised that's legal at all. We should make a policy to > > forbid subroutines named "sub" or "package" or "use", etc. > > Chris > >
> > At the moment, Subroutines::ProhibitBuiltinHomonyms only covers user > functions that have the same name as a builtin function. But we could > easily extend that to include user functions that have the same name as a > perl keyword. Or do you think that should be a separate Policy? > > -Jeff
To me, a user function hiding a Perl builtin function and thus introducing context-dependency should be rated with a higher severity than a method name with a namesake in a completely different syntactical domain. In Perl, method calls are always qualified with a class or variable name and thus easily distinguishable from everything else, so that naming policies might want to allow more freedom for methods than for, say, plain subroutines. To support this, Perl::Critic would need to tell methods from plain subroutines, which unfortunately is not always easy. A good guess might take into account whether the name is exported and how it is used locally. In conclusion, I'd prefer a separate policy. Either way, PPI should of course correctly parse method calls. -Martin
  Mon Jul 14 21:21:16 2008 perl [...] galumph.com - Correspondence added
Subject: Re: [rt.cpan.org #37632] Method call sub() confuses parser
Date: Mon, 14 Jul 2008 20:20:55 -0500
To: bug-Perl-Critic [...] rt.cpan.org
From: Elliot Shank <perl [...] galumph.com>
Martin Becker via RT wrote: Show quoted text
> Either way, PPI should of course correctly parse method calls.
But it doesn't. It just treats "foo bar" as a couple of barewords, regardless of whether there's a "sub foo {}" directly above it.
  Tue Jul 22 11:12:40 2008 perl [...] galumph.com - Correspondence added
Subject: Re: [rt.cpan.org #37632] Method call sub() confuses parser
Date: Tue, 22 Jul 2008 08:12:14 -0700
To: bug-Perl-Critic [...] rt.cpan.org
From: Elliot Shank <perl [...] galumph.com>
Jeffrey Ryan Thalhammer via RT wrote: Show quoted text
> As of rev 2589, I've expanded Subroutines::ProhibitBuiltinHomonyms to > forbid subroutines that have the same name as a perl keyword (like sub, > foreach, while).
Released in 1.090.
  Sat Jul 26 23:56:22 2008 thaljef [...] cpan.org - Correspondence added
Technically, PPI still misparses 'sub' in some cases, so I'll open a bug with Adam on that. But P::C now forbids making subroutines called 'sub' in the first place. So I'm marking this as 'resolved'.
  Sat Jul 26 23:56:24 2008 thaljef [...] cpan.org - Status changed from 'open' to 'resolved'