Merge pull request #2698 from PHPOffice/Xls-Reader-DataValidations-Refactoring

Move DataValidation switch statements into a dedicated helper class

Author: MarkBaker

phpstan-baseline.neon

@@ -2300,16 +2300,6 @@ parameters:
 			count: 2
 			path: src/PhpSpreadsheet/Reader/Xls.php
 
-		-
-			message: "#^Parameter \\#1 \\$errorStyle of method PhpOffice\\\\PhpSpreadsheet\\\\Cell\\\\DataValidation\\:\\:setErrorStyle\\(\\) expects string, int\\|string given\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Reader/Xls.php
-
-		-
-			message: "#^Parameter \\#1 \\$operator of method PhpOffice\\\\PhpSpreadsheet\\\\Cell\\\\DataValidation\\:\\:setOperator\\(\\) expects string, int\\|string given\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Reader/Xls.php
-
 		-
 			message: "#^Parameter \\#1 \\$showSummaryBelow of method PhpOffice\\\\PhpSpreadsheet\\\\Worksheet\\\\Worksheet\\:\\:setShowSummaryBelow\\(\\) expects bool, int given\\.$#"
 			count: 1
@@ -2320,11 +2310,6 @@ parameters:
 			count: 1
 			path: src/PhpSpreadsheet/Reader/Xls.php
 
-		-
-			message: "#^Parameter \\#1 \\$type of method PhpOffice\\\\PhpSpreadsheet\\\\Cell\\\\DataValidation\\:\\:setType\\(\\) expects string, int\\|string given\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Reader/Xls.php
-
 		-
 			message: "#^Parameter \\#2 \\$row of method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\IReadFilter\\:\\:readCell\\(\\) expects int, string given\\.$#"
 			count: 1
@@ -5484,3 +5469,4 @@ parameters:
 			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Writer\\\\Xlsx\\\\Xlfn\\:\\:addXlfn\\(\\) should return string but returns string\\|null\\.$#"
 			count: 1
 			path: src/PhpSpreadsheet/Writer/Xlsx/Xlfn.php
+

src/PhpSpreadsheet/Reader/Xls.php

@@ -4776,57 +4776,11 @@ private function readDataValidation(): void
 
         // bit: 0-3; mask: 0x0000000F; type
         $type = (0x0000000F & $options) >> 0;
-        switch ($type) {
-            case 0x00:
-                $type = DataValidation::TYPE_NONE;
-
-                break;
-            case 0x01:
-                $type = DataValidation::TYPE_WHOLE;
-
-                break;
-            case 0x02:
-                $type = DataValidation::TYPE_DECIMAL;
-
-                break;
-            case 0x03:
-                $type = DataValidation::TYPE_LIST;
-
-                break;
-            case 0x04:
-                $type = DataValidation::TYPE_DATE;
-
-                break;
-            case 0x05:
-                $type = DataValidation::TYPE_TIME;
-
-                break;
-            case 0x06:
-                $type = DataValidation::TYPE_TEXTLENGTH;
-
-                break;
-            case 0x07:
-                $type = DataValidation::TYPE_CUSTOM;
-
-                break;
-        }
+        $type = Xls\DataValidationHelper::type($type);
 
         // bit: 4-6; mask: 0x00000070; error type
         $errorStyle = (0x00000070 & $options) >> 4;
-        switch ($errorStyle) {
-            case 0x00:
-                $errorStyle = DataValidation::STYLE_STOP;
-
-                break;
-            case 0x01:
-                $errorStyle = DataValidation::STYLE_WARNING;
-
-                break;
-            case 0x02:
-                $errorStyle = DataValidation::STYLE_INFORMATION;
-
-                break;
-        }
+        $errorStyle = Xls\DataValidationHelper::errorStyle($errorStyle);
 
         // bit: 7; mask: 0x00000080; 1= formula is explicit (only applies to list)
         // I have only seen cases where this is 1
@@ -4846,39 +4800,10 @@ private function readDataValidation(): void
 
         // bit: 20-23; mask: 0x00F00000; condition operator
         $operator = (0x00F00000 & $options) >> 20;
