Skip to content

Commit

Permalink
Merge pull request #4073 from oleibman/issue1310
Browse files Browse the repository at this point in the history
Change Style Without Affecting Current Cell/Sheet, and Invalid Formulas
  • Loading branch information
oleibman authored Jul 6, 2024
2 parents 1b68270 + 2a7cbab commit 06737c1
Show file tree
Hide file tree
Showing 10 changed files with 114 additions and 11 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org).
### Added

- Xlsx Reader Optionally Ignore Rows With No Cells. [Issue #3982](https://github.com/PHPOffice/PhpSpreadsheet/issues/3982) [PR #4035](https://github.com/PHPOffice/PhpSpreadsheet/pull/4035)
- Means to change style without affecting current cell/sheet. [PR #4073](https://github.com/PHPOffice/PhpSpreadsheet/pull/4073)
- Option for CSV output file to have varying numbers of columns for each row. [Issue #1415](https://github.com/PHPOffice/PhpSpreadsheet/issues/1415) [PR #4076](https://github.com/PHPOffice/PhpSpreadsheet/pull/4076)

### Changed
Expand Down Expand Up @@ -38,6 +39,7 @@ and this project adheres to [Semantic Versioning](https://semver.org).
- Problem rendering line chart with missing plot label. [PR #4074](https://github.com/PHPOffice/PhpSpreadsheet/pull/4074)
- More RTL in Xlsx/Html Comments [Issue #4004](https://github.com/PHPOffice/PhpSpreadsheet/issues/4004) [PR #4065](https://github.com/PHPOffice/PhpSpreadsheet/pull/4065)
- Empty String in sharedStrings. [Issue #4063](https://github.com/PHPOffice/PhpSpreadsheet/issues/4063) [PR #4064](https://github.com/PHPOffice/PhpSpreadsheet/pull/4064)
- Treat invalid formulas as strings. [Issue #1310](https://github.com/PHPOffice/PhpSpreadsheet/issues/1310) [PR #4073](https://github.com/PHPOffice/PhpSpreadsheet/pull/4073)

## 2024-05-11 - 2.1.0

Expand Down
2 changes: 1 addition & 1 deletion src/PhpSpreadsheet/Calculation/Calculation.php
Original file line number Diff line number Diff line change
Expand Up @@ -4065,7 +4065,7 @@ private function internalParseFormula(string $formula, ?Cell $cell = null): bool
$opCharacter = $formula[$index]; // Get the first character of the value at the current index position

// Check for two-character operators (e.g. >=, <=, <>)
if ((isset(self::$comparisonOperators[$opCharacter])) && (strlen($formula) > $index) && (isset(self::$comparisonOperators[$formula[$index + 1]]))) {
if ((isset(self::$comparisonOperators[$opCharacter])) && (strlen($formula) > $index) && isset($formula[$index + 1], self::$comparisonOperators[$formula[$index + 1]])) {
$opCharacter .= $formula[++$index];
}
// Find out if we're currently at the beginning of a number, variable, cell/row/column reference,
Expand Down
6 changes: 6 additions & 0 deletions src/PhpSpreadsheet/Cell/Cell.php
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ public function setValue(mixed $value, ?IValueBinder $binder = null): self
public function setValueExplicit(mixed $value, string $dataType = DataType::TYPE_STRING): self
{
$oldValue = $this->value;
$quotePrefix = false;

// set the value according to data type
switch ($dataType) {
Expand All @@ -267,6 +268,10 @@ public function setValueExplicit(mixed $value, string $dataType = DataType::TYPE
// no break
case DataType::TYPE_STRING:
// Synonym for string
if (is_string($value) && strlen($value) > 1 && $value[0] === '=') {
$quotePrefix = true;
}
// no break
case DataType::TYPE_INLINE:
// Rich text
if ($value !== null && !is_scalar($value) && !($value instanceof Stringable)) {
Expand Down Expand Up @@ -312,6 +317,7 @@ public function setValueExplicit(mixed $value, string $dataType = DataType::TYPE
$this->updateInCollection();
$cellCoordinate = $this->getCoordinate();
self::updateIfCellIsTableHeader($this->getParent()?->getParent(), $this, $oldValue, $value);
$this->getWorksheet()->applyStylesFromArray($cellCoordinate, ['quotePrefix' => $quotePrefix]);

return $this->getParent()?->get($cellCoordinate) ?? $this;
}
Expand Down
19 changes: 19 additions & 0 deletions src/PhpSpreadsheet/Cell/DefaultValueBinder.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
namespace PhpOffice\PhpSpreadsheet\Cell;

use DateTimeInterface;
use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalculationException;
use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException;
use PhpOffice\PhpSpreadsheet\RichText\RichText;
use PhpOffice\PhpSpreadsheet\Shared\StringHelper;
Expand Down Expand Up @@ -68,6 +70,23 @@ public static function dataTypeForValue(mixed $value): string
throw new SpreadsheetException("unusable type $gettype");
}
if (strlen($value) > 1 && $value[0] === '=') {
$calculation = new Calculation();
$calculation->disableBranchPruning();

try {
if (empty($calculation->parseFormula($value))) {
return DataType::TYPE_STRING;
}
} catch (CalculationException $e) {
$message = $e->getMessage();
if (
$message === 'Formula Error: An unexpected error occurred'
|| str_contains($message, 'has no operands')
) {
return DataType::TYPE_STRING;
}
}

return DataType::TYPE_FORMULA;
}
if (preg_match('/^[\+\-]?(\d+\\.?\d*|\d*\\.?\d+)([Ee][\-\+]?[0-2]?\d{1,3})?$/', $value)) {
Expand Down
7 changes: 2 additions & 5 deletions src/PhpSpreadsheet/Cell/StringValueBinder.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
use PhpOffice\PhpSpreadsheet\Shared\StringHelper;
use Stringable;

class StringValueBinder implements IValueBinder
class StringValueBinder extends DefaultValueBinder implements IValueBinder
{
protected bool $convertNull = true;

Expand Down Expand Up @@ -87,12 +87,9 @@ public function bindValue(Cell $cell, mixed $value): bool
$cell->setValueExplicit($value, DataType::TYPE_BOOL);
} elseif ((is_int($value) || is_float($value)) && $this->convertNumeric === false) {
$cell->setValueExplicit($value, DataType::TYPE_NUMERIC);
} elseif (is_string($value) && strlen($value) > 1 && $value[0] === '=' && $this->convertFormula === false) {
} elseif (is_string($value) && strlen($value) > 1 && $value[0] === '=' && $this->convertFormula === false && parent::dataTypeForValue($value) === DataType::TYPE_FORMULA) {
$cell->setValueExplicit($value, DataType::TYPE_FORMULA);
} else {
if (is_string($value) && strlen($value) > 1 && $value[0] === '=') {
$cell->getStyle()->setQuotePrefix(true);
}
$cell->setValueExplicit((string) $value, DataType::TYPE_STRING);
}

Expand Down
15 changes: 15 additions & 0 deletions src/PhpSpreadsheet/Worksheet/Worksheet.php
Original file line number Diff line number Diff line change
Expand Up @@ -3673,4 +3673,19 @@ public function copyCells(string $fromCell, string $toCells, bool $copyStyle = t
}
}
}

public function applyStylesFromArray(string $coordinate, array $styleArray): bool
{
$spreadsheet = $this->parent;
if ($spreadsheet === null) {
return false;
}
$activeSheetIndex = $spreadsheet->getActiveSheetIndex();
$originalSelected = $this->selectedCells;
$this->getStyle($coordinate)->applyFromArray($styleArray);
$this->selectedCells = $originalSelected;
$spreadsheet->setActiveSheetIndex($activeSheetIndex);

return true;
}
}
35 changes: 31 additions & 4 deletions tests/PhpSpreadsheetTests/Calculation/Engine/RangeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace PhpOffice\PhpSpreadsheetTests\Calculation\Engine;

use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
use PhpOffice\PhpSpreadsheet\Calculation\Information\ExcelError;
use PhpOffice\PhpSpreadsheet\NamedRange;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
Expand All @@ -13,20 +14,31 @@ class RangeTest extends TestCase
{
private string $incompleteMessage = 'Must be revisited';

private Spreadsheet $spreadSheet;
private ?Spreadsheet $spreadSheet = null;

protected function setUp(): void
protected function getSpreadsheet(): Spreadsheet
{
$this->spreadSheet = new Spreadsheet();
$this->spreadSheet->getActiveSheet()
$spreadsheet = new Spreadsheet();
$spreadsheet->getActiveSheet()
->fromArray(array_chunk(range(1, 240), 6), null, 'A1', true);

return $spreadsheet;
}

protected function tearDown(): void
{
if ($this->spreadSheet !== null) {
$this->spreadSheet->disconnectWorksheets();
$this->spreadSheet = null;
}
}

/**
* @dataProvider providerRangeEvaluation
*/
public function testRangeEvaluation(string $formula, int|string $expectedResult): void
{
$this->spreadSheet = $this->getSpreadsheet();
$workSheet = $this->spreadSheet->getActiveSheet();
$workSheet->setCellValue('H1', $formula);

Expand Down Expand Up @@ -64,8 +76,20 @@ public static function providerRangeEvaluation(): array
];
}

public function test3dRangeParsing(): void
{
// This test shows that parsing throws exception.
// Next test shows that formula is still treated as a formula
// despite the parse failure.
$this->expectExceptionMessage('3D Range references are not yet supported');
$calculation = new Calculation();
$calculation->disableBranchPruning();
$calculation->parseFormula('=SUM(Worksheet!A1:Worksheet2!B3');
}

public function test3dRangeEvaluation(): void
{
$this->spreadSheet = $this->getSpreadsheet();
$workSheet = $this->spreadSheet->getActiveSheet();
$workSheet->setCellValue('E1', '=SUM(Worksheet!A1:Worksheet2!B3)');

Expand All @@ -78,6 +102,7 @@ public function test3dRangeEvaluation(): void
*/
public function testNamedRangeEvaluation(array $ranges, string $formula, int $expectedResult): void
{
$this->spreadSheet = $this->getSpreadsheet();
$workSheet = $this->spreadSheet->getActiveSheet();
foreach ($ranges as $id => $range) {
$this->spreadSheet->addNamedRange(new NamedRange('GROUP' . ++$id, $workSheet, $range));
Expand Down Expand Up @@ -116,6 +141,7 @@ public static function providerNamedRangeEvaluation(): array
*/
public function testUTF8NamedRangeEvaluation(array $names, array $ranges, string $formula, int $expectedResult): void
{
$this->spreadSheet = $this->getSpreadsheet();
$workSheet = $this->spreadSheet->getActiveSheet();
foreach ($names as $index => $name) {
$range = $ranges[$index];
Expand Down Expand Up @@ -144,6 +170,7 @@ public function testCompositeNamedRangeEvaluation(string $composite, int $expect
if ($this->incompleteMessage !== '') {
self::markTestIncomplete($this->incompleteMessage);
}
$this->spreadSheet = $this->getSpreadsheet();

$workSheet = $this->spreadSheet->getActiveSheet();
$this->spreadSheet->addNamedRange(new NamedRange('COMPOSITE', $workSheet, $composite));
Expand Down
28 changes: 28 additions & 0 deletions tests/PhpSpreadsheetTests/Cell/AdvancedValueBinderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use PhpOffice\PhpSpreadsheet\Cell\AdvancedValueBinder;
use PhpOffice\PhpSpreadsheet\Cell\Cell;
use PhpOffice\PhpSpreadsheet\Cell\DataType;
use PhpOffice\PhpSpreadsheet\Cell\IValueBinder;
use PhpOffice\PhpSpreadsheet\Settings;
use PhpOffice\PhpSpreadsheet\Shared\StringHelper;
Expand Down Expand Up @@ -232,4 +233,31 @@ public static function stringProvider(): array
["Hello\nWorld", true],
];
}

/**
* @dataProvider formulaProvider
*/
public function testFormula(string $value, string $dataType): void
{
$spreadsheet = new Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();

$sheet->getCell('A1')->setValue($value);
self::assertSame($dataType, $sheet->getCell('A1')->getDataType());
if ($dataType === DataType::TYPE_FORMULA) {
self::assertFalse($sheet->getStyle('A1')->getQuotePrefix());
} else {
self::assertTrue($sheet->getStyle('A1')->getQuotePrefix());
}

$spreadsheet->disconnectWorksheets();
}

public static function formulaProvider(): array
{
return [
'normal formula' => ['=SUM(A1:C3)', DataType::TYPE_FORMULA],
'issue 1310' => ['======', DataType::TYPE_STRING],
];
}
}
8 changes: 7 additions & 1 deletion tests/PhpSpreadsheetTests/Cell/StringValueBinderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -211,13 +211,19 @@ public function testStringValueBinderSuppressFormulaConversion(
$cell->setValue($value);
self::assertSame($expectedValue, $cell->getValue());
self::assertSame($expectedDataType, $cell->getDataType());
if ($expectedDataType === DataType::TYPE_FORMULA) {
self::assertFalse($sheet->getStyle('A1')->getQuotePrefix());
} else {
self::assertTrue($sheet->getStyle('A1')->getQuotePrefix());
}
$spreadsheet->disconnectWorksheets();
}

public static function providerDataValuesSuppressFormulaConversion(): array
{
return [
['=SUM(A1:C3)', '=SUM(A1:C3)', DataType::TYPE_FORMULA, false],
'normal formula' => ['=SUM(A1:C3)', '=SUM(A1:C3)', DataType::TYPE_FORMULA],
'issue 1310' => ['======', '======', DataType::TYPE_STRING],
];
}

Expand Down
3 changes: 3 additions & 0 deletions tests/data/Cell/DefaultValueBinder.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,4 +83,7 @@
's',
'1234567890123459012345689012345690',
],
'Issue 1310 Multiple = at start' => ['s', '======'],
'Issue 1310 Variant 1' => ['s', '= ====='],
'Issue 1310 Variant 2' => ['s', '=2*3='],
];

0 comments on commit 06737c1

Please sign in to comment.