bug symfony#10849 [WIP][Finder] Fix wrong implementation on sortable callback comparator (ProPheT777)

This PR was submitted for the master branch but it was merged into the 2.3 branch instead (closes symfony#10849).

Discussion
----------

[WIP][Finder] Fix wrong implementation on sortable callback comparator

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | N/A
| License       | MIT
| Doc PR        | no

Working on this issue, i find SORT_BY_ACCESSED_TIME, SORT_BY_CHANGED_TIME, SORT_BY_MODIFIED_TIME was not tested.

- [x] Add test for SORT_BY_ACCESSED_TIME case
- [x] Add test for SORT_BY_CHANGED_TIME case
- [x] add test for SORT_BY_MODIFIED_TIME case

Commits
-------

b965fa2 [WIP][Finder] Fix wrong implementation on sortable callback comparator

Author: fabpot

src/Symfony/Component/Finder/Iterator/SortableIterator.php

@@ -30,8 +30,8 @@ class SortableIterator implements \IteratorAggregate
     /**
      * Constructor.
      *
-     * @param \Traversable     $iterator The Iterator to filter
-     * @param int|callback     $sort     The sort type (SORT_BY_NAME, SORT_BY_TYPE, or a PHP callback)
+     * @param \Traversable $iterator The Iterator to filter
+     * @param int|callable $sort     The sort type (SORT_BY_NAME, SORT_BY_TYPE, or a PHP callback)
      *
      * @throws \InvalidArgumentException
      */
@@ -55,20 +55,20 @@ public function __construct(\Traversable $iterator, $sort)
             };
         } elseif (self::SORT_BY_ACCESSED_TIME === $sort) {
             $this->sort = function ($a, $b) {
-                return ($a->getATime() > $b->getATime());
+                return ($a->getATime() - $b->getATime());
             };
         } elseif (self::SORT_BY_CHANGED_TIME === $sort) {
             $this->sort = function ($a, $b) {
-                return ($a->getCTime() > $b->getCTime());
+                return ($a->getCTime() - $b->getCTime());
             };
         } elseif (self::SORT_BY_MODIFIED_TIME === $sort) {
             $this->sort = function ($a, $b) {
-                return ($a->getMTime() > $b->getMTime());
+                return ($a->getMTime() - $b->getMTime());
             };
         } elseif (is_callable($sort)) {
             $this->sort = $sort;
         } else {
-            throw new \InvalidArgumentException('The SortableIterator takes a PHP callback or a valid built-in sort algorithm as an argument.');
+            throw new \InvalidArgumentException('The SortableIterator takes a PHP callable or a valid built-in sort algorithm as an argument.');
         }
     }
 

src/Symfony/Component/Finder/Tests/Iterator/SortableIteratorTest.php

@@ -30,6 +30,26 @@ public function testConstructor()
      */
     public function testAccept($mode, $expected)
     {
+        if (!is_callable($mode)) {
+            switch ($mode) {
+                case SortableIterator::SORT_BY_ACCESSED_TIME :
+                    file_get_contents(self::toAbsolute('.git'));
+                    sleep(1);
+                    file_get_contents(self::toAbsolute('.bar'));
+                    break;
+                case SortableIterator::SORT_BY_CHANGED_TIME :
+                    file_put_contents(self::toAbsolute('test.php'), 'foo');
+                    sleep(1);
+                    file_put_contents(self::toAbsolute('test.py'), 'foo');
+                    break;
+                case SortableIterator::SORT_BY_MODIFIED_TIME :
+                    file_put_contents(self::toAbsolute('test.php'), 'foo');
+                    sleep(1);
+                    file_put_contents(self::toAbsolute('test.py'), 'foo');
+                    break;
+            }
+        }
+
         $inner = new Iterator(self::$files);
 
         $iterator = new SortableIterator($inner, $mode);
@@ -82,9 +102,54 @@ public function getAcceptData()
             'toto',
         );
 
+        $sortByAccessedTime = array(
+            'foo/bar.tmp',
+            'test.php',
+            'toto',
+            'foo bar',
+            'foo',
+            'test.py',
+            '.foo',
+            '.foo/.bar',
+            '.foo/bar',
+            '.git',
+            '.bar'
+        );
+
+        $sortByChangedTime = array(
+            'foo',
+            'foo/bar.tmp',
+            'toto',
+            '.git',
+            '.bar',
+            '.foo',
+            'foo bar',
+            '.foo/.bar',
+            '.foo/bar',
+            'test.php',
+            'test.py'
+        );
+
+        $sortByModifiedTime = array(
+            'foo/bar.tmp',
+            'foo',
+            'toto',
+            '.git',
+            '.bar',
+            '.foo',
+            'foo bar',
+            '.foo/.bar',
+            '.foo/bar',
+            'test.php',
+            'test.py'
+        );
+
         return array(
             array(SortableIterator::SORT_BY_NAME, $this->toAbsolute($sortByName)),
             array(SortableIterator::SORT_BY_TYPE, $this->toAbsolute($sortByType)),
+            array(SortableIterator::SORT_BY_ACCESSED_TIME, $this->toAbsolute($sortByAccessedTime)),
+            array(SortableIterator::SORT_BY_CHANGED_TIME, $this->toAbsolute($sortByChangedTime)),
+            array(SortableIterator::SORT_BY_MODIFIED_TIME, $this->toAbsolute($sortByModifiedTime)),
             array(function (\SplFileInfo $a, \SplFileInfo $b) { return strcmp($a->getRealpath(), $b->getRealpath()); }, $this->toAbsolute($customComparison)),
         );
     }