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: 50894
Status: open
Priority: 0/
Queue: Perl-Critic

People
Owner: Nobody in particular
Requestors: user42 [...] zip.com.au
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)

Attachments
RequireInitializationForLocalVars.output-vars.diff



Subject: RequireInitializationForLocalVars on $@ and $!
Date: Wed, 28 Oct 2009 12:00:22 +1100
To: bug-Perl-Critic [...] rt.cpan.org
From: Kevin Ryde <user42 [...] zip.com.au>
Download (untitled) / with headers
text/plain 405b
In the current perlcritic cvs RequireInitializationForLocalVars reports a violation on local $@; or local $!; where I hoped it might recognise them as output variables not normally given an initializer when localizing. (Localizing to avoid clobbering a caller's value, etc.) A bit of a grep suggests "local $@;" is quite common. $! less so but has the same rationale. Perhaps like below,
Index: lib/Perl/Critic/Policy/Variables/RequireInitializationForLocalVars.pm =================================================================== --- lib/Perl/Critic/Policy/Variables/RequireInitializationForLocalVars.pm (revision 3692) +++ lib/Perl/Critic/Policy/Variables/RequireInitializationForLocalVars.pm (working copy) @@ -33,6 +33,9 @@ sub violates { my ( $self, $elem, undef ) = @_; + + return if List::MoreUtils::all {_is_output_var($_)} $elem->variables; + if ( $elem->type() eq 'local' && !_is_initialized($elem) ) { return $self->violation( $DESC, $EXPL, $elem ); } @@ -41,6 +44,15 @@ #----------------------------------------------------------------------------- +# variables which are outputs of various things and therefore normally won't +# have initializers in a "local" +my %output_vars = ('$!' => 1, + '$@' => 1); +sub _is_output_var { + my ($str) = @_; + return $output_vars{$str}; +} + sub _is_initialized { my $elem = shift; my $wanted = sub { $_[1]->isa('PPI::Token::Operator') && $_[1] eq q{=} }; @@ -88,7 +100,15 @@ } +=head2 Allowed Forms +An initializer is not required for C<$@> and C<$!> since they're normally +outputs from a block of code or a function. + + local $@; # ok + eval { ... } + + =head1 CONFIGURATION This Policy is not configurable except for the standard options. Index: t/Variables/RequireInitializationForLocalVars.run =================================================================== --- t/Variables/RequireInitializationForLocalVars.run (revision 3692) +++ t/Variables/RequireInitializationForLocalVars.run (working copy) @@ -26,8 +26,22 @@ our $bar our ($foo, $bar); +local $!; +local $@; +local ($!, $@); + #----------------------------------------------------------------------------- +## name mixture of $! and other var +## failures 2 +## cut + +local ($!, $foo); +local ($var, $@); + + +#----------------------------------------------------------------------------- + ## name key named "local" ## failures 0 ## cut
Subject: Re: [rt.cpan.org #50894] RequireInitializationForLocalVars on $@ and $!
Date: Wed, 28 Oct 2009 09:39:59 -0700
To: bug-Perl-Critic [...] rt.cpan.org
From: Jeffrey Thalhammer <jeff [...] imaginative-software.com>
Download (untitled) / with headers
text/plain 358b
I'm not exactly sure what you mean by classifying these as "output variables." But near the top of page 79 of PBP, Conway recommends explicitly initializing local vars, even when setting the value to undef. Thus... local $@ = undef; local $! = undef; So if I understand your request correctly, then I think the policy should remain as-is. -Jeff
Subject: Re: [rt.cpan.org #50894] RequireInitializationForLocalVars on $@ and $!
Date: Wed, 28 Oct 2009 22:28:02 -0500
To: bug-Perl-Critic [...] rt.cpan.org
From: Elliot Shank <perl [...] galumph.com>
Download (untitled) / with headers
text/plain 641b
Kevin Ryde via RT wrote: Show quoted text
> In the current perlcritic cvs RequireInitializationForLocalVars reports > a violation on > > local $@; > > or > > local $!; > > where I hoped it might recognise them as output variables not normally > given an initializer when localizing. (Localizing to avoid clobbering a > caller's value, etc.) > > A bit of a grep suggests "local $@;" is quite common.
How common something is is not relevant. Just because "eval { ... }; if ($@) { ... }" is common, doesn't make it right. Conway wants you to explicitly assign undef to the variable so that the naive reader knows the value after the statement.
Subject: Re: [rt.cpan.org #50894] RequireInitializationForLocalVars on $@ and $!
Date: Sun, 01 Nov 2009 18:46:07 -0600
To: bug-Perl-Critic [...] rt.cpan.org
From: Elliot Shank <perl [...] galumph.com>
I'm closing this one because this policy works exactly as intended.
Subject: Re: [rt.cpan.org #50894] RequireInitializationForLocalVars on $@ and $!
Date: Tue, 03 Nov 2009 11:58:25 +1100
To: bug-Perl-Critic [...] rt.cpan.org
From: Kevin Ryde <user42 [...] zip.com.au>
Download (untitled) / with headers
text/plain 759b
"Jeffrey Thalhammer via RT" <bug-Perl-Critic@rt.cpan.org> writes: Show quoted text
> > I'm not exactly sure what you mean by classifying these as "output > variables."
Variables that you wouldn't normally initialize in a local, because you're not localizing to establish a value but instead to protect the variable from being overwritten in the output of eval or a syscall. Show quoted text
> But near the top of page 79 of PBP,
I didn't notice it was a pbp one. I see it in the themes, but do such ones normally cite a chapter and verse ...? What about an option that would tone it down? Then you can have strict pbp, or a middle ground. (Middle ground supported as I say by some 50-odd places in perl itself doing such a localize, in pretty reasonable fashion, as I understand it.)


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.