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: 82177
Status: open
Priority: 0/
Queue: Perl-Critic
People
Owner: Nobody in particular
Requestors: sandals [...] crustytoothpaste.net
Cc:
AdminCc:
Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)
Attachments
0001-Add-a-ProhibitUnlessElse-policy.patch
signature.asc

History Show all quoted text
  Sat Dec 22 17:31:28 2012 sandals [...] crustytoothpaste.net - Ticket created
Subject: [PATCH] a policy to prohibit unless-else
Date: Sat, 22 Dec 2012 22:31:04 +0000
To: bug-Perl-Critic [...] rt.cpan.org
From: "brian m. carlson" <sandals [...] crustytoothpaste.net>
Reading and understanding unless-elsif-else chains is difficult because one has to think about the inverse of the condition for the unless, and then the inverse of *that* for the else. It's simply clearer and more understandable to either invert the position of the blocks (for simple unless-else) and use if-else or to just use an appropriate negation in the initial condition for more complex chains. Included is a patch against 1.118 that provides a ProhibitUnlessElse policy that checks for this specific usage. Although I wrote this on my own, this policy implements a standard we have at $DAYJOB that I would very much like to have Perl::Critic check for. It's based off the ProhibitCascadingIfElse policy. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187

Message body is not shown because sender requested not to inline it.

Download signature.asc
application/pgp-signature 836b

Message body not shown because it is not plain text.

  Thu Jan 03 11:53:56 2013 jeff [...] imaginative-software.com - Correspondence added
Subject: Re: [rt.cpan.org #82177] [PATCH] a policy to prohibit unless-else
Date: Wed, 2 Jan 2013 14:32:01 -0800
To: bug-Perl-Critic [...] rt.cpan.org
From: Jeffrey Ryan Thalhammer <jeff [...] imaginative-software.com>
On Dec 22, 2012, at 2:31 PM, sandals@crustytoothpaste.net via RT wrote: Show quoted text
> Reading and understanding unless-elsif-else chains is difficult because > one has to think about the inverse of the condition for the unless, and > then the inverse of *that* for the else. It's simply clearer and more > understandable to either invert the position of the blocks (for simple > unless-else) and use if-else or to just use an appropriate negation in > the initial condition for more complex chains. > > Included is a patch against 1.118 that provides a ProhibitUnlessElse > policy that checks for this specific usage. Although I wrote this on my > own, this policy implements a standard we have at $DAYJOB that I would > very much like to have Perl::Critic check for. It's based off the > ProhibitCascadingIfElse policy.
This is a perfectly fine Policy, but ControlStructures::ProhibitUnlessBlocks already bans unless{...} entirely. We aspire to make all the Policies that ship with Perl::Critic consistent with each other, so you can comply with all of them at once. If we added this Policy to the core as well, then users would have to choose between ProhibitUnlessElse and ProhibitUnlessBlocks. Alternatively, you could make ProhibitUnlessBlocks configurable so that the Policy only applied when the unless{…} is followed by an else{…} or elsif{…}. The default would have to be false to stay compatible with the current behavior. I would take a patch for that (better still, I'd just give you a commit bit). But I think the best option is to ship your ProhibitUnlessElse Policy in a separate distribution. I'd be happy to reference it in the documentation for ProhibitUnlessBlocks. If you *really* don't want to take responsibility for owning a CPAN distribution, then we can adopt it into Perl-Critic-More. -Jeff
  Thu Jan 03 11:53:57 2013 The RT System itself - Status changed from 'new' to 'open'
  Thu Jan 03 19:21:51 2013 sandals [...] crustytoothpaste.net - Correspondence added
Subject: Re: [rt.cpan.org #82177] [PATCH] a policy to prohibit unless-else
Date: Fri, 4 Jan 2013 00:21:36 +0000
To: Jeffrey Thalhammer via RT <bug-Perl-Critic [...] rt.cpan.org>
From: "brian m. carlson" <sandals [...] crustytoothpaste.net>
On Thu, Jan 03, 2013 at 11:53:57AM -0500, Jeffrey Thalhammer via RT wrote: Show quoted text
> Alternatively, you could make ProhibitUnlessBlocks configurable so > that the Policy only applied when the unless{…} is followed by an > else{…} or elsif{…}. The default would have to be false to stay > compatible with the current behavior. I would take a patch for that > (better still, I'd just give you a commit bit).
I'll send you a patch for this at some point over the next week. I'm not interested in a commit bit at the moment, thanks. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187
Download signature.asc
application/pgp-signature 836b

Message body not shown because it is not plain text.

  Fri Jan 04 20:44:11 2013 BRIANC [...] cpan.org - Correspondence added
On Thu Jan 03 19:21:51 2013, sandals@crustytoothpaste.net wrote: Show quoted text
> On Thu, Jan 03, 2013 at 11:53:57AM -0500, Jeffrey Thalhammer via RT
wrote: Show quoted text
> > Alternatively, you could make ProhibitUnlessBlocks configurable so > > that the Policy only applied when the unless{…} is followed by an > > else{…} or elsif{…}. The default would have to be false to stay > > compatible with the current behavior. I would take a patch for that > > (better still, I'd just give you a commit bit).
> > I'll send you a patch for this at some point over the next week. I'm > not interested in a commit bit at the moment, thanks.
Actually, come to think of it, it's probably just better to ship it in Perl::Critic::More as you suggested.
  Fri Jan 04 21:15:38 2013 jeff [...] imaginative-software.com - Correspondence added
Subject: Re: [rt.cpan.org #82177] [PATCH] a policy to prohibit unless-else
Date: Fri, 4 Jan 2013 18:15:24 -0800
To: bug-Perl-Critic [...] rt.cpan.org
From: Jeffrey Ryan Thalhammer <jeff [...] imaginative-software.com>
On Jan 4, 2013, at 5:44 PM, brian m. carlson via RT wrote: Show quoted text
> Actually, come to think of it, it's probably just better to ship it in > Perl::Critic::More as you suggested.
So you're not interested in releasing this yourself?
  Fri Jan 04 21:44:03 2013 sandals [...] crustytoothpaste.net - Correspondence added
Subject: Re: [rt.cpan.org #82177] [PATCH] a policy to prohibit unless-else
Date: Sat, 5 Jan 2013 02:43:50 +0000
To: Jeffrey Thalhammer via RT <bug-Perl-Critic [...] rt.cpan.org>
From: "brian m. carlson" <sandals [...] crustytoothpaste.net>
On Fri, Jan 04, 2013 at 09:15:42PM -0500, Jeffrey Thalhammer via RT wrote: Show quoted text
> On Jan 4, 2013, at 5:44 PM, brian m. carlson via RT wrote: >
> > Actually, come to think of it, it's probably just better to ship it in > > Perl::Critic::More as you suggested.
> > So you're not interested in releasing this yourself?
Nope. It seems silly to have a CPAN distribution that contains just one Perl::Critic policy, and I can't think of any other policies that I'd want to write. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187