BaselineNeonErrorFormatter: Sort errors by normalized relative path, … #536
Conversation
…instead of original absolute path while in theory the net result should be the same (because the leading part of the path would be the same), / and \ have a different sorting order, so output gets sorted differently on Windows than on Linux. This fixes #5085.
|
Any change in src/ without change in tests/ is a red flag. Add a failing test first, thanks. |
ondrejmirtes
force-pushed
the
master
branch
2 times, most recently
from
efd31ce
to
0471f87
Compare
|
@dktapps Do you plan to finish this PR? Thanks. |
|
Sorry, I forgot all about it. I'll try and find some time to finish it today. |
|
As I recall, last time I tried to write tests with this, I had some problem with SimpleRelativePathHelper not normalizing the separator. |
|
I've added a test case. If you run it against the original code, the With the change it passes as expected. |
| @@ -207,6 +207,9 @@ public function outputOrderingProvider(): \Generator | |||
| new Error('Second error in a different file', 'TestfileB', 4), | |||
| new Error('Error #1 in a different file', 'TestfileB', 5), | |||
| new Error('Second error in a different file', 'TestfileB', 6), | |||
| new Error('Error with Windows path separators', 'TestFiles\\TestA', 1), | |||
There was a problem hiding this comment.
It should be named a "dir separator", not a "path separator" as the later means something else, see https://www.php.net/manual/en/dir.constants.php
| return str_replace('\\', '/', substr($filename, strlen($this->currentWorkingDirectory) + 1)); | ||
| } | ||
|
|
||
| return $filename; | ||
| return str_replace('\\', '/', $filename); |
There was a problem hiding this comment.
do we need the same fix in other implementations of RelativePathHelper ?
There was a problem hiding this comment.
Not that I'm aware of. The one used for regular analysis already performs dirsep normalization.
|
Test failure looks unrelated. |
|
Thank you! |
…instead of original absolute path
while in theory the net result should be the same (because the leading part of the path would be the same), / and \ have a different sorting order, so output gets sorted differently on Windows than on Linux.
This fixes phpstan/phpstan#5085.