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

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

Closed
wants to merge 1 commit into from

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Aug 14, 2023

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

  • 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

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:
```bash
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.
@jrfnl jrfnl changed the title PHP 8.3 | Ruleset: fix deprecation notice PHP 8.3 | Ruleset::explain() fix deprecation notice Aug 27, 2023
@DannyvdSluijs
Copy link

DannyvdSluijs commented Aug 28, 2023

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
Copy link
Contributor Author

jrfnl commented Dec 2, 2023

Closing as replaced by PHPCSStandards/PHP_CodeSniffer#84

@jrfnl
Copy link
Contributor Author

jrfnl commented Dec 8, 2023

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).

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