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

People
Owner: Nobody in particular
Requestors: mschwern [...] cpan.org
Cc:
AdminCc:

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



Subject: CodeLayout::ProhibitParensWithBuiltins exception when in ?:
Download (untitled) / with headers
text/plain 368b
CodeLayout::ProhibitParensWithBuiltins does not make an exception for built-ins used inside a ternary op. The code below illustrates: #!/usr/bin/perl -w sub foo { return @_ ? gmtime($_[0]) : gmtime(); } sub bar { return @_ ? gmtime $_[0] : gmtime; } Ternary ops are complicated enough that use of parens is certainly not a bad thing to clarify precedence.
Subject: Re: [rt.cpan.org #38646] CodeLayout::ProhibitParensWithBuiltins exception when in ?:
Date: Thu, 21 Aug 2008 17:43:28 -0500
To: bug-Perl-Critic [...] rt.cpan.org
From: Elliot Shank <perl [...] galumph.com>
Download (untitled) / with headers
text/plain 510b
Michael G Schwern via RT wrote: Show quoted text
> CodeLayout::ProhibitParensWithBuiltins does not make an exception for > built-ins used inside a ternary op. The code below illustrates: > > #!/usr/bin/perl -w > > sub foo { > return @_ ? gmtime($_[0]) : gmtime(); > } > > sub bar { > return @_ ? gmtime $_[0] : gmtime; > } > > Ternary ops are complicated enough that use of parens is certainly not a > bad thing to clarify precedence.
Part of the point of disallowing parens is to get people to learn precedence.
Subject: Re: [rt.cpan.org #38646] CodeLayout::ProhibitParensWithBuiltins exception when in ?:
Date: Thu, 21 Aug 2008 16:23:20 -0700
To: bug-Perl-Critic [...] rt.cpan.org
From: Michael G Schwern <schwern [...] pobox.com>
Download (untitled) / with headers
text/plain 2.3k
Elliot Shank via RT wrote: Show quoted text
> <URL: http://rt.cpan.org/Ticket/Display.html?id=38646 > > > Michael G Schwern via RT wrote:
>> CodeLayout::ProhibitParensWithBuiltins does not make an exception for >> built-ins used inside a ternary op. The code below illustrates: >> >> #!/usr/bin/perl -w >> >> sub foo { >> return @_ ? gmtime($_[0]) : gmtime(); >> } >> >> sub bar { >> return @_ ? gmtime $_[0] : gmtime; >> } >> >> Ternary ops are complicated enough that use of parens is certainly not a >> bad thing to clarify precedence.
> > Part of the point of disallowing parens is to get people to learn precedence.
The stated purpose of the policy is to "reduce visual clutter" and "disambiguate built-in functions from user functions". Not to force the user to study every line and puzzle out precedence rules in detail. There are complicated precedence cases where it's not worth the reader puzzling out the precedence rules, and possibly get it wrong. It creates a visual speed bump and a trap. It's much simpler to just throw in some disambiguating parens. Ternary ops are one of them. You can potentially justify using parens to disambiguate in almost any compound expression which is why I dislike this blanket rule. Hell, I couldn't list out the precedence table and I've been programming Perl for 12 years! I would not blink at: open( my $fh, "<", $file ) or die $!; "Reducing visual clutter" varies according to circumstance [1] and thus difficult to encode as a policy. "Disambiguating built-in functions" is of low value and can be overridden by the more important need to clarify the precedence of a complicated expression. I would put in a default exemption for a built-in being used as a condition (ternary op, if statement, etc...), and a configurable exemption for any time a built-in is used in anything but a simple statement. C<< $thing = function @args; >> would be simple. This would allow me to tweak the policy to allow the programmer to decide whether reducing visual clutter or disambiguation is more important while still enforcing the obviously undesirable case. [1] and destroyed if you need to put in a ## no critic on that line -- 24. Must not tell any officer that I am smarter than they are, especially if it's true. -- The 213 Things Skippy Is No Longer Allowed To Do In The U.S. Army http://skippyslist.com/list/
Subject: Re: [rt.cpan.org #38646] CodeLayout::ProhibitParensWithBuiltins exception when in ?:
Date: Thu, 21 Aug 2008 18:32:51 -0500
To: bug-Perl-Critic [...] rt.cpan.org
From: Elliot Shank <perl [...] galumph.com>
Download (untitled) / with headers
text/plain 418b
Michael G Schwern via RT wrote: Show quoted text
> "Reducing visual clutter" varies according to circumstance [1] and thus > difficult to encode as a policy. "Disambiguating built-in functions" is of > low value and can be overridden by the more important need to clarify the > precedence of a complicated expression.
Ummm... this is a severity 1 policy and thus most people won't even use it. If you don't like it, why enable it?
Subject: Re: [rt.cpan.org #38646] CodeLayout::ProhibitParensWithBuiltins exception when in ?:
Date: Thu, 21 Aug 2008 17:39:28 -0700
To: bug-Perl-Critic [...] rt.cpan.org
From: Michael G Schwern <schwern [...] pobox.com>
Download (untitled) / with headers
text/plain 1.2k
Elliot Shank via RT wrote: Show quoted text
> <URL: http://rt.cpan.org/Ticket/Display.html?id=38646 > > > Michael G Schwern via RT wrote:
>> "Reducing visual clutter" varies according to circumstance [1] and thus >> difficult to encode as a policy. "Disambiguating built-in functions" is of >> low value and can be overridden by the more important need to clarify the >> precedence of a complicated expression.
> > Ummm... this is a severity 1 policy and thus most people won't even use it. > If you don't like it, why enable it?
There are elements of this policy that I like. I agree with the general sense of the policy, that parens on built-ins are not necessary. I disagree that this should override using parens for disambiguation. What I would like is the ability to tweak policies to my project's desires. This is under the principle that the perlcritic project does not dictate policy, but allows projects to enforce policy. perlcritic just provides a good default. More config options can only help. This desire generally increases as the severity of the policy decreases, because low severity policies are generally the more debatable ones and thus more in need of tweaking. -- package Outer::Space; use Test::More tests => 9;
Subject: Re: [rt.cpan.org #38646] CodeLayout::ProhibitParensWithBuiltins exception when in ?:
Date: Thu, 21 Aug 2008 20:11:15 -0500
To: bug-Perl-Critic [...] rt.cpan.org
From: Elliot Shank <perl [...] galumph.com>
Download (untitled) / with headers
text/plain 234b
Michael G Schwern via RT wrote: Show quoted text
> This desire generally increases as the severity of the policy decreases, > because low severity policies are generally the more debatable ones and thus > more in need of tweaking.
Sounds reasonable.


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.