From 1c03d6f07602c68e66ae24b78e40cdf115e5cdf7 Mon Sep 17 00:00:00 2001 From: Bruce Weirdan Date: Sun, 2 Jun 2019 18:23:56 +0300 Subject: [PATCH] Implemented multiple composer roots for plugins (#1723) Refs vimeo/psalm#1710 --- .../Internal/PluginManager/ComposerLock.php | 39 +++++--- .../PluginManager/PluginListFactory.php | 58 +++++++++-- src/psalm_plugin.php | 5 +- tests/ComposerLockTest.php | 99 +++++++++++-------- 4 files changed, 138 insertions(+), 63 deletions(-) diff --git a/src/Psalm/Internal/PluginManager/ComposerLock.php b/src/Psalm/Internal/PluginManager/ComposerLock.php index 80f290369..1ac15a1a7 100644 --- a/src/Psalm/Internal/PluginManager/ComposerLock.php +++ b/src/Psalm/Internal/PluginManager/ComposerLock.php @@ -5,12 +5,13 @@ use RuntimeException; class ComposerLock { - /** @var string */ - private $file_name; + /** @var string[] */ + private $file_names; - public function __construct(string $file_name) + /** @param string[] $file_names */ + public function __construct(array $file_names) { - $this->file_name = $file_name; + $this->file_names = $file_names; } @@ -45,17 +46,17 @@ class ComposerLock return $ret; } - private function read(): array + private function read(string $file_name): array { /** @psalm-suppress MixedAssignment */ - $contents = json_decode(file_get_contents($this->file_name), true); + $contents = json_decode(file_get_contents($file_name), true); if ($error = json_last_error()) { throw new RuntimeException(json_last_error_msg(), $error); } if (!is_array($contents)) { - throw new RuntimeException('Malformed ' . $this->file_name . ', expecting JSON-encoded object'); + throw new RuntimeException('Malformed ' . $file_name . ', expecting JSON-encoded object'); } return $contents; @@ -79,13 +80,23 @@ class ComposerLock private function getAllPackages(): array { - $composer_lock_contents = $this->read(); - if (!isset($composer_lock_contents["packages"]) || !is_array($composer_lock_contents["packages"])) { - throw new RuntimeException('packages section is missing or not an array'); + $packages = []; + foreach ($this->file_names as $file_name) { + $composer_lock_contents = $this->read($file_name); + if (!isset($composer_lock_contents["packages"]) || !is_array($composer_lock_contents["packages"])) { + throw new RuntimeException('packages section is missing or not an array'); + } + if (!isset($composer_lock_contents["packages-dev"]) || !is_array($composer_lock_contents["packages-dev"])) { + throw new RuntimeException('packages-dev section is missing or not an array'); + } + $packages = array_merge( + $packages, + array_merge( + $composer_lock_contents["packages"], + $composer_lock_contents["packages-dev"] + ) + ); } - if (!isset($composer_lock_contents["packages-dev"]) || !is_array($composer_lock_contents["packages-dev"])) { - throw new RuntimeException('packages-dev section is missing or not an array'); - } - return array_merge($composer_lock_contents["packages"], $composer_lock_contents["packages-dev"]); + return $packages; } } diff --git a/src/Psalm/Internal/PluginManager/PluginListFactory.php b/src/Psalm/Internal/PluginManager/PluginListFactory.php index fad125f7d..525743bb7 100644 --- a/src/Psalm/Internal/PluginManager/PluginListFactory.php +++ b/src/Psalm/Internal/PluginManager/PluginListFactory.php @@ -3,19 +3,59 @@ namespace Psalm\Internal\PluginManager; class PluginListFactory { + /** @var string */ + private $project_root; + + /** @var string */ + private $psalm_root; + + public function __construct(string $project_root, string $psalm_root) + { + $this->project_root = $project_root; + $this->psalm_root = $psalm_root; + } + public function __invoke(string $current_dir, string $config_file_path = null): PluginList { - $stub_composer_lock = (object)[ - "packages" => [], - "packages-dev" => [], - ]; - $config_file = new ConfigFile($current_dir, $config_file_path); - $lock_file = is_readable('composer.lock') ? - 'composer.lock' : - 'data:application/json,' . urlencode(json_encode($stub_composer_lock)); + $composer_lock = new ComposerLock($this->findLockFiles()); - $composer_lock = new ComposerLock($lock_file); return new PluginList($config_file, $composer_lock); } + + /** @return non-empty-array */ + private function findLockFiles(): array + { + // use cases + // 1. plugins are installed into project vendors - composer.lock is PROJECT_ROOT/composer.lock + // 2. plugins are installed into separate composer environment (either global or bamarni-bin) + // - composer.lock is PSALM_ROOT/../../../composer.lock + // 3. plugins are installed into psalm vendors - composer.lock is PSALM_ROOT/composer.lock + // 4. none of the above - use stub (empty virtual composer.lock) + + if ($this->psalm_root === $this->project_root) { + // managing plugins for psalm itself + $composer_lock_filenames = [ + rtrim($this->psalm_root, DIRECTORY_SEPARATOR) . '/composer.lock', + ]; + } else { + $composer_lock_filenames = [ + rtrim($this->project_root, DIRECTORY_SEPARATOR) . '/composer.lock', + rtrim($this->psalm_root, DIRECTORY_SEPARATOR) . '/../../../composer.lock', + rtrim($this->psalm_root, DIRECTORY_SEPARATOR) . '/composer.lock', + ]; + } + + $composer_lock_filenames = array_filter($composer_lock_filenames, 'is_readable'); + + if (empty($composer_lock_filenames)) { + $stub_composer_lock = (object)[ + "packages" => [], + "packages-dev" => [], + ]; + $composer_lock_filenames[] = 'data:application/json,' . urlencode(json_encode($stub_composer_lock)); + } + + return $composer_lock_filenames; + } } diff --git a/src/psalm_plugin.php b/src/psalm_plugin.php index 544996573..49cf6bb4f 100644 --- a/src/psalm_plugin.php +++ b/src/psalm_plugin.php @@ -15,7 +15,10 @@ requireAutoloaders($current_dir, false, $vendor_dir); $app = new Application('psalm-plugin', (string) Versions::getVersion('vimeo/psalm')); -$plugin_list_factory = new PluginListFactory; +$psalm_root = dirname(__DIR__) . DIRECTORY_SEPARATOR; + +$plugin_list_factory = new PluginListFactory($current_dir, $psalm_root); + $app->addCommands([ new ShowCommand($plugin_list_factory), new EnableCommand($plugin_list_factory), diff --git a/tests/ComposerLockTest.php b/tests/ComposerLockTest.php index f7f5c58cf..0e54441ab 100644 --- a/tests/ComposerLockTest.php +++ b/tests/ComposerLockTest.php @@ -12,17 +12,8 @@ class ComposerLockTest extends TestCase */ public function pluginIsPackageOfTypePsalmPlugin() { - $lock = new ComposerLock($this->jsonFile((object)[])); - $this->assertTrue($lock->isPlugin([ - 'name' => 'vendor/package', - 'type' => 'psalm-plugin', - 'extra' => [ - 'psalm' => [ - 'pluginClass' => 'Some\Class', - ], - ], - ])); - + $lock = new ComposerLock([$this->jsonFile((object)[])]); + $this->assertTrue($lock->isPlugin($this->pluginEntry('vendor/package', 'Some\Class'))); // counterexamples $this->assertFalse($lock->isPlugin([]), 'Non-package should not be considered a plugin'); @@ -44,20 +35,12 @@ class ComposerLockTest extends TestCase */ public function seesNonDevPlugins() { - $lock = new ComposerLock($this->jsonFile((object)[ + $lock = new ComposerLock([$this->jsonFile((object)[ 'packages' => [ - (object)[ - 'name' => 'vendor/package', - 'type' => 'psalm-plugin', - 'extra' => (object)[ - 'psalm' => (object) [ - 'pluginClass' => 'Vendor\Package\PluginClass', - ], - ], - ], + (object)$this->pluginEntry('vendor/package', 'Vendor\Package\PluginClass') ], 'packages-dev' => [], - ])); + ])]); $plugins = $lock->getPlugins(); $this->assertArrayHasKey('vendor/package', $plugins); @@ -70,20 +53,12 @@ class ComposerLockTest extends TestCase */ public function seesDevPlugins() { - $lock = new ComposerLock($this->jsonFile((object)[ + $lock = new ComposerLock([$this->jsonFile((object)[ 'packages' => [], 'packages-dev' => [ - (object)[ - 'name' => 'vendor/package', - 'type' => 'psalm-plugin', - 'extra' => (object)[ - 'psalm' => (object)[ - 'pluginClass' => 'Vendor\Package\PluginClass', - ], - ], - ], + (object) $this->pluginEntry('vendor/package', 'Vendor\Package\PluginClass') ], - ])); + ])]); $plugins = $lock->getPlugins(); $this->assertArrayHasKey('vendor/package', $plugins); @@ -101,10 +76,10 @@ class ComposerLockTest extends TestCase 'type' => 'library', ]; - $lock = new ComposerLock($this->jsonFile((object)[ + $lock = new ComposerLock([$this->jsonFile((object)[ 'packages' => [$nonPlugin], 'packages-dev' => [$nonPlugin], - ])); + ])]); $this->assertEmpty($lock->getPlugins()); } @@ -114,7 +89,7 @@ class ComposerLockTest extends TestCase */ public function failsOnInvalidJson() { - $lock = new ComposerLock('data:application/json,['); + $lock = new ComposerLock(['data:application/json,[']); $this->expectException(\RuntimeException::class); $lock->getPlugins(); @@ -126,7 +101,7 @@ class ComposerLockTest extends TestCase */ public function failsOnNonObjectJson() { - $lock = new ComposerLock('data:application/json,null'); + $lock = new ComposerLock(['data:application/json,null']); $this->expectException(\RuntimeException::class); $lock->getPlugins(); @@ -141,7 +116,7 @@ class ComposerLockTest extends TestCase $noPackagesFile = $this->jsonFile((object)[ 'packages-dev' => [], ]); - $lock = new ComposerLock($noPackagesFile); + $lock = new ComposerLock([$noPackagesFile]); $this->expectException(\RuntimeException::class); $lock->getPlugins(); } @@ -155,11 +130,57 @@ class ComposerLockTest extends TestCase $noPackagesDevFile = $this->jsonFile((object)[ 'packages' => [], ]); - $lock = new ComposerLock($noPackagesDevFile); + $lock = new ComposerLock([$noPackagesDevFile]); $this->expectException(\RuntimeException::class); $lock->getPlugins(); } + /** @test */ + public function mergesMultipleComposerLockFiles(): void + { + $lock = new ComposerLock([ + $this->jsonFile([ + 'packages' => [ + (object) $this->pluginEntry('vendor/packageA', 'Vendor\PackageA\PluginClass') + ], + 'packages-dev' => [ + (object) $this->pluginEntry('vendor/packageB', 'Vendor\PackageB\PluginClass') + ], + ]), + $this->jsonFile([ + 'packages' => [ + (object) $this->pluginEntry('vendor/packageC', 'Vendor\PackageC\PluginClass') + ], + 'packages-dev' => [ + (object) $this->pluginEntry('vendor/packageD', 'Vendor\PackageD\PluginClass') + ], + ]), + ]); + + $this->assertEquals( + [ + 'vendor/packageA' => 'Vendor\PackageA\PluginClass', + 'vendor/packageB' => 'Vendor\PackageB\PluginClass', + 'vendor/packageC' => 'Vendor\PackageC\PluginClass', + 'vendor/packageD' => 'Vendor\PackageD\PluginClass', + ], + $lock->getPlugins() + ); + } + + private function pluginEntry(string $package_name, string $package_class): array + { + return [ + 'name' => $package_name, + 'type' => 'psalm-plugin', + 'extra' => [ + 'psalm' => [ + 'pluginClass' => $package_class, + ], + ], + ]; + } + /** @param mixed $data */ private function jsonFile($data): string {