bug #39703 [Finder] apply the sort callback on the whole search result (xabbuh)

fabpot · fabpot · commit 1dead9a51897 · 2021-01-05T07:04:52.000+01:00
This PR was merged into the 4.4 branch. Discussion ---------- [Finder] apply the sort callback on the whole search result | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #34879 | License | MIT | Doc PR | Commits ------- 1261051 apply the sort callback on the whole search result
diff --git a/src/Symfony/Component/Finder/Finder.php b/src/Symfony/Component/Finder/Finder.php
@@ -624,7 +624,13 @@ public function getIterator()
         }
 
         if (1 === \count($this->dirs) && 0 === \count($this->iterators)) {
-            return $this->searchInDirectory($this->dirs[0]);
+            $iterator = $this->searchInDirectory($this->dirs[0]);
+
+            if ($this->sort || $this->reverseSorting) {
+                $iterator = (new Iterator\SortableIterator($iterator, $this->sort, $this->reverseSorting))->getIterator();
+            }
+
+            return $iterator;
         }
 
         $iterator = new \AppendIterator();
@@ -636,6 +642,10 @@ public function getIterator()
             $iterator->append($it);
         }
 
+        if ($this->sort || $this->reverseSorting) {
+            $iterator = (new Iterator\SortableIterator($iterator, $this->sort, $this->reverseSorting))->getIterator();
+        }
+
         return $iterator;
     }
 
@@ -782,11 +792,6 @@ private function searchInDirectory(string $dir): \Iterator
             $iterator = new Iterator\PathFilterIterator($iterator, $this->paths, $notPaths);
         }
 
-        if ($this->sort || $this->reverseSorting) {
-            $iteratorAggregate = new Iterator\SortableIterator($iterator, $this->sort, $this->reverseSorting);
-            $iterator = $iteratorAggregate->getIterator();
-        }
-
         return $iterator;
     }
 
diff --git a/src/Symfony/Component/Finder/Tests/FinderTest.php b/src/Symfony/Component/Finder/Tests/FinderTest.php
@@ -832,6 +832,39 @@ public function testSort()
         ]), $finder->in(self::$tmpDir)->getIterator());
     }
 
+    public function testSortAcrossDirectories()
+    {
+        $finder = $this->buildFinder()
+            ->in([
+                self::$tmpDir,
+                self::$tmpDir.'/qux',
+                self::$tmpDir.'/foo',
+            ])
+            ->depth(0)
+            ->files()
+            ->filter(static function (\SplFileInfo $file): bool {
+                return '' !== $file->getExtension();
+            })
+            ->sort(static function (\SplFileInfo $a, \SplFileInfo $b): int {
+                return strcmp($a->getExtension(), $b->getExtension()) ?: strcmp($a->getFilename(), $b->getFilename());
+            })
+        ;
+
+        $this->assertOrderedIterator($this->toAbsolute([
+            'qux_0_1.php',
+            'qux_1000_1.php',
+            'qux_1002_0.php',
+            'qux_10_2.php',
+            'qux_12_0.php',
+            'qux_2_0.php',
+            'test.php',
+            'qux/baz_100_1.py',
+            'qux/baz_1_2.py',
+            'test.py',
+            'foo/bar.tmp',
+        ]), $finder->getIterator());
+    }
+
     public function testFilter()
     {
         $finder = $this->buildFinder();
diff --git a/src/Symfony/Component/Finder/Tests/Iterator/IteratorTestCase.php b/src/Symfony/Component/Finder/Tests/Iterator/IteratorTestCase.php
@@ -31,7 +31,8 @@ protected function assertIterator($expected, \Traversable $iterator)
 
     protected function assertOrderedIterator($expected, \Traversable $iterator)
     {
-        $values = array_map(function (\SplFileInfo $fileinfo) { return $fileinfo->getPathname(); }, iterator_to_array($iterator));
+        $values = array_map(function (\SplFileInfo $fileinfo) { return str_replace('/', \DIRECTORY_SEPARATOR, $fileinfo->getPathname()); }, iterator_to_array($iterator));
+        $expected = array_map(function ($path) { return str_replace('/', \DIRECTORY_SEPARATOR, $path); }, $expected);
 
         $this->assertEquals($expected, array_values($values));
     }

Original file line number	Diff line number	Diff line change
`@@ -624,7 +624,13 @@ public function getIterator()`
`624`	`624`	`}`
`625`	`625`
`626`	`626`	`if (1 === \count($this->dirs) && 0 === \count($this->iterators)) {`
`627`		`- return $this->searchInDirectory($this->dirs[0]);`
	`627`	`+ $iterator = $this->searchInDirectory($this->dirs[0]);`
	`628`	`+`
	`629`	`+ if ($this->sort \|\| $this->reverseSorting) {`
	`630`	`+ $iterator = (new Iterator\SortableIterator($iterator, $this->sort, $this->reverseSorting))->getIterator();`
	`631`	`+ }`
	`632`	`+`
	`633`	`+ return $iterator;`
`628`	`634`	`}`
`629`	`635`
`630`	`636`	`$iterator = new \AppendIterator();`
`@@ -636,6 +642,10 @@ public function getIterator()`
`636`	`642`	`$iterator->append($it);`
`637`	`643`	`}`
`638`	`644`
	`645`	`+ if ($this->sort \|\| $this->reverseSorting) {`
	`646`	`+ $iterator = (new Iterator\SortableIterator($iterator, $this->sort, $this->reverseSorting))->getIterator();`
	`647`	`+ }`
	`648`	`+`
`639`	`649`	`return $iterator;`
`640`	`650`	`}`
`641`	`651`
`@@ -782,11 +792,6 @@ private function searchInDirectory(string $dir): \Iterator`
`782`	`792`	`$iterator = new Iterator\PathFilterIterator($iterator, $this->paths, $notPaths);`
`783`	`793`	`}`
`784`	`794`
`785`		`- if ($this->sort \|\| $this->reverseSorting) {`
`786`		`- $iteratorAggregate = new Iterator\SortableIterator($iterator, $this->sort, $this->reverseSorting);`
`787`		`- $iterator = $iteratorAggregate->getIterator();`
`788`		`- }`
`789`		`-`
`790`	`795`	`return $iterator;`
`791`	`796`	`}`
`792`	`797`
Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,8 @@ protected function assertIterator($expected, \Traversable $iterator)`
`31`	`31`
`32`	`32`	`protected function assertOrderedIterator($expected, \Traversable $iterator)`
`33`	`33`	`{`
`34`		`- $values = array_map(function (\SplFileInfo $fileinfo) { return $fileinfo->getPathname(); }, iterator_to_array($iterator));`
	`34`	`+ $values = array_map(function (\SplFileInfo $fileinfo) { return str_replace('/', \DIRECTORY_SEPARATOR, $fileinfo->getPathname()); }, iterator_to_array($iterator));`
	`35`	`+ $expected = array_map(function ($path) { return str_replace('/', \DIRECTORY_SEPARATOR, $path); }, $expected);`
`35`	`36`
`36`	`37`	`$this->assertEquals($expected, array_values($values));`
`37`	`38`	`}`