Always treat stream wrappers as absolute paths #18
Conversation
src/ClassMapGenerator.php
Outdated
| } | ||
| } | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Please don't use tabs in files that use spaces for indentation..
There was a problem hiding this comment.
Ok, I’ll open another PR with .php-cs-fixer.php copied from the main project
There was a problem hiding this comment.
Or set your editor to autodetect what's in the file then you never have such issues.
src/ClassMapGenerator.php
Outdated
| @@ -142,18 +142,20 @@ public function scanPaths($path, ?string $excluded = null, string $autoloadType | |||
| continue; | |||
| } | |||
|
|
|||
| if (!self::isAbsolutePath($filePath)) { | |||
| if (!self::isAbsolutePath($filePath) && !self::isStreamWrapper($filePath)) { | |||
There was a problem hiding this comment.
I think this could be done by checking for || Preg::isMatch('{^[a-z0-9]+://}', $path) in isAbsolutePath, then we don't need to iterate all stream wrappers for every single file, because that seems kinda wasteful..
There was a problem hiding this comment.
True, it’s certainly not efficient how I’ve written it, but it does at least confirm that it’s a valid streamwrapper.
I was considering putting a static variable inside the function, but I know that’s a nightmare made for testing. I could put a regular instance variable array map of path:bool. Or maybe add ~implode(‘|’, stream_get_wrappers()) into that regex, and move the definition out of the loop.
There was a problem hiding this comment.
Yeah that sounds better for sure
src/ClassMapGenerator.php
Outdated
| } elseif(!self::isStreamWrapper($filePath)) { | ||
| $filePath = Preg::replace('{[\\\\/]{2,}}', '/', $filePath); |
There was a problem hiding this comment.
| } elseif(!self::isStreamWrapper($filePath)) { | |
| $filePath = Preg::replace('{[\\\\/]{2,}}', '/', $filePath); | |
| } else { | |
| $filePath = Preg::replace('{(?<!:)[\\\\/]{2,}}', '/', $filePath); |
This should prevent foo:// from being replaced
src/ClassMapGenerator.php
Outdated
| $realPath = !self::isStreamWrapper($filePath) | ||
| ? realpath($filePath) | ||
| : $filePath; |
There was a problem hiding this comment.
This one I'm not sure what to do about.. Is $file->getRealPath() also returning false for stream wrapper paths?
Anyway we could also do a check like I suggested above (Preg::isMatch('{^[a-z0-9]+://}', $filePath)) if needed..
There was a problem hiding this comment.
Yes: php/php-src#12118
|
Thanks, I think that looks fine now :) |
I'm trying to add a
--dry-runmode to BrianHenryIE/strauss. To achieve this I've written a FlySystem ReadOnlyFilesytem class which uses a couple of in-memory filesystems to emulate the process without ever writing to the true filesystem.Obviously, I can't pass a Flysystem instance to third party libraries, such as this.
elazar/flystream to the rescue! Which allows registering
myscheme://to then use your Flysystem of choice during normalfile_get_contents('myscheme://path/to/file.php')operations.Currently,
composer/class-map-generatordoes some operations, e.g.::isAbsolutePath(), that mis-characterisemyscheme://....This PR adds a
::isStreamWrapper()function which is used in theClassMapGenerator::scanPaths()function to enable using stream wrappers.The test creates a stream wrapper that reads e.g.
test://BackslashLineEndingString.phpfromfile://path/to/class-map-generator/tests/Fixtures/BackslashLineEndingString.php.