fix(PhpcsFormatter): remove last PHP_EOL before parsing json #684
Conversation
|
Any updates on this?? |
src/Formatter/PhpcsFormatter.php
Outdated
| @@ -24,7 +24,7 @@ public function format(Process $process): string | |||
| return $process->getErrorOutput(); | |||
| } | |||
|
|
|||
| $pos = strrpos($output, "\n"); | |||
| $pos = strrpos(trim($output), "\n"); | |||
There was a problem hiding this comment.
Wouldn't it be better to do this trim at line 22 instead?
That way, the code always works with the normalized version.
(Now $pos works on a different output value then the substr method)
There was a problem hiding this comment.
This patch works for me, where I have the same "rogue" JSON output. I'm using 0.16.1
There was a problem hiding this comment.
Until this is patched, we can run the following:
sed -i 's#$pos = strrpos($output, "\\n");#$pos = strrpos(trim($output), "\\n");#g' vendor/phpro/grumphp/src/Formatter/PhpcsFormatter.php
|
Thanks for reporting @matiasfuster! I added a small remark. Besides that : all good for me. |
|
Hi! Problem with adding it at 22 is that in older versions it would trim the last “}” of the json report. This way it just removes de EOL. Do you agree? |
|
No : trim only removes whitespace characters if you don't specify a character mask. |
|
You are right, i'm still asleep! Closing and resubmitting. |
5 months ago a commit changes how PHPCS ends the json report by adding a new line at the end. You can see here: https://github.com/squizlabs/PHP_CodeSniffer/blob/master/src/Reports/Json.php#L101
This messes up the PhpcsFormatter because the last "\n" that finds is the new PHP_EOL that is added to de report.
With this fix it won't print the json report and will prompt for autofixing like before.
No