Skip to content

Commit 4001c89

Browse files
oleibmanoleibman
authored
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.
1 parent fea0e34 commit 4001c89

File tree

6 files changed

+122
-155
lines changed

6 files changed

+122
-155
lines changed

‎src/PhpSpreadsheet/Calculation/Calculation.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4524,7 +4524,7 @@ private function processTokenStack($tokens, $cellID = null, ?Cell $pCell = null)
45244524
$sheet2 = $sheet1;
45254525
}
45264526

4527-
if ($sheet1 == $sheet2) {
4527+
if (trim($sheet1, "'") === trim($sheet2, "'")) {
45284528
if ($operand1Data['reference'] === null) {
45294529
if ((trim($operand1Data['value']) != '') && (is_numeric($operand1Data['value']))) {
45304530
$operand1Data['reference'] = $pCell->getColumn() . $operand1Data['value'];
@@ -4741,7 +4741,7 @@ private function processTokenStack($tokens, $cellID = null, ?Cell $pCell = null)
47414741
$cellValue = $this->extractCellRange($cellRef, $this->spreadsheet->getSheetByName($matches[2]), false);
47424742
$pCell->attach($pCellParent);
47434743
} else {
4744-
$cellRef = ($cellSheet !== null) ? "{$matches[2]}!{$cellRef}" : $cellRef;
4744+
$cellRef = ($cellSheet !== null) ? "'{$matches[2]}'!{$cellRef}" : $cellRef;
47454745
$cellValue = null;
47464746
}
47474747
} else {
@@ -5262,7 +5262,7 @@ public function extractCellRange(&$pRange = 'A1', ?Worksheet $pSheet = null, $re
52625262

52635263
// Extract range
52645264
$aReferences = Coordinate::extractAllCellReferencesInRange($pRange);
5265-
$pRange = $pSheetName . '!' . $pRange;
5265+
$pRange = "'" . $pSheetName . "'" . '!' . $pRange;
52665266
if (!isset($aReferences[1])) {
52675267
$currentCol = '';
52685268
$currentRow = 0;

‎src/PhpSpreadsheet/Calculation/Functions.php

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use PhpOffice\PhpSpreadsheet\Cell\Cell;
66
use PhpOffice\PhpSpreadsheet\Shared\Date;
7+
use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
78

89
class Functions
910
{
@@ -659,7 +660,7 @@ public static function flattenSingleValue($value = '')
659660
* ISFORMULA.
660661
*
661662
* @param mixed $cellReference The cell to check
662-
* @param Cell $pCell The current cell (containing this formula)
663+
* @param ?Cell $pCell The current cell (containing this formula)
663664
*
664665
* @return bool|string
665666
*/
@@ -668,6 +669,8 @@ public static function isFormula($cellReference = '', ?Cell $pCell = null)
668669
if ($pCell === null) {
669670
return self::REF();
670671
}
672+
$cellReference = self::expandDefinedName((string) $cellReference, $pCell);
673+
$cellReference = self::trimTrailingRange($cellReference);
671674

672675
preg_match('/^' . Calculation::CALCULATION_REGEXP_CELLREF . '$/i', $cellReference, $matches);
673676

@@ -680,4 +683,28 @@ public static function isFormula($cellReference = '', ?Cell $pCell = null)
680683

681684
return $worksheet->getCell($cellReference)->isFormula();
682685
}
686+
687+
public static function expandDefinedName(string $pCoordinate, Cell $pCell): string
688+
{
689+
$worksheet = $pCell->getWorksheet();
690+
$spreadsheet = $worksheet->getParent();
691+
// Uppercase coordinate
692+
$pCoordinatex = strtoupper($pCoordinate);
693+
// Eliminate leading equal sign
694+
$pCoordinatex = Worksheet::pregReplace('/^=/', '', $pCoordinatex);
695+
$defined = $spreadsheet->getDefinedName($pCoordinatex, $worksheet);
696+
if ($defined !== null) {
697+
$worksheet2 = $defined->getWorkSheet();
698+
if (!$defined->isFormula() && $worksheet2 !== null) {
699+
$pCoordinate = "'" . $worksheet2->getTitle() . "'!" . Worksheet::pregReplace('/^=/', '', $defined->getValue());
700+
}
701+
}
702+
703+
return $pCoordinate;
704+
}
705+
706+
public static function trimTrailingRange(string $pCoordinate): string
707+
{
708+
return Worksheet::pregReplace('/:[\\w\$]+$/', '', $pCoordinate);
709+
}
683710
}

‎src/PhpSpreadsheet/Worksheet/Worksheet.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2384,7 +2384,7 @@ private static function ensureString($str): string
23842384
return is_string($str) ? $str : '';
23852385
}
23862386

2387-
private static function pregReplace(string $pattern, string $replacement, string $subject): string
2387+
public static function pregReplace(string $pattern, string $replacement, string $subject): string
23882388
{
23892389
return self::ensureString(preg_replace($pattern, $replacement, $subject));
23902390
}

‎tests/PhpSpreadsheetTests/Calculation/FunctionsTest.php

Lines changed: 0 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,6 @@
33
namespace PhpOffice\PhpSpreadsheetTests\Calculation;
44

55
use PhpOffice\PhpSpreadsheet\Calculation\Functions;
6-
use PhpOffice\PhpSpreadsheet\Cell\Cell;
7-
use PhpOffice\PhpSpreadsheet\Spreadsheet;
8-
use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
96
use PHPUnit\Framework\TestCase;
107

118
class FunctionsTest extends TestCase
@@ -326,59 +323,6 @@ public function providerN(): array
326323
return require 'tests/data/Calculation/Functions/N.php';
327324
}
328325

329-
/**
330-
* @dataProvider providerIsFormula
331-
*
332-
* @param mixed $expectedResult
333-
* @param mixed $reference Reference to the cell we wish to test
334-
* @param mixed $value Value of the cell we wish to test
335-
*/
336-
public function testIsFormula($expectedResult, $reference, $value = 'undefined'): void
337-
{
338-
$ourCell = null;
339-
if ($value !== 'undefined') {
340-
$remoteCell = $this->getMockBuilder(Cell::class)
341-
->disableOriginalConstructor()
342-
->getMock();
343-
$remoteCell->method('isFormula')
344-
->willReturn(substr($value ?? '', 0, 1) == '=');
345-
346-
$remoteSheet = $this->getMockBuilder(Worksheet::class)
347-
->disableOriginalConstructor()
348-
->getMock();
349-
$remoteSheet->method('getCell')
350-
->willReturn($remoteCell);
351-
352-
$workbook = $this->getMockBuilder(Spreadsheet::class)
353-
->disableOriginalConstructor()
354-
->getMock();
355-
$workbook->method('getSheetByName')
356-
->willReturn($remoteSheet);
357-
358-
$sheet = $this->getMockBuilder(Worksheet::class)
359-
->disableOriginalConstructor()
360-
->getMock();
361-
$sheet->method('getCell')
362-
->willReturn($remoteCell);
363-
$sheet->method('getParent')
364-
->willReturn($workbook);
365-
366-
$ourCell = $this->getMockBuilder(Cell::class)
367-
->disableOriginalConstructor()
368-
->getMock();
369-
$ourCell->method('getWorksheet')
370-
->willReturn($sheet);
371-
}
372-
373-
$result = Functions::isFormula($reference, $ourCell);
374-
self::assertEqualsWithDelta($expectedResult, $result, 1E-8);
375-
}
376-
377-
public function providerIsFormula(): array
378-
{
379-
return require 'tests/data/Calculation/Functions/ISFORMULA.php';
380-
}
381-
382326
/**
383327
* @dataProvider providerIfCondition
384328
*

‎tests/PhpSpreadsheetTests/Calculation/IsFormulaTest.php

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
<?php
2+
3+
namespace PhpOffice\PhpSpreadsheetTests\Calculation;
4+
5+
use PhpOffice\PhpSpreadsheet\NamedRange as NamedRange;
6+
use PhpOffice\PhpSpreadsheet\Spreadsheet;
7+
use PHPUnit\Framework\TestCase;
8+
9+
class IsFormulaTest extends TestCase
10+
{
11+
public function testIsFormula(): void
12+
{
13+
$spreadsheet = new Spreadsheet();
14+
$sheet1 = $spreadsheet->getActiveSheet();
15+
$sheet1->setTitle('SheetOne'); // no space in sheet title
16+
$sheet2 = $spreadsheet->createSheet();
17+
$sheet2->setTitle('Sheet Two'); // space in sheet title
18+
$values = [
19+
[null, false],
20+
[-1, false],
21+
[0, false],
22+
[1, false],
23+
['', false],
24+
[false, false],
25+
[true, false],
26+
['=-1', true],
27+
['="ABC"', true],
28+
['=SUM(1,2,3)', true],
29+
];
30+
$row = 0;
31+
foreach ($values as $valArray) {
32+
++$row;
33+
if ($valArray[0] !== null) {
34+
$sheet1->getCell("A$row")->setValue($valArray[0]);
35+
}
36+
$sheet1->getCell("B$row")->setValue("=ISFORMULA(A$row)");
37+
self::assertSame($valArray[1], $sheet1->getCell("B$row")->getCalculatedValue(), "sheet1 error in B$row");
38+
}
39+
$row = 0;
40+
foreach ($values as $valArray) {
41+
++$row;
42+
if ($valArray[0] !== null) {
43+
$sheet2->getCell("A$row")->setValue($valArray[0]);
44+
}
45+
$sheet2->getCell("B$row")->setValue("=ISFORMULA(A$row)");
46+
self::assertSame($valArray[1], $sheet2->getCell("B$row")->getCalculatedValue(), "sheet2 error in B$row");
47+
}
48+
$sheet1->getCell('C1')->setValue(0);
49+
$sheet1->getCell('C2')->setValue('=0');
50+
$sheet2->getCell('C3')->setValue(0);
51+
$sheet2->getCell('C4')->setValue('=0');
52+
$sheet1->getCell('D1')->setValue('=ISFORMULA(SheetOne!C1)');
53+
$sheet1->getCell('D2')->setValue('=ISFORMULA(SheetOne!C2)');
54+
$sheet1->getCell('E1')->setValue('=ISFORMULA(\'SheetOne\'!C1)');
55+
$sheet1->getCell('E2')->setValue('=ISFORMULA(\'SheetOne\'!C2)');
56+
$sheet1->getCell('F1')->setValue('=ISFORMULA(\'Sheet Two\'!C3)');
57+
$sheet1->getCell('F2')->setValue('=ISFORMULA(\'Sheet Two\'!C4)');
58+
self::assertFalse($sheet1->getCell('D1')->getCalculatedValue());
59+
self::assertTrue($sheet1->getCell('D2')->getCalculatedValue());
60+
self::assertFalse($sheet1->getCell('E1')->getCalculatedValue());
61+
self::assertTrue($sheet1->getCell('E2')->getCalculatedValue());
62+
self::assertFalse($sheet1->getCell('F1')->getCalculatedValue());
63+
self::assertTrue($sheet1->getCell('F2')->getCalculatedValue());
64+
65+
$spreadsheet->addNamedRange(new NamedRange('range1f', $sheet1, '$C$1'));
66+
$spreadsheet->addNamedRange(new NamedRange('range1t', $sheet1, '$C$2'));
67+
$spreadsheet->addNamedRange(new NamedRange('range2f', $sheet2, '$C$3'));
68+
$spreadsheet->addNamedRange(new NamedRange('range2t', $sheet2, '$C$4'));
69+
$spreadsheet->addNamedRange(new NamedRange('range2ft', $sheet2, '$C$3:$C$4'));
70+
$sheet1->getCell('G1')->setValue('=ISFORMULA(ABCDEFG)');
71+
$sheet1->getCell('G3')->setValue('=ISFORMULA(range1f)');
72+
$sheet1->getCell('G4')->setValue('=ISFORMULA(range1t)');
73+
$sheet1->getCell('G5')->setValue('=ISFORMULA(range2f)');
74+
$sheet1->getCell('G6')->setValue('=ISFORMULA(range2t)');
75+
$sheet1->getCell('G7')->setValue('=ISFORMULA(range2ft)');
76+
self::assertSame('#NAME?', $sheet1->getCell('G1')->getCalculatedValue());
77+
self::assertFalse($sheet1->getCell('G3')->getCalculatedValue());
78+
self::assertTrue($sheet1->getCell('G4')->getCalculatedValue());
79+
self::assertFalse($sheet1->getCell('G5')->getCalculatedValue());
80+
self::assertTrue($sheet1->getCell('G6')->getCalculatedValue());
81+
self::assertFalse($sheet1->getCell('G7')->getCalculatedValue());
82+
83+
$sheet1->getCell('H1')->setValue('=ISFORMULA(C1:C2)');
84+
$sheet1->getCell('H3')->setValue('=ISFORMULA(C2:C3)');
85+
self::assertFalse($sheet1->getCell('H1')->getCalculatedValue());
86+
self::assertTrue($sheet1->getCell('H3')->getCalculatedValue());
87+
88+
$spreadsheet->disconnectWorksheets();
89+
}
90+
}

‎tests/data/Calculation/Functions/ISFORMULA.php

Lines changed: 0 additions & 94 deletions
This file was deleted.

0 commit comments

Comments
 (0)