From f1acb08ecda68112f7e747c431e1848d4d0b8d8a Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 4 Feb 2022 09:10:21 +0100 Subject: [PATCH 1/3] TestCase: new `recursiveDirectoryCopy()` method New helper method to recursively copy a (fixture) directory with all its files to another location, typically, one of the system temp test directories. If the `$srcDir` contains a `composer.json` file, it will get the same `TestCase::writeComposerJsonFile()` special treatment, meaning that the artifacts directory will be injected, as well as plugin permissions and more. --- tests/TestCase.php | 59 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/tests/TestCase.php b/tests/TestCase.php index 7309aea2..69396800 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -10,6 +10,8 @@ namespace Dealerdirect\Composer\Plugin\Installers\PHPCodeSniffer\Tests; +use RecursiveDirectoryIterator; +use RecursiveIteratorIterator; use RuntimeException; use Yoast\PHPUnitPolyfills\TestCases\TestCase as PolyfillTestCase; @@ -552,6 +554,63 @@ protected function createFile($path, $contents = '') } } + /** + * Helper function to recursively copy a directory with all its content to another location. + * + * Includes making any potentially needed tweaks to `composer.json` files. + * + * @param string $src Full path to the source directory. + * @param string $dest Full path to the destination directory. + * + * @return void + * + * @throws RuntimeException When either or the passed arguments is not a string. + * @throws RuntimeException When a (sub)directory could not be created. + * @throws RuntimeException When a file could not be copied. + */ + protected function recursiveDirectoryCopy($srcDir, $destDir) + { + if (is_string($srcDir) === false || is_dir($srcDir) === false) { + throw new RuntimeException('The source directory must be a string pointing to an existing directory.'); + } + + if (is_string($destDir) === false) { + throw new RuntimeException('The destination directory must be a string.'); + } + + $directory = new RecursiveDirectoryIterator( + realpath($srcDir), + RecursiveDirectoryIterator::SKIP_DOTS | RecursiveDirectoryIterator::UNIX_PATHS + ); + $iterator = new RecursiveIteratorIterator($directory, RecursiveIteratorIterator::SELF_FIRST); + + foreach ($iterator as $path => $fileInfo) { + $subPath = $iterator->getSubPathname(); + + // Special case the Composer config. + if ($subPath === 'composer.json') { + $composerConfig = json_decode(file_get_contents($fileInfo->getPathname()), true); + $this->writeComposerJsonFile($composerConfig, $destDir); + continue; + } + + $target = $destDir . '/' . $subPath; + + if ($fileInfo->isDir()) { + if (mkdir($target, 0766, true) === false || is_dir($target) === false) { + throw new RuntimeException("Failed to create the $target directory for the test"); + } + continue; + } + + if ($fileInfo->isFile()) { + if (copy($fileInfo->getPathname(), $target) === false || is_file($target) === false) { + throw new RuntimeException("Failed to copy $src to $target "); + } + } + } + } + /** * Retrieve a list of the standards recognized by PHPCS based on the output of `phpcs -i`. * From b0f58fa504f03f65ae1ace4ed022523e04ff677f Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 4 Feb 2022 07:58:22 +0100 Subject: [PATCH 2/3] Tests: add new `RootPackageHandlingTest` This new test class tests that the plugin correctly registers standards found in the root package if it is an external standard. Notes: * This test is about Composer and the plugin, so does not need to be tested against multiple PHPCS versions. This test class covers the following bugs previously reported: * Dealerdirect/phpcodesniffer-composer-installer issue 19 * Dealerdirect/phpcodesniffer-composer-installer PR 21 * Dealerdirect/phpcodesniffer-composer-installer issue 20 * Dealerdirect/phpcodesniffer-composer-installer PR 25 * Dealerdirect/phpcodesniffer-composer-installer issue 32 --- .../RootPackageHandlingTest.php | 145 ++++++++++++++++++ 1 file changed, 145 insertions(+) create mode 100644 tests/IntegrationTest/RootPackageHandlingTest.php diff --git a/tests/IntegrationTest/RootPackageHandlingTest.php b/tests/IntegrationTest/RootPackageHandlingTest.php new file mode 100644 index 00000000..47b0e51f --- /dev/null +++ b/tests/IntegrationTest/RootPackageHandlingTest.php @@ -0,0 +1,145 @@ +createTestEnvironment(); + } + + /** + * Clean up. + */ + protected function tear_down() + { + $this->removeTestEnvironment(); + } + + /** + * Test that the plugin registers a standard found in the root package. + * + * @return void + */ + public function testSetInstalledPathsWhenRootPackageIsExternalStandard() + { + /* + * Copy one of the fixtures to the test directory. + */ + $this->recursiveDirectoryCopy(dirname(__DIR__) . '/fixtures/multistandard/', static::$tempLocalPath); + + // Install the dependencies, including the plugin. + $command = sprintf('composer install -v --no-ansi --working-dir=%s', escapeshellarg(static::$tempLocalPath)); + $this->assertExecute( + $command, + 0, // Expected exit code. + Plugin::MESSAGE_RUNNING_INSTALLER, // Expected stdout. + null, // No stderr expectation. + 'Failed to install the plugin.' + ); + + // Verify that the root package standard is registered correctly. + $result = $this->executeCliCommand('"vendor/bin/phpcs" --config-show', static::$tempLocalPath); + $this->assertSame(0, $result['exitcode'], 'Exitcode for "phpcs --config-show" did not match 0 (first run)'); + + $this->assertSame( + 1, + preg_match('`installed_paths:\s+([^\n\r]+)\s+`', $result['stdout'], $matches), + 'Could not find the installed paths in the config' + ); + + // Work around differences in paths being returned between *nix and Windows. + $hasExpectedPath = false; + if ($matches[1] === '../../../') { // Most common. + $hasExpectedPath = true; + } else { + $needle = str_replace(sys_get_temp_dir(), '', static::$tempLocalPath); + if (substr_compare($matches[1], $needle, -\strlen($needle)) === 0) { + $hasExpectedPath = true; + } + } + + $this->assertTrue($hasExpectedPath, $matches[1], 'PHPCS configuration does not contain the root standard'); + } + + /** + * Test that the plugin does not act on root packages which aren't valid PHPCS external standards + * for the purposes of this plugin. + * + * @dataProvider dataInvalidRootPackages + * + * @param string $srcDir The path to the fixture to use as the root package. + * + * @return void + */ + public function testDontSetInstalledPathsForInvalidPackages($srcDir) + { + // Copy the fixture to the test directory. + $this->recursiveDirectoryCopy($srcDir, static::$tempLocalPath); + + // Install the dependencies, including the plugin. + $command = sprintf('composer install -v --no-ansi --working-dir=%s', escapeshellarg(static::$tempLocalPath)); + $this->assertExecute( + $command, + 0, // Expected exit code. + Plugin::MESSAGE_RUNNING_INSTALLER, // Expected stdout. + null, // No stderr expectation. + 'Failed to install the plugin.' + ); + + // Verify that the root package is not registered as a standard. + $result = $this->executeCliCommand('"vendor/bin/phpcs" --config-show', static::$tempLocalPath); + $this->assertSame(0, $result['exitcode'], 'Exitcode for "phpcs --config-show" did not match 0 (first run)'); + + // As the fixture doesn't contain other external standards, the installed_paths config should not exist. + $this->assertStringNotContainsString( + 'installed_paths:', + $result['stdout'], + 'Root package registered as an external standard with PHPCS, while it shouldn\'t be' + ); + } + + /** + * Data provider. + * + * @return array + */ + public function dataInvalidRootPackages() + { + return array( + 'Root package without ruleset file' => array( + 'srcDir' => dirname(__DIR__) . '/fixtures/no-ruleset/', + ), + 'Root package with incorrect type' => array( + 'srcDir' => dirname(__DIR__) . '/fixtures/incorrect-type/', + ), + ); + } +} From 2de5e4b4906a28cf854da24ae725357eb832c318 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 4 Feb 2022 13:23:16 +0100 Subject: [PATCH 3/3] Plugin: only search root package when it has the `phpcodesniffer-standard` type (bug fix) In the "olden days" of PHPCS 1.x, it was customary for projects to call their custom project ruleset `ruleset.xml` instead of `[.]phpcs.xml[.dist]`. This could lead to a root package with such an _old-style_ PHPCS ruleset being registered as if it were a proper PHPCS external standard. This was previously reported in issue 32. The fix I'm proposing now, applies the same rules as for `vendor` installed standards to the root package, i.e.: * Must have the `phpcodesniffer-standard` type set in the `composer.json` file AND * Must have a `ruleset.xml` file. Root packages which do not comply with _both_ these rules will no longer be considered for registration with PHPCS. This should also make the plugin slightly faster for those packages which do not have external standards, but do have the plugin installed. Includes removing some dead code (condition which could never be `true`) which was loosely related to this. Fixes 32 --- src/Plugin.php | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/Plugin.php b/src/Plugin.php index 1cb5af0f..c7a734f0 100644 --- a/src/Plugin.php +++ b/src/Plugin.php @@ -444,9 +444,17 @@ private function cleanInstalledPaths() */ private function updateInstalledPaths() { - $changes = false; + $changes = false; + $searchPaths = array(); + + // Add root package only if it has the expected package type. + if ( + $this->composer->getPackage() instanceof RootPackageInterface + && $this->composer->getPackage()->getType() === self::PACKAGE_TYPE + ) { + $searchPaths[] = $this->cwd; + } - $searchPaths = array($this->cwd); $codingStandardPackages = $this->getPHPCodingStandardPackages(); foreach ($codingStandardPackages as $package) { $installPath = $this->composer->getInstallationManager()->getInstallPath($package); @@ -458,6 +466,11 @@ private function updateInstalledPaths() $searchPaths[] = $installPath; } + // Nothing to do. + if ($searchPaths === array()) { + return false; + } + $finder = new Finder(); $finder->files() ->depth('<= ' . $this->getMaxDepth()) @@ -499,9 +512,6 @@ private function updateInstalledPaths() * Iterates through Composers' local repository looking for valid Coding * Standard packages. * - * If the package is the RootPackage (the one the plugin is installed into), - * the package is ignored for now since it needs a different install path logic. - * * @return array Composer packages containing coding standard(s) */ private function getPHPCodingStandardPackages() @@ -516,13 +526,6 @@ function (PackageInterface $package) { } ); - if ( - ! $this->composer->getPackage() instanceof RootPackageInterface - && $this->composer->getPackage()->getType() === self::PACKAGE_TYPE - ) { - $codingStandardPackages[] = $this->composer->getPackage(); - } - return $codingStandardPackages; }