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

NonPrintableCharacterFixer - fix for backslash and quotes when changing to escape sequences #4675

Merged
merged 1 commit into from
Jan 1, 2020
Merged

NonPrintableCharacterFixer - fix for backslash and quotes when changing to escape sequences #4675

merged 1 commit into from
Jan 1, 2020

Conversation

kubawerlos
Copy link
Contributor

@kubawerlos kubawerlos commented Nov 29, 2019

Fixes #4663 and replaces #4664.

Ping @mvorisek as reporter and @ivan1986 as the original author for review.

@kubawerlos kubawerlos changed the title Add tests for NonPrintableCharacterFixer NonPrintableCharacterFixer - fix for quotes when changing to escape sequences Nov 29, 2019
@mvorisek
Copy link
Contributor

mvorisek commented Nov 29, 2019

For me it seems like it does not handle backslashes correctly:

$content = "'\\\\\\\\'";
var_dump($content);

$content = str_replace("\\'", "'", $content);
$content = preg_replace('/([\\\\$])/', '\\\\$1', $content);
var_dump($content);

To replace escaped quotes (and not preceded by double/escaped backslash) you needs something like this: https://github.com/FriendsOfPHP/PHP-CS-Fixer/pull/4664/files#diff-778a67f6227ed439e1490963b2f0f94dR154

@ivan1986
Copy link
Contributor

Yes, need add tests

            [
                <<<'EXPECTED'
<?php echo "\\\u{200b}\"";
EXPECTED
                ,
                sprintf(<<<'INPUT'
<?php echo '\\%s"';
INPUT
                 , pack('H*', 'e2808b')),
            ],
            [
                <<<'EXPECTED'
<?php echo "\\\u{200b}'";
EXPECTED
                ,
                sprintf(<<<'INPUT'
<?php echo '\\%s\'';
INPUT
                 , pack('H*', 'e2808b')),
            ],

for check double slashes, it's fail now, or add slashes into exist tests

@kubawerlos kubawerlos changed the title NonPrintableCharacterFixer - fix for quotes when changing to escape sequences NonPrintableCharacterFixer - fix for backslash and quotes when changing to escape sequences Nov 30, 2019
@kubawerlos
Copy link
Contributor Author

For me it seems

@mvorisek take an example from @ivan1986 - presenting test case is much more valuable.

@ivan1986 added yours 2 - thanks - and few more for backslashes.

@julienfalque julienfalque added this to the 2.15.6 milestone Dec 1, 2019
@SpacePossum SpacePossum added the RTM Ready To Merge label Dec 17, 2019
@julienfalque
Copy link
Member

Thank you @kubawerlos.

julienfalque added a commit that referenced this pull request Jan 1, 2020
…hen changing to escape sequences (kubawerlos)

This PR was squashed before being merged into the 2.15 branch (closes #4675).

Discussion
----------

NonPrintableCharacterFixer - fix for backslash and quotes when changing to escape sequences

Fixes #4663 and replaces #4664.

Ping @mvorisek as reporter and @ivan1986 as the original author for review.

Commits
-------

6365103 NonPrintableCharacterFixer - fix for backslash and quotes when changing to escape sequences
@julienfalque julienfalque merged commit 6365103 into PHP-CS-Fixer:2.15 Jan 1, 2020
@julienfalque julienfalque removed the RTM Ready To Merge label Jan 1, 2020
@kubawerlos kubawerlos deleted the fix_NonPrintableCharacterFixer branch January 1, 2020 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants