-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
PEAR/FunctionDeclaration: examine arrow function declarations + fix fixer conflict #3661
base: master
Are you sure you want to change the base?
PEAR/FunctionDeclaration: examine arrow function declarations + fix fixer conflict #3661
Conversation
I'm not sure about enforcing the multi-line rules for arrow functions. I think the spacing after |
The changes I've made only kick in when arrow functions already use some sort of multi-line format for the parameters (not the function body). I agree that arrow functions will mostly be single-line, so chances that this sniff will kick in are slim anyway, but it felt like an oversight leading to inconsistency to not examine and fix the parameter lay-out when it is multi-line. |
87aa11f
to
e922ddf
Compare
I've added two more commits to this PR and updated the description to include those. |
e922ddf
to
fa2f2df
Compare
I've moved the last commit (preventing a fixer conflict for function declarations with return types) to PR #3739 as that's a straight forward bug fix and the discussion on whether on not the sniff should examine arrow functions is blocking that bug fix from going in. If no decision is taken about the sniff and arrow functions by the time #3739 has been merged, I will rebase this PR after the merge. |
Needs checking if these sniffs are used by PSR12 and if so, if PSR-PER has any additional information. |
PHP 7.4 arrow functions were not yet being taken into account for this sniff. Fixed now. Includes unit tests.
…arations The Squiz sniff inherits the change from the `PEAR.Functions.FunctionDeclaration` sniff. This commit adds tests to document the behaviour and safeguard the inherited change.
If a return type declaration was not confined to one line, the sniff could have a fixer conflict with itself. The fixer would also potentially remove a close curly on the same line, causing parse errors. Fixed now. The diff will be most straight forward to review while ignoring whitespace changes. Includes unit tests.
fa2f2df
to
d479811
Compare
Rebased without changed to get rid of conflicts after the merge of #3787. |
I have checked and this PR will need to be changed/updated to be in line with PSR-PER. I'm moving this PR to draft until it has been updated. |
PEAR/FunctionDeclaration: examine arrow function declarations
PHP 7.4 arrow functions were not yet being taken into account for this sniff.
Note: I've explicitly left the sniff to not have an opinion on the formatting for "arrow on same line/new line".
Fixed now.
Includes unit tests.
Squiz/MultiLineFunctionDeclaration: add tests for arrow function declarations
The Squiz sniff inherits the change from the
PEAR.Functions.FunctionDeclaration
sniff.This commit adds tests to document the behaviour and safeguard the inherited change.
PEAR/FunctionDeclaration: prevent fixer conflictIf a return type declaration was not confined to one line, the sniff could have a fixer conflict with itself.The fixer would also potentially remove a close curly on the same line, causing parse errors.
Fixed now. The diff will be most straight forward to review while ignoring whitespace changes.Includes unit tests.