-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[CS] Respect PSR2 4.2 #19176
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
[CS] Respect PSR2 4.2 #19176
Conversation
I'm :-0: on this one. |
public $bar = null; | ||
|
||
public $initialized = false; | ||
|
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.
We don't add blank lines between property declarations
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.
This LF is "inherited" from the original separator between the property declaration and the next class element. I'll add a test for this behavior.
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.
LGTM, after removing the blank lines also |
| Q | A | ------------- | --- | Branch | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | http://www.php-fig.org/psr/psr-2/#4-2-properties
e8bc2e9
to
d250b1d
Compare
I've updated the fixer behavior for line breaks. |
I thought this was a CS rule already 😀 (makes sense to me at least), ie i changed it for the workflow component just now #19187 (The blank lines that is) @nicolas-grekas that is part of the rule then too right? |
👍 |
Thank you @phansys. |
This PR was merged into the 2.7 branch. Discussion ---------- [CS] Respect PSR2 4.2 | Q | A | ------------- | --- | Branch | 2.7 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | http://www.php-fig.org/psr/psr-2/#4-2-properties I'm writing a [new fixer](PHP-CS-Fixer/PHP-CS-Fixer#1889) (`single_class_element_per_statement`), which intends to avoid multiple class elements in one statement. This PR is a POC about the result of this fixer, so I'd like to know what do you think about, since it will be also added to the `@Symfony` ruleset. Please note that class constants are considered too, but here isn't any offending code about that. Example output: ``` $ ./php-cs-fixer.phar fix . --rules=single_class_element_per_statement -vvv 1) src/Symfony/Component/Console/Tests/Input/InputDefinitionTest.php (single_class_element_per_statement) 2) src/Symfony/Component/DependencyInjection/Tests/ContainerTest.php (single_class_element_per_statement) 3) src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/foo.php (single_class_element_per_statement) 4) src/Symfony/Bridge/ProxyManager/Tests/LazyProxy/Fixtures/includes/foo.php (single_class_element_per_statement) ``` Commits ------- d250b1d [CS] Respect PSR2 4.2
http://www.php-fig.org/psr/psr-2/#4-2-properties
I'm writing a new fixer (
single_class_element_per_statement
), which intends to avoid multiple class elements in one statement.This PR is a POC about the result of this fixer, so I'd like to know what do you think about, since it will be also added to the
@Symfony
ruleset.Please note that class constants are considered too, but here isn't any offending code about that.
Example output: