Skip to content

Commit e404389

Browse files
oleibmanoleibman
committed
Change Style Without Affecting Current Cell/Sheet, and Invalid Formulas
Fix #1310, which was closed as stale in 2020, but which I will now reopen. Supersedes PR #1311 (@jaiminmoslake7020), from which I will remove the stale label but leave closed. The issue and the PR were too limited - they detected that the use of two equal signs at the start of a string made for an invalid formula, but there are variations, trivial and otherwise, which might also be detected. Using `setValue` with a string which starts with an equal sign will now attempt to parse (not evaluate) the formula; for certain situations in which the parser throws an exception, the string will be treated as a string rather than a formula. An example where it will still be treated as a formula is a 3D range reference, where the problem is not that it can't be parsed, but rather that the formula isn't supported (see unit test Calculation/Engine/RangeTest::test3dRangeEvaluation). Allowing such a formula might cause problems later on, but that is already what happens. A string beginning with an equal sign but which isn't treated as a formula will automatically set the `quotePrefix` attribute to `true`; all other `setValue` attempts will set it to `false`. This avoids the problem of a lingering value causing problems later on. It has long been a matter of discontent that setting a style can change the selected cells. A new method is added to `Worksheet`: ```php applyStylesFromArray(string $coordinate, array $styleArray) ``` This will attempt to guarantee that the active sheet in the current spreadsheet, and the selected cells in the current worksheet, remain undisturbed after the call. The setting of `quotePrefix` above is the first use of the new method.
1 parent 318a82e commit e404389

File tree

9 files changed

+126
-18
lines changed

9 files changed

+126
-18
lines changed

‎src/PhpSpreadsheet/Calculation/Calculation.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4065,7 +4065,7 @@ private function internalParseFormula(string $formula, ?Cell $cell = null): bool
40654065
$opCharacter = $formula[$index]; // Get the first character of the value at the current index position
40664066

