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.
This commit is contained in:
oleibman 2021-10-03 10:04:48 -07:00 committed by GitHub
parent fea0e34e90
commit 4001c89aaa
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 122 additions and 155 deletions

View File

@ -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;

View File

@ -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);
}
}

View File

@ -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));
}

View File

@ -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
*

View File

@ -0,0 +1,90 @@
<?php
namespace PhpOffice\PhpSpreadsheetTests\Calculation;
use PhpOffice\PhpSpreadsheet\NamedRange as NamedRange;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PHPUnit\Framework\TestCase;
class IsFormulaTest extends TestCase
{
public function testIsFormula(): void
{
$spreadsheet = new Spreadsheet();
$sheet1 = $spreadsheet->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();
}
}

View File

@ -1,94 +0,0 @@
<?php
return [
[
false,
'A1',
null,
],
[
false,
'A2',
-1,
],
[
false,
'A3',
0,
],
[
false,
'A4',
1,
],
[
false,
'A5',
'',
],
[
false,
'A6',
'2',
],
[
false,
'A7',
'#VALUE!',
],
[
false,
'A8',
'#N/A',
],
[
false,
'A9',
'TRUE',
],
[
false,
'A10',
true,
],
[
false,
'A11',
false,
],
[
true,
'A12',
'="ABC"',
],
[
true,
'A13',
'=A1',
],
[
true,
'A14',
'=\'Worksheet1\'!A1',
],
[
true,
'\'Worksheet1\'!A15',
'="HELLO WORLD"',
],
[
false,
'\'Worksheet1\'!A16',
'123',
],
[
true,
'A17',
'=\'Work sheet1\'!A1',
],
[
true,
'A18',
'=\'Work!sheet1\'!A1',
],
];