Skip to content

Commit

Permalink
Handle Booleans in Conditional Styles (#2654)
Browse files Browse the repository at this point in the history
You can set up a conditional style to, say, apply to cells equal to boolean values. For such conditions, the Excel XML specifies `TRUE` or `FALSE`. It is noteworthy that false matches empty cells as well as FALSE, but not 0; similarly TRUE does not match 1. The Xlsx Writer just casts these values to string, which will not work properly. The Xlsx Reader treats the values as strings, so it won't work properly either. This PR corrects both. Also the doc blocks in Style/Conditional allow bool in some places, but not in others; these are corrected but no executable code is changed there.
  • Loading branch information
oleibman authored Mar 10, 2022
1 parent 0d5d9eb commit bd2e7b6
Show file tree
Hide file tree
Showing 6 changed files with 121 additions and 43 deletions.
30 changes: 0 additions & 30 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -4125,16 +4125,6 @@ parameters:
count: 1
path: src/PhpSpreadsheet/Spreadsheet.php

-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\Conditional\\:\\:\\$condition \\(array\\<string\\>\\) does not accept array\\<bool\\|float\\|int\\|string\\>\\.$#"
count: 1
path: src/PhpSpreadsheet/Style/Conditional.php

-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\Conditional\\:\\:\\$style \\(PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\Style\\) does not accept PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\Style\\|null\\.$#"
count: 1
path: src/PhpSpreadsheet/Style/Conditional.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\ConditionalFormatting\\\\ConditionalDataBar\\:\\:setConditionalFormattingRuleExt\\(\\) has no return type specified\\.$#"
count: 1
Expand Down Expand Up @@ -4295,11 +4285,6 @@ parameters:
count: 1
path: src/PhpSpreadsheet/Style/ConditionalFormatting/ConditionalFormattingRuleExtension.php

-
message: "#^Parameter \\#1 \\$conditions of method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\Conditional\\:\\:setConditions\\(\\) expects array\\<string\\>\\|bool\\|float\\|int\\|string, array\\<int, float\\|int\\|string\\> given\\.$#"
count: 1
path: src/PhpSpreadsheet/Style/ConditionalFormatting/Wizard/CellValue.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\DateFormatter\\:\\:escapeQuotesCallback\\(\\) has parameter \\$matches with no type specified\\.$#"
count: 1
Expand Down Expand Up @@ -5810,11 +5795,6 @@ parameters:
count: 1
path: tests/PhpSpreadsheetTests/Functional/CommentsTest.php

-
message: "#^Parameter \\#1 \\$condition of method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\Conditional\\:\\:addCondition\\(\\) expects string, float given\\.$#"
count: 2
path: tests/PhpSpreadsheetTests/Functional/ConditionalStopIfTrueTest.php

-
message: "#^Cannot call method getPageSetup\\(\\) on PhpOffice\\\\PhpSpreadsheet\\\\Worksheet\\\\Worksheet\\|null\\.$#"
count: 5
Expand Down Expand Up @@ -5960,16 +5940,6 @@ parameters:
count: 1
path: tests/PhpSpreadsheetTests/SpreadsheetTest.php

-
message: "#^Parameter \\#1 \\$condition of method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\Conditional\\:\\:addCondition\\(\\) expects string, float given\\.$#"
count: 2
path: tests/PhpSpreadsheetTests/Style/ConditionalTest.php

-
message: "#^Parameter \\#1 \\$conditions of method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\Conditional\\:\\:setConditions\\(\\) expects array\\<string\\>\\|bool\\|float\\|int\\|string, array\\<int, float\\> given\\.$#"
count: 1
path: tests/PhpSpreadsheetTests/Style/ConditionalTest.php

-
message: "#^Parameter \\#2 \\$value of method PhpOffice\\\\PhpSpreadsheet\\\\Worksheet\\\\AutoFilter\\\\Column\\:\\:setAttribute\\(\\) expects string, int given\\.$#"
count: 1
Expand Down
15 changes: 11 additions & 4 deletions src/PhpSpreadsheet/Reader/Xlsx/ConditionalStyles.php
Original file line number Diff line number Diff line change
Expand Up @@ -196,12 +196,19 @@ private function readStyleRules($cfRules, $extLst)
$objConditional->setStopIfTrue(true);
}

