Display user friendly message instead of a fatal error when unable to change a sniff property
Is your feature request related to a problem?
PHPCS displays a fatal error when the user inadvertently tries to change a sniff property that can't be changed from outside the class. Consider the ruleset.xml file below:
<?xml version="1.0"?>
<ruleset xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="phpcs.xsd">
<rule ref="Generic.PHP.ForbiddenFunctions">
<properties>
<property name="patternMatch" value="true"/>
</properties>
</rule>
</ruleset>
It results in the following PHP fatal error:
PHP Fatal error: Uncaught Error: Cannot access protected property PHP_CodeSniffer\Standards\Generic\Sniffs\PHP\ForbiddenFunctionsSniff::$patternMatch in src/Ruleset.php:1620
Stack trace:
#0 src/Ruleset.php(1449): PHP_CodeSniffer\Ruleset->setSniffProperty()
#1 src/Ruleset.php(250): PHP_CodeSniffer\Ruleset->populateTokenListeners()
#2 src/Runner.php(348): PHP_CodeSniffer\Ruleset->__construct()
#3 src/Runner.php(76): PHP_CodeSniffer\Runner->init()
#4 bin/phpcs(14): PHP_CodeSniffer\Runner->runPHPCS()
#5 {main}
thrown in src/Ruleset.php on line 1620
Describe the solution you'd like
I want to suggest that instead of a fatal error, PHPCS display a user-friendly error message alerting that the property cannot be changed.
It needs to be determined in which cases PHPCS cannot change the sniff property. Here is a non-exhaustive list:
- The property is not public and the
__set()magic method does not exist in the sniff class. - The property is public but read-only (PHP >= 8.1).
Additional context
Here is the code that checks if the property exists and displays an error if it doesn't:
https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/95e3a355ffee6ae1336d08d0c60161e2bfbb1564/src/Ruleset.php#L1571-L1586
- [x] I have read the Contribution Guidelines and this is not a support question.
- [x] I intend to create a pull request to implement this feature.
I suppose a friendlier error message would be nice, but changing non-public properties was never supported and has never been a feature of PHPCS, so I'm not too concerned about people getting a fatal about that. I've also never seen anyone report seeing that fatal or even asking for support because of it.
The code you are referring to above was introduced to handle the PHP 8.2 dynamic properties deprecations and dynamic properties are public by nature, so that's a completely different matter.
All in all, while giving people friendly error messages is a good thing, I don't think we need to accommodate every single thing people may possibly try to do, even though it's never been supported.
I.e.: very low prio.