Fix package selector warnings to only show for versions that were actually skipped, #10942

src/Composer/Command/RequireCommand.php

@@ -240,7 +240,7 @@ EOT
         if (count($inconsistentRequireKeys) > 0) {
             foreach ($inconsistentRequireKeys as $package) {
                 $io->warning(sprintf(
-                    '%s is currently present in the %s key and you ran the command %s the --dev flag, which would move it to the %s key.',
+                    '%s is currently present in the %s key and you ran the command %s the --dev flag, which will move it to the %s key.',
                     $package,
                     $removeKey,
                     $input->getOption('dev') ? 'with' : 'without',

src/Composer/Package/Version/VersionSelector.php

@@ -86,86 +86,84 @@ class VersionSelector
 
         $constraint = $targetPackageVersion ? $this->getParser()->parseConstraints($targetPackageVersion) : null;
         $candidates = $this->repositorySet->findPackages(strtolower($packageName), $constraint, $repoSetFlags);
-        $skippedWarnings = [];
 
-        if ($this->platformConstraints && !($platformRequirementFilter instanceof IgnoreAllPlatformRequirementFilter)) {
+        $minPriority = BasePackage::$stabilities[$preferredStability];
-            $platformConstraints = $this->platformConstraints;
+        usort($candidates, function (PackageInterface $a, PackageInterface $b) use ($minPriority) {
-            $candidates = array_filter($candidates, static function ($pkg) use ($platformConstraints, $platformRequirementFilter, &$skippedWarnings): bool {
+            $aPriority = $a->getStabilityPriority();
+            $bPriority = $b->getStabilityPriority();
+
+            // A is less stable than our preferred stability,
+            // and B is more stable than A, select B
+            if ($minPriority < $aPriority && $bPriority < $aPriority) {
+                return 1;
+            }
+
+            // A is less stable than our preferred stability,
+            // and B is less stable than A, select A
+            if ($minPriority < $aPriority && $aPriority < $bPriority) {
+                return -1;
+            }
+
+            // A is more stable than our preferred stability,
+            // and B is less stable than preferred stability, select A
+            if ($minPriority >= $aPriority && $minPriority < $bPriority) {
+                return -1;
+            }
+
+            // select highest version of the two
+            return version_compare($b->getVersion(), $a->getVersion());
+        });
+
+        if (count($this->platformConstraints) > 0 && !($platformRequirementFilter instanceof IgnoreAllPlatformRequirementFilter)) {
+            /** @var array<string, true> $alreadyWarnedNames */
+            $alreadyWarnedNames = [];
+
+            foreach ($candidates as $pkg) {
                 $reqs = $pkg->getRequires();
-
                 foreach ($reqs as $name => $link) {
-                    if (!$platformRequirementFilter->isIgnored($name)) {
+                    if (!PlatformRepository::isPlatformPackage($name) || $platformRequirementFilter->isIgnored($name)) {
-                        if (isset($platformConstraints[$name])) {
+                        continue;
-                            foreach ($platformConstraints[$name] as $constraint) {
+                    }
-                                if ($link->getConstraint()->matches($constraint)) {
+                    if (isset($this->platformConstraints[$name])) {
+                        foreach ($this->platformConstraints[$name] as $providedConstraint) {
+                            if ($link->getConstraint()->matches($providedConstraint)) {
+                                // constraint satisfied, go to next require
                                 continue 2;
                             }
                         }
 
-                            $skippedWarnings[$pkg->getName()][] = ['version' => $pkg->getPrettyVersion(), 'link' => $link, 'reason' => 'is not satisfied by your platform'];
+                        // constraint not satisfied
-                            return false;
+                        $reason = 'is not satisfied by your platform';
-                        } elseif (PlatformRepository::isPlatformPackage($name)) {
+                    } else {
                         // Package requires a platform package that is unknown on current platform.
                         // It means that current platform cannot validate this constraint and so package is not installable.
-                            $skippedWarnings[$pkg->getName()][] = ['version' => $pkg->getPrettyVersion(), 'link' => $link, 'reason' => 'is missing from your platform'];
+                        $reason = 'is missing from your platform';
-                            return false;
-                        }
-                    }
                     }
 
-                return true;
+                    if ($io !== null) {
-            });
+                        $isFirst = !isset($alreadyWarnedNames[$pkg->getName()]);
-        }
+                        $alreadyWarnedNames[$pkg->getName()] = true;
-
+                        $latest = $isFirst ? "'s latest version" : '';
-        if (!$candidates) {
-            return false;
-        }
-
-        if (count($skippedWarnings) > 0 && $io !== null) {
-            foreach ($skippedWarnings as $name => $warnings) {
-                foreach ($warnings as $index => $warning) {
-                    $link = $warning['link'];
-                    $latest = $index === 0 ? "'s latest version" : '';
                         $io->writeError(
-                        '<warning>Cannot use '.$name.$latest.' '.$warning['version'].' as it '.$link->getDescription().' '.$link->getTarget().' '.$link->getPrettyConstraint().' which '.$warning['reason'].'.</>',
+                            '<warning>Cannot use '.$pkg->getPrettyName().$latest.' '.$pkg->getPrettyVersion().' as it '.$link->getDescription().' '.$link->getTarget().' '.$link->getPrettyConstraint().' which '.$reason.'.</>',
                             true,
-                        $index === 0 ? IOInterface::NORMAL : IOInterface::VERBOSE
+                            $isFirst ? IOInterface::NORMAL : IOInterface::VERBOSE
                         );
                     }
-            }
+
+                    // skip candidate
+                    continue 2;
                 }
 
-        // select highest version if we have many
+                $package = $pkg;
-        $package = reset($candidates);
+                break;
-        $minPriority = BasePackage::$stabilities[$preferredStability];
+            }
-        foreach ($candidates as $candidate) {
+        } else {
-            $candidatePriority = $candidate->getStabilityPriority();
+            $package = count($candidates) > 0 ? $candidates[0] : null;
-            $currentPriority = $package->getStabilityPriority();
-
-            // candidate is less stable than our preferred stability,
-            // and current package is more stable than candidate, skip it
-            if ($minPriority < $candidatePriority && $currentPriority < $candidatePriority) {
-                continue;
         }
 
-            // candidate is less stable than our preferred stability,
+        if (!isset($package)) {
-            // and current package is less stable than candidate, select candidate
+            return false;
-            if ($minPriority < $candidatePriority && $candidatePriority < $currentPriority) {
-                $package = $candidate;
-                continue;
-            }
-
-            // candidate is more stable than our preferred stability,
-            // and current package is less stable than preferred stability, select candidate
-            if ($minPriority >= $candidatePriority && $minPriority < $currentPriority) {
-                $package = $candidate;
-                continue;
-            }
-
-            // select highest version of the two
-            if (version_compare($package->getVersion(), $candidate->getVersion(), '<')) {
-                $package = $candidate;
-            }
         }
 
         // if we end up with 9999999-dev as selected package, make sure we use the original version instead of the alias

tests/Composer/Test/Package/Version/VersionSelectorTest.php

@@ -13,6 +13,8 @@
 namespace Composer\Test\Package\Version;
 
 use Composer\Filter\PlatformRequirementFilter\PlatformRequirementFilterFactory;
+use Composer\IO\BufferIO;
+use Composer\IO\IOInterface;
 use Composer\Package\Version\VersionSelector;
 use Composer\Package\Package;
 use Composer\Package\Link;
@@ -20,6 +22,7 @@ use Composer\Package\AliasPackage;
 use Composer\Repository\PlatformRepository;
 use Composer\Package\Version\VersionParser;
 use Composer\Test\TestCase;
+use Symfony\Component\Console\Output\StreamOutput;
 
 class VersionSelectorTest extends TestCase
 {
@@ -29,11 +32,11 @@ class VersionSelectorTest extends TestCase
 
     public function testLatestVersionIsReturned(): void
     {
-        $packageName = 'foobar';
+        $packageName = 'foo/bar';
 
-        $package1 = $this->createPackage('1.2.1');
+        $package1 = $this->getPackage('foo/bar', '1.2.1');
-        $package2 = $this->createPackage('1.2.2');
+        $package2 = $this->getPackage('foo/bar', '1.2.2');
-        $package3 = $this->createPackage('1.2.0');
+        $package3 = $this->getPackage('foo/bar', '1.2.0');
         $packages = array($package1, $package2, $package3);
 
         $repositorySet = $this->createMockRepositorySet();
@@ -51,42 +54,56 @@ class VersionSelectorTest extends TestCase
 
     public function testLatestVersionIsReturnedThatMatchesPhpRequirements(): void
     {
-        $packageName = 'foobar';
+        $packageName = 'foo/bar';
 
         $platform = new PlatformRepository(array(), array('php' => '5.5.0'));
         $repositorySet = $this->createMockRepositorySet();
         $versionSelector = new VersionSelector($repositorySet, $platform);
 
         $parser = new VersionParser;
-        $package1 = $this->createPackage('1.0.0');
+        $package1 = $this->getPackage('foo/bar', '1.0.0');
         $package1->setRequires(array('php' => new Link($packageName, 'php', $parser->parseConstraints('>=5.4'), Link::TYPE_REQUIRE, '>=5.4')));
-        $package2 = $this->createPackage('2.0.0');
+        $package2 = $this->getPackage('foo/bar', '2.0.0');
         $package2->setRequires(array('php' => new Link($packageName, 'php', $parser->parseConstraints('>=5.6'), Link::TYPE_REQUIRE, '>=5.6')));
-        $packages = array($package1, $package2);
+        $package3 = $this->getPackage('foo/bar', '2.1.0');
+        $package3->setRequires(array('php' => new Link($packageName, 'php', $parser->parseConstraints('>=5.6'), Link::TYPE_REQUIRE, '>=5.6')));
+        $packages = array($package1, $package2, $package3);
 
         $repositorySet->expects($this->any())
             ->method('findPackages')
             ->with($packageName, null)
             ->will($this->returnValue($packages));
 
-        $best = $versionSelector->findBestCandidate($packageName);
+        $io = new BufferIO();
-        $this->assertSame($package1, $best, 'Latest version supporting php 5.5 should be returned (1.0.0)');
+        $best = $versionSelector->findBestCandidate($packageName, null, 'stable', null, 0, $io);
+        $this->assertSame((string) $package1, (string) $best, 'Latest version supporting php 5.5 should be returned (1.0.0)');
+        self::assertSame("<warning>Cannot use foo/bar's latest version 2.1.0 as it requires php >=5.6 which is not satisfied by your platform.".PHP_EOL, $io->getOutput());
+
+        $io = new BufferIO('', StreamOutput::VERBOSITY_VERBOSE);
+        $best = $versionSelector->findBestCandidate($packageName, null, 'stable', null, 0, $io);
+        $this->assertSame((string) $package1, (string) $best, 'Latest version supporting php 5.5 should be returned (1.0.0)');
+        self::assertSame(
+            "<warning>Cannot use foo/bar's latest version 2.1.0 as it requires php >=5.6 which is not satisfied by your platform.".PHP_EOL
+            ."<warning>Cannot use foo/bar 2.0.0 as it requires php >=5.6 which is not satisfied by your platform.".PHP_EOL,
+            $io->getOutput()
+        );
+
         $best = $versionSelector->findBestCandidate($packageName, null, 'stable', PlatformRequirementFilterFactory::ignoreAll());
-        $this->assertSame($package2, $best, 'Latest version should be returned when ignoring platform reqs (2.0.0)');
+        $this->assertSame((string) $package3, (string) $best, 'Latest version should be returned when ignoring platform reqs (2.1.0)');
     }
 
     public function testLatestVersionIsReturnedThatMatchesExtRequirements(): void
     {
-        $packageName = 'foobar';
+        $packageName = 'foo/bar';
 
         $platform = new PlatformRepository(array(), array('ext-zip' => '5.3.0'));
         $repositorySet = $this->createMockRepositorySet();
         $versionSelector = new VersionSelector($repositorySet, $platform);
 
         $parser = new VersionParser;
-        $package1 = $this->createPackage('1.0.0');
+        $package1 = $this->getPackage('foo/bar', '1.0.0');
         $package1->setRequires(array('ext-zip' => new Link($packageName, 'ext-zip', $parser->parseConstraints('^5.2'), Link::TYPE_REQUIRE, '^5.2')));
-        $package2 = $this->createPackage('2.0.0');
+        $package2 = $this->getPackage('foo/bar', '2.0.0');
         $package2->setRequires(array('ext-zip' => new Link($packageName, 'ext-zip', $parser->parseConstraints('^5.4'), Link::TYPE_REQUIRE, '^5.4')));
         $packages = array($package1, $package2);
 
@@ -103,15 +120,15 @@ class VersionSelectorTest extends TestCase
 
     public function testLatestVersionIsReturnedThatMatchesPlatformExt(): void
     {
-        $packageName = 'foobar';
+        $packageName = 'foo/bar';
 
         $platform = new PlatformRepository();
         $repositorySet = $this->createMockRepositorySet();
         $versionSelector = new VersionSelector($repositorySet, $platform);
 
         $parser = new VersionParser;
-        $package1 = $this->createPackage('1.0.0');
+        $package1 = $this->getPackage('foo/bar', '1.0.0');
-        $package2 = $this->createPackage('2.0.0');
+        $package2 = $this->getPackage('foo/bar', '2.0.0');
         $package2->setRequires(array('ext-barfoo' => new Link($packageName, 'ext-barfoo', $parser->parseConstraints('*'), Link::TYPE_REQUIRE, '*')));
         $packages = array($package1, $package2);
 
@@ -128,16 +145,16 @@ class VersionSelectorTest extends TestCase
 
     public function testLatestVersionIsReturnedThatMatchesComposerRequirements(): void
     {
-        $packageName = 'foobar';
+        $packageName = 'foo/bar';
 
         $platform = new PlatformRepository(array(), array('composer-runtime-api' => '1.0.0'));
         $repositorySet = $this->createMockRepositorySet();
         $versionSelector = new VersionSelector($repositorySet, $platform);
 
         $parser = new VersionParser;
-        $package1 = $this->createPackage('1.0.0');
+        $package1 = $this->getPackage('foo/bar', '1.0.0');
         $package1->setRequires(array('composer-runtime-api' => new Link($packageName, 'composer-runtime-api', $parser->parseConstraints('^1.0'), Link::TYPE_REQUIRE, '^1.0')));
-        $package2 = $this->createPackage('1.1.0');
+        $package2 = $this->getPackage('foo/bar', '1.1.0');
         $package2->setRequires(array('composer-runtime-api' => new Link($packageName, 'composer-runtime-api', $parser->parseConstraints('^2.0'), Link::TYPE_REQUIRE, '^2.0')));
         $packages = array($package1, $package2);
 
@@ -154,10 +171,10 @@ class VersionSelectorTest extends TestCase
 
     public function testMostStableVersionIsReturned(): void
     {
-        $packageName = 'foobar';
+        $packageName = 'foo/bar';
 
-        $package1 = $this->createPackage('1.0.0');
+        $package1 = $this->getPackage('foo/bar', '1.0.0');
-        $package2 = $this->createPackage('1.1.0-beta');
+        $package2 = $this->getPackage('foo/bar', '1.1.0-beta');
         $packages = array($package1, $package2);
 
         $repositorySet = $this->createMockRepositorySet();
@@ -174,10 +191,10 @@ class VersionSelectorTest extends TestCase
 
     public function testMostStableVersionIsReturnedRegardlessOfOrder(): void
     {
-        $packageName = 'foobar';
+        $packageName = 'foo/bar';
 
-        $package1 = $this->createPackage('2.x-dev');
+        $package1 = $this->getPackage('foo/bar', '2.x-dev');
-        $package2 = $this->createPackage('2.0.0-beta3');
+        $package2 = $this->getPackage('foo/bar', '2.0.0-beta3');
         $packages = array($package1, $package2);
 
         $repositorySet = $this->createMockRepositorySet();
@@ -199,10 +216,10 @@ class VersionSelectorTest extends TestCase
 
     public function testHighestVersionIsReturned(): void
     {
-        $packageName = 'foobar';
+        $packageName = 'foo/bar';
 
-        $package1 = $this->createPackage('1.0.0');
+        $package1 = $this->getPackage('foo/bar', '1.0.0');
-        $package2 = $this->createPackage('1.1.0-beta');
+        $package2 = $this->getPackage('foo/bar', '1.1.0-beta');
         $packages = array($package1, $package2);
 
         $repositorySet = $this->createMockRepositorySet();
@@ -219,11 +236,11 @@ class VersionSelectorTest extends TestCase
 
     public function testHighestVersionMatchingStabilityIsReturned(): void
     {
-        $packageName = 'foobar';
+        $packageName = 'foo/bar';
 
-        $package1 = $this->createPackage('1.0.0');
+        $package1 = $this->getPackage('foo/bar', '1.0.0');
-        $package2 = $this->createPackage('1.1.0-beta');
+        $package2 = $this->getPackage('foo/bar', '1.1.0-beta');
-        $package3 = $this->createPackage('1.2.0-alpha');
+        $package3 = $this->getPackage('foo/bar', '1.2.0-alpha');
         $packages = array($package1, $package2, $package3);
 
         $repositorySet = $this->createMockRepositorySet();
@@ -240,10 +257,10 @@ class VersionSelectorTest extends TestCase
 
     public function testMostStableUnstableVersionIsReturned(): void
     {
-        $packageName = 'foobar';
+        $packageName = 'foo/bar';
 
-        $package2 = $this->createPackage('1.1.0-beta');
+        $package2 = $this->getPackage('foo/bar', '1.1.0-beta');
-        $package3 = $this->createPackage('1.2.0-alpha');
+        $package3 = $this->getPackage('foo/bar', '1.2.0-alpha');
         $packages = array($package2, $package3);
 
         $repositorySet = $this->createMockRepositorySet();
@@ -260,10 +277,10 @@ class VersionSelectorTest extends TestCase
 
     public function testDefaultBranchAliasIsNeverReturned(): void
     {
-        $packageName = 'foobar';
+        $packageName = 'foo/bar';
 
-        $package = $this->createPackage('1.1.0-beta');
+        $package = $this->getPackage('foo/bar', '1.1.0-beta');
-        $package2 = $this->createPackage('dev-main');
+        $package2 = $this->getPackage('foo/bar', 'dev-main');
         $package2Alias = new AliasPackage($package2, VersionParser::DEFAULT_BRANCH_ALIAS, VersionParser::DEFAULT_BRANCH_ALIAS);
         $packages = array($package, $package2Alias);
 
@@ -357,18 +374,6 @@ class VersionSelectorTest extends TestCase
         );
     }
 
-    /**
-     * @param string $version
-     *
-     * @return Package
-     */
-    private function createPackage(string $version): Package
-    {
-        $parser = new VersionParser();
-
-        return new Package('foo', $parser->normalize($version), $version);
-    }
-
     /**
      * @return \PHPUnit\Framework\MockObject\MockObject&\Composer\Repository\RepositorySet
      */