Phpunit 10 and Phpstan 1.11

oleibman · oleibman · commit 9b239ebdb01e · 2024-06-01T17:39:39.000-07:00
Dependabot suggested some changes this month which required an unusual effort to implement successfully. With the elimination of Php 8.0 as a supported environment, it became possible to use Phpunit 10 rather than 9. Among other considerations, the configuration file for Phpunit is changed. I preserved the Phpunit 9 version under a different name. Aside from the configuration change, several other changes needed to be made to accommodate the change: - Fix #3993. A peculiar problem indeed. One of the reporters said it had something to do with mocking, but I couldn't duplicate it. But Phpunit 10 revealed the problem in one test (Reader/Xlsx/AutoFilterTest), and that was sufficient for me to apply the trivial source code change to Worksheet/Worksheet. - Some extra stringency on data providers required extra work in Calculation/CalculationFunctionListTest. - There was a misplaced label in Calculation/ParseFormulaTest. Likewise in the data member CellGetRangeBoundaries. - More stringency required changes to data members Shared/Trend/ExponentialBestFit and Shared/Trend/LinearBestFit. - Issue3982Test testLoadAllRows seemed to go into a memory-acquiring loop in 10 that was not evident in 9. This particular test uses a lot of memory by design, but was included only to establish a base level for the other tests in that member. I feel it is acceptable to skip it for 10. Phpstan errors with a new release are not unusual. One of the problems this time around was, however, unusual - it is fixed when Phpstan runs under Php8.3, but not for earlier Php releases. Our tools currently use Php 8.1. It is on my to-do list to get Phpstan and other test tools running under 8.3 before 8.1 goes EOL. There are also some configuration file changes needed for Phpstan. Php-cs-fixer has been taking an increasingly long time to run. They've added an experimental option to permit it to run its checks in parallel. I've changed its configuration to use that option.
diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php
@@ -10,6 +10,7 @@
 $config
     ->setRiskyAllowed(true)
     ->setFinder($finder)
