From 4e6d54b7311bf8035aeba7a5b07753b72d147420 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Mon, 3 Jan 2022 15:40:32 +0100 Subject: [PATCH] Fix all 5.3 $this-in-closure usages --- composer.json | 9 +--- .../src/AnonymousFunctionWithThisRule.php | 46 ---------------- .../AnonymousFunctionWithThisRuleTest.php | 28 ---------- phpstan/Rules/tests/data/method-with-this.php | 34 ------------ phpstan/config.neon | 3 -- src/Composer/Command/InitCommand.php | 5 +- src/Composer/Config.php | 6 +-- .../DependencyResolver/DefaultPolicy.php | 9 ++-- src/Composer/Downloader/ArchiveDownloader.php | 21 ++++---- src/Composer/Downloader/DownloadManager.php | 11 ++-- src/Composer/Downloader/FileDownloader.php | 34 ++++-------- src/Composer/Downloader/ZipDownloader.php | 19 +++---- .../Installer/InstallationManager.php | 9 ++-- src/Composer/Installer/LibraryInstaller.php | 11 +--- src/Composer/Installer/PluginInstaller.php | 28 +++------- src/Composer/Json/JsonManipulator.php | 11 ++-- .../Repository/ComposerRepository.php | 54 +++++++------------ src/Composer/Util/Filesystem.php | 6 +-- src/Composer/Util/HttpDownloader.php | 16 ++---- src/Composer/Util/ProcessExecutor.php | 7 +-- .../Test/Autoload/AutoloadGeneratorTest.php | 15 +++--- .../Test/Downloader/FileDownloaderTest.php | 28 +++++----- .../Composer/Test/Package/BasePackageTest.php | 13 +++-- .../Repository/ComposerRepositoryTest.php | 17 ++---- tests/Composer/Test/Util/GitTest.php | 10 ++-- 25 files changed, 120 insertions(+), 330 deletions(-) delete mode 100644 phpstan/Rules/src/AnonymousFunctionWithThisRule.php delete mode 100644 phpstan/Rules/tests/AnonymousFunctionWithThisRuleTest.php delete mode 100644 phpstan/Rules/tests/data/method-with-this.php diff --git a/composer.json b/composer.json index 92bbfdc8c..366cd7e64 100644 --- a/composer.json +++ b/composer.json @@ -69,13 +69,8 @@ }, "autoload-dev": { "psr-4": { - "Composer\\Test\\": "tests/Composer/Test", - "Composer\\PHPStanRules\\": "phpstan/Rules/src", - "Composer\\PHPStanRulesTests\\": "phpstan/Rules/tests" - }, - "classmap": [ - "phpstan/Rules/tests/data" - ] + "Composer\\Test\\": "tests/Composer/Test" + } }, "bin": [ "bin/composer" diff --git a/phpstan/Rules/src/AnonymousFunctionWithThisRule.php b/phpstan/Rules/src/AnonymousFunctionWithThisRule.php deleted file mode 100644 index d9a44ecb5..000000000 --- a/phpstan/Rules/src/AnonymousFunctionWithThisRule.php +++ /dev/null @@ -1,46 +0,0 @@ - - */ -final class AnonymousFunctionWithThisRule implements Rule -{ - /** - * @inheritDoc - */ - public function getNodeType(): string - { - return \PhpParser\Node\Expr\Variable::class; - } - - /** - * @inheritDoc - */ - public function processNode(Node $node, Scope $scope): array - { - if (!\is_string($node->name) || $node->name !== 'this') { - return []; - } - - if ($scope->isInClosureBind()) { - return []; - } - - if (!$scope->isInClass()) { - // reported in other standard rule on level 0 - return []; - } - - if ($scope->isInAnonymousFunction()) { - return ['Using $this inside anonymous function is prohibited because of PHP 5.3 support.']; - } - - return []; - } -} diff --git a/phpstan/Rules/tests/AnonymousFunctionWithThisRuleTest.php b/phpstan/Rules/tests/AnonymousFunctionWithThisRuleTest.php deleted file mode 100644 index add285d17..000000000 --- a/phpstan/Rules/tests/AnonymousFunctionWithThisRuleTest.php +++ /dev/null @@ -1,28 +0,0 @@ - - */ -final class AnonymousFunctionWithThisRuleTest extends RuleTestCase -{ - /** - * @inheritDoc - */ - protected function getRule(): \PHPStan\Rules\Rule - { - return new AnonymousFunctionWithThisRule(); - } - - public function testWithThis(): void - { - $this->analyse([__DIR__ . '/data/method-with-this.php'], [ - ['Using $this inside anonymous function is prohibited because of PHP 5.3 support.', 13], - ['Using $this inside anonymous function is prohibited because of PHP 5.3 support.', 17], - ]); - } -} diff --git a/phpstan/Rules/tests/data/method-with-this.php b/phpstan/Rules/tests/data/method-with-this.php deleted file mode 100644 index e0b15bffa..000000000 --- a/phpstan/Rules/tests/data/method-with-this.php +++ /dev/null @@ -1,34 +0,0 @@ -firstProp; - }; - - call_user_func(function() { - $this->funMethod(); - }, $this); - - $bind = 'bind'; - function() use($bind) { - - }; - } -} - -function global_ok() { - $_SERVER['REMOTE_ADDR']; -} - -function global_this() { - // not checked by our rule, it is checked with standard phpstan rule on level 0 - $this['REMOTE_ADDR']; -} diff --git a/phpstan/config.neon b/phpstan/config.neon index 69d7df43d..990a6b0b6 100644 --- a/phpstan/config.neon +++ b/phpstan/config.neon @@ -56,6 +56,3 @@ parameters: - Composer\Composer::VERSION - Composer\Composer::RELEASE_DATE - Composer\Composer::SOURCE_VERSION - -rules: - - Composer\PHPStanRules\AnonymousFunctionWithThisRule diff --git a/src/Composer/Command/InitCommand.php b/src/Composer/Command/InitCommand.php index 099ec2a2e..5f30d7a04 100644 --- a/src/Composer/Command/InitCommand.php +++ b/src/Composer/Command/InitCommand.php @@ -348,15 +348,14 @@ EOT } } - $self = $this; $author = $io->askAndValidate( 'Author ['.$author.', n to skip]: ', - function ($value) use ($self, $author) { + function ($value) use ($author) { if ($value === 'n' || $value === 'no') { return; } $value = $value ?: $author; - $author = $self->parseAuthorString($value); + $author = $this->parseAuthorString($value); return sprintf('%s <%s>', $author['name'], $author['email']); }, diff --git a/src/Composer/Config.php b/src/Composer/Config.php index 6b00ba46c..af2b2edc3 100644 --- a/src/Composer/Config.php +++ b/src/Composer/Config.php @@ -514,14 +514,12 @@ class Config */ private function process($value, $flags) { - $config = $this; - if (!is_string($value)) { return $value; } - return Preg::replaceCallback('#\{\$(.+)\}#', function ($match) use ($config, $flags) { - return $config->get($match[1], $flags); + return Preg::replaceCallback('#\{\$(.+)\}#', function ($match) use ($flags) { + return $this->get($match[1], $flags); }, $value); } diff --git a/src/Composer/DependencyResolver/DefaultPolicy.php b/src/Composer/DependencyResolver/DefaultPolicy.php index 4d7aa55b4..74e60d31a 100644 --- a/src/Composer/DependencyResolver/DefaultPolicy.php +++ b/src/Composer/DependencyResolver/DefaultPolicy.php @@ -64,11 +64,10 @@ class DefaultPolicy implements PolicyInterface public function selectPreferredPackages(Pool $pool, array $literals, $requiredPackage = null) { $packages = $this->groupLiteralsByName($pool, $literals); - $policy = $this; foreach ($packages as &$nameLiterals) { - usort($nameLiterals, function ($a, $b) use ($policy, $pool, $requiredPackage) { - return $policy->compareByPriority($pool, $pool->literalToPackage($a), $pool->literalToPackage($b), $requiredPackage, true); + usort($nameLiterals, function ($a, $b) use ($pool, $requiredPackage) { + return $this->compareByPriority($pool, $pool->literalToPackage($a), $pool->literalToPackage($b), $requiredPackage, true); }); } @@ -80,8 +79,8 @@ class DefaultPolicy implements PolicyInterface $selected = \call_user_func_array('array_merge', array_values($packages)); // now sort the result across all packages to respect replaces across packages - usort($selected, function ($a, $b) use ($policy, $pool, $requiredPackage) { - return $policy->compareByPriority($pool, $pool->literalToPackage($a), $pool->literalToPackage($b), $requiredPackage); + usort($selected, function ($a, $b) use ($pool, $requiredPackage) { + return $this->compareByPriority($pool, $pool->literalToPackage($a), $pool->literalToPackage($b), $requiredPackage); }); return $selected; diff --git a/src/Composer/Downloader/ArchiveDownloader.php b/src/Composer/Downloader/ArchiveDownloader.php index 570de63d8..ddbea79c0 100644 --- a/src/Composer/Downloader/ArchiveDownloader.php +++ b/src/Composer/Downloader/ArchiveDownloader.php @@ -28,9 +28,8 @@ abstract class ArchiveDownloader extends FileDownloader { /** * @var array - * @protected */ - public $cleanupExecuted = array(); + protected $cleanupExecuted = array(); /** * @return PromiseInterface|null @@ -92,22 +91,20 @@ abstract class ArchiveDownloader extends FileDownloader $fileName = $this->getFileName($package, $path); $filesystem = $this->filesystem; - $self = $this; - $cleanup = function () use ($path, $filesystem, $temporaryDir, $package, $self) { + $cleanup = function () use ($path, $filesystem, $temporaryDir, $package) { // remove cache if the file was corrupted - $self->clearLastCacheWrite($package); + $this->clearLastCacheWrite($package); // clean up $filesystem->removeDirectory($temporaryDir); if (is_dir($path) && realpath($path) !== getcwd()) { $filesystem->removeDirectory($path); } - $self->removeCleanupPath($package, $temporaryDir); - $self->removeCleanupPath($package, realpath($path)); + $this->removeCleanupPath($package, $temporaryDir); + $this->removeCleanupPath($package, realpath($path)); }; - $promise = null; try { $promise = $this->extract($package, $fileName, $temporaryDir); } catch (\Exception $e) { @@ -119,7 +116,7 @@ abstract class ArchiveDownloader extends FileDownloader $promise = \React\Promise\resolve(); } - return $promise->then(function () use ($self, $package, $filesystem, $fileName, $temporaryDir, $path) { + return $promise->then(function () use ($package, $filesystem, $fileName, $temporaryDir, $path) { $filesystem->unlink($fileName); /** @@ -203,9 +200,9 @@ abstract class ArchiveDownloader extends FileDownloader $promise = $filesystem->removeDirectoryAsync($temporaryDir); - return $promise->then(function () use ($self, $package, $path, $temporaryDir) { - $self->removeCleanupPath($package, $temporaryDir); - $self->removeCleanupPath($package, $path); + return $promise->then(function () use ($package, $path, $temporaryDir) { + $this->removeCleanupPath($package, $temporaryDir); + $this->removeCleanupPath($package, $path); }); }, function ($e) use ($cleanup) { $cleanup(); diff --git a/src/Composer/Downloader/DownloadManager.php b/src/Composer/Downloader/DownloadManager.php index 8e457f117..71eb87ee6 100644 --- a/src/Composer/Downloader/DownloadManager.php +++ b/src/Composer/Downloader/DownloadManager.php @@ -192,16 +192,15 @@ class DownloadManager $sources = $this->getAvailableSources($package, $prevPackage); $io = $this->io; - $self = $this; - $download = function ($retry = false) use (&$sources, $io, $package, $self, $targetDir, &$download, $prevPackage) { + $download = function ($retry = false) use (&$sources, $io, $package, $targetDir, &$download, $prevPackage) { $source = array_shift($sources); if ($retry) { $io->writeError(' Now trying to download from ' . $source . ''); } $package->setInstallationSource($source); - $downloader = $self->getDownloaderForPackage($package); + $downloader = $this->getDownloaderForPackage($package); if (!$downloader) { return \React\Promise\resolve(); } @@ -332,10 +331,8 @@ class DownloadManager // we wipe the dir and do a new install instead of updating it $promise = $initialDownloader->remove($initial, $targetDir); if ($promise) { - $self = $this; - - return $promise->then(function ($res) use ($self, $target, $targetDir) { - return $self->install($target, $targetDir); + return $promise->then(function ($res) use ($target, $targetDir) { + return $this->install($target, $targetDir); }); } diff --git a/src/Composer/Downloader/FileDownloader.php b/src/Composer/Downloader/FileDownloader.php index feefe6421..f4bd777df 100644 --- a/src/Composer/Downloader/FileDownloader.php +++ b/src/Composer/Downloader/FileDownloader.php @@ -150,11 +150,10 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface $httpDownloader = $this->httpDownloader; $eventDispatcher = $this->eventDispatcher; $filesystem = $this->filesystem; - $self = $this; $accept = null; $reject = null; - $download = function () use ($io, $output, $httpDownloader, $cache, $cacheKeyGenerator, $eventDispatcher, $package, $fileName, &$urls, &$accept, &$reject, $self) { + $download = function () use ($io, $output, $httpDownloader, $cache, $cacheKeyGenerator, $eventDispatcher, $package, $fileName, &$urls, &$accept, &$reject) { /** @var array{base: string, processed: string, cacheKey: string} $url */ $url = reset($urls); $index = key($urls); @@ -184,7 +183,7 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface // the cache is corrupt the archive will be deleted and the next attempt will re-download it // see https://github.com/composer/composer/issues/10028 if (!$cache->isReadOnly()) { - $self->lastCacheWrites[$package->getName()] = $cacheKey; + $this->lastCacheWrites[$package->getName()] = $cacheKey; } $result = \React\Promise\resolve($fileName); } else { @@ -222,13 +221,13 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface }); }; - $accept = function ($response) use ($cache, $package, $fileName, $self, &$urls) { + $accept = function ($response) use ($cache, $package, $fileName, &$urls) { $url = reset($urls); $cacheKey = $url['cacheKey']; FileDownloader::$downloadMetadata[$package->getName()] = @filesize($fileName) ?: $response->getHeader('Content-Length') ?: '?'; if ($cache && !$cache->isReadOnly()) { - $self->lastCacheWrites[$package->getName()] = $cacheKey; + $this->lastCacheWrites[$package->getName()] = $cacheKey; $cache->copyFrom($cacheKey, $fileName); } @@ -237,12 +236,12 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface return $fileName; }; - $reject = function ($e) use ($io, &$urls, $download, $fileName, $package, &$retries, $filesystem, $self) { + $reject = function ($e) use ($io, &$urls, $download, $fileName, $package, &$retries, $filesystem) { // clean up if (file_exists($fileName)) { $filesystem->unlink($fileName); } - $self->clearLastCacheWrite($package); + $this->clearLastCacheWrite($package); if ($e instanceof IrrecoverableDownloadException) { throw $e; @@ -361,12 +360,9 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface } /** - * TODO mark private in v3 - * @protected This is public due to PHP 5.3 - * * @return void */ - public function clearLastCacheWrite(PackageInterface $package) + protected function clearLastCacheWrite(PackageInterface $package) { if ($this->cache && isset($this->lastCacheWrites[$package->getName()])) { $this->cache->remove($this->lastCacheWrites[$package->getName()]); @@ -375,27 +371,21 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface } /** - * TODO mark private in v3 - * @protected This is public due to PHP 5.3 - * * @param string $path * * @return void */ - public function addCleanupPath(PackageInterface $package, $path) + protected function addCleanupPath(PackageInterface $package, $path) { $this->additionalCleanupPaths[$package->getName()][] = $path; } /** - * TODO mark private in v3 - * @protected This is public due to PHP 5.3 - * * @param string $path * * @return void */ - public function removeCleanupPath(PackageInterface $package, $path) + protected function removeCleanupPath(PackageInterface $package, $path) { if (isset($this->additionalCleanupPaths[$package->getName()])) { $idx = array_search($path, $this->additionalCleanupPaths[$package->getName()]); @@ -416,11 +406,9 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface if (!$promise instanceof PromiseInterface) { $promise = \React\Promise\resolve(); } - $self = $this; - $io = $this->io; - return $promise->then(function () use ($self, $target, $path) { - $promise = $self->install($target, $path, false); + return $promise->then(function () use ($target, $path) { + $promise = $this->install($target, $path, false); return $promise; }); diff --git a/src/Composer/Downloader/ZipDownloader.php b/src/Composer/Downloader/ZipDownloader.php index a86d1da6b..0405cdf9c 100644 --- a/src/Composer/Downloader/ZipDownloader.php +++ b/src/Composer/Downloader/ZipDownloader.php @@ -126,9 +126,8 @@ class ZipDownloader extends ArchiveDownloader $executable = $commandSpec[0]; - $self = $this; $io = $this->io; - $tryFallback = function ($processError) use ($isLastChance, $io, $self, $file, $path, $package, $executable) { + $tryFallback = function ($processError) use ($isLastChance, $io, $file, $path, $package, $executable) { if ($isLastChance) { throw $processError; } @@ -143,15 +142,15 @@ class ZipDownloader extends ArchiveDownloader $io->writeError(' Unzip with '.$executable.' command failed, falling back to ZipArchive class'); } - return $self->extractWithZipArchive($package, $file, $path); + return $this->extractWithZipArchive($package, $file, $path); }; try { $promise = $this->process->executeAsync($command); - return $promise->then(function ($process) use ($tryFallback, $command, $package, $file, $self) { + return $promise->then(function ($process) use ($tryFallback, $command, $package, $file) { if (!$process->isSuccessful()) { - if (isset($self->cleanupExecuted[$package->getName()])) { + if (isset($this->cleanupExecuted[$package->getName()])) { throw new \RuntimeException('Failed to extract '.$package->getName().' as the installation was aborted by another package operation.'); } @@ -174,11 +173,8 @@ class ZipDownloader extends ArchiveDownloader * @param string $file File to extract * @param string $path Path where to extract file * @return PromiseInterface - * - * TODO v3 should make this private once we can drop PHP 5.3 support - * @protected */ - public function extractWithZipArchive(PackageInterface $package, $file, $path) + private function extractWithZipArchive(PackageInterface $package, $file, $path) { $processError = null; $zipArchive = $this->zipArchiveObject ?: new ZipArchive(); @@ -214,11 +210,8 @@ class ZipDownloader extends ArchiveDownloader * @param string $file File to extract * @param string $path Path where to extract file * @return PromiseInterface|null - * - * TODO v3 should make this private once we can drop PHP 5.3 support - * @protected */ - public function extract(PackageInterface $package, $file, $path) + protected function extract(PackageInterface $package, $file, $path) { return $this->extractWithSystemUnzip($package, $file, $path); } diff --git a/src/Composer/Installer/InstallationManager.php b/src/Composer/Installer/InstallationManager.php index e1d3d2ee2..71aa3d986 100644 --- a/src/Composer/Installer/InstallationManager.php +++ b/src/Composer/Installer/InstallationManager.php @@ -436,7 +436,6 @@ class InstallationManager } $dispatcher = $this->eventDispatcher; - $installManager = $this; $io = $this->io; $promise = $installer->prepare($opType, $package, $initialPackage); @@ -444,11 +443,11 @@ class InstallationManager $promise = \React\Promise\resolve(); } - $promise = $promise->then(function () use ($opType, $installManager, $repo, $operation) { - return $installManager->$opType($repo, $operation); + $promise = $promise->then(function () use ($opType, $repo, $operation) { + return $this->$opType($repo, $operation); })->then($cleanupPromises[$index]) - ->then(function () use ($installManager, $devMode, $repo) { - $repo->write($devMode, $installManager); + ->then(function () use ($devMode, $repo) { + $repo->write($devMode, $this); }, function ($e) use ($opType, $package, $io) { $io->writeError(' ' . ucfirst($opType) .' of '.$package->getPrettyName().' failed'); diff --git a/src/Composer/Installer/LibraryInstaller.php b/src/Composer/Installer/LibraryInstaller.php index 71f45e8e4..7490e11a5 100644 --- a/src/Composer/Installer/LibraryInstaller.php +++ b/src/Composer/Installer/LibraryInstaller.php @@ -287,15 +287,8 @@ class LibraryInstaller implements InstallerInterface, BinaryPresenceInterface $promise = \React\Promise\resolve(); } - $self = $this; - - return $promise->then(function () use ($self, $target) { - $reflMethod = new \ReflectionMethod($self, 'installCode'); - $reflMethod->setAccessible(true); - - // equivalent of $this->installCode($target) with php 5.3 support - // TODO remove this once 5.3 support is dropped - return $reflMethod->invoke($self, $target); + return $promise->then(function () use ($target) { + return $this->installCode($target); }); } diff --git a/src/Composer/Installer/PluginInstaller.php b/src/Composer/Installer/PluginInstaller.php index 3fc09638b..e4d0f0404 100644 --- a/src/Composer/Installer/PluginInstaller.php +++ b/src/Composer/Installer/PluginInstaller.php @@ -70,15 +70,12 @@ class PluginInstaller extends LibraryInstaller $promise = \React\Promise\resolve(); } - $pluginManager = $this->composer->getPluginManager(); - $self = $this; - - return $promise->then(function () use ($self, $pluginManager, $package, $repo) { + return $promise->then(function () use ($package, $repo) { try { Platform::workaroundFilesystemIssues(); - $pluginManager->registerPackage($package, true); + $this->composer->getPluginManager()->registerPackage($package, true); } catch (\Exception $e) { - $self->rollbackInstall($e, $repo, $package); + $this->rollbackInstall($e, $repo, $package); } }); } @@ -93,16 +90,13 @@ class PluginInstaller extends LibraryInstaller $promise = \React\Promise\resolve(); } - $pluginManager = $this->composer->getPluginManager(); - $self = $this; - - return $promise->then(function () use ($self, $pluginManager, $initial, $target, $repo) { + return $promise->then(function () use ($initial, $target, $repo) { try { Platform::workaroundFilesystemIssues(); - $pluginManager->deactivatePackage($initial); - $pluginManager->registerPackage($target, true); + $this->composer->getPluginManager()->deactivatePackage($initial); + $this->composer->getPluginManager()->registerPackage($target, true); } catch (\Exception $e) { - $self->rollbackInstall($e, $repo, $target); + $this->rollbackInstall($e, $repo, $target); } }); } @@ -114,13 +108,7 @@ class PluginInstaller extends LibraryInstaller return parent::uninstall($repo, $package); } - /** - * TODO v3 should make this private once we can drop PHP 5.3 support - * @private - * - * @return void - */ - public function rollbackInstall(\Exception $e, InstalledRepositoryInterface $repo, PackageInterface $package) + private function rollbackInstall(\Exception $e, InstalledRepositoryInterface $repo, PackageInterface $package): void { $this->io->writeError('Plugin initialization failed ('.$e->getMessage().'), uninstalling plugin'); parent::uninstall($repo, $package); diff --git a/src/Composer/Json/JsonManipulator.php b/src/Composer/Json/JsonManipulator.php index f1fcdf520..1e51f4dcc 100644 --- a/src/Composer/Json/JsonManipulator.php +++ b/src/Composer/Json/JsonManipulator.php @@ -294,12 +294,10 @@ class JsonManipulator return false; } - $that = $this; - // child exists $childRegex = '{'.self::$DEFINES.'(?P"'.preg_quote($name).'"\s*:\s*)(?P(?&json))(?P,?)}x'; if (Preg::isMatch($childRegex, $children, $matches)) { - $children = Preg::replaceCallback($childRegex, function ($matches) use ($subName, $value, $that) { + $children = Preg::replaceCallback($childRegex, function ($matches) use ($subName, $value) { if ($subName !== null) { $curVal = json_decode($matches['content'], true); if (!is_array($curVal)) { @@ -309,7 +307,7 @@ class JsonManipulator $value = $curVal; } - return $matches['start'] . $that->format($value, 1) . $matches['end']; + return $matches['start'] . $this->format($value, 1) . $matches['end']; }, $children); } else { Preg::match('#^{ (?P\s*?) (?P\S+.*?)? (?P\s*) }$#sx', $children, $match); @@ -452,12 +450,11 @@ class JsonManipulator return true; } - $that = $this; - $this->contents = Preg::replaceCallback($nodeRegex, function ($matches) use ($that, $name, $subName, $childrenClean) { + $this->contents = Preg::replaceCallback($nodeRegex, function ($matches) use ($name, $subName, $childrenClean) { if ($subName !== null) { $curVal = json_decode($matches['content'], true); unset($curVal[$name][$subName]); - $childrenClean = $that->format($curVal); + $childrenClean = $this->format($curVal); } return $matches['start'] . $childrenClean . $matches['end']; diff --git a/src/Composer/Repository/ComposerRepository.php b/src/Composer/Repository/ComposerRepository.php index 8c30935f3..5dba6ca22 100644 --- a/src/Composer/Repository/ComposerRepository.php +++ b/src/Composer/Repository/ComposerRepository.php @@ -106,28 +106,23 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito private $partialPackagesByName = null; /** - * TODO v3 should make this private once we can drop PHP 5.3 support - * @private * @var array list of package names which are fresh and can be loaded from the cache directly in case loadPackage is called several times * useful for v2 metadata repositories with lazy providers * @phpstan-var array */ - public $freshMetadataUrls = array(); + private $freshMetadataUrls = array(); /** - * TODO v3 should make this private once we can drop PHP 5.3 support - * @private * @var array list of package names which returned a 404 and should not be re-fetched in case loadPackage is called several times * useful for v2 metadata repositories with lazy providers * @phpstan-var array */ - public $packagesNotFoundCache = array(); + private $packagesNotFoundCache = array(); + /** - * TODO v3 should make this private once we can drop PHP 5.3 support - * @private * @var VersionParser */ - public $versionParser; + private $versionParser; /** * @param array $repoConfig @@ -867,7 +862,6 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito $packages = array(); $namesFound = array(); $promises = array(); - $repo = $this; if (!$this->lazyProvidersUrl) { throw new \LogicException('loadAsyncPackages only supports v2 protocol composer repos with a metadata-url'); @@ -903,7 +897,7 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito } $promises[] = $this->asyncFetchFile($url, $cacheKey, $lastModified) - ->then(function ($response) use (&$packages, &$namesFound, $url, $cacheKey, $contents, $realName, $constraint, $repo, $acceptableStabilities, $stabilityFlags, $alreadyLoaded) { + ->then(function ($response) use (&$packages, &$namesFound, $url, $cacheKey, $contents, $realName, $constraint, $acceptableStabilities, $stabilityFlags, $alreadyLoaded) { $packagesSource = 'downloaded file ('.Url::sanitize($url).')'; if (true === $response) { @@ -925,10 +919,10 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito $versionsToLoad = array(); foreach ($versions as $version) { if (!isset($version['version_normalized'])) { - $version['version_normalized'] = $repo->versionParser->normalize($version['version']); + $version['version_normalized'] = $this->versionParser->normalize($version['version']); } elseif ($version['version_normalized'] === VersionParser::DEFAULT_BRANCH_ALIAS) { // handling of existing repos which need to remain composer v1 compatible, in case the version_normalized contained VersionParser::DEFAULT_BRANCH_ALIAS, we renormalize it - $version['version_normalized'] = $repo->versionParser->normalize($version['version']); + $version['version_normalized'] = $this->versionParser->normalize($version['version']); } // avoid loading packages which have already been loaded @@ -936,18 +930,18 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito continue; } - if ($repo->isVersionAcceptable($constraint, $realName, $version, $acceptableStabilities, $stabilityFlags)) { + if ($this->isVersionAcceptable($constraint, $realName, $version, $acceptableStabilities, $stabilityFlags)) { $versionsToLoad[] = $version; } } - $loadedPackages = $repo->createPackages($versionsToLoad, $packagesSource); + $loadedPackages = $this->createPackages($versionsToLoad, $packagesSource); foreach ($loadedPackages as $package) { - $package->setRepository($repo); + $package->setRepository($this); $packages[spl_object_hash($package)] = $package; if ($package instanceof AliasPackage && !isset($packages[spl_object_hash($package->getAliasOf())])) { - $package->getAliasOf()->setRepository($repo); + $package->getAliasOf()->setRepository($this); $packages[spl_object_hash($package->getAliasOf())] = $package->getAliasOf(); } } @@ -961,10 +955,6 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito } /** - * TODO v3 should make this private once we can drop PHP 5.3 support - * - * @private - * * @param ConstraintInterface|null $constraint * @param string $name package name (must be lowercased already) * @param array $versionData @@ -975,7 +965,7 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito * * @return bool */ - public function isVersionAcceptable($constraint, $name, $versionData, array $acceptableStabilities = null, array $stabilityFlags = null) + private function isVersionAcceptable($constraint, $name, $versionData, array $acceptableStabilities = null, array $stabilityFlags = null) { $versions = array($versionData['version_normalized']); @@ -1248,15 +1238,12 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito } /** - * TODO v3 should make this private once we can drop PHP 5.3 support - * @private - * * @param mixed[] $packages * @param string|null $source * * @return list */ - public function createPackages(array $packages, $source = null) + private function createPackages(array $packages, $source = null) { if (!$packages) { return array(); @@ -1499,25 +1486,24 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito $cache = $this->cache; $degradedMode = &$this->degradedMode; $eventDispatcher = $this->eventDispatcher; - $repo = $this; - $accept = function ($response) use ($io, $url, $filename, $cache, $cacheKey, $eventDispatcher, $repo) { + $accept = function ($response) use ($io, $url, $filename, $cache, $cacheKey, $eventDispatcher) { // package not found is acceptable for a v2 protocol repository if ($response->getStatusCode() === 404) { - $repo->packagesNotFoundCache[$filename] = true; + $this->packagesNotFoundCache[$filename] = true; return array('packages' => array()); } $json = (string) $response->getBody(); if ($json === '' && $response->getStatusCode() === 304) { - $repo->freshMetadataUrls[$filename] = true; + $this->freshMetadataUrls[$filename] = true; return true; } if ($eventDispatcher) { - $postFileDownloadEvent = new PostFileDownloadEvent(PluginEvents::POST_FILE_DOWNLOAD, null, null, $filename, 'metadata', array('response' => $response, 'repository' => $repo)); + $postFileDownloadEvent = new PostFileDownloadEvent(PluginEvents::POST_FILE_DOWNLOAD, null, null, $filename, 'metadata', array('response' => $response, 'repository' => $this)); $eventDispatcher->dispatch($postFileDownloadEvent->getName(), $postFileDownloadEvent); } @@ -1533,14 +1519,14 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito if (!$cache->isReadOnly()) { $cache->write($cacheKey, $json); } - $repo->freshMetadataUrls[$filename] = true; + $this->freshMetadataUrls[$filename] = true; return $data; }; - $reject = function ($e) use ($filename, $accept, $io, $url, &$degradedMode, $repo, $lastModifiedTime) { + $reject = function ($e) use ($filename, $accept, $io, $url, &$degradedMode, $lastModifiedTime) { if ($e instanceof TransportException && $e->getStatusCode() === 404) { - $repo->packagesNotFoundCache[$filename] = true; + $this->packagesNotFoundCache[$filename] = true; return false; } diff --git a/src/Composer/Util/Filesystem.php b/src/Composer/Util/Filesystem.php index 91b1cbe48..3bd1c86fa 100644 --- a/src/Composer/Util/Filesystem.php +++ b/src/Composer/Util/Filesystem.php @@ -157,9 +157,7 @@ class Filesystem $promise = $this->getProcess()->executeAsync($cmd); - $self = $this; - - return $promise->then(function ($process) use ($directory, $self) { + return $promise->then(function ($process) use ($directory) { // clear stat cache because external processes aren't tracked by the php stat cache clearstatcache(); @@ -169,7 +167,7 @@ class Filesystem } } - return \React\Promise\resolve($self->removeDirectoryPhp($directory)); + return \React\Promise\resolve($this->removeDirectoryPhp($directory)); }); } diff --git a/src/Composer/Util/HttpDownloader.php b/src/Composer/Util/HttpDownloader.php index 6b3e7d61d..d1c03b957 100644 --- a/src/Composer/Util/HttpDownloader.php +++ b/src/Composer/Util/HttpDownloader.php @@ -264,7 +264,6 @@ class HttpDownloader }; } - $downloader = $this; $curl = $this->curl; $canceler = function () use (&$job, $curl) { @@ -282,19 +281,18 @@ class HttpDownloader }; $promise = new Promise($resolver, $canceler); - $promise = $promise->then(function ($response) use (&$job, $downloader) { + $promise = $promise->then(function ($response) use (&$job) { $job['status'] = HttpDownloader::STATUS_COMPLETED; $job['response'] = $response; - // TODO 3.0 this should be done directly on $this when PHP 5.3 is dropped - $downloader->markJobDone(); + $this->markJobDone(); return $response; - }, function ($e) use (&$job, $downloader) { + }, function ($e) use (&$job) { $job['status'] = HttpDownloader::STATUS_FAILED; $job['exception'] = $e; - $downloader->markJobDone(); + $this->markJobDone(); throw $e; }); @@ -351,11 +349,7 @@ class HttpDownloader } } - /** - * @private - * @return void - */ - public function markJobDone() + private function markJobDone(): void { $this->runningJobs--; } diff --git a/src/Composer/Util/ProcessExecutor.php b/src/Composer/Util/ProcessExecutor.php index f7efe23de..45c9957e6 100644 --- a/src/Composer/Util/ProcessExecutor.php +++ b/src/Composer/Util/ProcessExecutor.php @@ -344,12 +344,7 @@ class ProcessExecutor return $active; } - /** - * @private - * - * @return void - */ - public function markJobDone() + private function markJobDone(): void { $this->runningJobs--; } diff --git a/tests/Composer/Test/Autoload/AutoloadGeneratorTest.php b/tests/Composer/Test/Autoload/AutoloadGeneratorTest.php index 40046caec..623544c72 100644 --- a/tests/Composer/Test/Autoload/AutoloadGeneratorTest.php +++ b/tests/Composer/Test/Autoload/AutoloadGeneratorTest.php @@ -92,7 +92,6 @@ class AutoloadGeneratorTest extends TestCase protected function setUp(): void { $this->fs = new Filesystem; - $that = $this; $this->workingDir = $this->getUniqueTmpDirectory(); $this->vendorDir = $this->workingDir.DIRECTORY_SEPARATOR.'composer-test-autoload'; @@ -101,8 +100,8 @@ class AutoloadGeneratorTest extends TestCase $this->config = $this->getMockBuilder('Composer\Config')->getMock(); $this->configValueMap = array( - 'vendor-dir' => function () use ($that) { - return $that->vendorDir; + 'vendor-dir' => function () { + return $this->vendorDir; }, 'platform-check' => function () { return true; @@ -111,10 +110,10 @@ class AutoloadGeneratorTest extends TestCase $this->config->expects($this->atLeastOnce()) ->method('get') - ->will($this->returnCallback(function ($arg) use ($that) { + ->will($this->returnCallback(function ($arg) { $ret = null; - if (isset($that->configValueMap[$arg])) { - $ret = $that->configValueMap[$arg]; + if (isset($this->configValueMap[$arg])) { + $ret = $this->configValueMap[$arg]; if (is_callable($ret)) { $ret = $ret(); } @@ -131,10 +130,10 @@ class AutoloadGeneratorTest extends TestCase ->getMock(); $this->im->expects($this->any()) ->method('getInstallPath') - ->will($this->returnCallback(function ($package) use ($that) { + ->will($this->returnCallback(function ($package) { $targetDir = $package->getTargetDir(); - return $that->vendorDir.'/'.$package->getName() . ($targetDir ? '/'.$targetDir : ''); + return $this->vendorDir.'/'.$package->getName() . ($targetDir ? '/'.$targetDir : ''); })); $this->repository = $this->getMockBuilder('Composer\Repository\InstalledRepositoryInterface')->getMock(); $this->repository->expects($this->any()) diff --git a/tests/Composer/Test/Downloader/FileDownloaderTest.php b/tests/Composer/Test/Downloader/FileDownloaderTest.php index 0a7e05c60..1a7ed8d24 100644 --- a/tests/Composer/Test/Downloader/FileDownloaderTest.php +++ b/tests/Composer/Test/Downloader/FileDownloaderTest.php @@ -180,8 +180,6 @@ class FileDownloaderTest extends TestCase public function testDownloadWithCustomProcessedUrl() { - $self = $this; - $path = $this->getUniqueTmpDirectory(); $packageMock = $this->getMockBuilder('Composer\Package\PackageInterface')->getMock(); @@ -232,16 +230,16 @@ class FileDownloaderTest extends TestCase $cacheMock ->expects($this->any()) ->method('copyTo') - ->will($this->returnCallback(function ($cacheKey) use ($self, $expectedCacheKey) { - $self->assertEquals($expectedCacheKey, $cacheKey, 'Failed assertion on $cacheKey argument of Cache::copyTo method:'); + ->will($this->returnCallback(function ($cacheKey) use ($expectedCacheKey) { + $this->assertEquals($expectedCacheKey, $cacheKey, 'Failed assertion on $cacheKey argument of Cache::copyTo method:'); return false; })); $cacheMock ->expects($this->any()) ->method('copyFrom') - ->will($this->returnCallback(function ($cacheKey) use ($self, $expectedCacheKey) { - $self->assertEquals($expectedCacheKey, $cacheKey, 'Failed assertion on $cacheKey argument of Cache::copyFrom method:'); + ->will($this->returnCallback(function ($cacheKey) use ($expectedCacheKey) { + $this->assertEquals($expectedCacheKey, $cacheKey, 'Failed assertion on $cacheKey argument of Cache::copyFrom method:'); return false; })); @@ -250,8 +248,8 @@ class FileDownloaderTest extends TestCase $httpDownloaderMock ->expects($this->any()) ->method('addCopy') - ->will($this->returnCallback(function ($url) use ($self, $expectedUrl) { - $self->assertEquals($expectedUrl, $url, 'Failed assertion on $url argument of HttpDownloader::addCopy method:'); + ->will($this->returnCallback(function ($url) use ($expectedUrl) { + $this->assertEquals($expectedUrl, $url, 'Failed assertion on $url argument of HttpDownloader::addCopy method:'); return \React\Promise\resolve( new Response(array('url' => 'http://example.org/'), 200, array(), 'file~') @@ -281,8 +279,6 @@ class FileDownloaderTest extends TestCase public function testDownloadWithCustomCacheKey() { - $self = $this; - $path = $this->getUniqueTmpDirectory(); $packageMock = $this->getMockBuilder('Composer\Package\PackageInterface')->getMock(); @@ -334,16 +330,16 @@ class FileDownloaderTest extends TestCase $cacheMock ->expects($this->any()) ->method('copyTo') - ->will($this->returnCallback(function ($cacheKey) use ($self, $expectedCacheKey) { - $self->assertEquals($expectedCacheKey, $cacheKey, 'Failed assertion on $cacheKey argument of Cache::copyTo method:'); + ->will($this->returnCallback(function ($cacheKey) use ($expectedCacheKey) { + $this->assertEquals($expectedCacheKey, $cacheKey, 'Failed assertion on $cacheKey argument of Cache::copyTo method:'); return false; })); $cacheMock ->expects($this->any()) ->method('copyFrom') - ->will($this->returnCallback(function ($cacheKey) use ($self, $expectedCacheKey) { - $self->assertEquals($expectedCacheKey, $cacheKey, 'Failed assertion on $cacheKey argument of Cache::copyFrom method:'); + ->will($this->returnCallback(function ($cacheKey) use ($expectedCacheKey) { + $this->assertEquals($expectedCacheKey, $cacheKey, 'Failed assertion on $cacheKey argument of Cache::copyFrom method:'); return false; })); @@ -352,8 +348,8 @@ class FileDownloaderTest extends TestCase $httpDownloaderMock ->expects($this->any()) ->method('addCopy') - ->will($this->returnCallback(function ($url) use ($self, $expectedUrl) { - $self->assertEquals($expectedUrl, $url, 'Failed assertion on $url argument of HttpDownloader::addCopy method:'); + ->will($this->returnCallback(function ($url) use ($expectedUrl) { + $this->assertEquals($expectedUrl, $url, 'Failed assertion on $url argument of HttpDownloader::addCopy method:'); return \React\Promise\resolve( new Response(array('url' => 'http://example.org/'), 200, array(), 'file~') diff --git a/tests/Composer/Test/Package/BasePackageTest.php b/tests/Composer/Test/Package/BasePackageTest.php index 3a2449bb0..b1609b957 100644 --- a/tests/Composer/Test/Package/BasePackageTest.php +++ b/tests/Composer/Test/Package/BasePackageTest.php @@ -79,13 +79,12 @@ class BasePackageTest extends TestCase ), ); - $self = $this; - $createPackage = function ($arr) use ($self) { - $package = $self->getMockForAbstractClass('\Composer\Package\BasePackage', array(), '', false); - $package->expects($self->once())->method('isDev')->will($self->returnValue(true)); - $package->expects($self->any())->method('getSourceType')->will($self->returnValue('git')); - $package->expects($self->once())->method('getPrettyVersion')->will($self->returnValue('PrettyVersion')); - $package->expects($self->any())->method('getSourceReference')->will($self->returnValue($arr['sourceReference'])); + $createPackage = function ($arr) { + $package = $this->getMockForAbstractClass('\Composer\Package\BasePackage', array(), '', false); + $package->expects($this->once())->method('isDev')->will($this->returnValue(true)); + $package->expects($this->any())->method('getSourceType')->will($this->returnValue('git')); + $package->expects($this->once())->method('getPrettyVersion')->will($this->returnValue('PrettyVersion')); + $package->expects($this->any())->method('getSourceReference')->will($this->returnValue($arr['sourceReference'])); return array($package, $arr['truncate'], $arr['expected']); }; diff --git a/tests/Composer/Test/Repository/ComposerRepositoryTest.php b/tests/Composer/Test/Repository/ComposerRepositoryTest.php index 3460ec2d8..6da4e3815 100644 --- a/tests/Composer/Test/Repository/ComposerRepositoryTest.php +++ b/tests/Composer/Test/Repository/ComposerRepositoryTest.php @@ -37,7 +37,7 @@ class ComposerRepositoryTest extends TestCase ); $repository = $this->getMockBuilder('Composer\Repository\ComposerRepository') - ->onlyMethods(array('loadRootServerFile', 'createPackages')) + ->onlyMethods(array('loadRootServerFile')) ->setConstructorArgs(array( $repoConfig, new NullIO, @@ -52,22 +52,15 @@ class ComposerRepositoryTest extends TestCase ->method('loadRootServerFile') ->will($this->returnValue($repoPackages)); - $stubs = array(); - foreach ($expected as $at => $arg) { - $stubs[] = $this->getPackage('stub/stub', '1.0.0'); - } - - $repository - ->expects($this->once()) - ->method('createPackages') - ->with($this->identicalTo($expected), $this->equalTo('root file (http://example.org/packages.json)')) - ->will($this->returnValue($stubs)); - // Triggers initialization $packages = $repository->getPackages(); // Final sanity check, ensure the correct number of packages were added. $this->assertCount(count($expected), $packages); + + foreach ($expected as $index => $pkg) { + self::assertSame($pkg['name'].' '.$pkg['version'], $packages[$index]->getName().' '.$packages[$index]->getPrettyVersion()); + } } public function loadDataProvider() diff --git a/tests/Composer/Test/Util/GitTest.php b/tests/Composer/Test/Util/GitTest.php index 8e3928a62..9379d5b2e 100644 --- a/tests/Composer/Test/Util/GitTest.php +++ b/tests/Composer/Test/Util/GitTest.php @@ -49,9 +49,8 @@ class GitTest extends TestCase */ public function testRunCommandPublicGitHubRepositoryNotInitialClone($protocol, $expectedUrl) { - $that = $this; - $commandCallable = function ($url) use ($that, $expectedUrl) { - $that->assertSame($expectedUrl, $url); + $commandCallable = function ($url) use ($expectedUrl) { + $this->assertSame($expectedUrl, $url); return 'git command'; }; @@ -75,9 +74,8 @@ class GitTest extends TestCase { self::expectException('RuntimeException'); - $that = $this; - $commandCallable = function ($url) use ($that) { - $that->assertSame('https://github.com/acme/repo', $url); + $commandCallable = function ($url) { + $this->assertSame('https://github.com/acme/repo', $url); return 'git command'; };