Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow single line closures #1580

Closed
CarsonF opened this issue Jul 28, 2017 · 4 comments
Closed

Allow single line closures #1580

CarsonF opened this issue Jul 28, 2017 · 4 comments

Comments

@CarsonF
Copy link

CarsonF commented Jul 28, 2017

This code:

$func = function ($item) { return $item['foo']; };

Triggers:

  • Generic.Formatting.DisallowMultipleStatements.SameLine
  • PEAR.WhiteSpace.ScopeClosingBrace.Line
  • Squiz.WhiteSpace.ScopeClosingBrace.ContentBefore
  • Squiz.Functions.MultiLineFunctionDeclaration.ContentAfterBrace

IMO the DisallowMultipleStatements error is a false positive. The code above is not the same as

$a = ''; $b = '';

For the rest of the errors, it would be nice to the have an option to allow single statement methods to be on one line (PHPStorm has this option). Maybe another option to further restrict it to only closures.

@tgr
Copy link

tgr commented Oct 7, 2017

Referenced in https://phabricator.wikimedia.org/T154789

tgr added a commit to tgr/PHP_CodeSniffer that referenced this issue Oct 8, 2017
…ontentBefore

Allows non-awkward single line syntax for empty functions/classes.

Fixes one of the cases mentioned in squizlabs#1580.
tgr added a commit to tgr/PHP_CodeSniffer that referenced this issue Oct 8, 2017
…ontentBefore

Allows non-awkward single line syntax for empty functions/classes.

Fixes one of the cases mentioned in squizlabs#1580.
@claytonrcarter
Copy link

@gsherwood would you entertain a PR for this? If so, do you have any tips, suggestions or pointers for implementing support for single line closures? Would you prefer that each of the indicated Sniffs be updated to allow SLCs, or is there another tactic that I should look into? I might be biting off more than I can chew, but I would like to try working on this.

@gsherwood
Copy link
Member

would you entertain a PR for this?

I can't see how this could be done with the existing sniffs. This is a specific rule for closures that does not exist in any of the included standards and is probably best served using a custom sniff. Including a rule like "Allow single line closures if they only have one statement" in these sniffs is far too specific.

@gsherwood
Copy link
Member

Closing due to lack of additional interest in a new sniff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants