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

Fix bug with handling bad symlinks #1938

Merged
merged 5 commits into from
Mar 23, 2022

Conversation

inderpreet99
Copy link
Contributor

When the path being operated upon has a file with a bad symlink on Mac OSX (somewhere in the hierarchy), rector will throw up the below cryptic error. This is due to the FilesFinder code expecting empty string from SplFileInfo->getRealPath() for bad symlinks/files. That function however actually only outputs false or the real string path.

Ref: https://www.php.net/manual/en/splfileinfo.getrealpath.php

✗ vendor/bin/rector process /Users/me/proj
PHP Fatal error:  Uncaught TypeError: str_replace(): Argument #3 ($subject) must be of type array|string, bool given in /Users/me/proj/vendor/rector/rector/src/FileSystem/FilesFinder.php:130
Stack trace:
#0 /Users/me/proj/vendor/rector/rector/src/FileSystem/FilesFinder.php(130): str_replace('\\', '/', false)
#1 /Users/me/proj/vendor/rector/rector/vendor/symfony/finder/Iterator/CustomFilterIterator.php(52): Rector\Core\FileSystem\FilesFinder->Rector\Core\FileSystem\{closure}(Object(RectorPrefix20220303\Symfony\Component\Finder\SplFileInfo))
#2 [internal function]: RectorPrefix20220303\Symfony\Component\Finder\Iterator\CustomFilterIterator->accept()
#3 [internal function]: FilterIterator->next()
#4 [internal function]: FilterIterator->next()
#5 /Users/me/proj/vendor/rector/rector/vendor/symfony/finder/Iterator/SortableIterator.php(91): iterator_to_array(Object(RectorPrefix20220303\Symfony\Component\Finder\Iterator\PathFilterIterator), true)
#6 /Users/me/proj/vendor/rector/rector/vendor/symfony/finder/Finder.php(614): RectorPrefix20220303\Symfony\Component\Finder\Iterator\SortableIterator->getIterator()
#7 /Users/me/proj/vendor/rector/rector/vendor/symplify/smart-file-system/src/Finder/FinderSanitizer.php(23): RectorPrefix20220303\Symfony\Component\Finder\Finder->getIterator()
#8 /Users/me/proj/vendor/rector/rector/src/FileSystem/FilesFinder.php(106): RectorPrefix20220303\Symplify\SmartFileSystem\Finder\FinderSanitizer->sanitize(Object(RectorPrefix20220303\Symfony\Component\Finder\Finder))
#9 /Users/me/proj/vendor/rector/rector/src/FileSystem/FilesFinder.php(85): Rector\Core\FileSystem\FilesFinder->findInDirectories(Array, Array)
#10 /Users/me/proj/vendor/rector/rector/src/FileSystem/PhpFilesFinder.php(33): Rector\Core\FileSystem\FilesFinder->findInDirectoriesAndFiles(Array)
#11 /Users/me/proj/vendor/rector/rector/src/StaticReflection/DynamicSourceLocatorDecorator.php(48): Rector\Core\FileSystem\PhpFilesFinder->findInPaths(Array)
#12 /Users/me/proj/vendor/rector/rector/src/Console/Command/ProcessCommand.php(140): Rector\Core\StaticReflection\DynamicSourceLocatorDecorator->addPaths(Array)
#13 /Users/me/proj/vendor/rector/rector/vendor/symfony/console/Command/Command.php(282): Rector\Core\Console\Command\ProcessCommand->execute(Object(RectorPrefix20220303\Symfony\Component\Console\Input\ArgvInput), Object(RectorPrefix20220303\Symfony\Component\Console\Output\ConsoleOutput))
#14 /Users/me/proj/vendor/rector/rector/vendor/symfony/console/Application.php(861): RectorPrefix20220303\Symfony\Component\Console\Command\Command->run(Object(RectorPrefix20220303\Symfony\Component\Console\Input\ArgvInput), Object(RectorPrefix20220303\Symfony\Component\Console\Output\ConsoleOutput))
#15 /Users/me/proj/vendor/rector/rector/vendor/symfony/console/Application.php(289): RectorPrefix20220303\Symfony\Component\Console\Application->doRunCommand(Object(Rector\Core\Console\Command\ProcessCommand), Object(RectorPrefix20220303\Symfony\Component\Console\Input\ArgvInput), Object(RectorPrefix20220303\Symfony\Component\Console\Output\ConsoleOutput))
#16 /Users/me/proj/vendor/rector/rector/src/Console/ConsoleApplication.php(66): RectorPrefix20220303\Symfony\Component\Console\Application->doRun(Object(RectorPrefix20220303\Symfony\Component\Console\Input\ArgvInput), Object(RectorPrefix20220303\Symfony\Component\Console\Output\ConsoleOutput))
#17 /Users/me/proj/vendor/rector/rector/vendor/symfony/console/Application.php(187): Rector\Core\Console\ConsoleApplication->doRun(Object(RectorPrefix20220303\Symfony\Component\Console\Input\ArgvInput), Object(RectorPrefix20220303\Symfony\Component\Console\Output\ConsoleOutput))
#18 /Users/me/proj/vendor/rector/rector/bin/rector.php(57): RectorPrefix20220303\Symfony\Component\Console\Application->run()
#19 /Users/me/proj/vendor/rector/rector/bin/rector(5): require_once('/Users/isingh/p...')
#20 /Users/me/proj/vendor/bin/rector(117): include('/Users/isingh/p...')
#21 {main}
  thrown in /Users/me/proj/vendor/rector/rector/src/FileSystem/FilesFinder.php on line 130
