Start using the new exceptions #599
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jrfnl
changed the title
jrfnl
force-pushed
the
feature/start-using-the-new-exceptions
branch
2 times, most recently
from
10aa322 to
5680552
Compare
jrfnl
force-pushed
the
feature/introduce-better-exceptions
branch
from
d06975f to
8b08c1e
Compare
This implements the use of the new exceptions introduced in 598 in all the right places in PHPCSUtils. Notes: * The `BCFile` class is explicitly exempt from this change as the methods in that class emulate the PHPCS native methods, which means they should also throw the PHPCS native exception. * Includes switching a number of test class from the `UtilityMethodTestCase` parent class to the `PolyfilledTestCase` parent class - which is basically the `UtilityMethodTestCase` + the PHPUnit Polyfills -. This allows for expecting the exceptions without having to jump through hoops for PHPUnit cross-version support. * Includes adding select extra tests where needed.
jrfnl
force-pushed
the
feature/start-using-the-new-exceptions
branch
from
5680552 to
18fb796
Compare
This changes the exceptions being caught in various `catch` statements to more specific ones.
This means that errors which should always have been thrown, will now throw and only the potentially expected (and acceptable) exceptions will now be caught.
Note:
* For the `Namespaces::getDeclaredName()` method, the `catch` has not been changed (other than switching from the PHPCS native `RuntimeException` to the PHPCSUtils one).
The reason for this is that the method is explicitly documented as returning `false` for non-existent tokens.
While this behaviour is not in line with other methods in PHPCSUtils, changing the behaviour could be seen as a breaking change, so should be done in a major release.
Includes test for where the behaviour of the functions is now different.
|
Rebased without changes, other than squashing the "catch change/test" commit into the commit making the catch changes. |
jrfnl
force-pushed
the
feature/start-using-the-new-exceptions
branch
from
18fb796 to
1c9176d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Start using the new exceptions throughout PHPCSUtils
This implements the use of the new exceptions introduced in #598 in all the right places in PHPCSUtils.
Notes:
BCFileclass is explicitly exempt from this change as the methods in that class emulate the PHPCS native methods, which means they should also throw the PHPCS native exception.UtilityMethodTestCaseparent class to thePolyfilledTestCaseparent class - which is basically theUtilityMethodTestCase+ the PHPUnit Polyfills -. This allows for expecting the exceptions without having to jump through hoops for PHPUnit cross-version support.Only catch what should be caught
This changes the exceptions being caught in various
catchstatements to more specific ones.This means that errors which should always have been thrown, will now throw and only the potentially expected (and acceptable) exceptions will now be caught.
Note:
Namespaces::getDeclaredName()method, thecatchhas not been changed (other than switching from the PHPCS nativeRuntimeExceptionto the PHPCSUtils one).The reason for this is that the method is explicitly documented as returning
falsefor non-existent tokens.While this behaviour is not in line with other methods in PHPCSUtils, changing the behaviour could be seen as a breaking change, so should be done in a major release.
Includes test for where the behaviour of the functions is now different.