From 4001c89aaa03e915de0b1cd24907ff21e473b9ba Mon Sep 17 00:00:00 2001 From: oleibman Date: Sun, 3 Oct 2021 10:04:48 -0700 Subject: [PATCH] isFormula Referencing Sheet With Space in Title (#2306) * isFormula Referencing Sheet With Space in Title See issue #2304. User sets a cell to `ISFORMULA(cell)`, where `cell` exists on a sheet whose title contains a space, and receives an error. Coordinates are not being passed correctly to Functions::isFormula; in particular, the sheet name is not enclosed in apostrophes, e.g. `Sheet Name!A1` rather than `'Sheet Name'!A1`. (Note that sheet name was not specified in Cell; PhpSpreadsheet adds it before calling isFormula.) Sheets without embedded spaces (or other non-word characters) are handled correctly with or without apostrophes, but spaces require surrounding apostrophes. As part of this investigation, I determined that Excel handles defined names and cell ranges in ISFORMULA (subject to spills), and that PhpSpreadsheet does not. It is changed to handle them. In the absence of spill support, it will use only the first cell in the range. Existing tests for ISFORMULA used mocking unneccesarily. They are moved to a separate test member, and mocking is no longer used. * PhpUnit and Jpgraph 35_Char_render.php had previously been a problem only for PHP8+. It is now a problem for PHP7.4, and will therefore be skipped all the time. --- .../Calculation/Calculation.php | 6 +- src/PhpSpreadsheet/Calculation/Functions.php | 29 +++++- src/PhpSpreadsheet/Worksheet/Worksheet.php | 2 +- .../Calculation/FunctionsTest.php | 56 ----------- .../Calculation/IsFormulaTest.php | 90 ++++++++++++++++++ .../data/Calculation/Functions/ISFORMULA.php | 94 ------------------- 6 files changed, 122 insertions(+), 155 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Calculation/IsFormulaTest.php delete mode 100644 tests/data/Calculation/Functions/ISFORMULA.php diff --git a/src/PhpSpreadsheet/Calculation/Calculation.php b/src/PhpSpreadsheet/Calculation/Calculation.php index e6b79c7a..506264a3 100644 --- a/src/PhpSpreadsheet/Calculation/Calculation.php +++ b/src/PhpSpreadsheet/Calculation/Calculation.php @@ -4524,7 +4524,7 @@ class Calculation $sheet2 = $sheet1; } - if ($sheet1 == $sheet2) { + if (trim($sheet1, "'") === trim($sheet2, "'")) { if ($operand1Data['reference'] === null) { if ((trim($operand1Data['value']) != '') && (is_numeric($operand1Data['value']))) { $operand1Data['reference'] = $pCell->getColumn() . $operand1Data['value']; @@ -4741,7 +4741,7 @@ class Calculation $cellValue = $this->extractCellRange($cellRef, $this->spreadsheet->getSheetByName($matches[2]), false); $pCell->attach($pCellParent); } else { - $cellRef = ($cellSheet !== null) ? "{$matches[2]}!{$cellRef}" : $cellRef; + $cellRef = ($cellSheet !== null) ? "'{$matches[2]}'!{$cellRef}" : $cellRef; $cellValue = null; } } else { @@ -5262,7 +5262,7 @@ class Calculation // Extract range $aReferences = Coordinate::extractAllCellReferencesInRange($pRange); - $pRange = $pSheetName . '!' . $pRange; + $pRange = "'" . $pSheetName . "'" . '!' . $pRange; if (!isset($aReferences[1])) { $currentCol = ''; $currentRow = 0; diff --git a/src/PhpSpreadsheet/Calculation/Functions.php b/src/PhpSpreadsheet/Calculation/Functions.php index 9b56aad4..b334c0a1 100644 --- a/src/PhpSpreadsheet/Calculation/Functions.php +++ b/src/PhpSpreadsheet/Calculation/Functions.php @@ -4,6 +4,7 @@ namespace PhpOffice\PhpSpreadsheet\Calculation; use PhpOffice\PhpSpreadsheet\Cell\Cell; use PhpOffice\PhpSpreadsheet\Shared\Date; +use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet; class Functions { @@ -659,7 +660,7 @@ class Functions * ISFORMULA. * * @param mixed $cellReference The cell to check - * @param Cell $pCell The current cell (containing this formula) + * @param ?Cell $pCell The current cell (containing this formula) * * @return bool|string */ @@ -668,6 +669,8 @@ class Functions if ($pCell === null) { return self::REF(); } + $cellReference = self::expandDefinedName((string) $cellReference, $pCell); + $cellReference = self::trimTrailingRange($cellReference); preg_match('/^' . Calculation::CALCULATION_REGEXP_CELLREF . '$/i', $cellReference, $matches); @@ -680,4 +683,28 @@ class Functions return $worksheet->getCell($cellReference)->isFormula(); } + + public static function expandDefinedName(string $pCoordinate, Cell $pCell): string + { + $worksheet = $pCell->getWorksheet(); + $spreadsheet = $worksheet->getParent(); + // Uppercase coordinate + $pCoordinatex = strtoupper($pCoordinate); + // Eliminate leading equal sign + $pCoordinatex = Worksheet::pregReplace('/^=/', '', $pCoordinatex); + $defined = $spreadsheet->getDefinedName($pCoordinatex, $worksheet); + if ($defined !== null) { + $worksheet2 = $defined->getWorkSheet(); + if (!$defined->isFormula() && $worksheet2 !== null) { + $pCoordinate = "'" . $worksheet2->getTitle() . "'!" . Worksheet::pregReplace('/^=/', '', $defined->getValue()); + } + } + + return $pCoordinate; + } + + public static function trimTrailingRange(string $pCoordinate): string + { + return Worksheet::pregReplace('/:[\\w\$]+$/', '', $pCoordinate); + } } diff --git a/src/PhpSpreadsheet/Worksheet/Worksheet.php b/src/PhpSpreadsheet/Worksheet/Worksheet.php index 29522463..f7f28be6 100644 --- a/src/PhpSpreadsheet/Worksheet/Worksheet.php +++ b/src/PhpSpreadsheet/Worksheet/Worksheet.php @@ -2384,7 +2384,7 @@ class Worksheet implements IComparable return is_string($str) ? $str : ''; } - private static function pregReplace(string $pattern, string $replacement, string $subject): string + public static function pregReplace(string $pattern, string $replacement, string $subject): string { return self::ensureString(preg_replace($pattern, $replacement, $subject)); } diff --git a/tests/PhpSpreadsheetTests/Calculation/FunctionsTest.php b/tests/PhpSpreadsheetTests/Calculation/FunctionsTest.php index ea629183..c03167ca 100644 --- a/tests/PhpSpreadsheetTests/Calculation/FunctionsTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/FunctionsTest.php @@ -3,9 +3,6 @@ namespace PhpOffice\PhpSpreadsheetTests\Calculation; use PhpOffice\PhpSpreadsheet\Calculation\Functions; -use PhpOffice\PhpSpreadsheet\Cell\Cell; -use PhpOffice\PhpSpreadsheet\Spreadsheet; -use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet; use PHPUnit\Framework\TestCase; class FunctionsTest extends TestCase @@ -326,59 +323,6 @@ class FunctionsTest extends TestCase return require 'tests/data/Calculation/Functions/N.php'; } - /** - * @dataProvider providerIsFormula - * - * @param mixed $expectedResult - * @param mixed $reference Reference to the cell we wish to test - * @param mixed $value Value of the cell we wish to test - */ - public function testIsFormula($expectedResult, $reference, $value = 'undefined'): void - { - $ourCell = null; - if ($value !== 'undefined') { - $remoteCell = $this->getMockBuilder(Cell::class) - ->disableOriginalConstructor() - ->getMock(); - $remoteCell->method('isFormula') - ->willReturn(substr($value ?? '', 0, 1) == '='); - - $remoteSheet = $this->getMockBuilder(Worksheet::class) - ->disableOriginalConstructor() - ->getMock(); - $remoteSheet->method('getCell') - ->willReturn($remoteCell); - - $workbook = $this->getMockBuilder(Spreadsheet::class) - ->disableOriginalConstructor() - ->getMock(); - $workbook->method('getSheetByName') - ->willReturn($remoteSheet); - - $sheet = $this->getMockBuilder(Worksheet::class) - ->disableOriginalConstructor() - ->getMock(); - $sheet->method('getCell') - ->willReturn($remoteCell); - $sheet->method('getParent') - ->willReturn($workbook); - - $ourCell = $this->getMockBuilder(Cell::class) - ->disableOriginalConstructor() - ->getMock(); - $ourCell->method('getWorksheet') - ->willReturn($sheet); - } - - $result = Functions::isFormula($reference, $ourCell); - self::assertEqualsWithDelta($expectedResult, $result, 1E-8); - } - - public function providerIsFormula(): array - { - return require 'tests/data/Calculation/Functions/ISFORMULA.php'; - } - /** * @dataProvider providerIfCondition * diff --git a/tests/PhpSpreadsheetTests/Calculation/IsFormulaTest.php b/tests/PhpSpreadsheetTests/Calculation/IsFormulaTest.php new file mode 100644 index 00000000..40e0d949 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Calculation/IsFormulaTest.php @@ -0,0 +1,90 @@ +getActiveSheet(); + $sheet1->setTitle('SheetOne'); // no space in sheet title + $sheet2 = $spreadsheet->createSheet(); + $sheet2->setTitle('Sheet Two'); // space in sheet title + $values = [ + [null, false], + [-1, false], + [0, false], + [1, false], + ['', false], + [false, false], + [true, false], + ['=-1', true], + ['="ABC"', true], + ['=SUM(1,2,3)', true], + ]; + $row = 0; + foreach ($values as $valArray) { + ++$row; + if ($valArray[0] !== null) { + $sheet1->getCell("A$row")->setValue($valArray[0]); + } + $sheet1->getCell("B$row")->setValue("=ISFORMULA(A$row)"); + self::assertSame($valArray[1], $sheet1->getCell("B$row")->getCalculatedValue(), "sheet1 error in B$row"); + } + $row = 0; + foreach ($values as $valArray) { + ++$row; + if ($valArray[0] !== null) { + $sheet2->getCell("A$row")->setValue($valArray[0]); + } + $sheet2->getCell("B$row")->setValue("=ISFORMULA(A$row)"); + self::assertSame($valArray[1], $sheet2->getCell("B$row")->getCalculatedValue(), "sheet2 error in B$row"); + } + $sheet1->getCell('C1')->setValue(0); + $sheet1->getCell('C2')->setValue('=0'); + $sheet2->getCell('C3')->setValue(0); + $sheet2->getCell('C4')->setValue('=0'); + $sheet1->getCell('D1')->setValue('=ISFORMULA(SheetOne!C1)'); + $sheet1->getCell('D2')->setValue('=ISFORMULA(SheetOne!C2)'); + $sheet1->getCell('E1')->setValue('=ISFORMULA(\'SheetOne\'!C1)'); + $sheet1->getCell('E2')->setValue('=ISFORMULA(\'SheetOne\'!C2)'); + $sheet1->getCell('F1')->setValue('=ISFORMULA(\'Sheet Two\'!C3)'); + $sheet1->getCell('F2')->setValue('=ISFORMULA(\'Sheet Two\'!C4)'); + self::assertFalse($sheet1->getCell('D1')->getCalculatedValue()); + self::assertTrue($sheet1->getCell('D2')->getCalculatedValue()); + self::assertFalse($sheet1->getCell('E1')->getCalculatedValue()); + self::assertTrue($sheet1->getCell('E2')->getCalculatedValue()); + self::assertFalse($sheet1->getCell('F1')->getCalculatedValue()); + self::assertTrue($sheet1->getCell('F2')->getCalculatedValue()); + + $spreadsheet->addNamedRange(new NamedRange('range1f', $sheet1, '$C$1')); + $spreadsheet->addNamedRange(new NamedRange('range1t', $sheet1, '$C$2')); + $spreadsheet->addNamedRange(new NamedRange('range2f', $sheet2, '$C$3')); + $spreadsheet->addNamedRange(new NamedRange('range2t', $sheet2, '$C$4')); + $spreadsheet->addNamedRange(new NamedRange('range2ft', $sheet2, '$C$3:$C$4')); + $sheet1->getCell('G1')->setValue('=ISFORMULA(ABCDEFG)'); + $sheet1->getCell('G3')->setValue('=ISFORMULA(range1f)'); + $sheet1->getCell('G4')->setValue('=ISFORMULA(range1t)'); + $sheet1->getCell('G5')->setValue('=ISFORMULA(range2f)'); + $sheet1->getCell('G6')->setValue('=ISFORMULA(range2t)'); + $sheet1->getCell('G7')->setValue('=ISFORMULA(range2ft)'); + self::assertSame('#NAME?', $sheet1->getCell('G1')->getCalculatedValue()); + self::assertFalse($sheet1->getCell('G3')->getCalculatedValue()); + self::assertTrue($sheet1->getCell('G4')->getCalculatedValue()); + self::assertFalse($sheet1->getCell('G5')->getCalculatedValue()); + self::assertTrue($sheet1->getCell('G6')->getCalculatedValue()); + self::assertFalse($sheet1->getCell('G7')->getCalculatedValue()); + + $sheet1->getCell('H1')->setValue('=ISFORMULA(C1:C2)'); + $sheet1->getCell('H3')->setValue('=ISFORMULA(C2:C3)'); + self::assertFalse($sheet1->getCell('H1')->getCalculatedValue()); + self::assertTrue($sheet1->getCell('H3')->getCalculatedValue()); + + $spreadsheet->disconnectWorksheets(); + } +} diff --git a/tests/data/Calculation/Functions/ISFORMULA.php b/tests/data/Calculation/Functions/ISFORMULA.php deleted file mode 100644 index 60d9c07f..00000000 --- a/tests/data/Calculation/Functions/ISFORMULA.php +++ /dev/null @@ -1,94 +0,0 @@ -