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

AbstractClassRestrictionsSniff: fix insufficient defensive coding #2500

Merged

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Oct 3, 2024

This commit fixes the test failure seen in PR #2499.

The test failure was surfaced due to a new exception in PHPCS (see: PHPCSStandards/PHP_CodeSniffer#524), which will be included in PHPCS 3.11.0.

The test failure highlighted that the above mentioned PHPCS PR needs a follow-up with some extra defensive coding, however, that defensive coding is needed in a part in PHPCS which does the error handling for PHP notices being encountered, i.e. the throwing of Internal.Exception errors.

This implied there was also an underlying issue in WPCS causing the Internal.Exception, which apparently was only triggered when the fixer was being run.

Note: the fact that Internal.Exceptions during a PHPCBF run are being suppressed is a known issue, but fixing that is not that easy, so that definitely won't happen before PHPCS 4.0 (and may not even make it into 4.0). See: squizlabs/PHP_CodeSniffer#2871

Either way, I've looked into the Internal.Exception now. The error was as follows:

An error occurred during processing; checking has been aborted. The error message was: PHPCSUtils\Utils\GetTokensAsString::getString(): Argument #2 ($start) must be a stack pointer which exists in the $phpcsFile object, 8 given.
The error originated in the AbstractClassRestrictionsSniff.php sniff on line 131.

The additional defensive coding added in this commit, fixes the issue.

This commit fixes the test failure seen in PR 2499.

The test failure was surfaced due to a new exception in PHPCS (see: PHPCSStandards/PHP_CodeSniffer 524), which will be included in PHPCS 3.11.0.

The test failure highlighted that the above mentioned PHPCS PR needs a follow-up with some extra defensive coding, however, that defensive coding is needed in a part in PHPCS which does the error handling for PHP notices being encountered, i.e. the throwing of `Internal.Exception` errors.

This implied there was also an underlying issue in WPCS causing the `Internal.Exception`, which apparently was only triggered when the fixer was being run.

> Note: the fact that `Internal.Exception`s during a PHPCBF run are being suppressed is a known issue, but fixing that is not that easy, so that definitely won't happen before PHPCS 4.0 (and may not even make it into 4.0). See: squizlabs/PHP_CodeSniffer 2871

Either way, I've looked into the `Internal.Exception` now.
The error was as follows:
```
An error occurred during processing; checking has been aborted. The error message was: PHPCSUtils\Utils\GetTokensAsString::getString(): Argument #2 ($start) must be a stack pointer which exists in the $phpcsFile object, 8 given.
The error originated in the AbstractClassRestrictionsSniff.php sniff on line 131.
```

The additional defensive coding added in this commit, fixes the issue.
@jrfnl
Copy link
Member Author

jrfnl commented Oct 3, 2024

Related upstream fix has also been pulled: PHPCSStandards/PHP_CodeSniffer#625

@dingo-d dingo-d merged commit 0988a5a into develop Oct 4, 2024
51 checks passed
@dingo-d dingo-d deleted the feature/abstractclassrestrictions-extra-defensive-coding branch October 4, 2024 19:05
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