From ef14320117b4aa04268429efa38041c94fd8747d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillaume=20Perr=C3=A9al?= Date: Mon, 18 Nov 2019 11:16:25 +0100 Subject: [PATCH] Add an option `--include-dev` to check the development code. --- README.md | 13 +++ .../Cli/CheckCommand.php | 12 ++- ...erPackageDirectDependenciesSourceFiles.php | 24 +++++- .../LocateComposerPackageSourceFiles.php | 33 ++++++-- .../Cli/CheckCommandTest.php | 17 ++++ ...ckageDirectDependenciesSourceFilesTest.php | 84 ++++++++++++++++++- .../LocateComposerPackageSourceFilesTest.php | 53 +++++++++++- 7 files changed, 224 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 8fc1e86f..3b324d0d 100644 --- a/README.md +++ b/README.md @@ -51,6 +51,19 @@ If this is already done, run it like this: composer-require-checker check /path/to/your/project/composer.json ``` +### Check development code and dependencies (optional) + +By default, Composer require checker only checks the source code listed in the `autoload` section of `composer.json` +against the dependencies listed in the `require` section. This checks that there are no indirect dependencies in the +*production* code. + +To check the *development* code, use the `--include-dev` option. This will scan the source code from both the +`autoload` and `autoload-dev` section and will search for symbols in dependencies from both the `require` and `require-dev` +section. + +Please note these two checks are really differents: a successful `--include-dev` check does not guarantee there are no +no indirect dependencies in the *production* code. You probably want to do both. + ## Configuration Composer require checker is configured to whitelist some symbols per default. Have a look at the diff --git a/src/ComposerRequireChecker/Cli/CheckCommand.php b/src/ComposerRequireChecker/Cli/CheckCommand.php index b6e26c44..ca4c867a 100644 --- a/src/ComposerRequireChecker/Cli/CheckCommand.php +++ b/src/ComposerRequireChecker/Cli/CheckCommand.php @@ -48,6 +48,12 @@ protected function configure() InputOption::VALUE_NONE, 'this will cause ComposerRequireChecker to ignore errors when files cannot be parsed, otherwise' . ' errors will be thrown' + ) + ->addOption( + 'include-dev', + null, + InputOption::VALUE_NONE, + 'also check the development sources and the development dependencies' ); } @@ -63,9 +69,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int } $composerData = $this->getComposerData($composerJson); + $includeDev = (bool)$input->getOption('include-dev'); + $options = $this->getCheckOptions($input); - $getPackageSourceFiles = new LocateComposerPackageSourceFiles(); + $getPackageSourceFiles = new LocateComposerPackageSourceFiles($includeDev); $getAdditionalSourceFiles = new LocateFilesByGlobPattern(); $sourcesASTs = $this->getASTFromFilesLocator($input); @@ -75,7 +83,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int (new ComposeGenerators())->__invoke( $getAdditionalSourceFiles($options->getScanFiles(), dirname($composerJson)), $getPackageSourceFiles($composerData, dirname($composerJson)), - (new LocateComposerPackageDirectDependenciesSourceFiles())->__invoke($composerJson) + (new LocateComposerPackageDirectDependenciesSourceFiles($includeDev))->__invoke($composerJson) ) )); $this->verbose("found " . count($definedVendorSymbols) . " symbols.", $output, true); diff --git a/src/ComposerRequireChecker/FileLocator/LocateComposerPackageDirectDependenciesSourceFiles.php b/src/ComposerRequireChecker/FileLocator/LocateComposerPackageDirectDependenciesSourceFiles.php index f318978f..147a22e5 100644 --- a/src/ComposerRequireChecker/FileLocator/LocateComposerPackageDirectDependenciesSourceFiles.php +++ b/src/ComposerRequireChecker/FileLocator/LocateComposerPackageDirectDependenciesSourceFiles.php @@ -12,6 +12,21 @@ final class LocateComposerPackageDirectDependenciesSourceFiles { + /** + * @var bool + */ + private $includeDev; + + /** + * LocateComposerPackageDirectDependenciesSourceFiles constructor. + * + * @param bool $includeDev Wether to include packages from require-dev or not. + */ + public function __construct(bool $includeDev) + { + $this->includeDev = $includeDev; + } + public function __invoke(string $composerJsonPath): Generator { $packageDir = dirname($composerJsonPath); @@ -21,7 +36,12 @@ public function __invoke(string $composerJsonPath): Generator $vendorDirs = []; foreach ($composerJson['require'] ?? [] as $vendorName => $vendorRequiredVersion) { $vendorDirs[$vendorName] = $packageDir . '/' . $configVendorDir . '/' . $vendorName; - }; + } + if ($this->includeDev) { + foreach ($composerJson['require-dev'] ?? [] as $vendorName => $vendorRequiredVersion) { + $vendorDirs[$vendorName] = $packageDir . '/' . $configVendorDir . '/' . $vendorName; + } + } $installedPackages = $this->getInstalledPackages($packageDir . '/' . $configVendorDir); @@ -30,7 +50,7 @@ public function __invoke(string $composerJsonPath): Generator continue; } - yield from (new LocateComposerPackageSourceFiles())->__invoke($installedPackages[$vendorName], $vendorDir); + yield from (new LocateComposerPackageSourceFiles(false))->__invoke($installedPackages[$vendorName], $vendorDir); } } diff --git a/src/ComposerRequireChecker/FileLocator/LocateComposerPackageSourceFiles.php b/src/ComposerRequireChecker/FileLocator/LocateComposerPackageSourceFiles.php index d9c33e36..d3484413 100644 --- a/src/ComposerRequireChecker/FileLocator/LocateComposerPackageSourceFiles.php +++ b/src/ComposerRequireChecker/FileLocator/LocateComposerPackageSourceFiles.php @@ -6,6 +6,21 @@ final class LocateComposerPackageSourceFiles { + /** + * @var bool + */ + private $includeDev; + + /** + * LocateComposerPackageSourceFiles constructor. + * + * @param bool $includeDev Wether to liste the files from the autoload-dev configuration. + */ + public function __construct(bool $includeDev) + { + $this->includeDev = $includeDev; + } + /** * @param mixed[] $composerData The contents of composer.json for a package * @param string $packageDir The path to package @@ -14,22 +29,30 @@ final class LocateComposerPackageSourceFiles */ public function __invoke(array $composerData, string $packageDir): Generator { - $blacklist = $composerData['autoload']['exclude-from-classmap'] ?? null; + yield from $this->iterateAutoload($composerData['autoload'] ?? [], $packageDir); + if ($this->includeDev) { + yield from $this->iterateAutoload($composerData['autoload-dev'] ?? [], $packageDir); + } + } + + private function iterateAutoload(array $autoloadData, string $packageDir): Generator + { + $blacklist = $autoloadData['exclude-from-classmap'] ?? null; yield from $this->locateFilesInClassmapDefinitions( - $this->getFilePaths($composerData['autoload']['classmap'] ?? [], $packageDir), + $this->getFilePaths($autoloadData['classmap'] ?? [], $packageDir), $blacklist ); yield from $this->locateFilesInFilesInFilesDefinitions( - $this->getFilePaths($composerData['autoload']['files'] ?? [], $packageDir), + $this->getFilePaths($autoloadData['files'] ?? [], $packageDir), $blacklist ); yield from $this->locateFilesInPsr0Definitions( - $this->getFilePaths($composerData['autoload']['psr-0'] ?? [], $packageDir), + $this->getFilePaths($autoloadData['psr-0'] ?? [], $packageDir), $blacklist ); yield from $this->locateFilesInPsr4Definitions( - $this->getFilePaths($composerData['autoload']['psr-4'] ?? [], $packageDir), + $this->getFilePaths($autoloadData['psr-4'] ?? [], $packageDir), $blacklist ); } diff --git a/test/ComposerRequireCheckerTest/Cli/CheckCommandTest.php b/test/ComposerRequireCheckerTest/Cli/CheckCommandTest.php index 37c032a6..19118a7a 100644 --- a/test/ComposerRequireCheckerTest/Cli/CheckCommandTest.php +++ b/test/ComposerRequireCheckerTest/Cli/CheckCommandTest.php @@ -48,6 +48,23 @@ public function testSelfCheckShowsNoErrors(): void $this->assertNotRegExp('/Collecting used symbols... found \d+ symbols./', $this->commandTester->getDisplay()); } + public function testSelfCheckIncludingDevShowsNoErrors(): void + { + $this->commandTester->execute([ + // that's our own composer.json, lets be sure our self check does not throw errors + 'composer-json' => dirname(__DIR__, 3) . '/composer.json' + ], [ + 'include-dev' => true + ]); + + $this->assertSame(0, $this->commandTester->getStatusCode()); + $this->assertStringContainsString('no unknown symbols found', $this->commandTester->getDisplay()); + + // verbose output should not be shown + $this->assertNotRegExp('/Collecting defined (vendor|extension) symbols... found \d+ symbols./', $this->commandTester->getDisplay()); + $this->assertNotRegExp('/Collecting used symbols... found \d+ symbols./', $this->commandTester->getDisplay()); + } + public function testVerboseSelfCheckShowsCounts(): void { $this->commandTester->execute([ diff --git a/test/ComposerRequireCheckerTest/FileLocator/LocateComposerPackageDirectDependenciesSourceFilesTest.php b/test/ComposerRequireCheckerTest/FileLocator/LocateComposerPackageDirectDependenciesSourceFilesTest.php index 0c82ea99..7f2cc751 100644 --- a/test/ComposerRequireCheckerTest/FileLocator/LocateComposerPackageDirectDependenciesSourceFilesTest.php +++ b/test/ComposerRequireCheckerTest/FileLocator/LocateComposerPackageDirectDependenciesSourceFilesTest.php @@ -21,7 +21,7 @@ protected function setUp(): void { parent::setUp(); - $this->locator = new LocateComposerPackageDirectDependenciesSourceFiles(); + $this->locator = new LocateComposerPackageDirectDependenciesSourceFiles(false); $this->root = vfsStream::setup(); } @@ -182,11 +182,91 @@ public function testOldInstalledJsonUsedAsFallback(): void $this->assertFalse($this->root->hasChild('vendor/foo/bar/composer.json')); } + public function testWithoutDevDependencies(): void + { + vfsStream::create([ + 'composer.json' => '{"require":{"foo/bar": "^1.0"},"require-dev":{"foo/baz": "^1.0"}}', + 'vendor' => [ + 'composer' => [ + 'installed.json' => + '{"packages":[' . + '{"name": "foo/bar", "autoload":{"psr-4":{"":"src"}}},' . + '{"name": "foo/baz", "autoload":{"psr-4":{"":"src"}}}' . + ']}', + ], + 'foo' => [ + 'bar' => [ + 'src' => [ + 'MyClass.php' => '', + ], + ], + 'baz' => [ + 'src' => [ + 'BazClass.php' => '', + ], + ], + ], + ], + ]); + + $files = $this->locate($this->root->getChild('composer.json')->url()); + + $this->assertCount(1, $files); + + $expectedFile = $this->root->getChild('vendor/foo/bar/src/MyClass.php')->url(); + $actualFile = str_replace('\\', '/', reset($files)); + $this->assertSame($expectedFile, $actualFile); + } + + public function testWithDevDependencies(): void + { + $this->locator = new LocateComposerPackageDirectDependenciesSourceFiles(true); + + vfsStream::create([ + 'composer.json' => '{"require":{"foo/bar": "^1.0"},"require-dev":{"foo/baz": "^1.0"}}', + 'vendor' => [ + 'composer' => [ + 'installed.json' => <<<'INSTALLEDJSON' +{ + "packages": [ + {"name": "foo/bar", "autoload": {"psr-4":{"":"src"}}}, + {"name": "foo/baz", "autoload": {"psr-4":{"":"src"}}} + ] +} +INSTALLEDJSON + ], + 'foo' => [ + 'bar' => [ + 'src' => [ + 'MyClass.php' => '', + ], + ], + 'baz' => [ + 'src' => [ + 'BazClass.php' => '', + ], + ], + ], + ], + ]); + + $files = $this->locate($this->root->getChild('composer.json')->url()); + + $this->assertCount(2, $files); + + $this->assertContains($this->root->getChild('vendor/foo/bar/src/MyClass.php')->url(), $files); + $this->assertContains($this->root->getChild('vendor/foo/baz/src/BazClass.php')->url(), $files); + } /** * @return string[] */ private function locate(string $composerJson): array { - return iterator_to_array(($this->locator)($composerJson)); + $files = []; + $generator = ($this->locator)($composerJson); + foreach ($generator as $file) { + $files[] = $file; + } + return $files; } } diff --git a/test/ComposerRequireCheckerTest/FileLocator/LocateComposerPackageSourceFilesTest.php b/test/ComposerRequireCheckerTest/FileLocator/LocateComposerPackageSourceFilesTest.php index 0b3a2969..e3a20bb0 100644 --- a/test/ComposerRequireCheckerTest/FileLocator/LocateComposerPackageSourceFilesTest.php +++ b/test/ComposerRequireCheckerTest/FileLocator/LocateComposerPackageSourceFilesTest.php @@ -22,7 +22,7 @@ protected function setUp(): void { parent::setUp(); - $this->locator = new LocateComposerPackageSourceFiles(); + $this->locator = new LocateComposerPackageSourceFiles(false); $this->root = vfsStream::setup(); } @@ -180,6 +180,57 @@ public function testFromPsr4WithExcludeFromClassmap(array $excludedPattern, arra } } + public function testFromClassmapWithoutDev(): void + { + vfsStream::create([ + 'composer.json' => <<<'COMPOSERJSON' +{ + "autoload": {"classmap": ["src/MyClassA.php"]}, + "autoload-dev": {"classmap": ["tests/MyClassATest.php"]} +} +COMPOSERJSON + , + 'src' => [ + 'MyClassA.php' => ' [ + 'MyClassATest.php' => 'files($this->root->getChild('composer.json')->url()); + + $this->assertCount(1, $files); + $this->assertContains($this->root->getChild('src/MyClassA.php')->url(), $files); + } + + public function testFromClassmapWithDev(): void + { + $this->locator = new LocateComposerPackageSourceFiles(true); + + vfsStream::create([ + 'composer.json' => <<<'COMPOSERJSON' +{ + "autoload": {"classmap": ["src/MyClassA.php"]}, + "autoload-dev": {"classmap": ["tests/MyClassATest.php"]} +} +COMPOSERJSON +, + 'src' => [ + 'MyClassA.php' => ' [ + 'MyClassATest.php' => 'files($this->root->getChild('composer.json')->url()); + + $this->assertCount(2, $files); + $this->assertContains($this->root->getChild('src/MyClassA.php')->url(), $files); + $this->assertContains($this->root->getChild('tests/MyClassATest.php')->url(), $files); + } + /** * @return array[] */