Fatal error: Uncaught TypeError: str_replace(): Argument #3 ($subject) must be of type array|string, bool given in /Users/me/proj/vendor/rector/rector/src/FileSystem/FilesFinder.php:130
Stack trace:
#0 /Users/me/proj/vendor/rector/rector/src/FileSystem/FilesFinder.php(130): str_replace('\\', '/', false)
#1 /Users/me/proj/vendor/rector/rector/vendor/symfony/finder/Iterator/CustomFilterIterator.php(52): Rector\Core\FileSystem\FilesFinder->Rector\Core\FileSystem\{closure}(Object(RectorPrefix20220303\Symfony\Component\Finder\SplFileInfo))
#2 [internal function]: RectorPrefix20220303\Symfony\Component\Finder\Iterator\CustomFilterIterator->accept()
#3 [internal function]: FilterIterator->next()
#4 [internal function]: FilterIterator->next()
#5 /Users/me/proj/vendor/rector/rector/vendor/symfony/finder/Iterator/SortableIterator.php(91): iterator_to_array(Object(RectorPrefix20220303\Symfony\Component\Finder\Iterator\PathFilterIterator), true)
#6 /Users/me/proj/vendor/rector/rector/vendor/symfony/finder/Finder.php(614): RectorPrefix20220303\Symfony\Component\Finder\Iterator\SortableIterator->getIterator()
#7 /Users/me/proj/vendor/rector/rector/vendor/symplify/smart-file-system/src/Finder/FinderSanitizer.php(23): RectorPrefix20220303\Symfony\Component\Finder\Finder->getIterator()
#8 /Users/me/proj/vendor/rector/rector/src/FileSystem/FilesFinder.php(106): RectorPrefix20220303\Symplify\SmartFileSystem\Finder\FinderSanitizer->sanitize(Object(RectorPrefix20220303\Symfony\Component\Finder\Finder))
#9 /Users/me/proj/vendor/rector/rector/src/FileSystem/FilesFinder.php(85): Rector\Core\FileSystem\FilesFinder->findInDirectories(Array, Array)
#10 /Users/me/proj/vendor/rector/rector/src/FileSystem/PhpFilesFinder.php(33): Rector\Core\FileSystem\FilesFinder->findInDirectoriesAndFiles(Array)
#11 /Users/me/proj/vendor/rector/rector/src/StaticReflection/DynamicSourceLocatorDecorator.php(48): Rector\Core\FileSystem\PhpFilesFinder->findInPaths(Array)
#12 /Users/me/proj/vendor/rector/rector/src/Console/Command/ProcessCommand.php(140): Rector\Core\StaticReflection\DynamicSourceLocatorDecorator->addPaths(Array)
#13 /Users/me/proj/vendor/rector/rector/vendor/symfony/console/Command/Command.php(282): Rector\Core\Console\Command\ProcessCommand->execute(Object(RectorPrefix20220303\Symfony\Component\Console\Input\ArgvInput), Object(RectorPrefix20220303\Symfony\Component\Console\Output\ConsoleOutput))
#14 /Users/me/proj/vendor/rector/rector/vendor/symfony/console/Application.php(861): RectorPrefix20220303\Symfony\Component\Console\Command\Command->run(Object(RectorPrefix20220303\Symfony\Component\Console\Input\ArgvInput), Object(RectorPrefix20220303\Symfony\Component\Console\Output\ConsoleOutput))
#15 /Users/me/proj/vendor/rector/rector/vendor/symfony/console/Application.php(289): RectorPrefix20220303\Symfony\Component\Console\Application->doRunCommand(Object(Rector\Core\Console\Command\ProcessCommand), Object(RectorPrefix20220303\Symfony\Component\Console\Input\ArgvInput), Object(RectorPrefix20220303\Symfony\Component\Console\Output\ConsoleOutput))
#16 /Users/me/proj/vendor/rector/rector/src/Console/ConsoleApplication.php(66): RectorPrefix20220303\Symfony\Component\Console\Application->doRun(Object(RectorPrefix20220303\Symfony\Component\Console\Input\ArgvInput), Object(RectorPrefix20220303\Symfony\Component\Console\Output\ConsoleOutput))
#17 /Users/me/proj/vendor/rector/rector/vendor/symfony/console/Application.php(187): Rector\Core\Console\ConsoleApplication->doRun(Object(RectorPrefix20220303\Symfony\Component\Console\Input\ArgvInput), Object(RectorPrefix20220303\Symfony\Component\Console\Output\ConsoleOutput))
#18 /Users/me/proj/vendor/rector/rector/bin/rector.php(57): RectorPrefix20220303\Symfony\Component\Console\Application->run()
#19 /Users/me/proj/vendor/rector/rector/bin/rector(5): require_once('/Users/isingh/p...')
#20 /Users/me/proj/vendor/bin/rector(117): include('/Users/isingh/p...')
#21 {main}
  thrown in /Users/me/proj/vendor/rector/rector/src/FileSystem/FilesFinder.php on line 130

