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

Ruleset::explain(): use natural sorting #3868

Closed
wants to merge 1 commit into from

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Aug 6, 2023

Description

Use natural sorting for the list of included sniffs.

The SORT_NATURAL and the SORT_FLAG_CASE flag are available since PHP 5.4, so can be used without issue.

Suggested changelog entry

The -e (explain) command will now list sniffs in natural order.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
    • This change is only breaking for integrators, not for external standards or end-users.
  • Documentation improvement

... for the list of included sniffs.

The `SORT_NATURAL` and the `SORT_FLAG_CASE` flag are available since PHP 5.4, so can be used without issue.
@jrfnl jrfnl force-pushed the feature/ruleset-explain-use-natural-sort branch from f468ab7 to 1757c04 Compare August 6, 2023 14:53
@@ -238,7 +238,7 @@ public function __construct(Config $config)
public function explain()
{
$sniffs = array_keys($this->sniffCodes);
sort($sniffs);
sort($sniffs, (SORT_NATURAL | SORT_FLAG_CASE));
Copy link
Contributor

Choose a reason for hiding this comment

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

Would using natcasesort() be more legible here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I'll need to check if that function was available in PHP 5.4.

Copy link
Contributor Author

@jrfnl jrfnl Aug 14, 2023

Choose a reason for hiding this comment

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

Just checked, yes, the function is available, but is still not the right choice.

natcasesort() maintains key association. sort() does not.
The foreach() loop used in the function uses the (numeric) key to determine whether the end of the array has been reached, so the array not being re-indexed could cause it to stop the loop too early or even to display sniffs with the wrong standard.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yes, I'd forgotten about that inconsistency. Thanks for checking / remembering!

Copy link

@DannyvdSluijs DannyvdSluijs left a comment

Choose a reason for hiding this comment

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

Approved. The output now shows the PSR2 before the PSR12 sniffs.

--- /tmp/master.out     2023-08-28 20:52:17
+++ /tmp/diff.out       2023-08-28 20:53:08
@@ -43,16 +43,16 @@
   PEAR.WhiteSpace.ScopeClosingBrace
   PEAR.WhiteSpace.ScopeIndent
 
-PSR12 (1 sniff)
-----------------
-  PSR12.Files.OpenTag
-
 PSR2 (3 sniffs)
 ---------------
   PSR2.Classes.PropertyDeclaration
   PSR2.Files.EndFileNewline
   PSR2.Methods.MethodDeclaration
 
+PSR12 (1 sniff)
+----------------
+  PSR12.Files.OpenTag
+
 Squiz (25 sniffs)
 -----------------
   Squiz.Arrays.ArrayBracketSpacing

@jrfnl
Copy link
Contributor Author

jrfnl commented Dec 2, 2023

Closing as replaced by PHPCSStandards/PHP_CodeSniffer#79

@jrfnl jrfnl closed this Dec 2, 2023
@jrfnl jrfnl deleted the feature/ruleset-explain-use-natural-sort branch December 2, 2023 02:32
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.

3 participants