Handle ConditionalStyle NumberFormat When Reading Xlsx File (#1296)

* Handle ConditionalStyle NumberFormat When Reading Xlsx File

ReadStyle in Reader/Xlsx/Styles.php expects numberFormat to be a string.
However, when reading conditional style in Xlsx file, NumberFormat
   is actually a SimpleXMLElement, so is not handled correctly.
While testing this change, it turned out that reader always expects
   that there is a "SharedString" portion of the XML, which is not
   true for spreadsheets with no string data, which causes a
   run-time message.
Likewise, when conditional number format is not one of the built-in
   formats, a run-time message is issued because 'isset' is used
   to determine existence rather than 'array_key_exists'.
The new workbook added to the testing data demonstrates both those
   problems (prior to the code changes).

* Move Comment to Resolve Conflict

Github reports conflict involving placement of one comment statement.

* Respond to Scrutinizer Style Suggestion

Change detection for empty SimpleXMLElement.

src/PhpSpreadsheet/Reader/Xlsx/Styles.php

@@ -8,6 +8,7 @@ use PhpOffice\PhpSpreadsheet\Style\Borders;
 use PhpOffice\PhpSpreadsheet\Style\Color;
 use PhpOffice\PhpSpreadsheet\Style\Fill;
 use PhpOffice\PhpSpreadsheet\Style\Font;
+use PhpOffice\PhpSpreadsheet\Style\NumberFormat;
 use PhpOffice\PhpSpreadsheet\Style\Protection;
 use PhpOffice\PhpSpreadsheet\Style\Style;
 
@@ -71,6 +72,17 @@ class Styles extends BaseParserClass
         }
     }
 
+    private static function readNumberFormat(NumberFormat $numfmtStyle, \SimpleXMLElement $numfmtStyleXml)
+    {
+        if ($numfmtStyleXml->count() === 0) {
+            return;
+        }
+        $numfmt = $numfmtStyleXml->attributes();
+        if ($numfmt->count() > 0 && isset($numfmt['formatCode'])) {
+            $numfmtStyle->setFormatCode((string) $numfmt['formatCode']);
+        }
+    }
+
     private static function readFillStyle(Fill $fillStyle, \SimpleXMLElement $fillStyleXml)
     {
         if ($fillStyleXml->gradientFill) {
@@ -149,7 +161,11 @@ class Styles extends BaseParserClass
 
     private function readStyle(Style $docStyle, $style)
     {
+        if ($style->numFmt instanceof \SimpleXMLElement) {
+            self::readNumberFormat($docStyle->getNumberFormat(), $style->numFmt);
+        } else {
             $docStyle->getNumberFormat()->setFormatCode($style->numFmt);
+        }
 
         if (isset($style->font)) {
             self::readFontStyle($docStyle->getFont(), $style->font);

src/PhpSpreadsheet/Style/NumberFormat.php

@@ -367,7 +367,7 @@ class NumberFormat extends Supervisor
         self::fillBuiltInFormatCodes();
 
         // Lookup format code
-        if (isset(self::$flippedBuiltInFormats[$formatCode])) {
+        if (array_key_exists($formatCode, self::$flippedBuiltInFormats)) {
             return self::$flippedBuiltInFormats[$formatCode];
         }
 

tests/PhpSpreadsheetTests/Reader/CondNumFmtTest.php

@@ -0,0 +1,39 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests\Reader;
+
+use PhpOffice\PhpSpreadsheet\Reader\Xlsx;
+use PhpOffice\PhpSpreadsheet\Style\Conditional;
+use PHPUnit\Framework\TestCase;
+
+class CondNumFmtTest extends TestCase
+{
+    public function testLoadCondNumFmt()
+    {
+        $filename = './data/Reader/XLSX/condfmtnum.xlsx';
+        $reader = new Xlsx();
+        $spreadsheet = $reader->load($filename);
+
+        $worksheet = $spreadsheet->getActiveSheet();
+        // NumberFormat explicitly set in following conditional style
+        $conditionalStyle = $worksheet->getConditionalStyles('A1:A3');
+        self::assertNotEmpty($conditionalStyle);
+        $conditionalRule = $conditionalStyle[0];
+        $conditions = $conditionalRule->getConditions();
+        self::assertNotEmpty($conditions);
+        self::assertEquals(Conditional::CONDITION_EXPRESSION, $conditionalRule->getConditionType());
+        self::assertEquals('MONTH(A1)=10', $conditions[0]);
+        $numfmt = $conditionalRule->getStyle()->getNumberFormat()->getFormatCode();
+        self::assertEquals('yyyy/mm/dd', $numfmt);
+        // NumberFormat not set in following conditional style
+        $conditionalStyle = $worksheet->getConditionalStyles('B1');
+        self::assertNotEmpty($conditionalStyle);
+        $conditionalRule = $conditionalStyle[0];
+        $conditions = $conditionalRule->getConditions();
+        self::assertNotEmpty($conditions);
+        self::assertEquals(Conditional::CONDITION_EXPRESSION, $conditionalRule->getConditionType());
+        self::assertEquals('AND(B1>=2000,B1<3000)', $conditions[0]);
+        $numfmt = $conditionalRule->getStyle()->getNumberFormat()->getFormatCode();
+        self::assertEquals('', $numfmt);
+    }
+}