-
Notifications
You must be signed in to change notification settings - Fork 51
Add rule to ensure Arrow Functions declaration format #175
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
Conversation
03de28b
to
0324f09
Compare
This new rule enforces format for already declared `arrow functions` (this sniff does not convert anonymous function to arrow functions).
0324f09
to
7e99a44
Compare
@@ -4,6 +4,7 @@ PHP CODE SNIFFER REPORT SUMMARY | |||
FILE ERRORS WARNINGS | |||
---------------------------------------------------------------------- | |||
tests/input/array_indentation.php 10 0 | |||
tests/input/arrow-functions-format.php 10 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Errors:
FILE: tests/input/arrow-functions-format.php
-------------------------------------------------------------------------------------------------
FOUND 10 ERRORS AFFECTING 7 LINES
-------------------------------------------------------------------------------------------------
5 | ERROR | [x] Closure not using "$this" should be declared static.
7 | ERROR | [x] Useless parentheses.
9 | ERROR | [x] There must be no whitespace between closing parenthesis and return type colon.
11 | ERROR | [x] Expected 0 spaces between argument "$a" and comma; 1 found
13 | ERROR | [x] There must be exactly 1 whitespace after "fn" keyword.
13 | ERROR | [x] There must be exactly 1 whitespace before =>.
13 | ERROR | [x] There must be exactly 1 whitespace after =>.
15 | ERROR | [x] There must be exactly 1 whitespace before =>.
15 | ERROR | [x] There must be exactly 1 whitespace after =>.
40 | ERROR | [x] Multi-line arrays must have a trailing comma after the last element.
-------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 10 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------------------------------
Time: 139ms; Memory: 10MB
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer fn($x)
which is also the winning option from your twitter poll
Should we have a different space handling compared to anonymous functions? |
There is also an option to change spacing of normal closures, so we can have no space in both. I would welcome that personally. |
@ostrolucky We can't, the |
@doctrine/coding-standard-approvers Mind to review this one? I'd like to get it in for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the interest of consistency, I'd prefer fn ($x)
. However, I also wouldn't object to a PR that enforces fn($x)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as @alcaeus said.
@malarzm Would you agree with us merging this PR given the arguments above? :) |
@carusogabriel I'm the only one objecting and there are already 4 approvals. Feel free to merge, I'm in minority anyway :) |
We can't click the button if changes are requested
Thanks @carusogabriel ! |
This new rule enforces format for already declared
arrow functions
(this sniffdoes not convert anonymous function to arrow functions).
Note for reviewers: the idea of having
1 space
set forspacesCountAfterKeyword
is that we follow the current standards for function calls.Pool on Twitter for reference: https://twitter.com/carusogabriel/status/1251576089497010177