Skip to content

Commit

Permalink
Bug fix: loadInstalledPaths() very very broken since PHPCS 3.1.0
Browse files Browse the repository at this point in the history
The PHPCS `--config-show` command shows **all** config settings set, not just the `installed_paths` and since PHPCS 3.1.0, it also includes a line to indicate which configuration file is being used to help with debugging configuration issues.

In PHPCS 2.0 - 3.0.1, if any [other configuration options](https://github.com/squizlabs/PHP_CodeSniffer/wiki/Configuration-Options) were set, the result of the `loadInstalledPaths()` command would be broken.

In PHPCS 3.1.0+, the result would always be broken, no matter what.

In effect this means that the `installed_paths` would (nearly) always be set as if the config was previously non-existent as the `cleanInstalledPaths()` method would empty out the array as the values contained therein would be invalid anyway.

It would be lovely if it were possible to add unit tests, but as we're talking about a `private` method setting a `private` property...

So, for now, to proof the bug and validate the fix, I've set up a "fake" unit test to demonstrate what the previous behaviour was and what the behaviour will be with this fix in place: https://3v4l.org/XKMIf
  • Loading branch information
jrfnl committed Jan 19, 2020
1 parent bb64a27 commit 5e8a561
Showing 1 changed file with 22 additions and 14 deletions.
36 changes: 22 additions & 14 deletions src/Plugin.php
Expand Up @@ -46,6 +46,7 @@ class Plugin implements PluginInterface, EventSubscriberInterface
const PACKAGE_NAME = 'squizlabs/php_codesniffer';
const PACKAGE_TYPE = 'phpcodesniffer-standard';

const PHPCS_CONFIG_REGEX = '`%s:[^\r\n]+`';
const PHPCS_CONFIG_KEY = 'installed_paths';

/**
Expand Down Expand Up @@ -199,22 +200,29 @@ public function onDependenciesChangedEvent()
*/
private function loadInstalledPaths()
{
if ($this->isPHPCodeSnifferInstalled() === true) {
$this->processExecutor->execute(
sprintf(
'phpcs --config-show %s',
self::PHPCS_CONFIG_KEY
),
$output,
$this->composer->getConfig()->get('bin-dir')
);
if ($this->isPHPCodeSnifferInstalled() !== true) {
return;
}

$this->processExecutor->execute(
sprintf(
'phpcs --config-show %s',
self::PHPCS_CONFIG_KEY
),
$output,
$this->composer->getConfig()->get('bin-dir')
);

$phpcsInstalledPaths = str_replace(self::PHPCS_CONFIG_KEY . ': ', '', $output);
$phpcsInstalledPaths = trim($phpcsInstalledPaths);
$regex = sprintf(self::PHPCS_CONFIG_REGEX, self::PHPCS_CONFIG_KEY);
if (preg_match($regex, $output, $match) !== 1) {
return;
}

if ($phpcsInstalledPaths !== '') {
$this->installedPaths = explode(',', $phpcsInstalledPaths);
}
$phpcsInstalledPaths = str_replace(self::PHPCS_CONFIG_KEY . ': ', '', $match[0]);
$phpcsInstalledPaths = trim($phpcsInstalledPaths);

if ($phpcsInstalledPaths !== '') {
$this->installedPaths = explode(',', $phpcsInstalledPaths);
}
}

Expand Down

0 comments on commit 5e8a561

Please sign in to comment.