+    ->setParallelConfig(PhpCsFixer\Runner\Parallel\ParallelConfigFactory::detect())
     ->setCacheFile(sys_get_temp_dir() . '/php-cs-fixer' . preg_replace('~\W~', '-', __DIR__))
     ->setRules([
         'align_multiline_comment' => true,
diff --git a/composer.json b/composer.json
@@ -95,7 +95,7 @@
         "phpcompatibility/php-compatibility": "^9.3",
         "phpstan/phpstan": "^1.1",
         "phpstan/phpstan-phpunit": "^1.0",
-        "phpunit/phpunit": "^9.6",
+        "phpunit/phpunit": "^9.6 || ^10.5",
         "squizlabs/php_codesniffer": "^3.7",
         "tecnickcom/tcpdf": "^6.5"
     },
diff --git a/composer.lock b/composer.lock
diff --git a/phpstan.neon.dist b/phpstan.neon.dist
@@ -22,9 +22,9 @@ parameters:
         - src/PhpSpreadsheet/Writer/ZipStream3.php
     parallel:
         processTimeout: 300.0
-    checkMissingIterableValueType: false
-    checkGenericClassInNonGenericObjectType: false
     ignoreErrors:
         # Accept a bit anything for assert methods
         - '~^Parameter \#2 .* of static method PHPUnit\\Framework\\Assert\:\:assert\w+\(\) expects .*, .* given\.$~'
         - '~^Variable \$helper might not be defined\.$~'
+        - identifier: missingType.iterableValue
+        - identifier: missingType.generics
diff --git a/phpunit.xml.dist b/phpunit.xml.dist
@@ -1,15 +1,15 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd" bootstrap="./tests/bootstrap.php" backupGlobals="true" colors="true" cacheResultFile="/tmp/.phpspreadsheet.phpunit.result.cache" convertDeprecationsToExceptions="true">
-  <coverage>
-    <include>
-      <directory suffix=".php">./src</directory>
-    </include>
-  </coverage>
+<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/10.1/phpunit.xsd" bootstrap="./tests/bootstrap.php" backupGlobals="true" colors="true" cacheResultFile="/tmp/.phpspreadsheet.phpunit.result.cache">
+  <coverage/>
   <php>
     <ini name="memory_limit" value="2048M"/>
-    <ini name="error_reporting" value="E_ALL"/>
   </php>
   <testsuite name="PhpSpreadsheet Unit Test Suite">
     <directory>./tests/PhpSpreadsheetTests</directory>
   </testsuite>
+  <source>
+    <include>
+      <directory suffix=".php">./src</directory>
+    </include>
+  </source>
 </phpunit>
diff --git a/phpunit9.xml.dist b/phpunit9.xml.dist
@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd" bootstrap="./tests/bootstrap.php" backupGlobals="true" colors="true" cacheResultFile="/tmp/.phpspreadsheet.phpunit.result.cache" convertDeprecationsToExceptions="true">
+  <coverage>
+    <include>
+      <directory suffix=".php">./src</directory>
+    </include>
+  </coverage>
+  <php>
+    <ini name="memory_limit" value="2048M"/>
+    <ini name="error_reporting" value="E_ALL"/>
+  </php>
+  <testsuite name="PhpSpreadsheet Unit Test Suite">
+    <directory>./tests/PhpSpreadsheetTests</directory>
+  </testsuite>
+</phpunit>
diff --git a/src/PhpSpreadsheet/Helper/Html.php b/src/PhpSpreadsheet/Helper/Html.php
@@ -694,7 +694,7 @@ private function rgbToColour(string $rgbValue): string
     {
         preg_match_all('/\d+/', $rgbValue, $values);
         foreach ($values[0] as &$value) {
-            $value = str_pad(dechex($value), 2, '0', STR_PAD_LEFT);
+            $value = str_pad(dechex((int) $value), 2, '0', STR_PAD_LEFT);
         }
 
         return implode('', $values[0]);
diff --git a/src/PhpSpreadsheet/Reader/Html.php b/src/PhpSpreadsheet/Reader/Html.php
@@ -169,7 +169,8 @@ private function readBeginning(): string
     private function readEnding(): string
     {
         $meta = stream_get_meta_data($this->fileHandle);
-        $filename = $meta['uri'];
+        // Phpstan incorrectly flags following line for Php8.2-, corrected in 8.3
+        $filename = $meta['uri']; //@phpstan-ignore-line
 
         $size = (int) filesize($filename);
         if ($size === 0) {
diff --git a/src/PhpSpreadsheet/Worksheet/Worksheet.php b/src/PhpSpreadsheet/Worksheet/Worksheet.php
@@ -67,7 +67,7 @@ class Worksheet implements IComparable
     /**
      * Parent spreadsheet.
      */
-    private ?Spreadsheet $parent;
+    private ?Spreadsheet $parent = null;
 
     /**
      * Collection of cells.
diff --git a/src/PhpSpreadsheet/Writer/Xls/Worksheet.php b/src/PhpSpreadsheet/Writer/Xls/Worksheet.php
@@ -2374,7 +2374,7 @@ public function processBitmap(string $bitmap): array
     {
         // Open file.
         $bmp_fd = @fopen($bitmap, 'rb');
-        if ($bmp_fd === false) {
+        if ($bmp_fd === false || 0 === (int) filesize($bitmap)) {
             throw new WriterException("Couldn't import $bitmap");
         }
 
diff --git a/tests/PhpSpreadsheetTests/Calculation/CalculationFunctionListTest.php b/tests/PhpSpreadsheetTests/Calculation/CalculationFunctionListTest.php
@@ -33,14 +33,20 @@ protected function tearDown(): void
     /**
      * @dataProvider providerGetFunctions
      */
-    public function testGetFunctions(string $category, array|string $functionCall): void
+    public function testGetFunctions(array|string $functionCall): void
     {
         self::assertIsCallable($functionCall);
     }
 
     public static function providerGetFunctions(): array
     {
-        return Calculation::getInstance()->getFunctions();
+        $returnFunctions = [];
+        $functionList = Calculation::getInstance()->getFunctions();
+        foreach ($functionList as $functionName => $functionArray) {
+            $returnFunctions[$functionName]['functionCall'] = $functionArray['functionCall'];
+        }
+
+        return $returnFunctions;
     }
 
     public function testIsImplemented(): void
diff --git a/tests/PhpSpreadsheetTests/Calculation/ParseFormulaTest.php b/tests/PhpSpreadsheetTests/Calculation/ParseFormulaTest.php
@@ -287,8 +287,8 @@ public static function providerBinaryOperations(): array
                 ],
                 '=DeptSales[[#Headers],[Region]:[Commission Amount]]',
             ],
-            [
-                'Multi-RowGroup Fully Qualified Nested Structured Reference' => [
+            'Multi-RowGroup Fully Qualified Nested Structured Reference' => [
+                [
                     ['type' => 'Structured Reference', 'value' => new StructuredReference('DeptSales[[#Headers],[#Data],[% Commission]]'), 'reference' => null],
                 ],
                 '=DeptSales[[#Headers],[#Data],[% Commission]]',
diff --git a/tests/PhpSpreadsheetTests/Reader/Xlsx/Issue3982Test.php b/tests/PhpSpreadsheetTests/Reader/Xlsx/Issue3982Test.php
@@ -7,13 +7,17 @@
 use PhpOffice\PhpSpreadsheet\IOFactory;
 use PhpOffice\PhpSpreadsheet\Reader\IReader;
 use PhpOffice\PhpSpreadsheet\Reader\Xlsx as XlsxReader;
+use PHPUnit\Framework\TestCase;
 
-class Issue3982Test extends \PHPUnit\Framework\TestCase
+class Issue3982Test extends TestCase
 {
     private static string $testbook = 'tests/data/Reader/XLSX/issue.3982.xlsx';
 
     public function testLoadAllRows(): void
     {
+        if (!method_exists(TestCase::class, 'setOutputCallback')) {
+            self::markTestSkipped('Memory loop in Phpunit 10');
+        }
         $spreadsheet = IOFactory::load(self::$testbook);
         $sheet = $spreadsheet->getActiveSheet();
         $data = $sheet->toArray(null, true, false, true);
diff --git a/tests/data/CellGetRangeBoundaries.php b/tests/data/CellGetRangeBoundaries.php
@@ -3,8 +3,8 @@
 declare(strict_types=1);
 
 return [
-    [
-        'Cell Range' => [
+    'Cell Range' => [
+        [
             ['B', 4],
             ['E', 9],
         ],
diff --git a/tests/data/Shared/Trend/ExponentialBestFit.php b/tests/data/Shared/Trend/ExponentialBestFit.php
@@ -4,11 +4,11 @@
 
 return [
     [
-        'slope' => [0.8, 0.813512072856517],
-        'intersect' => [20.7, 20.671878197177865],
-        'goodnessOfFit' => [0.904868, 0.9048681877346413],
-        'equation' => 'Y = 20.67 * 0.81^X',
-        [3, 10, 3, 6, 8, 12, 1, 4, 9, 14],
-        [8, 2, 11, 6, 5, 4, 12, 9, 6, 1],
+        'expectedSlope' => [0.8, 0.813512072856517],
+        'expectedIntersect' => [20.7, 20.671878197177865],
+        'expectedGoodnessOfFit' => [0.904868, 0.9048681877346413],
+        'expectedEquation' => 'Y = 20.67 * 0.81^X',
+        'yValues' => [3, 10, 3, 6, 8, 12, 1, 4, 9, 14],
+        'xValues' => [8, 2, 11, 6, 5, 4, 12, 9, 6, 1],
     ],
 ];
diff --git a/tests/data/Shared/Trend/LinearBestFit.php b/tests/data/Shared/Trend/LinearBestFit.php
@@ -4,19 +4,19 @@
 
 return [
     [
-        'slope' => [-1.1, -1.1064189189190],
-        'intersect' => [14.1, 14.081081081081],
-        'goodnessOfFit' => [0.873138, 0.8731378215564962],
-        'equation' => 'Y = 14.08 + -1.11 * X',
-        [3, 10, 3, 6, 8, 12, 1, 4, 9, 14],
-        [8, 2, 11, 6, 5, 4, 12, 9, 6, 1],
+        'expectedSlope' => [-1.1, -1.1064189189190],
+        'expectedIntersect' => [14.1, 14.081081081081],
+        'expectedGoodnessOfFit' => [0.873138, 0.8731378215564962],
+        'expectedEquation' => 'Y = 14.08 + -1.11 * X',
+        'yValues' => [3, 10, 3, 6, 8, 12, 1, 4, 9, 14],
+        'xValues' => [8, 2, 11, 6, 5, 4, 12, 9, 6, 1],
     ],
     [
-        'slope' => [1.0, 1.0],
-        'intersect' => [-2.0, -2.0],
-        'goodnessOfFit' => [1.0, 1.0],
-        'equation' => 'Y = -2 + 1 * X',
-        [1, 2, 3, 4, 5],
-        [3, 4, 5, 6, 7],
+        'expectedSlope' => [1.0, 1.0],
+        'expectedIntersect' => [-2.0, -2.0],
+        'expectedGoodnessOfFit' => [1.0, 1.0],
+        'expectedEquation' => 'Y = -2 + 1 * X',
+        'yValues' => [1, 2, 3, 4, 5],
+        'xValues' => [3, 4, 5, 6, 7],
     ],
 ];

Original file line number	Diff line number	Diff line change
`@@ -694,7 +694,7 @@ private function rgbToColour(string $rgbValue): string`
`694`	`694`	`{`
`695`	`695`	`preg_match_all('/\d+/', $rgbValue, $values);`
`696`	`696`	`foreach ($values[0] as &$value) {`
`697`		`- $value = str_pad(dechex($value), 2, '0', STR_PAD_LEFT);`
	`697`	`+ $value = str_pad(dechex((int) $value), 2, '0', STR_PAD_LEFT);`
`698`	`698`	`}`
`699`	`699`
`700`	`700`	`return implode('', $values[0]);`
Original file line number	Diff line number	Diff line change
`@@ -169,7 +169,8 @@ private function readBeginning(): string`
`169`	`169`	`private function readEnding(): string`
`170`	`170`	`{`
`171`	`171`	`$meta = stream_get_meta_data($this->fileHandle);`
`172`		`- $filename = $meta['uri'];`
	`172`	`+ // Phpstan incorrectly flags following line for Php8.2-, corrected in 8.3`
	`173`	`+ $filename = $meta['uri']; //@phpstan-ignore-line`
`173`	`174`
`174`	`175`	`$size = (int) filesize($filename);`
`175`	`176`	`if ($size === 0) {`
Original file line number	Diff line number	Diff line change
`@@ -2374,7 +2374,7 @@ public function processBitmap(string $bitmap): array`
`2374`	`2374`	`{`
`2375`	`2375`	`// Open file.`
`2376`	`2376`	`$bmp_fd = @fopen($bitmap, 'rb');`
`2377`		`- if ($bmp_fd === false) {`
	`2377`	`+ if ($bmp_fd === false \|\| 0 === (int) filesize($bitmap)) {`
`2378`	`2378`	`throw new WriterException("Couldn't import $bitmap");`
`2379`	`2379`	`}`
`2380`	`2380`