Fix handling of dev versions and consolidate logic, refs #7119

src/Composer/Downloader/FileDownloader.php

@@ -19,6 +19,7 @@ use Composer\IO\IOInterface;
 use Composer\IO\NullIO;
 use Composer\Package\Comparer\Comparer;
 use Composer\Package\PackageInterface;
+use Composer\Package\Version\VersionParser;
 use Composer\Plugin\PluginEvents;
 use Composer\Plugin\PreFileDownloadEvent;
 use Composer\EventDispatcher\EventDispatcher;
@@ -218,7 +219,7 @@ class FileDownloader implements DownloaderInterface
         $from = $initial->getPrettyVersion();
         $to = $target->getPrettyVersion();
 
-        $actionName = version_compare($from, $to, '<') ? 'Updating' : 'Downgrading';
+        $actionName = VersionParser::isUpgrade($initial->getVersion(), $target->getVersion()) ? 'Updating' : 'Downgrading';
         $this->io->writeError("  - " . $actionName . " <info>" . $name . "</info> (<comment>" . $from . "</comment> => <comment>" . $to . "</comment>): ", false);
 
         $this->remove($initial, $path, false);

src/Composer/Downloader/VcsDownloader.php

@@ -130,7 +130,7 @@ abstract class VcsDownloader implements DownloaderInterface, ChangeReportInterfa
             $to = $target->getFullPrettyVersion();
         }
 
-        $actionName = $this->packageCompare($initial, $target, '>') ? 'Downgrading' : 'Updating';
+        $actionName = VersionParser::isUpgrade($initial->getVersion(), $target->getVersion()) ? 'Updating' : 'Downgrading';
         $this->io->writeError("  - " . $actionName . " <info>" . $name . "</info> (<comment>" . $from . "</comment> => <comment>" . $to . "</comment>): ", false);
 
         $this->cleanChanges($initial, $path, true);
@@ -243,23 +243,6 @@ abstract class VcsDownloader implements DownloaderInterface, ChangeReportInterfa
         }
     }
 
-    /**
-     * Compare two packages. Always false if both versions are references
-     *
-     * @param  PackageInterface $initial
-     * @param  PackageInterface $target
-     * @param  string           $operation
-     * @return bool
-     */
-    protected function packageCompare(PackageInterface $initial, PackageInterface $target, $operation = '<')
-    {
-        if ($initial->getPrettyVersion() == $target->getPrettyVersion()) {
-            return false;
-        }
-
-        return version_compare($initial->getFullPrettyVersion(), $target->getFullPrettyVersion(), $operation);
-    }
-
     /**
      * Guarantee that no changes have been made to the local copy
      *

src/Composer/Package/Version/VersionParser.php

@@ -14,6 +14,7 @@ namespace Composer\Package\Version;
 
 use Composer\Repository\PlatformRepository;
 use Composer\Semver\VersionParser as SemverVersionParser;
+use Composer\Semver\Semver;
 
 class VersionParser extends SemverVersionParser
 {
@@ -63,4 +64,14 @@ class VersionParser extends SemverVersionParser
 
         return $result;
     }
+
+    /**
+     * @return bool
+     */
+    public static function isUpgrade($normalizedFrom, $normalizedTo)
+    {
+        $sorted = Semver::sort(array($normalizedTo, $normalizedFrom));
+
+        return $sorted[0] === $normalizedFrom;
+    }
 }

tests/Composer/Test/Downloader/FileDownloaderTest.php

@@ -211,18 +211,24 @@ class FileDownloaderTest extends TestCase
         $oldPackage = $this->getMock('Composer\Package\PackageInterface');
         $oldPackage->expects($this->once())
             ->method('getPrettyVersion')
-            ->will($this->returnValue('1.0.0'));
-        $oldPackage->expects($this->any())
-            ->method('getDistUrl')
-            ->will($this->returnValue($distUrl = 'http://example.com/script.js'));
+            ->will($this->returnValue('1.2.0'));
         $oldPackage->expects($this->once())
-            ->method('getDistUrls')
-            ->will($this->returnValue(array($distUrl)));
+            ->method('getVersion')
+            ->will($this->returnValue('1.2.0.0'));
 
         $newPackage = $this->getMock('Composer\Package\PackageInterface');
         $newPackage->expects($this->once())
             ->method('getPrettyVersion')
