Merge pull request #8723 from helhum/fix-package-sorting

Fix package sorting

src/Composer/Util/PackageSorter.php

@@ -55,9 +55,9 @@ class PackageSorter
 
         $weightList = array();
 
-        foreach ($packages as $name => $package) {
+        foreach ($packages as $index => $package) {
-            $weight = $computeImportance($name);
+            $weight = $computeImportance($package->getName());
-            $weightList[$name] = $weight;
+            $weightList[$index] = $weight;
         }
 
         $stable_sort = function (&$array) {
@@ -84,8 +84,8 @@ class PackageSorter
 
         $sortedPackages = array();
 
-        foreach (array_keys($weightList) as $name) {
+        foreach (array_keys($weightList) as $index) {
-            $sortedPackages[] = $packages[$name];
+            $sortedPackages[] = $packages[$index];
         }
         return $sortedPackages;
     }

tests/Composer/Test/Util/PackageSorterTest.php

@@ -0,0 +1,129 @@
+<?php
+
+/*
+ * This file is part of Composer.
+ *
+ * (c) Nils Adermann <naderman@naderman.de>
+ *     Jordi Boggiano <j.boggiano@seld.be>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Composer\Test\Util;
+
+use Composer\Package\Link;
+use Composer\Package\Package;
+use Composer\Test\TestCase;
+use Composer\Util\PackageSorter;
+
+class PackageSorterTest extends TestCase
+{
+    public function testSortingDoesNothingWithNoDependencies()
+    {
+        $packages[] = $this->createPackage('foo/bar1', array());
+        $packages[] = $this->createPackage('foo/bar2', array());
+        $packages[] = $this->createPackage('foo/bar3', array());
+        $packages[] = $this->createPackage('foo/bar4', array());
+
+        $sortedPackages = PackageSorter::sortPackages($packages);
+
+        self::assertSame($packages, $sortedPackages);
+    }
+
+    public function sortingOrdersDependenciesHigherThanPackageDataProvider()
+    {
+        return array(
+            'one package is dep' => array(
+                array(
+                    $this->createPackage('foo/bar1', array('foo/bar4')),
+                    $this->createPackage('foo/bar2', array('foo/bar4')),
+                    $this->createPackage('foo/bar3', array('foo/bar4')),
+                    $this->createPackage('foo/bar4', array()),
+                ),
+                array(
+                    'foo/bar4',
+                    'foo/bar1',
+                    'foo/bar2',
+                    'foo/bar3',
+                ),
+            ),
+            'one package has more deps' => array(
+                array(
+                    $this->createPackage('foo/bar1', array('foo/bar2')),
+                    $this->createPackage('foo/bar2', array('foo/bar4')),
+                    $this->createPackage('foo/bar3', array('foo/bar4')),
+                    $this->createPackage('foo/bar4', array()),
+                ),
+                array(
+                    'foo/bar4',
+                    'foo/bar2',
+                    'foo/bar1',
+                    'foo/bar3',
+                ),
+            ),
+            'package is required by many, but requires one other' => array(
+                array(
+                    $this->createPackage('foo/bar1', array('foo/bar3')),
+                    $this->createPackage('foo/bar2', array('foo/bar3')),
+                    $this->createPackage('foo/bar3', array('foo/bar4')),
+                    $this->createPackage('foo/bar4', array()),
+                    $this->createPackage('foo/bar5', array('foo/bar3')),
+                    $this->createPackage('foo/bar6', array('foo/bar3')),
+                ),
+                array(
+                    'foo/bar4',
+                    'foo/bar3',
+                    'foo/bar1',
+                    'foo/bar2',
+                    'foo/bar5',
+                    'foo/bar6',
+                ),
+            ),
+            'one package has many requires' => array(
+                array(
+                    $this->createPackage('foo/bar1', array('foo/bar2')),
+                    $this->createPackage('foo/bar2', array()),
+                    $this->createPackage('foo/bar3', array('foo/bar4')),
+                    $this->createPackage('foo/bar4', array()),
+                    $this->createPackage('foo/bar5', array('foo/bar2')),
+                    $this->createPackage('foo/bar6', array('foo/bar2')),
+                ),
+                array(
+                    'foo/bar2',
+                    'foo/bar4',
+                    'foo/bar1',
+                    'foo/bar3',
+                    'foo/bar5',
+                    'foo/bar6',
+                ),
+            ),
+        );
+    }
+
+    /**
+     * @dataProvider sortingOrdersDependenciesHigherThanPackageDataProvider
+     * @param array $packages
+     * @param array $expectedOrderedList
+     */
+    public function testSortingOrdersDependenciesHigherThanPackage($packages, $expectedOrderedList)
+    {
+        $sortedPackages = PackageSorter::sortPackages($packages);
+        $sortedPackageNames = array_map(function ($package) { return $package->getName(); }, $sortedPackages);
+
+        self::assertSame($expectedOrderedList, $sortedPackageNames);
+    }
+
+    private function createPackage($name, $requires)
+    {
+        $package = new Package($name, '1.0.0.0', '1.0.0');
+
+        $links = array();
+        foreach ($requires as $requireName) {
+            $links[] = new Link($package->getName(), $requireName);
+        }
+        $package->setRequires($links);
+
+        return $package;
+    }
+}