-
-
Notifications
You must be signed in to change notification settings - Fork 8
Description
PHPCSUtils 1.1.0 introduced a number of new exceptions via #598 (and started using them via #599).
These exceptions currently all extend the PHPCSUtils PHPCSUtils\Exceptions\RuntimeException, which in turn extends the PHPCS native PHP_CodeSniffer\Exceptions\RuntimeException.
This was done in this way to allow for introducing these exceptions into pre-existing functionality, which already threw a RuntimeException, without this being a breaking change.
However, strictly speaking, a number of these exceptions should extend a different PHP native exception for improved semantics.
Case(s) in point:
- The PHPCSUtils
LogicExceptionwould be better off extending the PHP nativeLogicException. - The PHPCSUtils
MissingArgumentErrorwould be better off extending the PHP nativeArgumentCountError. - The PHPCSUtils
OutOfBoundsStackPtrwould be better off extending the PHP nativeOutOfBoundsException. - The PHPCSUtils
TypeErrorwould be better off extending the PHP nativeTypeError. - The PHPCSUtils
UnexpectedTokenTypewould be better off extending the PHP nativeInvalidArgumentException. - The PHPCSUtils
ValueErrorwould be better off extending the PHP nativeInvalidArgumentExceptionor the PHP 8.0+ nativeValueError.
Having said that, making these changes would be a breaking change and the current exceptions are already much more descriptive than the original RuntimeException, so addressing this is not urgent and may not be needed at all.
Opening this issue to solicit opinions and as a reminder to think this over again before the next major release.