-            ->will($this->returnValue('1.2.0'));
+            ->will($this->returnValue('1.0.0'));
+        $newPackage->expects($this->once())
+            ->method('getVersion')
+            ->will($this->returnValue('1.0.0.0'));
+        $newPackage->expects($this->any())
+            ->method('getDistUrl')
+            ->will($this->returnValue($distUrl = 'http://example.com/script.js'));
+        $newPackage->expects($this->once())
+            ->method('getDistUrls')
+            ->will($this->returnValue(array($distUrl)));
 
         $ioMock = $this->getMock('Composer\IO\IOInterface');
         $ioMock->expects(($this->at(0)))
@@ -237,6 +243,6 @@ class FileDownloaderTest extends TestCase
             ->will($this->returnValue(true));
 
         $downloader = $this->getDownloader($ioMock, null, null, null, null, $filesystem);
-        $downloader->update($newPackage, $oldPackage, $path);
+        $downloader->update($oldPackage, $newPackage, $path);
     }
 }

tests/Composer/Test/Downloader/FossilDownloaderTest.php

@@ -123,6 +123,9 @@ class FossilDownloaderTest extends TestCase
         $packageMock->expects($this->any())
             ->method('getSourceUrls')
             ->will($this->returnValue(array('http://fossil.kd2.org/kd2fw/')));
+        $packageMock->expects($this->any())
+            ->method('getVersion')
+            ->will($this->returnValue('1.0.0.0'));
         $processExecutor = $this->getMockBuilder('Composer\Util\ProcessExecutor')->getMock();
 
         $expectedFossilCommand = $this->getCmd("fossil changes");

tests/Composer/Test/Downloader/GitDownloaderTest.php

@@ -390,8 +390,8 @@ class GitDownloaderTest extends TestCase
             ->method('getSourceUrls')
             ->will($this->returnValue(array('https://github.com/composer/composer')));
         $packageMock->expects($this->any())
-            ->method('getPrettyVersion')
-            ->will($this->returnValue('1.0.0'));
+            ->method('getVersion')
+            ->will($this->returnValue('1.0.0.0'));
         $processExecutor = $this->getMockBuilder('Composer\Util\ProcessExecutor')->getMock();
         $processExecutor->expects($this->at(0))
             ->method('execute')
@@ -442,8 +442,8 @@ class GitDownloaderTest extends TestCase
             ->method('getSourceUrl')
             ->will($this->returnValue('https://github.com/composer/composer'));
         $packageMock->expects($this->any())
-            ->method('getPrettyVersion')
-            ->will($this->returnValue('1.0.0'));
+            ->method('getVersion')
+            ->will($this->returnValue('1.0.0.0'));
         $processExecutor = $this->getMockBuilder('Composer\Util\ProcessExecutor')->getMock();
         $processExecutor->expects($this->at(0))
             ->method('execute')
@@ -510,6 +510,9 @@ composer https://github.com/old/url (push)
         $packageMock->expects($this->any())
             ->method('getSourceUrls')
             ->will($this->returnValue(array('https://github.com/composer/composer')));
+        $packageMock->expects($this->any())
+            ->method('getVersion')
+            ->will($this->returnValue('1.0.0.0'));
         $processExecutor = $this->getMockBuilder('Composer\Util\ProcessExecutor')->getMock();
         $processExecutor->expects($this->at(0))
             ->method('execute')
@@ -546,6 +549,9 @@ composer https://github.com/old/url (push)
         $packageMock->expects($this->any())
             ->method('getSourceReference')
             ->will($this->returnValue('ref'));
+        $packageMock->expects($this->any())
+            ->method('getVersion')
+            ->will($this->returnValue('1.0.0.0'));
         $packageMock->expects($this->any())
             ->method('getSourceUrls')
             ->will($this->returnValue(array('/foo/bar', 'https://github.com/composer/composer')));
