8000 [CS] Respect PSR2 4.2 by phansys · Pull Request #19176 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[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

Merged

Conversation

phansys
Copy link
Contributor
@phansys phansys commented Jun 25, 2016
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 (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)

@fabpot
Copy link
Member
fabpot commented Jun 25, 2016

I'm :-0: on this one.

public $bar = null;

public $initialized = false;

Copy link
Member

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

Copy link
Contributor Author

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicolas-grekas
Copy link
Member
nicolas-grekas commented Jun 25, 2016

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
@phansys phansys force-pushed the cs_fixer_single_class_element_per_statement branch from e8bc2e9 to d250b1d Compare June 25, 2016 16:31
@phansys
Copy link
Contributor Author
phansys commented Jun 25, 2016

I've updated the fixer behavior for line breaks.

@ro0NL
Copy link
Contributor
ro0NL commented Jun 26, 2016

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?

@dunglas
Copy link
Member
dunglas commented Jun 26, 2016

👍

@fabpot
Copy link
Member
fabpot commented Jun 27, 2016

Thank you @phansys.

@fabpot fabpot merged commit d250b1d into symfony:2.7 Jun 27, 2016
fabpot added a commit that referenced this pull request Jun 27, 2016
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
@phansys phansys deleted the cs_fixer_single_class_element_per_statement branch June 27, 2016 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants
0