PHP 8.3 | Ruleset::explain() fix deprecation notice

jrfnl commented 1 year ago

Description

On PHP 8.3, using the -e (explain) command would show a Deprecated: Increment on non-alphanumeric string is deprecated in path/to/src/Ruleset.php on line 297 notice after the very first sniff being listed.

As the $lastCount variable is used as a counter, it should have an integer default (start) value, not a text string.

Fixed now.

Testable via the command-line using the following command:

phpcs -e --standard=PSR1

Output without this fix:

The PSR1 standard contains 8 sniffs

Generic (4 sniffs)
------------------
  Generic.Files.ByteOrderMark

Deprecated: Increment on non-alphanumeric string is deprecated in D:\000_GitHub\PHPCS\PHP_CodeSniffer\src\Ruleset.php on line 297
  Generic.NamingConventions.UpperCaseConstantName
  Generic.PHP.DisallowAlternativePHPTags
  Generic.PHP.DisallowShortOpenTag

PSR1 (3 sniffs)
---------------
  PSR1.Classes.ClassDeclaration
  PSR1.Files.SideEffects
  PSR1.Methods.CamelCapsMethodName

Squiz (1 sniff)
----------------
  Squiz.Classes.ValidClassName

Output with this fix:

The PSR1 standard contains 8 sniffs

Generic (4 sniffs)
------------------
  Generic.Files.ByteOrderMark
  Generic.NamingConventions.UpperCaseConstantName
  Generic.PHP.DisallowAlternativePHPTags
  Generic.PHP.DisallowShortOpenTag

PSR1 (3 sniffs)
---------------
  PSR1.Classes.ClassDeclaration
  PSR1.Files.SideEffects
  PSR1.Methods.CamelCapsMethodName

Squiz (1 sniff)
----------------
  Squiz.Classes.ValidClassName

👉🏻 Note: yes, I'd like to add some tests for this, but it would be much easier to do so without the (partially unclosed) output buffers being created by this function, so I'm pulling this fix first and will refactor the method and add some tests at some point in the future.

Suggested changelog entry

Explain command: fixed PHP 8.3 deprecation notice

Related issues/external references

Refs: https://wiki.php.net/rfc/saner-inc-dec-operators

Types of changes

[x] 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

DannyvdSluijs commented 1 year ago

Tested these changes running phpcs from my local clone. Using Laravel Herd I could easily switch to PHP 8.3 beta. On the master branch I could see the mentioned deprecation whereas on the PR branch this message is gone. Secondly the code review looks like what you would expect from the fix.

jrfnl commented 9 months ago

Closing as replaced by https://github.com/PHPCSStandards/PHP_CodeSniffer/pull/84

jrfnl commented 8 months ago

FYI: this fix is included in today's PHP_CodeSniffer 3.8.0 release.

As per #3932, development on PHP_CodeSniffer will continue in the PHPCSStandards/PHP_CodeSniffer repository. If you want to stay informed, you may want to start "watching" that repo (or watching releases from that repo).

squizlabs / PHP_CodeSniffer