@@ -600,11 +606,11 @@ composer https://github.com/old/url (push)
     {
         $oldPackage = $this->getMock('Composer\Package\PackageInterface');
         $oldPackage->expects($this->any())
-            ->method('getPrettyVersion')
-            ->will($this->returnValue('1.0.0'));
+            ->method('getVersion')
+            ->will($this->returnValue('1.2.0.0'));
         $oldPackage->expects($this->any())
             ->method('getFullPrettyVersion')
-            ->will($this->returnValue('1.0.0'));
+            ->will($this->returnValue('1.2.0'));
         $oldPackage->expects($this->any())
             ->method('getSourceReference')
             ->will($this->returnValue('ref'));
@@ -620,11 +626,11 @@ composer https://github.com/old/url (push)
             ->method('getSourceUrls')
             ->will($this->returnValue(array('https://github.com/composer/composer')));
         $newPackage->expects($this->any())
-            ->method('getPrettyVersion')
-            ->will($this->returnValue('1.2.0'));
+            ->method('getVersion')
+            ->will($this->returnValue('1.0.0.0'));
         $newPackage->expects($this->any())
             ->method('getFullPrettyVersion')
-            ->will($this->returnValue('1.2.0'));
+            ->will($this->returnValue('1.0.0'));
 
         $processExecutor = $this->getMock('Composer\Util\ProcessExecutor');
         $processExecutor->expects($this->any())
@@ -638,15 +644,15 @@ composer https://github.com/old/url (push)
 
         $this->fs->ensureDirectoryExists($this->workingDir.'/.git');
         $downloader = $this->getDownloaderMock($ioMock, null, $processExecutor);
-        $downloader->update($newPackage, $oldPackage, $this->workingDir);
+        $downloader->update($oldPackage, $newPackage, $this->workingDir);
     }
 
     public function testNotUsingDowngradingWithReferences()
     {
         $oldPackage = $this->getMock('Composer\Package\PackageInterface');
         $oldPackage->expects($this->any())
-            ->method('getPrettyVersion')
-            ->will($this->returnValue('ref'));
+            ->method('getVersion')
+            ->will($this->returnValue('dev-ref'));
         $oldPackage->expects($this->any())
             ->method('getSourceReference')
             ->will($this->returnValue('ref'));
@@ -662,8 +668,8 @@ composer https://github.com/old/url (push)
             ->method('getSourceUrls')
             ->will($this->returnValue(array('https://github.com/composer/composer')));
         $newPackage->expects($this->any())
-            ->method('getPrettyVersion')
-            ->will($this->returnValue('ref'));
+            ->method('getVersion')
+            ->will($this->returnValue('dev-ref2'));
 
         $processExecutor = $this->getMock('Composer\Util\ProcessExecutor');
         $processExecutor->expects($this->any())
@@ -677,7 +683,7 @@ composer https://github.com/old/url (push)
 
         $this->fs->ensureDirectoryExists($this->workingDir.'/.git');
         $downloader = $this->getDownloaderMock($ioMock, null, $processExecutor);
-        $downloader->update($newPackage, $oldPackage, $this->workingDir);
+        $downloader->update($oldPackage, $newPackage, $this->workingDir);
     }
 
     public function testRemove()

tests/Composer/Test/Downloader/HgDownloaderTest.php

@@ -109,6 +109,9 @@ class HgDownloaderTest extends TestCase
         $packageMock->expects($this->any())
             ->method('getSourceReference')
             ->will($this->returnValue('ref'));
+        $packageMock->expects($this->any())
+            ->method('getVersion')
+            ->will($this->returnValue('1.0.0.0'));
         $packageMock->expects($this->any())
             ->method('getSourceUrls')
             ->will($this->returnValue(array('https://github.com/l3l0/composer')));

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

@@ -35,4 +35,26 @@ class VersionParserTest extends TestCase
             array(array('php', 'ext-apcu'), array(array('name' => 'php'), array('name' => 'ext-apcu'))),
         );
     }
+
+    /**
+     * @dataProvider getIsUpgradeTests
+     */
+    public function testIsUpgrade($from, $to, $expected)
+    {
+        $this->assertSame($expected, VersionParser::isUpgrade($from, $to));
+    }
+
+    public function getIsUpgradeTests()
+    {
+        return array(
+            array('0.9.0.0', '1.0.0.0', true),
+            array('1.0.0.0', '0.9.0.0', false),
+            array('1.0.0.0', '9999999-dev', true),
+            array('9999999-dev', '9999999-dev', true),
+            array('9999999-dev', '1.0.0.0', false),
+            array('1.0.0.0', 'dev-foo', true),
+            array('dev-foo', 'dev-foo', true),
+            array('dev-foo', '1.0.0.0', true),
+        );
+    }
 }