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

Alphabetical use sniff doesn't differentiate between functions/constants/etc #143

Closed
rmccue opened this issue May 24, 2019 · 3 comments
Closed
Labels

Comments

@rmccue
Copy link
Member

rmccue commented May 24, 2019

The sniff we're using for the alphabetical use order from psr2r-sniffer compares use statements with direct string comparison. This means that the order is expected to be:

use A;
use const A;
use D;
use function A;
use G;

This doesn't really make much sense. The slevomat standard instead sorts classes, then functions, then constants (you can flip the last two via config).

We should look at switching to this, and clarifying the order in the handbook; if we instead want (e.g.) constants, then functions, then classes, we may need to fork it.

(Side note: there are some neat other sniffs in their standard we could incorporate.)

@pdewouters
Copy link
Contributor

Looking at the source code, there is a comment referencing the cakePHP codebase and I found this bug report cakephp/cakephp-codesniffer#246

So now if you compare these sniffs between the package we use https://github.com/php-fig-rectified/psr2r-sniffer/blob/master/PSR2R/Sniffs/Namespaces/UseInAlphabeticalOrderSniff.php and the original that was patched https://github.com/cakephp/cakephp-codesniffer/blob/7f467fee00fd016b62cf8c85a57750ab2a895e81/CakePHP/Sniffs/Formatting/UseInAlphabeticalOrderSniff.php there's a difference.

The way the sniff works at the moment is couter productive so I think I'll just disable it by default in my custom configs.

Can we remove it or suggest the fix be ported to the package we use?

@mikeselander
Copy link
Contributor

This sniff is a very constant pain to me and I'd definitely like to see it improved 👍 I think there's a basis for alphabetically ordering use statements so I'd personally argue for keeping it but trying to get it ported into the package we use.

@kadamwhite
Copy link
Contributor

#188 would disable use of function anyway; we generally would recommend avoiding const as well.

As such we're going to mark this as "won't fix" since we should be avoiding the constructs that cause this rule to misalphabetize.

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

No branches or pull requests

4 participants