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 | Generic/ScopeIndent: bug fix - missing defensive coding #3857

Closed
wants to merge 1 commit into from

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Jul 20, 2023

Description

As of PHP 8.3, PHP will throw a Warning: Decrement on type bool has no effect, this will change in the next major version of PHP notice.

A test run with PHP 8.3 showed this deprecation notice being thrown in the Generic.WhiteSpace.ScopeIndent sniff.

Investigation of the notice showed that this was actually a bug due to too little defensive coding.

The sniff tries to skip over multi-line/multi-token text strings, but the findNext() will return false for a single-line/single-token text string, which would lead to $i being reset to 0.

This commit fixes this by only changing $i when the return from the call to findNext() is not false.

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

Note: I've not added any extra tests as this issue was discovered via the pre-existing tests, so is already covered.

Suggested changelog entry

Generic.WhiteSpace.ScopeIndent: bug fix preventing a deprecation notice on PHP 8.3

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

As of PHP 8.3, PHP will throw a `Warning: Decrement on type bool has no effect, this will change in the next major version of PHP` notice.

A test run with PHP 8.3 showed this deprecation notice being thrown in the `Generic.WhiteSpace.ScopeIndent` sniff.

Investigation of the notice showed that this was actually a bug due to too little defensive coding.

The sniff tries to skip over multi-line/multi-token text strings, but the `findNext()` will return `false` for a single-line/single-token text string, which would lead to `$i` being reset to `0`.

This commit fixes this by only changing `$i` when the return from the call to `findNext()` is not `false`.

Ref: https://wiki.php.net/rfc/saner-inc-dec-operators
@jrfnl
Copy link
Contributor Author

jrfnl commented Dec 2, 2023

Closing as replaced by PHPCSStandards/PHP_CodeSniffer#76

@jrfnl jrfnl closed this Dec 2, 2023
@jrfnl jrfnl deleted the php-8.3/generic-scopeindent-bugfix branch December 2, 2023 02:30
@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.

2 participants