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: handle invalid sniffs more graciously #873

Merged
merged 1 commit into from
Mar 17, 2025

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Mar 12, 2025

Description

As things were, a ruleset loading an invalid sniff - a sniff which doesn't implement the Sniff interface and is missing either the register() or process() method, or both -, would result in fatal errors which are unfriendly to the end-user.

Now, while this will hopefully be a very rare occurrence and should no longer be possible as of PHPCS 4.0, which intends to remove support for sniffs not implementing the Sniff interface, I still believe it prudent to show more user-friendly and more informative error messages to the end-user until that time.

This commit implements this and executes step 2 to address issue #694.

Includes tests.

Output of commands involving various invalid sniffs before this PR:

$ phpcs -ps . --standard=./tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterTest.xml

Fatal error: Uncaught Error: Call to undefined method TestStandard\Sniffs\InvalidSniffError\NoImplementsNoRegisterSniff::register() in path/to/PHP_CodeSniffer/src/Ruleset.php:1505
Stack trace:
  thrown in path/to/PHP_CodeSniffer/src/Ruleset.php on line 1505
$ phpcs -ps . --standard=./tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoProcessTest.xml

PHP_CodeSniffer version 3.11.1 (stable) by Squiz and PHPCSStandards

Fatal error: Uncaught Error: Call to undefined method TestStandard\Sniffs\InvalidSniffError\NoImplementsNoProcessSniff::process() in path/to/PHP_CodeSniffer/src/Files/File.php:519
Stack trace:
  thrown in path/to/PHP_CodeSniffer/src/Files/File.php on line 519
$ phpcs -ps . --standard=./tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterOrProcessTest.xml

PHP_CodeSniffer version 3.11.1 (stable) by Squiz and PHPCSStandards

Fatal error: Uncaught Error: Call to undefined method TestStandard\Sniffs\InvalidSniffError\NoImplementsNoRegisterOrProcessSniff::register() in path/to/PHP_CodeSniffer/src/Ruleset.php:1505
Stack trace:
  thrown in path/to/PHP_CodeSniffer/src/Ruleset.php on line 1505

Output of commands involving various invalid sniffs after this PR:

$ phpcs -ps . --standard=./tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterTest.xml

ERROR: Sniff class TestStandard\Sniffs\InvalidSniffError\NoImplementsNoRegisterSniff is missing required method register().
$ phpcs -ps . --standard=./tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoProcessTest.xml

ERROR: Sniff class TestStandard\Sniffs\InvalidSniffError\NoImplementsNoProcessSniff is missing required method process().
$ phpcs -ps . --standard=./tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterOrProcessTest.xml

ERROR: Sniff class TestStandard\Sniffs\InvalidSniffError\NoImplementsNoRegisterOrProcessSniff is missing required method register().
ERROR: Sniff class TestStandard\Sniffs\InvalidSniffError\NoImplementsNoRegisterOrProcessSniff is missing required method process().

Suggested changelog entry

The user will be shown an informative error message for sniffs are missing one of the required methods.
Previously this would result in a fatal error.

Related issues/external references

Related to #694

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
As things were, a ruleset loading an invalid sniff - a sniff which doesn't implement the `Sniff` interface and is missing either the `register()` or `process()` method, or both -, would result in fatal errors which are unfriendly to the end-user.

Now, while this will hopefully be a very rare occurrence and should no longer be possible as of PHPCS 4.0, which intends to remove support for sniffs not implementing the `Sniff` interface, I still believe it prudent to show more user-friendly and more informative error messages to the end-user until that time.

This commit implements this and executes step 2 to address issue 694..

Includes tests.

Output of commands involving various invalid sniffs **before** this PR:

```
$ phpcs -ps . --standard=./tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterTest.xml

Fatal error: Uncaught Error: Call to undefined method TestStandard\Sniffs\InvalidSniffError\NoImplementsNoRegisterSniff::register() in path/to/PHP_CodeSniffer/src/Ruleset.php:1505
Stack trace:
  thrown in path/to/PHP_CodeSniffer/src/Ruleset.php on line 1505
```

```
$ phpcs -ps . --standard=./tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoProcessTest.xml

PHP_CodeSniffer version 3.11.1 (stable) by Squiz and PHPCSStandards

Fatal error: Uncaught Error: Call to undefined method TestStandard\Sniffs\InvalidSniffError\NoImplementsNoProcessSniff::process() in path/to/PHP_CodeSniffer/src/Files/File.php:519
Stack trace:
  thrown in path/to/PHP_CodeSniffer/src/Files/File.php on line 519
```

```
$ phpcs -ps . --standard=./tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterOrProcessTest.xml

PHP_CodeSniffer version 3.11.1 (stable) by Squiz and PHPCSStandards

Fatal error: Uncaught Error: Call to undefined method TestStandard\Sniffs\InvalidSniffError\NoImplementsNoRegisterOrProcessSniff::register() in path/to/PHP_CodeSniffer/src/Ruleset.php:1505
Stack trace:
  thrown in path/to/PHP_CodeSniffer/src/Ruleset.php on line 1505
```

Output of commands involving various invalid sniffs **after** this PR:
```
$ phpcs -ps . --standard=./tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterTest.xml

ERROR: Sniff class TestStandard\Sniffs\InvalidSniffError\NoImplementsNoRegisterSniff is missing required method register().
```

```
$ phpcs -ps . --standard=./tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoProcessTest.xml

ERROR: Sniff class TestStandard\Sniffs\InvalidSniffError\NoImplementsNoProcessSniff is missing required method process().
```

```
$ phpcs -ps . --standard=./tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterOrProcessTest.xml

ERROR: Sniff class TestStandard\Sniffs\InvalidSniffError\NoImplementsNoRegisterOrProcessSniff is missing required method register().
ERROR: Sniff class TestStandard\Sniffs\InvalidSniffError\NoImplementsNoRegisterOrProcessSniff is missing required method process().
```
@jrfnl jrfnl merged commit 97e340e into master Mar 17, 2025
61 checks passed
@jrfnl jrfnl deleted the feature/694-2-ruleset-ignore-broken-sniffs branch March 17, 2025 14:44
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.

None yet

2 participants