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: 42254
Status: open
Priority: 0/
Queue: Perl-Critic
People
Owner: Nobody in particular
Requestors: krasnoroot [...] mail.ru
Cc:
AdminCc:
Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)

History Show all quoted text
  Fri Jan 09 09:11:34 2009 krasnoroot [...] mail.ru - Ticket created
Subject: several errors (false-negative results)
Date: Fri, 09 Jan 2009 19:13:06 +0500
To: bug-Perl-Critic [...] rt.cpan.org
From: Alexander Krasnorutsky <krasnoroot [...] mail.ru>
Hello! Thanks for a very useful and powerful tool with a lot of excellent features. But, unfortunately, there are some bugs (?) (at least they seems to be errors). 1. false-negative - Variables::RequireLocalizedPunctuationVars with `@a' 2. false-negative - Subroutines::ProhibitNestedSubs and `BEGIN { sub t{ } }' 3. false-negative - Subroutines::ProhibitAmpersandSigils with `defined &func' 4. false-negative - CodeLayout::ProhibitParensWithBuiltins and `Tk::exit(1)' 5. InputOutput::RequireCheckedSyscalls - why checks `close' (even if the handle is really a socket) but not `shutdown' ? Should this policy be applied only when the severity level is 1 (the most strict) ? I think that it is more important (IMHO). 6. ErrorHandling::RequireCheckingReturnValueOfEval - the bug really is in object destructor which should localise global variable $@ before implicitly using it (encapsulation ...) but not in code which checks $@, is not it ? 7. InputOutput::ProhibitExplicitStdin - of course, `<>' is a convenient thing but without some additional checks it is insecure! Something like `|rm -rf /' in arguments will cause a disaster (but user could want just to open a file with such a `magic' name). And it is not very good to mix I/O and handling of command line arguments. 8. RegularExpressions::RequireExtendedFormatting - is it really needed for very short patterns ? Excuse me for this critique(!) of the PBP (but it is inspired by the name of this module ...). $VERSION = 1.094001; perl 5.8.6; PPI-1.203 Best regards -- Alexander Krasnorutsky.
  Fri Jan 09 12:35:53 2009 jeff [...] imaginative-software.com - Correspondence added
