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

SyntaxError::getNormalizedMessage(): improve tests + 2 bug fixes #118

Merged
merged 5 commits into from Apr 19, 2022

Conversation

jrfnl
Copy link
Collaborator

@jrfnl jrfnl commented Apr 5, 2022

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.

  • 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

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.

@jrfnl jrfnl added this to the 2.0.0 milestone Apr 5, 2022
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 jrfnl force-pushed the feature/tests-getnormalized-message-improvements branch from 351ba3d to 0da93c9 Compare April 8, 2022 11:45
@jrfnl jrfnl marked this pull request as ready for review April 8, 2022 11:47
@jrfnl jrfnl requested a review from grogy April 8, 2022 11:47
Copy link
Member

@grogy grogy left a 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 👍

@grogy grogy merged commit 41a9c77 into develop Apr 19, 2022
@grogy grogy deleted the feature/tests-getnormalized-message-improvements branch April 19, 2022 12:42
@jrfnl jrfnl modified the milestones: 2.0.0, 1.3.x Next Mar 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

File path and line nr does not get stripped when passing relative path with Windows slashes
2 participants