Merge pull request #2673 from PHPOffice/Resolve-CF-Issues-with-Xls-Writer

Resolve Conditional Formatting issues with Xls writer

CHANGELOG.md

@@ -39,6 +39,8 @@ and this project adheres to [Semantic Versioning](https://semver.org).
 
 ### Fixed
 
+- Fix bug in Conditional Formatting in the Xls Writer that resulted in a broken file when there were multiple conditional ranges in a worksheet.
+- Fix Conditional Formatting in the Xls Writer to work with rules that contain string literals, cell references and formulae.
 - Fix for setting Active Sheet to the first loaded worksheet when bookViews element isn't defined [Issue #2666](https://github.com/PHPOffice/PhpSpreadsheet/issues/2666) [PR #2669](https://github.com/PHPOffice/PhpSpreadsheet/pull/2669)
 - Fixed behaviour of XLSX font style vertical align settings.
 - Resolved formula translations to handle separators (row and column) for array functions as well as for function argument separators; and cleanly handle nesting levels.

samples/ConditionalFormatting/01_Basic_Comparisons.php

@@ -30,7 +30,8 @@ $spreadsheet->getActiveSheet()
     ->setCellValue('A1', 'Literal Value Comparison')
     ->setCellValue('A9', 'Value Comparison with Absolute Cell Reference $H$9')
     ->setCellValue('A17', 'Value Comparison with Relative Cell References')
-    ->setCellValue('A23', 'Value Comparison with Formula based on AVERAGE() ± STDEV()');
+    ->setCellValue('A23', 'Value Comparison with Formula based on AVERAGE() ± STDEV()')
+    ->setCellValue('A30', 'Literal String Value Comparison');
 
 $dataArray = [
     [-2, -1, 0, 1, 2],
@@ -45,11 +46,18 @@ $betweenDataArray = [
     [4, 3, 8],
 ];
 
+$stringArray = [
+    ['I'],
+    ['Love'],
+    ['PHP'],
+];
+
 $spreadsheet->getActiveSheet()
     ->fromArray($dataArray, null, 'A2', true)
     ->fromArray($dataArray, null, 'A10', true)
     ->fromArray($betweenDataArray, null, 'A18', true)
     ->fromArray($dataArray, null, 'A24', true)
+    ->fromArray($stringArray, null, 'A31', true)
     ->setCellValue('H9', 1);
 
 // Set title row bold
@@ -58,21 +66,31 @@ $spreadsheet->getActiveSheet()->getStyle('A1:E1')->getFont()->setBold(true);
 $spreadsheet->getActiveSheet()->getStyle('A9:E9')->getFont()->setBold(true);
 $spreadsheet->getActiveSheet()->getStyle('A17:E17')->getFont()->setBold(true);
 $spreadsheet->getActiveSheet()->getStyle('A23:E23')->getFont()->setBold(true);
+$spreadsheet->getActiveSheet()->getStyle('A30:E30')->getFont()->setBold(true);
 
 // Define some styles for our Conditionals
 $helper->log('Define some styles for our Conditionals');
 $yellowStyle = new Style(false, true);
 $yellowStyle->getFill()
     ->setFillType(Fill::FILL_SOLID)
+    ->getStartColor()->setARGB(Color::COLOR_YELLOW);
+$yellowStyle->getFill()
     ->getEndColor()->setARGB(Color::COLOR_YELLOW);
+$yellowStyle->getFont()->setColor(new Color(Color::COLOR_BLUE));
 $greenStyle = new Style(false, true);
 $greenStyle->getFill()
     ->setFillType(Fill::FILL_SOLID)
+    ->getStartColor()->setARGB(Color::COLOR_GREEN);
+$greenStyle->getFill()
     ->getEndColor()->setARGB(Color::COLOR_GREEN);
+$greenStyle->getFont()->setColor(new Color(Color::COLOR_DARKRED));
 $redStyle = new Style(false, true);
 $redStyle->getFill()
     ->setFillType(Fill::FILL_SOLID)
+    ->getStartColor()->setARGB(Color::COLOR_RED);
+$redStyle->getFill()
     ->getEndColor()->setARGB(Color::COLOR_RED);
+$redStyle->getFont()->setColor(new Color(Color::COLOR_GREEN));
 
 // Set conditional formatting rules and styles
 $helper->log('Define conditional formatting and set styles');
@@ -166,6 +184,32 @@ $cellWizard->lessThan('AVERAGE(' . $formulaRange . ')-STDEV(' . $formulaRange .
     ->setStyle($redStyle);
 $conditionalStyles[] = $cellWizard->getConditional();
 
+$spreadsheet->getActiveSheet()
+    ->getStyle($cellWizard->getCellRange())
+    ->setConditionalStyles($conditionalStyles);
+
+// Set rules for Value Comparison with String Literal
+$cellRange = 'A31:A33';
+$formulaRange = implode(
+    ':',
+    array_map(
+        [Coordinate::class, 'absoluteCoordinate'],
+        Coordinate::splitRange($cellRange)[0]
+    )
+);
+$conditionalStyles = [];
+$wizardFactory = new Wizard($cellRange);
+/** @var Wizard\CellValue $cellWizard */
+$cellWizard = $wizardFactory->newRule(Wizard::CELL_VALUE);
+
+$cellWizard->equals('LOVE')
+    ->setStyle($redStyle);
+$conditionalStyles[] = $cellWizard->getConditional();
+
+$cellWizard->equals('PHP')
+    ->setStyle($greenStyle);
+$conditionalStyles[] = $cellWizard->getConditional();
+
 $spreadsheet->getActiveSheet()
     ->getStyle($cellWizard->getCellRange())
     ->setConditionalStyles($conditionalStyles);

samples/ConditionalFormatting/02_Text_Comparisons.php

@@ -74,14 +74,17 @@ $yellowStyle = new Style(false, true);
 $yellowStyle->getFill()
     ->setFillType(Fill::FILL_SOLID)
     ->getEndColor()->setARGB(Color::COLOR_YELLOW);
+$yellowStyle->getFont()->setColor(new Color(Color::COLOR_BLUE));
 $greenStyle = new Style(false, true);
 $greenStyle->getFill()
     ->setFillType(Fill::FILL_SOLID)
     ->getEndColor()->setARGB(Color::COLOR_GREEN);
+$greenStyle->getFont()->setColor(new Color(Color::COLOR_DARKRED));
 $redStyle = new Style(false, true);
 $redStyle->getFill()
     ->setFillType(Fill::FILL_SOLID)
     ->getEndColor()->setARGB(Color::COLOR_RED);
+$redStyle->getFont()->setColor(new Color(Color::COLOR_GREEN));
 
 // Set conditional formatting rules and styles
 $helper->log('Define conditional formatting and set styles');

samples/ConditionalFormatting/03_Blank_Comparisons.php

@@ -46,10 +46,12 @@ $greenStyle = new Style(false, true);
 $greenStyle->getFill()
     ->setFillType(Fill::FILL_SOLID)
     ->getEndColor()->setARGB(Color::COLOR_GREEN);
+$greenStyle->getFont()->setColor(new Color(Color::COLOR_DARKRED));
 $redStyle = new Style(false, true);
 $redStyle->getFill()
     ->setFillType(Fill::FILL_SOLID)
     ->getEndColor()->setARGB(Color::COLOR_RED);
+$redStyle->getFont()->setColor(new Color(Color::COLOR_GREEN));
 
 // Set conditional formatting rules and styles
 $helper->log('Define conditional formatting and set styles');

samples/ConditionalFormatting/04_Error_Comparisons.php

@@ -49,10 +49,12 @@ $greenStyle = new Style(false, true);
 $greenStyle->getFill()
     ->setFillType(Fill::FILL_SOLID)
     ->getEndColor()->setARGB(Color::COLOR_GREEN);
+$greenStyle->getFont()->setColor(new Color(Color::COLOR_DARKRED));
 $redStyle = new Style(false, true);
 $redStyle->getFill()
     ->setFillType(Fill::FILL_SOLID)
     ->getEndColor()->setARGB(Color::COLOR_RED);
+$redStyle->getFont()->setColor(new Color(Color::COLOR_GREEN));
 
 // Set conditional formatting rules and styles
 $helper->log('Define conditional formatting and set styles');

samples/ConditionalFormatting/05_Date_Comparisons.php

@@ -108,12 +108,11 @@ $spreadsheet->getActiveSheet()->getStyle('B1:K1')->getAlignment()->setHorizontal
 
 // Define some styles for our Conditionals
 $helper->log('Define some styles for our Conditionals');
-
 $yellowStyle = new Style(false, true);
 $yellowStyle->getFill()
     ->setFillType(Fill::FILL_SOLID)
     ->getEndColor()->setARGB(Color::COLOR_YELLOW);
-$yellowStyle->getNumberFormat()->setFormatCode('ddd dd-mmm-yyyy');
+$yellowStyle->getFont()->setColor(new Color(Color::COLOR_BLUE));
 
 // Set conditional formatting rules and styles
 $helper->log('Define conditional formatting and set styles');

samples/ConditionalFormatting/06_Duplicate_Comparisons.php

@@ -51,14 +51,16 @@ $spreadsheet->getActiveSheet()->getStyle('A1:C1')->getFont()->setBold(true);
 
 // Define some styles for our Conditionals
 $helper->log('Define some styles for our Conditionals');
-$greenStyle = new Style(false, true);
-$greenStyle->getFill()
-    ->setFillType(Fill::FILL_SOLID)
-    ->getEndColor()->setARGB(Color::COLOR_GREEN);
 $yellowStyle = new Style(false, true);
 $yellowStyle->getFill()
     ->setFillType(Fill::FILL_SOLID)
     ->getEndColor()->setARGB(Color::COLOR_YELLOW);
+$yellowStyle->getFont()->setColor(new Color(Color::COLOR_BLUE));
+$greenStyle = new Style(false, true);
+$greenStyle->getFill()
+    ->setFillType(Fill::FILL_SOLID)
+    ->getEndColor()->setARGB(Color::COLOR_GREEN);
+$greenStyle->getFont()->setColor(new Color(Color::COLOR_DARKRED));
 
 // Set conditional formatting rules and styles
 $helper->log('Define conditional formatting and set styles');

samples/ConditionalFormatting/07_Expression_Comparisons.php

@@ -28,6 +28,7 @@ $helper->log('Add data');
 $spreadsheet->setActiveSheetIndex(0);
 $spreadsheet->getActiveSheet()
     ->setCellValue('A1', 'Odd/Even Expression Comparison')
+    ->setCellValue('A4', 'Note that these functions are not available for Xls files')
     ->setCellValue('A15', 'Sales Grid Expression Comparison')
     ->setCellValue('A25', 'Sales Grid Multiple Expression Comparison');
 
@@ -69,14 +70,16 @@ $spreadsheet->getActiveSheet()->getStyle('A25:D26')->getFont()->setBold(true);
 
 // Define some styles for our Conditionals
 $helper->log('Define some styles for our Conditionals');
-$greenStyle = new Style(false, true);
-$greenStyle->getFill()
-    ->setFillType(Fill::FILL_SOLID)
-    ->getEndColor()->setARGB(Color::COLOR_GREEN);
 $yellowStyle = new Style(false, true);
 $yellowStyle->getFill()
     ->setFillType(Fill::FILL_SOLID)
     ->getEndColor()->setARGB(Color::COLOR_YELLOW);
+$yellowStyle->getFont()->setColor(new Color(Color::COLOR_BLUE));
+$greenStyle = new Style(false, true);
+$greenStyle->getFill()
+    ->setFillType(Fill::FILL_SOLID)
+    ->getEndColor()->setARGB(Color::COLOR_GREEN);
+$greenStyle->getFont()->setColor(new Color(Color::COLOR_DARKRED));
 
 $greenStyleMoney = clone $greenStyle;
 $greenStyleMoney->getNumberFormat()->setFormatCode(NumberFormat::FORMAT_ACCOUNTING_USD);

src/PhpSpreadsheet/Style/ConditionalFormatting/Wizard/WizardAbstract.php

@@ -122,7 +122,7 @@ abstract class WizardAbstract
         return "{$worksheet}{$column}{$row}";
     }
 
-    protected static function reverseAdjustCellRef(string $condition, string $cellRange): string
+    public static function reverseAdjustCellRef(string $condition, string $cellRange): string
     {
         $conditionalRange = Coordinate::splitRange(str_replace('$', '', strtoupper($cellRange)));
         [$referenceCell] = $conditionalRange[0];

src/PhpSpreadsheet/Writer/Xls/ConditionalHelper.php

@@ -0,0 +1,76 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheet\Writer\Xls;
+
+use PhpOffice\PhpSpreadsheet\Exception as PhpSpreadsheetException;
+use PhpOffice\PhpSpreadsheet\Style\ConditionalFormatting\Wizard;
+
+class ConditionalHelper
+{
+    /**
+     * Formula parser.
+     *
+     * @var Parser
+     */
+    protected $parser;
+
+    /**
+     * @var mixed
+     */
+    protected $condition;
+
+    /**
+     * @var string
+     */
+    protected $cellRange;
+
+    /**
+     * @var null|string
+     */
+    protected $tokens;
+
+    /**
+     * @var int
+     */
+    protected $size;
+
+    public function __construct(Parser $parser)
+    {
+        $this->parser = $parser;
+    }
+
+    /**
+     * @param mixed $condition
+     */
+    public function processCondition($condition, string $cellRange): void
+    {
+        $this->condition = $condition;
+        $this->cellRange = $cellRange;
+
+        if (is_int($condition) || is_float($condition)) {
+            $this->size = ($condition <= 65535 ? 3 : 0x0000);
+            $this->tokens = pack('Cv', 0x1E, $condition);
+        } else {
+            try {
+                $formula = Wizard\WizardAbstract::reverseAdjustCellRef((string) $condition, $cellRange);
+                $this->parser->parse($formula);
+                $this->tokens = $this->parser->toReversePolish();
+                $this->size = strlen($this->tokens ?? '');
+            } catch (PhpSpreadsheetException $e) {
+                // In the event of a parser error with a formula value, we set the expression to ptgInt + 0
+                $this->tokens = pack('Cv', 0x1E, 0);
+                $this->size = 3;
+            }
+        }
+    }
+
+    public function tokens(): ?string
+    {
+        return $this->tokens;
+    }
+
+    public function size(): int
+    {
+        return $this->size;
+    }
+}

src/PhpSpreadsheet/Writer/Xls/Worksheet.php

@@ -539,37 +539,44 @@ class Worksheet extends BIFFwriter
         $this->writeSheetProtection();
         $this->writeRangeProtection();
 
-        $arrConditionalStyles = $phpSheet->getConditionalStylesCollection();
+        // Write Conditional Formatting Rules and Styles
+        $this->writeConditionalFormatting();
+
+        $this->storeEof();
+    }
+
+    private function writeConditionalFormatting(): void
+    {
+        $conditionalFormulaHelper = new ConditionalHelper($this->parser);
+
+        $arrConditionalStyles = $this->phpSheet->getConditionalStylesCollection();
         if (!empty($arrConditionalStyles)) {
             $arrConditional = [];
 
-            $cfHeaderWritten = false;
             // Write ConditionalFormattingTable records
             foreach ($arrConditionalStyles as $cellCoordinate => $conditionalStyles) {
+                $cfHeaderWritten = false;
                 foreach ($conditionalStyles as $conditional) {
                     /** @var Conditional $conditional */
                     if (
-                        $conditional->getConditionType() == Conditional::CONDITION_EXPRESSION ||
+                        $conditional->getConditionType() === Conditional::CONDITION_EXPRESSION ||
-                        $conditional->getConditionType() == Conditional::CONDITION_CELLIS
+                        $conditional->getConditionType() === Conditional::CONDITION_CELLIS
                     ) {
                         // Write CFHEADER record (only if there are Conditional Styles that we are able to write)
                         if ($cfHeaderWritten === false) {
-                            $this->writeCFHeader();
+                            $cfHeaderWritten = $this->writeCFHeader($cellCoordinate, $conditionalStyles);
-                            $cfHeaderWritten = true;
                         }
-                        if (!isset($arrConditional[$conditional->getHashCode()])) {
+                        if ($cfHeaderWritten === true && !isset($arrConditional[$conditional->getHashCode()])) {
                             // This hash code has been handled
                             $arrConditional[$conditional->getHashCode()] = true;
 
                             // Write CFRULE record
-                            $this->writeCFRule($conditional);
+                            $this->writeCFRule($conditionalFormulaHelper, $conditional, $cellCoordinate);
                         }
                     }
                 }
             }
         }
-
-        $this->storeEof();
     }
 
     /**
@@ -2774,8 +2781,11 @@ class Worksheet extends BIFFwriter
     /**
      * Write CFRule Record.
      */
-    private function writeCFRule(Conditional $conditional): void
+    private function writeCFRule(
-    {
+        ConditionalHelper $conditionalFormulaHelper,
+        Conditional $conditional,
+        string $cellRange
+    ): void {
         $record = 0x01B1; // Record identifier
         $type = null; // Type of the CF
         $operatorType = null; // Comparison operator
@@ -2827,21 +2837,23 @@ class Worksheet extends BIFFwriter
         // $szValue2 : size of the formula data for second value or formula
         $arrConditions = $conditional->getConditions();
         $numConditions = count($arrConditions);
-        if ($numConditions == 1) {
+
-            $szValue1 = ($arrConditions[0] <= 65535 ? 3 : 0x0000);
-            $szValue2 = 0x0000;
-            $operand1 = pack('Cv', 0x1E, $arrConditions[0]);
-            $operand2 = null;
-        } elseif ($numConditions == 2 && ($conditional->getOperatorType() == Conditional::OPERATOR_BETWEEN)) {
-            $szValue1 = ($arrConditions[0] <= 65535 ? 3 : 0x0000);
-            $szValue2 = ($arrConditions[1] <= 65535 ? 3 : 0x0000);
-            $operand1 = pack('Cv', 0x1E, $arrConditions[0]);
-            $operand2 = pack('Cv', 0x1E, $arrConditions[1]);
-        } else {
         $szValue1 = 0x0000;
         $szValue2 = 0x0000;
         $operand1 = null;
         $operand2 = null;
+
+        if ($numConditions === 1) {
+            $conditionalFormulaHelper->processCondition($arrConditions[0], $cellRange);
+            $szValue1 = $conditionalFormulaHelper->size();
+            $operand1 = $conditionalFormulaHelper->tokens();
+        } elseif ($numConditions === 2 && ($conditional->getOperatorType() === Conditional::OPERATOR_BETWEEN)) {
+            $conditionalFormulaHelper->processCondition($arrConditions[0], $cellRange);
+            $szValue1 = $conditionalFormulaHelper->size();
+            $operand1 = $conditionalFormulaHelper->tokens();
+            $conditionalFormulaHelper->processCondition($arrConditions[1], $cellRange);
+            $szValue2 = $conditionalFormulaHelper->size();
+            $operand2 = $conditionalFormulaHelper->tokens();
         }
 
         // $flags : Option flags
@@ -3127,8 +3139,10 @@ class Worksheet extends BIFFwriter
 
     /**
      * Write CFHeader record.
+     *
+     * @param Conditional[] $conditionalStyles
      */
-    private function writeCFHeader(): void
+    private function writeCFHeader(string $cellCoordinate, array $conditionalStyles): bool
     {
         $record = 0x01B0; // Record identifier
         $length = 0x0016; // Bytes to follow
@@ -3137,13 +3151,9 @@ class Worksheet extends BIFFwriter
         $numColumnMax = null;
         $numRowMin = null;
         $numRowMax = null;
+
         $arrConditional = [];
-        foreach ($this->phpSheet->getConditionalStylesCollection() as $cellCoordinate => $conditionalStyles) {
         foreach ($conditionalStyles as $conditional) {
-                if (
-                    $conditional->getConditionType() == Conditional::CONDITION_EXPRESSION ||
-                    $conditional->getConditionType() == Conditional::CONDITION_CELLIS
-                ) {
             if (!in_array($conditional->getHashCode(), $arrConditional)) {
                 $arrConditional[] = $conditional->getHashCode();
             }
@@ -3162,8 +3172,11 @@ class Worksheet extends BIFFwriter
                 $numRowMax = (int) $rangeCoordinates[1][1];
             }
         }
+
+        if (count($arrConditional) === 0) {
+            return false;
         }
-        }
+
         $needRedraw = 1;
         $cellRange = pack('vvvv', $numRowMin - 1, $numRowMax - 1, $numColumnMin - 1, $numColumnMax - 1);
 
@@ -3173,6 +3186,8 @@ class Worksheet extends BIFFwriter
         $data .= pack('v', 0x0001);
         $data .= $cellRange;
         $this->append($header . $data);
+
+        return true;
     }
 
     private function getDataBlockProtection(Conditional $conditional): int