Advanced Value Binder improvements (#1863)

* Refactor times, and add unit tests

src/PhpSpreadsheet/Cell/AdvancedValueBinder.php

@@ -83,29 +83,12 @@ class AdvancedValueBinder extends DefaultValueBinder implements IValueBinder
 
             // Check for time without seconds e.g. '9:45', '09:45'
             if (preg_match('/^(\d|[0-1]\d|2[0-3]):[0-5]\d$/', $value)) {
-                // Convert value to number
+                return $this->setTimeHoursMinutes($value, $cell);
-                [$h, $m] = explode(':', $value);
-                $days = $h / 24 + $m / 1440;
-                $cell->setValueExplicit($days, DataType::TYPE_NUMERIC);
-                // Set style
-                $cell->getWorksheet()->getStyle($cell->getCoordinate())
-                    ->getNumberFormat()->setFormatCode(NumberFormat::FORMAT_DATE_TIME3);
-
-                return true;
             }
 
             // Check for time with seconds '9:45:59', '09:45:59'
             if (preg_match('/^(\d|[0-1]\d|2[0-3]):[0-5]\d:[0-5]\d$/', $value)) {
-                // Convert value to number
+                return $this->setTimeHoursMinutesSeconds($value, $cell);
-                [$h, $m, $s] = explode(':', $value);
-                $days = $h / 24 + $m / 1440 + $s / 86400;
-                // Convert value to number
-                $cell->setValueExplicit($days, DataType::TYPE_NUMERIC);
-                // Set style
-                $cell->getWorksheet()->getStyle($cell->getCoordinate())
-                    ->getNumberFormat()->setFormatCode(NumberFormat::FORMAT_DATE_TIME4);
-
-                return true;
             }
 
             // Check for datetime, e.g. '2008-12-31', '2008-12-31 15:59', '2008-12-31 15:59:10'
@@ -191,4 +174,32 @@ class AdvancedValueBinder extends DefaultValueBinder implements IValueBinder
 
         return true;
     }
+
+    protected function setTimeHoursMinutes(string $value, Cell $cell): bool
+    {
+        // Convert value to number
+        [$hours, $minutes] = explode(':', $value);
+        $days = ($hours / 24) + ($minutes / 1440);
+        $cell->setValueExplicit($days, DataType::TYPE_NUMERIC);
+
+        // Set style
+        $cell->getWorksheet()->getStyle($cell->getCoordinate())
+            ->getNumberFormat()->setFormatCode(NumberFormat::FORMAT_DATE_TIME3);
+
+        return true;
+    }
+
+    protected function setTimeHoursMinutesSeconds(string $value, Cell $cell): bool
+    {
+        // Convert value to number
+        [$hours, $minutes, $seconds] = explode(':', $value);
+        $days = ($hours / 24) + ($minutes / 1440) + ($seconds / 86400);
+        $cell->setValueExplicit($days, DataType::TYPE_NUMERIC);
+
+        // Set style
+        $cell->getWorksheet()->getStyle($cell->getCoordinate())
+            ->getNumberFormat()->setFormatCode(NumberFormat::FORMAT_DATE_TIME4);
+
+        return true;
+    }
 }

tests/PhpSpreadsheetTests/Cell/AdvancedValueBinderTest.php

@@ -200,6 +200,59 @@ class AdvancedValueBinderTest extends TestCase
             ['10%', 0.1, NumberFormat::FORMAT_PERCENTAGE_00],
             ['-12%', -0.12, NumberFormat::FORMAT_PERCENTAGE_00],
             ['120%', 1.2, NumberFormat::FORMAT_PERCENTAGE_00],
+            ['12.5%', 0.125, NumberFormat::FORMAT_PERCENTAGE_00],
+        ];
+    }
+
+    /**
+     * @dataProvider timeProvider
+     *
+     * @param mixed $value
+     * @param mixed $valueBinded
+     * @param mixed $format
+     */
+    public function testTimes($value, $valueBinded, $format): void
+    {
+        $sheet = $this->getMockBuilder(Worksheet::class)
+            ->setMethods(['getStyle', 'getNumberFormat', 'setFormatCode', 'getCellCollection'])
+            ->getMock();
+
+        $cellCollection = $this->getMockBuilder(Cells::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $cellCollection->expects(self::any())
+            ->method('getParent')
+            ->willReturn($sheet);
+
+        $sheet->expects(self::once())
+            ->method('getStyle')
+            ->willReturnSelf();
+        $sheet->expects(self::once())
+            ->method('getNumberFormat')
+            ->willReturnSelf();
+        $sheet->expects(self::once())
+            ->method('setFormatCode')
+            ->with($format)
+            ->willReturnSelf();
+        $sheet->expects(self::any())
+            ->method('getCellCollection')
+            ->willReturn($cellCollection);
+
+        $cell = new Cell(null, DataType::TYPE_STRING, $sheet);
+
+        $binder = new AdvancedValueBinder();
+        $binder->bindValue($cell, $value);
+        self::assertEquals($valueBinded, $cell->getValue());
+    }
+
+    public function timeProvider()
+    {
+        return [
+            ['1:20', 0.05555555556, NumberFormat::FORMAT_DATE_TIME3],
+            ['09:17', 0.386805555556, NumberFormat::FORMAT_DATE_TIME3],
+            ['15:00', 0.625, NumberFormat::FORMAT_DATE_TIME3],
+            ['17:12:35', 0.71707175926, NumberFormat::FORMAT_DATE_TIME4],
+            ['23:58:20', 0.99884259259, NumberFormat::FORMAT_DATE_TIME4],
         ];
     }