if (count($cfRule->formula) > 1) {
foreach ($cfRule->formula as $formula) {
$objConditional->addCondition((string) $formula);
if (count($cfRule->formula) >= 1) {
foreach ($cfRule->formula as $formulax) {
$formula = (string) $formulax;
if ($formula === 'TRUE') {
$objConditional->addCondition(true);
} elseif ($formula === 'FALSE') {
$objConditional->addCondition(false);
} else {
$objConditional->addCondition($formula);
}
}
} else {
$objConditional->addCondition((string) $cfRule->formula);
$objConditional->addCondition('');
}

if (isset($cfRule->dataBar)) {
Expand Down
10 changes: 5 additions & 5 deletions src/PhpSpreadsheet/Style/Conditional.php
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ class Conditional implements IComparable
/**
* Condition.
*
* @var string[]
* @var (bool|float|int|string)[]
*/
private $condition = [];

Expand Down Expand Up @@ -223,7 +223,7 @@ public function setStopIfTrue($stopIfTrue)
/**
* Get Conditions.
*
* @return string[]
* @return (bool|float|int|string)[]
*/
public function getConditions()
{
Expand All @@ -233,7 +233,7 @@ public function getConditions()
/**
* Set Conditions.
*
* @param bool|float|int|string|string[] $conditions Condition
* @param bool|float|int|string|(bool|float|int|string)[] $conditions Condition
*
* @return $this
*/
Expand All @@ -250,7 +250,7 @@ public function setConditions($conditions)
/**
* Add Condition.
*
* @param string $condition Condition
* @param bool|float|int|string $condition Condition
*
* @return $this
*/
Expand All @@ -276,7 +276,7 @@ public function getStyle()
*
* @return $this
*/
public function setStyle(?Style $style = null)
public function setStyle(Style $style)
{
$this->style = $style;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public static function fromConditional(Conditional $conditional, string $cellRan
$wizard = new self($cellRange);
$wizard->style = $conditional->getStyle();
$wizard->stopIfTrue = $conditional->getStopIfTrue();
$wizard->expression = self::reverseAdjustCellRef($conditional->getConditions()[0], $cellRange);
$wizard->expression = self::reverseAdjustCellRef((string) ($conditional->getConditions()[0]), $cellRange);

return $wizard;
}
Expand Down
9 changes: 6 additions & 3 deletions src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,10 @@ private static function writeOtherCondElements(XMLWriter $objWriter, Conditional
) {
foreach ($conditions as $formula) {
// Formula
$objWriter->writeElement('formula', Xlfn::addXlfn($formula));
if (is_bool($formula)) {
$formula = $formula ? 'TRUE' : 'FALSE';
}
$objWriter->writeElement('formula', Xlfn::addXlfn("$formula"));
}
} else {
if ($conditional->getConditionType() == Conditional::CONDITION_CONTAINSBLANKS) {
Expand Down Expand Up @@ -525,7 +528,7 @@ private static function writeTimePeriodCondElements(XMLWriter $objWriter, Condit
$objWriter->writeElement('formula', 'AND(MONTH(' . $cellCoordinate . ')=MONTH(EDATE(TODAY(),0+1)),YEAR(' . $cellCoordinate . ')=YEAR(EDATE(TODAY(),0+1)))');
}
} else {
$objWriter->writeElement('formula', $conditional->getConditions()[0]);
$objWriter->writeElement('formula', (string) ($conditional->getConditions()[0]));
}
}
}
Expand All @@ -546,7 +549,7 @@ private static function writeTextCondElements(XMLWriter $objWriter, Conditional
$objWriter->writeElement('formula', 'ISERROR(SEARCH("' . $txt . '",' . $cellCoordinate . '))');
}
} else {
$objWriter->writeElement('formula', $conditional->getConditions()[0]);
$objWriter->writeElement('formula', (string) ($conditional->getConditions()[0]));
}
}
}
Expand Down
98 changes: 98 additions & 0 deletions tests/PhpSpreadsheetTests/Style/ConditionalBoolTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
<?php

namespace PhpOffice\PhpSpreadsheetTests\Style;

use PhpOffice\PhpSpreadsheet\Reader\Xlsx as XlsxReader;
use PhpOffice\PhpSpreadsheet\Shared\File;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PhpOffice\PhpSpreadsheet\Style\Conditional;
use PhpOffice\PhpSpreadsheet\Style\Fill;
use PhpOffice\PhpSpreadsheet\Writer\Xlsx as XlsxWriter;
use PHPUnit\Framework\TestCase;

class ConditionalBoolTest extends TestCase
{
/** @var string */
private $outfile = '';

protected function tearDown(): void
{
if ($this->outfile !== '') {
unlink($this->outfile);
$this->outfile = '';
}
}

public function testBool(): void
{
$spreadsheet = new Spreadsheet();

$sheet = $spreadsheet->getActiveSheet();
$condition1 = new Conditional();
$condition1->setConditionType(Conditional::CONDITION_CELLIS);
$condition1->setOperatorType(Conditional::OPERATOR_EQUAL);
$condition1->addCondition(false);
$condition1->getStyle()->getFill()
->setFillType(Fill::FILL_SOLID)
->getEndColor()->setARGB('FFFFFF00');
$conditionalStyles = $sheet->getStyle('A1:A10')->getConditionalStyles();
$conditionalStyles[] = $condition1;
$sheet->getStyle('A1:A20')->setConditionalStyles($conditionalStyles);
$sheet->setCellValue('A1', 1);
$sheet->setCellValue('A2', true);
$sheet->setCellValue('A3', false);
$sheet->setCellValue('A4', 0.6);
$sheet->setCellValue('A6', 0);
$sheet->setSelectedCell('B1');

$sheet = $spreadsheet->createSheet();
$condition1 = new Conditional();
$condition1->setConditionType(Conditional::CONDITION_CELLIS);
$condition1->setOperatorType(Conditional::OPERATOR_EQUAL);
$condition1->addCondition(true);
$condition1->getStyle()->getFill()
->setFillType(Fill::FILL_SOLID)
->getEndColor()->setARGB('FF00FF00');
$conditionalStyles = $sheet->getStyle('A1:A10')->getConditionalStyles();
$conditionalStyles[] = $condition1;
$sheet->getStyle('A1:A20')->setConditionalStyles($conditionalStyles);
$sheet->setCellValue('A1', 1);
$sheet->setCellValue('A2', true);
$sheet->setCellValue('A3', false);
$sheet->setCellValue('A4', 0.6);
$sheet->setCellValue('A6', 0);
$sheet->setSelectedCell('B1');

$writer = new XlsxWriter($spreadsheet);
$this->outfile = File::temporaryFilename();
$writer->save($this->outfile);
$spreadsheet->disconnectWorksheets();

$file = 'zip://' . $this->outfile . '#xl/worksheets/sheet1.xml';
$contents = file_get_contents($file);
self::assertNotFalse($contents);
self::assertStringContainsString('<formula>FALSE</formula>', $contents);
$file = 'zip://' . $this->outfile . '#xl/worksheets/sheet2.xml';
$contents = file_get_contents($file);
self::assertNotFalse($contents);
self::assertStringContainsString('<formula>TRUE</formula>', $contents);

$reader = new XlsxReader();
$spreadsheet2 = $reader->load($this->outfile);
$sheet1 = $spreadsheet2->getSheet(0);
$condArray = $sheet1->getStyle('A1:A20')->getConditionalStyles();
self::assertNotEmpty($condArray);
$cond1 = $condArray[0];
self::assertSame(Conditional::CONDITION_CELLIS, $cond1->getConditionType());
self::assertSame(Conditional::OPERATOR_EQUAL, $cond1->getOperatorType());
self::assertFalse(($cond1->getConditions())[0]);
$sheet2 = $spreadsheet2->getSheet(1);
$condArray = $sheet2->getStyle('A1:A20')->getConditionalStyles();
self::assertNotEmpty($condArray);
$cond1 = $condArray[0];
self::assertSame(Conditional::CONDITION_CELLIS, $cond1->getConditionType());
self::assertSame(Conditional::OPERATOR_EQUAL, $cond1->getOperatorType());
self::assertTrue(($cond1->getConditions())[0]);
$spreadsheet2->disconnectWorksheets();
}
}

0 comments on commit bd2e7b6

Please sign in to comment.