Subject: Re: [rt.cpan.org #42254] several errors (false-negative results)
Date: Fri, 9 Jan 2009 09:35:36 -0800
To: bug-Perl-Critic [...] rt.cpan.org
From: "Jeffrey Thalhammer" <jeff [...] imaginative-software.com>
Hi Alexander- Thank you for the excellent bug report! We might have some questions and/or different opinions about some of these things. But we will investigate them as soon as possible. Thanks again! -Jeff
  Fri Jan 09 12:35:54 2009 The RT System itself - Status changed from 'new' to 'open'
  Sat Jan 10 14:44:10 2009 krasnoroot [...] mail.ru - Correspondence added
Subject: Apology [rt.cpan.org #42254]
Date: Sat, 10 Jan 2009 23:19:14 +0500
To: bug-Perl-Critic [...] rt.cpan.org
From: Alexander Krasnorutsky <krasnoroot [...] mail.ru>
Greetings! I ought to read documentation of all these policies before sending a bug report. RegularExpressions::RequireExtendedFormatting is configurable, but why the default value of the minimal length is 0? InputOutput::RequireCheckedSyscalls is also configurable (user can choice appropriate functions). However, "close SOCKET" is always successful (at least, documentation (BSD Man Page shutdown(2)) tell us that similar `shutdown' system call return -1 only if the argument is not a socket or this socket is not connected; `close' works also and on unconnected sockets); if POSIX and MSDN say the same then checking of return value only will make the program more slow; if "close SOCKET" returns false (interrupted by a signal???) we can only try to close it again or `die $!'. It is absolutely impossible to determine EXACTLY if given file handle is a socket but heuristics can help. Obviously, it is not a bug in this policy but detecting sockets will be an improvement (maybe it is impossible with current version of PPI). Subroutines::ProhibitNestedSubs - `BEGIN' is a (special) subroutine but if you place a subroutine inside it you do it not to `localise' it but to execute (or just to define) it `before the rest of the containing file (or string) is parsed' (but formally it is a nested subroutine). Best regards -- Alexander Krasnorutsky.
  Thu Jan 15 07:44:02 2009 EDAVIS [...] cpan.org - Correspondence added
Show quoted text
>3. false-negative - Subroutines::ProhibitAmpersandSigils with `defined >&func'
I believe this is intentional - see #38855.
  Thu Jan 15 13:54:18 2009 krasnoroot [...] mail.ru - Correspondence added
Subject: [rt.cpan.org #42254] [rt.cpan.org #24619]
Date: Thu, 15 Jan 2009 23:53:37 +0500
To: bug-Perl-Critic [...] rt.cpan.org
From: Alexander Krasnorutsky <krasnoroot [...] mail.ru>
Hello! I have read ticket #24619 but I do not agree because the built-in function "defined" cannot be overridden without using source filters (it has no prototype and all my attempts to override it have failed: these examples do not work). perl -we 'print prototype "defined"' # prints "Use of uninitialized value in print at -e line 1." pe -we 'use subs "defined"; sub t { print "My sub" }; sub defined { print "Overriden" }; 1 if defined &t' pe -we 'use subs "defined"; sub t { print "My sub" }; sub defined { print "Overriden" }; 1 if defined(&t)' # the previous 2 examples print nothing pe -we 'BEGIN { package Another;*{"::defined"} = sub { print "Overriden" } } sub t { print "My sub" } 1 if defined(&t)' pe -we 'BEGIN { package Another;*{"::defined"} = sub { print "Overriden" } } sub t { print "My sub" } 1 if defined &t' # these programs also print nothing So "defined &func" is definitely not an error and every time when we see it in a program we know exactly what does it mean: it means a built-in test. Best regards -- Alexander Krasnorutsky.
  Fri Jan 16 10:57:35 2009 krasnoroot [...] mail.ru - Correspondence added
Subject: [rt.cpan.org #42254]
Date: Fri, 16 Jan 2009 17:59:43 +0500
To: bug-Perl-Critic [...] rt.cpan.org
From: Alexander Krasnorutsky <krasnoroot [...] mail.ru>
Hello! There was a mistake in my previous correspondence, we should test "CORE::defined", not "defined": perl -we 'print prototype "CORE::defined"' # also prints "Use of uninitialized value in print at -e line 1." . However, the results are the same. Best regards -- Alexander Krasnorutsky.
  Sat Jan 24 23:57:06 2009 thaljef [...] cpan.org - Correspondence added
Show quoted text
> 4. false-negative - CodeLayout::ProhibitParensWithBuiltins and
`Tk::exit(1)' Why do you think this is a false negative? Tk::exit(1) is not the same thing as the Perl built-in. Please help me understand.
  Sun Jan 25 04:57:55 2009 krasnoroot [...] mail.ru - Correspondence added
Subject: Re: [rt.cpan.org #42254] several errors (false-negative results)
Date: Sun, 25 Jan 2009 13:18:50 +0500
To: bug-Perl-Critic [...] rt.cpan.org
From: Alexander Krasnorutsky <krasnoroot [...] mail.ru>
On Sat, 2009-01-24 at 23:57 -0500, Jeffrey Ryan Thalhammer via RT wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=42254 > >
> > 4. false-negative - CodeLayout::ProhibitParensWithBuiltins and
> `Tk::exit(1)' > > > Why do you think this is a false negative? Tk::exit(1) is not the same > thing as the Perl built-in. Please help me understand. > >
Sorry, it is my error. This warning was about code in the another line; the code was like this: #============================================================= print "Internal error\n"; Tk::exit(ERROR_CODE) if defined(&Tk::exit); #============================================================= but "perlcritic" told: "Builtin function called with parentheses at line 11, near 'Tk::exit(ERROR_CODE)'". It concerned "defined(...)", not "Tk::exit". So there is no any bugs here. -- Krasnorutsky Alexander.
  Fri Mar 06 01:39:56 2009 thaljef [...] cpan.org - Correspondence added
If I understand correctly, these all turned out to be false alarms, so I've marked this ticket as "rejected".
  Fri Mar 06 01:39:57 2009 thaljef [...] cpan.org - Status changed from 'open' to 'rejected'
  Fri Mar 06 09:11:50 2009 krasnoroot [...] mail.ru - Correspondence added
Subject: Re: [rt.cpan.org #42254] several errors (false-negative results)
Date: Fri, 06 Mar 2009 19:10:34 +0500
To: bug-Perl-Critic [...] rt.cpan.org
From: Alexander Krasnorutsky <krasnoroot [...] mail.ru>
On Fri, 2009-03-06 at 01:39 -0500, Jeffrey Ryan Thalhammer via RT wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=42254 > > > If I understand correctly, these all turned out to be false alarms, so > I've marked this ticket as "rejected". >
Yes, some of these things are really `false alarms', but the first (about RequireLocalizedPunctuationVars with `@a') and the second (Subroutines::ProhibitNestedSubs and `BEGIN { sub t{ } }') issues are bugs (I think). Issues #5,6,7 are not important. -- Alexander Krasnorutsky.
  Fri Mar 06 09:11:51 2009 The RT System itself - Status changed from 'rejected' to 'open'
  Fri Mar 06 21:37:00 2009 wyant [...] cpan.org - Correspondence added
On Fri Mar 06 06:11:50 2009, krasnoroot@mail.ru wrote: Show quoted text
> On Fri, 2009-03-06 at 01:39 -0500, Jeffrey Ryan Thalhammer via RT wrote:
> > <URL: https://rt.cpan.org/Ticket/Display.html?id=42254 > > > > > If I understand correctly, these all turned out to be false alarms, so > > I've marked this ticket as "rejected". > >
> > Yes, some of these things are really `false alarms', but the first > (about RequireLocalizedPunctuationVars with `@a') and the second > (Subroutines::ProhibitNestedSubs and `BEGIN { sub t{ } }') issues are > bugs (I think). Issues #5,6,7 are not important. > >
I have some sympathy with Mr. Krasnorutsky on @a; it's not $a. But the way the Perl symbol table works @a and $a may well be inseparable. The ProhibitNestedSubs issue is discussed in RT 33936. Tom Wyant
  Sat Mar 07 01:59:48 2009 krasnoroot [...] mail.ru - Correspondence added
Subject: Re: [rt.cpan.org #42254]
Date: Sat, 07 Mar 2009 11:59:47 +0500
To: bug-Perl-Critic [...] rt.cpan.org
From: Alexander Krasnorutsky <krasnoroot [...] mail.ru>
On Fri, 2009-03-06 at 21:37 -0500, Tom Wyant via RT wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=42254 > > ... @a; it's not $a. But the > way the Perl symbol table works @a and $a may well be inseparable.
*a{SCALAR} ("magic") and *a{ARRAY} (ordinary) are different things so it is possible to distinguish them (if $] >= 5.003; if $] >= 5 the older method (*a = \$bar) works).