-        switch ($operator) {
-            case 0x00:
-                $operator = DataValidation::OPERATOR_BETWEEN;
-
-                break;
-            case 0x01:
-                $operator = DataValidation::OPERATOR_NOTBETWEEN;
-
-                break;
-            case 0x02:
-                $operator = DataValidation::OPERATOR_EQUAL;
-
-                break;
-            case 0x03:
-                $operator = DataValidation::OPERATOR_NOTEQUAL;
+        $operator = Xls\DataValidationHelper::operator($operator);
 
-                break;
-            case 0x04:
-                $operator = DataValidation::OPERATOR_GREATERTHAN;
-
-                break;
-            case 0x05:
-                $operator = DataValidation::OPERATOR_LESSTHAN;
-
-                break;
-            case 0x06:
-                $operator = DataValidation::OPERATOR_GREATERTHANOREQUAL;
-
-                break;
-            case 0x07:
-                $operator = DataValidation::OPERATOR_LESSTHANOREQUAL;
-
-                break;
+        if ($type === null || $errorStyle === null || $operator === null) {
+            return;
         }
 
         // offset: 4; size: var; title of the prompt box

src/PhpSpreadsheet/Reader/Xls/DataValidationHelper.php

@@ -0,0 +1,72 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheet\Reader\Xls;
+
+use PhpOffice\PhpSpreadsheet\Cell\DataValidation;
+
+class DataValidationHelper
+{
+    /**
+     * @var array<int, string>
+     */
+    private static $types = [
+        0x00 => DataValidation::TYPE_NONE,
+        0x01 => DataValidation::TYPE_WHOLE,
+        0x02 => DataValidation::TYPE_DECIMAL,
+        0x03 => DataValidation::TYPE_LIST,
+        0x04 => DataValidation::TYPE_DATE,
+        0x05 => DataValidation::TYPE_TIME,
+        0x06 => DataValidation::TYPE_TEXTLENGTH,
+        0x07 => DataValidation::TYPE_CUSTOM,
+    ];
+
+    /**
+     * @var array<int, string>
+     */
+    private static $errorStyles = [
+        0x00 => DataValidation::STYLE_STOP,
+        0x01 => DataValidation::STYLE_WARNING,
+        0x02 => DataValidation::STYLE_INFORMATION,
+    ];
+
+    /**
+     * @var array<int, string>
+     */
+    private static $operators = [
+        0x00 => DataValidation::OPERATOR_BETWEEN,
+        0x01 => DataValidation::OPERATOR_NOTBETWEEN,
+        0x02 => DataValidation::OPERATOR_EQUAL,
+        0x03 => DataValidation::OPERATOR_NOTEQUAL,
+        0x04 => DataValidation::OPERATOR_GREATERTHAN,
+        0x05 => DataValidation::OPERATOR_LESSTHAN,
+        0x06 => DataValidation::OPERATOR_GREATERTHANOREQUAL,
+        0x07 => DataValidation::OPERATOR_LESSTHANOREQUAL,
+    ];
+
+    public static function type(int $type): ?string
+    {
+        if (isset(self::$types[$type])) {
+            return self::$types[$type];
+        }
+
+        return null;
+    }
+
+    public static function errorStyle(int $errorStyle): ?string
+    {
+        if (isset(self::$errorStyles[$errorStyle])) {
+            return self::$errorStyles[$errorStyle];
+        }
+
+        return null;
+    }
+
+    public static function operator(int $operator): ?string
+    {
+        if (isset(self::$operators[$operator])) {
+            return self::$operators[$operator];
+        }
+
+        return null;
+    }
+}

tests/PhpSpreadsheetTests/Reader/Xls/DataValidationTest.php

@@ -0,0 +1,60 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests\Reader\Xls;
+
+use PhpOffice\PhpSpreadsheet\Cell\DataValidation;
+use PhpOffice\PhpSpreadsheet\Reader\Xls;
+use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
+use PHPUnit\Framework\TestCase;
+
+class DataValidationTest extends TestCase
+{
+    /**
+     * @var Worksheet
+     */
+    protected $sheet;
+
+    protected function setUp(): void
+    {
+        $filename = 'tests/data/Reader/XLS/DataValidation.xls';
+        $reader = new Xls();
+        $spreadsheet = $reader->load($filename);
+        $this->sheet = $spreadsheet->getActiveSheet();
+    }
+
+    /**
+     * @dataProvider dataValidationProvider
+     */
+    public function testDataValidation(string $expectedRange, array $expectedRule): void
+    {
+        $hasDataValidation = $this->sheet->dataValidationExists($expectedRange);
+        self::assertTrue($hasDataValidation);
+
+        $dataValidation = $this->sheet->getDataValidation($expectedRange);
+        self::assertSame($expectedRule['type'], $dataValidation->getType());
+        self::assertSame($expectedRule['operator'], $dataValidation->getOperator());
+        self::assertSame($expectedRule['formula'], $dataValidation->getFormula1());
+    }
+
+    public function dataValidationProvider(): array
+    {
+        return [
+            [
+                'B2',
+                [
+                    'type' => DataValidation::TYPE_WHOLE,
+                    'operator' => DataValidation::OPERATOR_GREATERTHANOREQUAL,
+                    'formula' => '18',
+                ],
+            ],
+            [
+                'B3',
+                [
+                    'type' => DataValidation::TYPE_LIST,
+                    'operator' => DataValidation::OPERATOR_BETWEEN,
+                    'formula' => '"Blocked,Pending,Approved"',
+                ],
+            ],
+        ];
+    }
+}