-
-
Notifications
You must be signed in to change notification settings - Fork 89
Description
Summary
Came across this by chance. Documenting the error here as this needs further investigation.
Looks like PHPCS tries to create a LocalFile object for symlinked directories on Windows, which results in a Failed to open stream: Permission denied in path/to/PHP_CodeSniffer/src/Files/LocalFile.php error.
To my surprise, it also looks like this is related to a change in PHP itself as the issue is not reproducable on PHP < 8.1 and consistently reproducible on PHP 8.1+.
Detailed outline of the problem
Given a composer.json like this (minimal example based on Symfony - take note of the repositories entries using "type": "path"):
{
"type": "library",
"require": {
"symfony/contracts": "^3.6",
},
"require-dev": {
"symfony/runtime": "self.version",
},
"repositories": [
{
"type": "path",
"url": "src/Symfony/Contracts",
"options": {
"versions": {
"symfony/contracts": "3.6.x-dev"
}
}
},
{
"type": "path",
"url": "src/Symfony/Component/Runtime"
}
],
"minimum-stability": "dev"
}... Composer will create symlinked directories for the symfony/contracts and symfony/runtime dependencies in the vendor directory.
On Windows, I'm observing that it also copies over the files, but that's irrelevant for the current issue.
On Windows, that symlinked directory path will identify as readable, but opening the "file" will result in a Failed to open stream: Permission denied warning, which is then caught by PHPCS and turned into a fatal error by the PHPCS error handler in the Runner class.
This is the error I've seen:
Fatal error: Uncaught PHP_CodeSniffer\Exceptions\RuntimeException: fopen(path/to/symfony/vendor/symfony/contracts): Failed to open stream: Permission denied in path/to/PHP_CodeSniffer/src/Files/LocalFile.php on line 47 in path/to/PHP_CodeSniffer/src/Runner.php:543
Stack trace:
#0 [internal function]: PHP_CodeSniffer\Runner->handleErrors(2, 'fopen(path/to/...', 'path/to/...', 47)
#1 path/to/PHP_CodeSniffer/src/Files/LocalFile.php(47): fopen('path/to/...', 'r')
#2 path/to/PHP_CodeSniffer/src/Files/FileList.php(201): PHP_CodeSniffer\Files\LocalFile->__construct('path/to/...', Object(PHP_CodeSniffer\Ruleset), Object(PHP_CodeSniffer\Config))
#3 path/to/PHP_CodeSniffer/src/Runner.php(379): PHP_CodeSniffer\Files\FileList->current()
#4 path/to/PHP_CodeSniffer/src/Runner.php(122): PHP_CodeSniffer\Runner->run()
#5 path/to/PHP_CodeSniffer/bin/phpcs(30): PHP_CodeSniffer\Runner->runPHPCS()
#6 {main}
thrown in path/to/PHP_CodeSniffer/src/Runner.php on line 543
To reproduce
- Clone the Symfony repo
git@github.com:symfony/symfony.git - Run
composer install - Using a local clone of PHPCS on PHP < 8.1, run
phpcs -ps ./vendor/symfony/ --standard=generic --report=summary --extensions=php --sniffs=generic.files.byteordermarkand observe no errors. - Switch to PHP 8.1 or higher.
- Run the same PHPCS command again and observe the error.
Analysis
The error occurs when PHPCS tries to create a LocalFile object for the symlinked directory, but PHPCS shouldn't create File objects for directories to begin with.
Changing line
PHP_CodeSniffer/src/Files/LocalFile.php
Line 33 in 0ca8684
| if (Common::isReadable($this->path) === false) { |
- if (Common::isReadable($this->path) === false) {
+ if (Common::isReadable($this->path) === false || @is_file($this->path) === false) {gets rid of the error, but might have side-effects.
Changing Common::isReadable() to only apply to files is not a viable solution direction, as the method is also used in the Config class to verify if directories are readable when that class is searching for a ruleset file.
PHP_CodeSniffer/src/Config.php
Line 429 in 0ca8684
| } while ($currentDir !== '.' && $currentDir !== $lastDir && Common::isReadable($currentDir) === true); |
Moreover, the FileList iterator class should probably only contain file references and discard directories, which should prevent the problem from occurring to begin with. However, if the class wasn't set up like that from the start, changing this may again have side-effects.
Next steps
- Some digging needs to be done through both the
FileListclass code as well as through its history to see why the symlinked directory is included in theFileListiterator and whether that is correct or not. - Some digging needs to be done to find out what change from PHP 8.1 is causing the issue and what the typical solution is to deal with that PHP 8.1 change.
That research should inform the solution for the issue.
This also needs a good think about how we can safeguard any fix for this issue via tests.
Note: this part of the codebase is (again) barely covered by tests, so while tests just covering this issue would be a good starting point, more comprehensive tests would be better.
Other open questions
- Does this issue only occur on Windows or also on Linux ?
- Is this error specifically related to how these symlinks are created by Composer ? Or does this apply to all symlinked directories on Windows ?
Versions (please complete the following information)
| Operating System | Windows 10 |
| PHP version | PHP >= 8.1 |
| PHP_CodeSniffer version | 3.x + 4.x, I've been able to reproduce with 3.5.0 and haven't tried further back |
| Standard | not relevant |
| Install type | not relevant |
Please confirm
- I have searched the issue list and am not opening a duplicate issue.
- I have read the Contribution Guidelines and this is not a support question.
- I confirm that this bug is a bug in PHP_CodeSniffer and not in one of the external standards.
- I have verified the issue still exists in the
4.xbranch of PHP_CodeSniffer.