Merge pull request #8 from willemstuursma/master

Minor performance improvements
mediact · Jul 9, 2018 · b3c77d4 · b3c77d4
2 parents 8340e9a + 31b9586
commit b3c77d4
Show file tree

Hide file tree

Showing 8 changed files with 76 additions and 119 deletions.
diff --git a/src/Candidate/CandidateExtractor.php b/src/Candidate/CandidateExtractor.php
@@ -46,27 +46,14 @@ public function extract(
             $packages[$package][] = $symbol;
         }
 
+        $installed = $repository->getPackages();
+
         $candidates = [];
-        $installed  = $repository->getPackages();
 
         foreach ($packages as $name => $symbols) {
-            $package = array_reduce(
-                $installed,
-                function (
-                    ?PackageInterface $carry,
-                    PackageInterface $package
-                ) use (
-                    $name
-                ): ?PackageInterface {
-                    return $carry ?? (
-                        $package->getName() === $name
-                            ? $package
-                            : null
-                        );
-                }
-            );
+            $package = $this->getPackageByName($installed, $name);
 
-            if (!$package instanceof PackageInterface) {
+            if ($package === null) {
                 continue;
             }
 
@@ -79,6 +66,23 @@ function (
         return $candidates;
     }
 
+    /**
+     * @param PackageInterface[]|iterable $packages
+     * @param string                      $name
+     *
+     * @return PackageInterface|null
+     */
+    private function getPackageByName(iterable $packages, string $name): ?PackageInterface
+    {
+        foreach ($packages as $package) {
+            if ($package->getName() === $name) {
+                return $package;
+            }
+        }
+
+        return null;
+    }
+
     /**
      * Extract the package name from the given PHP symbol.
      *

diff --git a/src/Php/Filter/SymbolFilterChain.php b/src/Php/Filter/SymbolFilterChain.php
@@ -30,17 +30,12 @@ public function __construct(SymbolFilterInterface ...$filters)
      */
     public function __invoke(string $symbol): bool
     {
-        return array_reduce(
-            $this->filters,
-            function (
-                bool $carry,
-                SymbolFilterInterface $filter
-            ) use (
-                $symbol
-            ) : bool {
-                return $carry && $filter($symbol);
-            },
-            true
-        );
+        foreach ($this->filters as $filter) {
+            if (!$filter->__invoke($symbol)) {
+                return false;
+            }
+        }
+
+        return true;
     }
 }
diff --git a/src/Php/SymbolExtractor.php b/src/Php/SymbolExtractor.php
@@ -48,17 +48,18 @@ public function extract(
         $symbols = [];
 
         foreach ($files as $file) {
-            if (!$file->isFile() || !$file->isReadable()) {
-                continue;
-            }
-
             try {
-                $handle     = $file->openFile('r');
-                $contents   = implode('', iterator_to_array($handle));
+                $handle   = $file->openFile('rb');
+                $contents = $handle->fread($file->getSize());
+
+                if (!$contents) { // phpstan thinks this can only return string.
+                    continue;
+                }
+
                 $statements = $this->parser->parse($contents);
             } catch (Error $e) {
-                // Either not a PHP file or the broken file should be detected by other
-                // tooling entirely.
+                // Either not a file, file is not readable, not a PHP file or
+                // the broken file should be detected by other tooling entirely.
                 continue;
             }
 

diff --git a/src/Php/SymbolTracker.php b/src/Php/SymbolTracker.php
@@ -72,16 +72,13 @@ public function enterNode(Node $node): void
      */
     public function getSymbols(): iterable
     {
-        return array_reduce(
-            array_filter($this->symbols),
-            function (array $carry, array $names) : array {
-                foreach ($names as $name) {
-                    $carry[] = $name;
-                }
-
-                return $carry;
-            },
-            []
+        return new \CallbackFilterIterator(
+            new \RecursiveIteratorIterator(
+                new \RecursiveArrayIterator($this->symbols, \RecursiveArrayIterator::CHILD_ARRAYS_ONLY)
+            ),
+            function ($each): bool {
+                return $each !== false;
+            }
         );
     }
 }
diff --git a/src/Violation/Filter/ViolationFilterChain.php b/src/Violation/Filter/ViolationFilterChain.php
@@ -32,17 +32,12 @@ public function __construct(ViolationFilterInterface ...$filters)
      */
     public function __invoke(ViolationInterface $violation): bool
     {
-        return array_reduce(
-            $this->filters,
-            function (
-                bool $carry,
-                ViolationFilterInterface $filter
-            ) use (
-                $violation
-            ) : bool {
-                return $carry && $filter($violation);
-            },
-            true
-        );
+        foreach ($this->filters as $filter) {
+            if (!$filter->__invoke($violation)) {
+                return false;
+            }
+        }
+
+        return true;
     }
 }
diff --git a/tests/Candidate/CandidateExtractorTest.php b/tests/Candidate/CandidateExtractorTest.php
@@ -35,6 +35,7 @@ class CandidateExtractorTest extends TestCase
      *
      * @covers ::extract
      * @covers ::extractPackage
+     * @covers ::getPackageByName
      */
     public function testExtract(
         Composer $composer,

diff --git a/tests/Php/SymbolExtractorTest.php b/tests/Php/SymbolExtractorTest.php
@@ -45,8 +45,7 @@ public function testConstructor(): void
 
     /**
      * @dataProvider emptyProvider
-     * @dataProvider illegalFilesProvider
-     * @dataProvider emptyFilesProvider
+     * @dataProvider unparsableFilesProvider
      * @dataProvider filledFilesProvider
      *
      * @param Parser                $parser
@@ -121,84 +120,47 @@ public function emptyProvider(): array
     }
 
     /**
-     * @return SplFileInfo
-     */
-    private function createDirectory(): SplFileInfo
-    {
-        /** @var SplFileInfo|MockObject $directory */
-        $directory = $this->createMock(SplFileInfo::class);
-
-        $directory
-            ->expects(self::any())
-            ->method('isFile')
-            ->willReturn(false);
-
-        return $directory;
-    }
-
-    /**
-     * @param string|null $content
+     * @param string $content
      *
-     * @return SplFileInfo
+     * @return SplFileInfo|\PHPUnit_Framework_MockObject_MockObject
      */
-    private function createFile(string $content = null): SplFileInfo
+    private function createFile(string $content): SplFileInfo
     {
         /** @var SplFileInfo|MockObject $fileInfo */
         $fileInfo = $this->createMock(SplFileInfo::class);
 
         $fileInfo
-            ->expects(self::any())
             ->method('isFile')
             ->willReturn(true);
 
         $fileInfo
-            ->expects(self::any())
             ->method('isReadable')
             ->willReturn($content !== null);
 
         $fileInfo
-            ->expects(self::any())
-            ->method('openFile')
-            ->with(self::isType('string'))
-            ->willReturnCallback(
-                function (string $mode) use ($content) {
-                    $file = new SplFileObject('php://memory', $mode);
-                    $file->fwrite((string)$content);
+            ->method("getSize")
+            ->willReturn(\strlen($content));
 
-                    return $file;
-                }
-            );
+        $handle = $this->getMockBuilder(\SplFileObject::class)
+            ->enableOriginalConstructor()
+            ->setConstructorArgs([tempnam(sys_get_temp_dir(), "")])
+            ->getMock();
 
-        return $fileInfo;
-    }
+        $handle
+            ->method("fread")
+            ->willReturn($content);
 
-    /**
-     * @return Parser[][]|FileIteratorInterface[][]
-     */
-    public function illegalFilesProvider(): array
-    {
-        $parser = $this->createMock(Parser::class);
-
-        $parser
-            ->expects(self::never())
-            ->method('parse')
-            ->with(self::anything());
+        $fileInfo
+            ->method('openFile')
+            ->willReturn($handle);
 
-        return [
-            [
-                $parser,
-                $this->createFileIterator(
-                    $this->createDirectory(),
-                    $this->createFile()
-                )
-            ]
-        ];
+        return $fileInfo;
     }
 
     /**
      * @return Parser[][]|FileIteratorInterface[][]
      */
-    public function emptyFilesProvider(): array
+    public function unparsableFilesProvider(): array
     {
         $parser = $this->createMock(Parser::class);
 
@@ -214,9 +176,9 @@ public function emptyFilesProvider(): array
             [
                 $parser,
                 $this->createFileIterator(
-                    $this->createFile(''),
-                    $this->createFile(''),
-                    $this->createFile('')
+                    $this->createFile('x'),
+                    $this->createFile('y'),
+                    $this->createFile('z')
                 )
             ]
         ];

diff --git a/tests/Php/SymbolTrackerTest.php b/tests/Php/SymbolTrackerTest.php
@@ -43,7 +43,9 @@ public function testGetSymbols(
             $subject->enterNode($node);
         }
 
-        $this->assertEquals($expected, $subject->getSymbols());
+        $actual = iterator_to_array($subject->getSymbols());
+
+        $this->assertSame($expected, $actual);
     }
 
     /**