-
Notifications
You must be signed in to change notification settings - Fork 22
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
SyntaxError::getNormalizedMessage(): improve tests + 2 bug fixes #118
Merged
grogy
merged 5 commits into
develop
from
feature/tests-getnormalized-message-improvements
Apr 19, 2022
Merged
SyntaxError::getNormalizedMessage(): improve tests + 2 bug fixes #118
grogy
merged 5 commits into
develop
from
feature/tests-getnormalized-message-improvements
Apr 19, 2022
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
Refactor the tests in the `SyntaxErrorGetNormalizeMessageTest` class to use data providers, which allow for adding more tests in a straight forward manner. Note: this is a 1-on-1 refactor of the test with all existing test cases still being tested but no other changes.
…egex delimiter If a file name would contain the delimiter used in the replacement regex, it would cause a PHP error like `preg_replace(): Unknown modifier 'f'`. Fixed by telling `preg_quote()` explicitly which delimiter is being used. Includes unit test. Ref: https://www.php.net/manual/en/function.preg-quote.php
…n't always get stripped In certain cases, PHP puts the full file name in the error message. In those cases the "in filename.php on line .." trailing part of the error message did not get stripped off. Also, in some cases, when Windows slashes are used in the file path, the "in filename.php on line .." trailing part of the error message may not get stripped off. This last case will be exceedingly rare as on Windows, those file paths are handled correctly and the chances of a non-Windows user passing a path using Windows slashes will be minuscule. Even so, I've fixed both cases by making the path handling in the function more robust. * When the initial stripping of the trailing part of the error message fails, it will be retried up to two times. * The first time, if Windows slashes would be found in the file path _after_ the `basename()` function has been applied, a "manual" basename extraction is done and the stripping of the trailing part is retried. * The second time, the full file path is used in a last attempt to strip the trailing part of the error message. Includes unit tests. Fixes 94
Including documenting that a `Deprecated:` prefix will not be removed.
By using `?:` at the start of a parenthesis group, the group will not be "remembered"/saved to memory. As this regex does not use the subgroups anyway, we don't need to remember them.
jrfnl
force-pushed
the
feature/tests-getnormalized-message-improvements
branch
from
April 8, 2022 11:45
351ba3d
to
0da93c9
Compare
grogy
approved these changes
Apr 19, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes with tests cases makes me a sense . Thank you 👍
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.
SyntaxErrorGetNormalizeMessageTest: refactor to dataprovider
Refactor the tests in the
SyntaxErrorGetNormalizeMessageTest
class to use data providers, which allow for adding more tests in a straight forward manner.Note: this is a 1-on-1 refactor of the test with all existing test cases still being tested but no other changes.
SyntaxError::getNormalizedMessage(): bug fix - file name containing regex delimiter
If a file name would contain the delimiter used in the replacement regex, it would cause a PHP error like
preg_replace(): Unknown modifier 'f'
.Fixed by telling
preg_quote()
explicitly which delimiter is being used.Includes unit test.
Ref: https://www.php.net/manual/en/function.preg-quote.php
SyntaxError::getNormalizedMessage(): bug fix - "in file on line" doesn't always get stripped
In certain cases, PHP puts the full file name in the error message. In those cases the "in filename.php on line .." trailing part of the error message did not get stripped off.
Also, in some cases, when Windows slashes are used in the file path, the "in filename.php on line .." trailing part of the error message may not get stripped off.
This last case will be exceedingly rare as on Windows, those file paths are handled correctly and the chances of a non-Windows user passing a path using Windows slashes will be minuscule.
Even so, I've fixed both cases by making the path handling in the function more robust.
basename()
function has been applied, a "manual" basename extraction is done and the stripping of the trailing part is retried.Includes unit tests.
Fixes #94
SyntaxErrorGetNormalizeMessageTest: add some additional test cases
Including documenting that a
Deprecated:
prefix will not be removed.SyntaxError::getNormalizedMessage(): minor regex tweak
By using
?:
at the start of a parenthesis group, the group will not be "remembered"/saved to memory.As this regex does not use the subgroups anyway, we don't need to remember them.