40674067
// Check for two-character operators (e.g. >=, <=, <>)
4068-
if ((isset(self::$comparisonOperators[$opCharacter])) && (strlen($formula) > $index) && (isset(self::$comparisonOperators[$formula[$index + 1]]))) {
4068+
if ((isset(self::$comparisonOperators[$opCharacter])) && (strlen($formula) > $index) && isset($formula[$index + 1], self::$comparisonOperators[$formula[$index + 1]])) {
40694069
$opCharacter .= $formula[++$index];
40704070
}
40714071
// Find out if we're currently at the beginning of a number, variable, cell/row/column reference,

‎src/PhpSpreadsheet/Cell/Cell.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,7 @@ public function setValue(mixed $value, ?IValueBinder $binder = null): self
248248
public function setValueExplicit(mixed $value, string $dataType = DataType::TYPE_STRING): self
249249
{
250250
$oldValue = $this->value;
251+
$quotePrefix = false;
251252

252253
// set the value according to data type
253254
switch ($dataType) {
@@ -260,6 +261,10 @@ public function setValueExplicit(mixed $value, string $dataType = DataType::TYPE
260261
// no break
261262
case DataType::TYPE_STRING:
262263
// Synonym for string
264+
if (is_string($value) && strlen($value) > 1 && $value[0] === '=') {
265+
$quotePrefix = true;
266+
}
267+
// no break
263268
case DataType::TYPE_INLINE:
264269
// Rich text
265270
$this->value = DataType::checkString($value);
@@ -299,6 +304,7 @@ public function setValueExplicit(mixed $value, string $dataType = DataType::TYPE
299304
$this->updateInCollection();
300305
$cellCoordinate = $this->getCoordinate();
301306
self::updateIfCellIsTableHeader($this->getParent()?->getParent(), $this, $oldValue, $value);
307+
$this->getWorksheet()->applyStylesFromArray($cellCoordinate, ['quotePrefix' => $quotePrefix]);
302308

303309
return $this->getParent()?->get($cellCoordinate) ?? $this;
304310
}

‎src/PhpSpreadsheet/Cell/DefaultValueBinder.php

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
namespace PhpOffice\PhpSpreadsheet\Cell;
44

55
use DateTimeInterface;
6+
use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
7+
use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalculationException;
68
use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException;
79
use PhpOffice\PhpSpreadsheet\RichText\RichText;
810
use PhpOffice\PhpSpreadsheet\Shared\StringHelper;
@@ -46,17 +48,40 @@ public static function dataTypeForValue(mixed $value): string
4648
// Match the value against a few data types
4749
if ($value === null) {
4850
return DataType::TYPE_NULL;
49-
} elseif (is_float($value) || is_int($value)) {
51+
}
52+
if (is_float($value) || is_int($value)) {
5053
return DataType::TYPE_NUMERIC;
51-
} elseif (is_bool($value)) {
54+
}
55+
if (is_bool($value)) {
5256
return DataType::TYPE_BOOL;
53-
} elseif ($value === '') {
57+
}
58+
if ($value === '') {
5459
return DataType::TYPE_STRING;
55-
} elseif ($value instanceof RichText) {
60+
}
61+
if ($value instanceof RichText) {
5662
return DataType::TYPE_INLINE;
57-
} elseif (is_string($value) && strlen($value) > 1 && $value[0] === '=') {
63+
}
64+
if (is_string($value) && strlen($value) > 1 && $value[0] === '=') {
65+
$calculation = new Calculation();
66+
$calculation->disableBranchPruning();
67+
68+
try {
69+
if (empty($calculation->parseFormula($value))) {
70+
return DataType::TYPE_STRING;
71+
}
72+
} catch (CalculationException $e) {
73+
$message = $e->getMessage();
74+
if (
75+
$message === 'Formula Error: An unexpected error occurred'
76+
|| str_contains($message, 'has no operands')
77+
) {
78+
return DataType::TYPE_STRING;
79+
}
80+
}
81+
5882
return DataType::TYPE_FORMULA;
59-
} elseif (preg_match('/^[\+\-]?(\d+\\.?\d*|\d*\\.?\d+)([Ee][\-\+]?[0-2]?\d{1,3})?$/', $value)) {
83+
}
84+
if (preg_match('/^[\+\-]?(\d+\\.?\d*|\d*\\.?\d+)([Ee][\-\+]?[0-2]?\d{1,3})?$/', $value)) {
6085
$tValue = ltrim($value, '+-');
6186
if (is_string($value) && strlen($tValue) > 1 && $tValue[0] === '0' && $tValue[1] !== '.') {
6287
return DataType::TYPE_STRING;
@@ -67,7 +92,8 @@ public static function dataTypeForValue(mixed $value): string
6792
}
6893

6994
return DataType::TYPE_NUMERIC;
70-
} elseif (is_string($value)) {
95+
}
96+
if (is_string($value)) {
7197
$errorCodes = DataType::getErrorCodes();
7298
if (isset($errorCodes[$value])) {
7399
return DataType::TYPE_ERROR;

‎src/PhpSpreadsheet/Cell/StringValueBinder.php

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
use PhpOffice\PhpSpreadsheet\Shared\StringHelper;
99
use Stringable;
1010

11-
class StringValueBinder implements IValueBinder
11+
class StringValueBinder extends DefaultValueBinder implements IValueBinder
1212
{
1313
protected bool $convertNull = true;
1414

@@ -87,12 +87,9 @@ public function bindValue(Cell $cell, mixed $value): bool
8787
$cell->setValueExplicit($value, DataType::TYPE_BOOL);
8888
} elseif ((is_int($value) || is_float($value)) && $this->convertNumeric === false) {
8989
$cell->setValueExplicit($value, DataType::TYPE_NUMERIC);
90-
} elseif (is_string($value) && strlen($value) > 1 && $value[0] === '=' && $this->convertFormula === false) {
90+
} elseif (is_string($value) && strlen($value) > 1 && $value[0] === '=' && $this->convertFormula === false && parent::dataTypeForValue($value) === DataType::TYPE_FORMULA) {
9191
$cell->setValueExplicit($value, DataType::TYPE_FORMULA);
9292
} else {
93-
if (is_string($value) && strlen($value) > 1 && $value[0] === '=') {
94-
$cell->getStyle()->setQuotePrefix(true);
95-
}
9693
$cell->setValueExplicit((string) $value, DataType::TYPE_STRING);
9794
}
9895

‎src/PhpSpreadsheet/Worksheet/Worksheet.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3673,4 +3673,19 @@ public function copyCells(string $fromCell, string $toCells, bool $copyStyle = t
36733673
}
36743674
}
36753675
}
3676+
3677+
public function applyStylesFromArray(string $coordinate, array $styleArray): bool
3678+
{
3679+
$spreadsheet = $this->parent;
3680+
if ($spreadsheet === null) {
3681+
return false;
3682+
}
3683+
$activeSheetIndex = $spreadsheet->getActiveSheetIndex();
3684+
$originalSelected = $this->selectedCells;
3685+
$this->getStyle($coordinate)->applyFromArray($styleArray);
3686+
$this->selectedCells = $originalSelected;
3687+
$spreadsheet->setActiveSheetIndex($activeSheetIndex);
3688+
3689+
return true;
3690+
}
36763691
}

‎tests/PhpSpreadsheetTests/Calculation/Engine/RangeTest.php

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
namespace PhpOffice\PhpSpreadsheetTests\Calculation\Engine;
66

7+
use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
78
use PhpOffice\PhpSpreadsheet\Calculation\Information\ExcelError;
89
use PhpOffice\PhpSpreadsheet\NamedRange;
910
use PhpOffice\PhpSpreadsheet\Spreadsheet;
@@ -13,20 +14,31 @@ class RangeTest extends TestCase
1314
{
1415
private string $incompleteMessage = 'Must be revisited';
1516

16-
private Spreadsheet $spreadSheet;
17+
private ?Spreadsheet $spreadSheet = null;
1718

18-
protected function setUp(): void
19+
protected function getSpreadsheet(): Spreadsheet
1920
{
20-
$this->spreadSheet = new Spreadsheet();
21-
$this->spreadSheet->getActiveSheet()
21+
$spreadsheet = new Spreadsheet();
22+
$spreadsheet->getActiveSheet()
2223
->fromArray(array_chunk(range(1, 240), 6), null, 'A1', true);
24+
25+
return $spreadsheet;
26+
}
27+
28+
protected function tearDown(): void
29+
{
30+
if ($this->spreadSheet !== null) {
31+
$this->spreadSheet->disconnectWorksheets();
32+
$this->spreadSheet = null;
33+
}
2334
}
2435

2536
/**
2637
* @dataProvider providerRangeEvaluation
2738
*/
2839
public function testRangeEvaluation(string $formula, int|string $expectedResult): void
2940
{
41+
$this->spreadSheet = $this->getSpreadsheet();
3042
$workSheet = $this->spreadSheet->getActiveSheet();
3143
$workSheet->setCellValue('H1', $formula);
3244

@@ -64,8 +76,20 @@ public static function providerRangeEvaluation(): array
6476
];
6577
}
6678

79+
public function test3dRangeParsing(): void
80+
{
81+
// This test shows that parsing throws exception.
82+
// Next test shows that formula is still treated as a formula
83+
// despite the parse failure.
84+
$this->expectExceptionMessage('3D Range references are not yet supported');
85+
$calculation = new Calculation();
86+
$calculation->disableBranchPruning();
87+
$calculation->parseFormula('=SUM(Worksheet!A1:Worksheet2!B3');
88+
}
89+
6790
public function test3dRangeEvaluation(): void
6891
{
92+
$this->spreadSheet = $this->getSpreadsheet();
6993
$workSheet = $this->spreadSheet->getActiveSheet();
7094
$workSheet->setCellValue('E1', '=SUM(Worksheet!A1:Worksheet2!B3)');
7195

@@ -78,6 +102,7 @@ public function test3dRangeEvaluation(): void
78102
*/
79103
public function testNamedRangeEvaluation(array $ranges, string $formula, int $expectedResult): void
80104
{
105+
$this->spreadSheet = $this->getSpreadsheet();
81106
$workSheet = $this->spreadSheet->getActiveSheet();
82107
foreach ($ranges as $id => $range) {
83108
$this->spreadSheet->addNamedRange(new NamedRange('GROUP' . ++$id, $workSheet, $range));
@@ -116,6 +141,7 @@ public static function providerNamedRangeEvaluation(): array
116141
*/
117142
public function testUTF8NamedRangeEvaluation(array $names, array $ranges, string $formula, int $expectedResult): void
118143
{
144+
$this->spreadSheet = $this->getSpreadsheet();
119145
$workSheet = $this->spreadSheet->getActiveSheet();
120146
foreach ($names as $index => $name) {
121147
$range = $ranges[$index];
@@ -144,6 +170,7 @@ public function testCompositeNamedRangeEvaluation(string $composite, int $expect
144170
if ($this->incompleteMessage !== '') {
145171
self::markTestIncomplete($this->incompleteMessage);
146172
}
173+
$this->spreadSheet = $this->getSpreadsheet();
147174

148175
$workSheet = $this->spreadSheet->getActiveSheet();
149176
$this->spreadSheet->addNamedRange(new NamedRange('COMPOSITE', $workSheet, $composite));

‎tests/PhpSpreadsheetTests/Cell/AdvancedValueBinderTest.php

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
use PhpOffice\PhpSpreadsheet\Cell\AdvancedValueBinder;
88
use PhpOffice\PhpSpreadsheet\Cell\Cell;
9+
use PhpOffice\PhpSpreadsheet\Cell\DataType;
910
use PhpOffice\PhpSpreadsheet\Cell\IValueBinder;
1011
use PhpOffice\PhpSpreadsheet\Settings;
1112
use PhpOffice\PhpSpreadsheet\Shared\StringHelper;
@@ -232,4 +233,31 @@ public static function stringProvider(): array
232233
["Hello\nWorld", true],
233234
];
234235
}
236+
237+
/**
238+
* @dataProvider formulaProvider
239+
*/
240+
public function testFormula(string $value, string $dataType): void
241+
{
242+
$spreadsheet = new Spreadsheet();
243+
$sheet = $spreadsheet->getActiveSheet();
244+
245+
$sheet->getCell('A1')->setValue($value);
246+
self::assertSame($dataType, $sheet->getCell('A1')->getDataType());
247+
if ($dataType === DataType::TYPE_FORMULA) {
248+
self::assertFalse($sheet->getStyle('A1')->getQuotePrefix());
249+
} else {
250+
self::assertTrue($sheet->getStyle('A1')->getQuotePrefix());
251+
}
252+
253+
$spreadsheet->disconnectWorksheets();
254+
}
255+
256+
public static function formulaProvider(): array
257+
{
258+
return [
259+
'normal formula' => ['=SUM(A1:C3)', DataType::TYPE_FORMULA],
260+
'issue 1310' => ['======', DataType::TYPE_STRING],
261+
];
262+
}
235263
}

‎tests/PhpSpreadsheetTests/Cell/StringValueBinderTest.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,13 +211,19 @@ public function testStringValueBinderSuppressFormulaConversion(
211211
$cell->setValue($value);
212212
self::assertSame($expectedValue, $cell->getValue());
213213
self::assertSame($expectedDataType, $cell->getDataType());
214+
if ($expectedDataType === DataType::TYPE_FORMULA) {
215+
self::assertFalse($sheet->getStyle('A1')->getQuotePrefix());
216+
} else {
217+
self::assertTrue($sheet->getStyle('A1')->getQuotePrefix());
218+
}
214219
$spreadsheet->disconnectWorksheets();
215220
}
216221

217222
public static function providerDataValuesSuppressFormulaConversion(): array
218223
{
219224
return [
220-
['=SUM(A1:C3)', '=SUM(A1:C3)', DataType::TYPE_FORMULA, false],
225+
'normal formula' => ['=SUM(A1:C3)', '=SUM(A1:C3)', DataType::TYPE_FORMULA],
226+
'issue 1310' => ['======', '======', DataType::TYPE_STRING],
221227
];
222228
}
223229

‎tests/data/Cell/DefaultValueBinder.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,4 +83,7 @@
8383
's',
8484
'1234567890123459012345689012345690',
8585
],
86+
'Issue 1310 Multiple = at start' => ['s', '======'],
87+
'Issue 1310 Variant 1' => ['s', '= ====='],
88+
'Issue 1310 Variant 2' => ['s', '=2*3='],
8689
];

0 commit comments

Comments
 (0)