@samsonasik
Copy link
Member

@inderpreet99 CI restarted, your change seems make PHPStan notice:

vendor/bin/phpstan analyse --ansi --error-format symplify
Note: Using configuration file /home/runner/work/rector-src/rector-src/phpstan.neon.
    0/1508 [░░░░░░░░░░░░░░░░░░░░░░░░░░░░]   0%
  620/1508 [▓▓▓▓▓▓▓▓▓▓▓░░░░░░░░░░░░░░░░░]  41%
 1360/1508 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓░░░]  90%
 1508/1508 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------------------------------------------------------------------------
  src/FileSystem/FilesFinder.php:120
 ------------------------------------------------------------------------
  - '#Strict comparison using \=\=\= between string and false will always evaluate to false#'
 ------------------------------------------------------------------------

The SplFileInfo here is came from symfony's Finder's SplFileInfo that extends the native \SplFileInfo so that's possibly make different behaviour.

@inderpreet99
Copy link
Contributor Author

inderpreet99 commented Mar 17, 2022

I thought Symfony's SplFileInfo is inherited from \SplFileInfo, so it should behave the same. Then I also found some legacy php5 Zend docs that claim it should be a string always, so maybe phpstan is just going for the least-common-denominator.

Anyhow I like your suggestion in this case @samsonasik, so I'll accept the change to cast it.

@samsonasik
Copy link
Member

It seems phpstan show another notice:

  src/FileSystem/FilesFinder.php:119
 ------------------------------------------------------------------------
  - '#Casting to string something that's already string#'
 ------------------------------------------------------------------------

Could you provide a failing fixture or sample repository for it that demo-ing your use case? Thank you.

@samsonasik
Copy link
Member

Please add @var string|false $realPath above $realPath variable to fix phpstan notice

@samsonasik samsonasik merged commit a2bc818 into rectorphp:main Mar 23, 2022
@samsonasik
Copy link
Member

Thank you @inderpreet99

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants