Skip to content

Commit 14a989a

Browse files
oleibmanoleibman
committed
Handle Missing Defined Names Better
The original issue leading to this PR was fixed by correctly treating a token in Calculator as a function rather than a defined name. However, it *should* have worked even when treating it as a defined name. There was a problem because Calculator was raising an exception for a missing defined name rather than returning `#NAME?`. It is now changed to return the error. That change initially had some adverse affects for functions ROW, ROWS, COLUMN, and COLUMNS. Those are fixed to handle the change correctly. As a bonus, a test for each which had been commented out, because it didn't work, is now uncommented and works correctly. One test for ISFORMULA and one for ISREF were also changed - the old expected result did not reflect Excel's behavior and the new one does. Sheet title and Defined Name matching used `strtoupper` to achieve case-insensitive compares. This handles only ASCII characters. They are changed to use a conversion routine which handles non-ASCII UTF-8 character sets.
1 parent f51ec36 commit 14a989a

File tree

12 files changed

+127
-26
lines changed

12 files changed

+127
-26
lines changed

‎src/PhpSpreadsheet/Calculation/Calculation.php

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1727,6 +1727,16 @@ private function processTokenStack(false|array $tokens, ?string $cellID = null,
17271727
break;
17281728
// Binary Operators
17291729
case ':': // Range
1730+
if ($operand1Data['type'] === 'Error') {
1731+
$stack->push($operand1Data['type'], $operand1Data['value'], null);
1732+
1733+
break;
1734+
}
1735+
if ($operand2Data['type'] === 'Error') {
1736+
$stack->push($operand2Data['type'], $operand2Data['value'], null);
1737+
1738+
break;
1739+
}
17301740
if ($operand1Data['type'] === 'Defined Name') {
17311741
/** @var array{reference: string} $operand1Data */
17321742
if (preg_match('/$' . self::CALCULATION_REGEXP_DEFINEDNAME . '^/mui', $operand1Data['reference']) !== false && $this->spreadsheet !== null) {
@@ -2087,6 +2097,13 @@ private function processTokenStack(false|array $tokens, ?string $cellID = null,
20872097
$nextArg = '';
20882098
}
20892099
}
2100+
} elseif (($arg['type'] ?? '') === 'Error') {
2101+
$argValue = $arg['value'];
2102+
if (is_scalar($argValue)) {
2103+
$nextArg = $argValue;
2104+
} elseif (empty($argValue)) {
2105+
$nextArg = '';
2106+
}
20902107
}
20912108
$args[] = $nextArg;
20922109
if ($functionName !== 'MKMATRIX') {
@@ -2217,11 +2234,13 @@ private function processTokenStack(false|array $tokens, ?string $cellID = null,
22172234
}
22182235
}
22192236
if ($namedRange === null) {
2220-
return $this->raiseFormulaError("undefined name '$definedName'");
2237+
$result = ExcelError::NAME();
2238+
$stack->push('Error', $result, null);
2239+
$this->debugLog->writeDebugLog("Error $result");
2240+
} else {
2241+
$result = $this->evaluateDefinedName($cell, $namedRange, $pCellWorksheet, $stack, $specifiedWorksheet !== '');
22212242
}
22222243

2223-
$result = $this->evaluateDefinedName($cell, $namedRange, $pCellWorksheet, $stack, $specifiedWorksheet !== '');
2224-
22252244
if (isset($storeKey)) {
22262245
$branchStore[$storeKey] = $result;
22272246
}
@@ -2487,6 +2506,8 @@ protected function raiseFormulaError(string $errorMessage, int $code = 0, ?Throw
24872506
$this->formulaError = $errorMessage;
24882507
$this->cyclicReferenceStack->clear();
24892508
$suppress = $this->suppressFormulaErrors;
2509+
$suppressed = $suppress ? ' $suppressed' : '';
2510+
$this->debugLog->writeDebugLog("Raise Error$suppressed $errorMessage");
24902511
if (!$suppress) {
24912512
throw new Exception($errorMessage, $code, $exception);
24922513
}
@@ -2788,7 +2809,13 @@ private function evaluateDefinedName(Cell $cell, DefinedName $namedRange, Worksh
27882809
$this->debugLog->writeDebugLog('Evaluation Result for Named %s %s is %s', $definedNameType, $namedRange->getName(), $this->showTypeDetails($result));
27892810
}
27902811

2791-
$stack->push('Defined Name', $result, $namedRange->getName());
2812+
$y = $namedRange->getWorksheet()?->getTitle();
2813+
$x = $namedRange->getLocalOnly();
2814+
if ($x && $y !== null) {
2815+
$stack->push('Defined Name', $result, "'$y'!" . $namedRange->getName());
2816+
} else {
2817+
$stack->push('Defined Name', $result, $namedRange->getName());
2818+
}
27922819

27932820
return $result;
27942821
}

‎src/PhpSpreadsheet/Calculation/Information/Value.php

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use PhpOffice\PhpSpreadsheet\Calculation\Functions;
88
use PhpOffice\PhpSpreadsheet\Cell\Cell;
99
use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
10+
use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException;
1011
use PhpOffice\PhpSpreadsheet\NamedRange;
1112
use PhpOffice\PhpSpreadsheet\Shared\StringHelper;
1213
use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
@@ -226,8 +227,15 @@ public static function isFormula(mixed $cellReference = '', ?Cell $cell = null):
226227
$worksheet = (!empty($worksheetName))
227228
? $cell->getWorksheet()->getParentOrThrow()->getSheetByName($worksheetName)
228229
: $cell->getWorksheet();
230+
if ($worksheet === null) {
231+
return ExcelError::REF();
232+
}
229233

230-
return ($worksheet !== null) ? $worksheet->getCell($fullCellReference)->isFormula() : ExcelError::REF();
234+
try {
235+
return $worksheet->getCell($fullCellReference)->isFormula();
236+
} catch (SpreadsheetException) {
237+
return true;
238+
}
231239
}
232240

233241
/**

‎src/PhpSpreadsheet/Calculation/LookupRef/RowColumnInformation.php

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@
33
namespace PhpOffice\PhpSpreadsheet\Calculation\LookupRef;
44

55
use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
6+
use PhpOffice\PhpSpreadsheet\Calculation\Information\ErrorValue;
67
use PhpOffice\PhpSpreadsheet\Calculation\Information\ExcelError;
78
use PhpOffice\PhpSpreadsheet\Cell\Cell;
89
use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
10+
use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException;
911
use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
1012

1113
class RowColumnInformation
@@ -40,9 +42,9 @@ private static function cellColumn(?Cell $cell): int
4042
*
4143
* @param null|mixed[]|string $cellAddress A reference to a range of cells for which you want the column numbers
4244
*
43-
* @return int|int[]
45+
* @return int|int[]|string
4446
*/
45-
public static function COLUMN($cellAddress = null, ?Cell $cell = null): int|array
47+
public static function COLUMN($cellAddress = null, ?Cell $cell = null): int|string|array
4648
{
4749
if (self::cellAddressNullOrWhitespace($cellAddress)) {
4850
return self::cellColumn($cell);
@@ -79,7 +81,11 @@ public static function COLUMN($cellAddress = null, ?Cell $cell = null): int|arra
7981

8082
$cellAddress = (string) preg_replace('/[^a-z]/i', '', $cellAddress);
8183

82-
return Coordinate::columnIndexFromString($cellAddress);
84+
try {
85+
return Coordinate::columnIndexFromString($cellAddress);
86+
} catch (SpreadsheetException) {
87+
return ExcelError::NAME();
88+
}
8389
}
8490

8591
/**
@@ -100,6 +106,9 @@ public static function COLUMNS($cellAddress = null)
100106
if (self::cellAddressNullOrWhitespace($cellAddress)) {
101107
return 1;
102108
}
109+
if (is_string($cellAddress) && ErrorValue::isError($cellAddress)) {
110+
return $cellAddress;
111+
}
103112
if (!is_array($cellAddress)) {
104113
return ExcelError::VALUE();
105114
}
@@ -115,9 +124,18 @@ public static function COLUMNS($cellAddress = null)
115124
return $columns;
116125
}
117126

118-
private static function cellRow(?Cell $cell): int
127+
private static function cellRow(?Cell $cell): int|string
128+
{
129+
return ($cell !== null) ? self::convert0ToName($cell->getRow()) : 1;
130+
}
131+
132+
private static function convert0ToName(int|string $result): int|string
119133
{
120-
return ($cell !== null) ? $cell->getRow() : 1;
134+
if (is_int($result) && ($result <= 0 || $result > 1048576)) {
135+
return ExcelError::NAME();
136+
}
137+
138+
return $result;
121139
}
122140

123141
/**
@@ -135,9 +153,9 @@ private static function cellRow(?Cell $cell): int
135153
*
136154
* @param null|mixed[][]|string $cellAddress A reference to a range of cells for which you want the row numbers
137155
*
138-
* @return int|mixed[]
156+
* @return int|mixed[]|string
139157
*/
140-
public static function ROW($cellAddress = null, ?Cell $cell = null): int|array
158+
public static function ROW($cellAddress = null, ?Cell $cell = null): int|string|array
141159
{
142160
if (self::cellAddressNullOrWhitespace($cellAddress)) {
143161
return self::cellRow($cell);
@@ -172,7 +190,7 @@ public static function ROW($cellAddress = null, ?Cell $cell = null): int|array
172190
}
173191
[$cellAddress] = explode(':', $cellAddress);
174192

175-
return (int) preg_replace('/\D/', '', $cellAddress);
193+
return self::convert0ToName((int) preg_replace('/\D/', '', $cellAddress));
176194
}
177195

178196
/**
@@ -193,6 +211,9 @@ public static function ROWS($cellAddress = null)
193211
if (self::cellAddressNullOrWhitespace($cellAddress)) {
194212
return 1;
195213
}
214+
if (is_string($cellAddress) && ErrorValue::isError($cellAddress)) {
215+
return $cellAddress;
216+
}
196217
if (!is_array($cellAddress)) {
197218
return ExcelError::VALUE();
198219
}

‎src/PhpSpreadsheet/Spreadsheet.php

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -691,9 +691,9 @@ public function getAllSheets(): array
691691
*/
692692
public function getSheetByName(string $worksheetName): ?Worksheet
693693
{
694-
$trimWorksheetName = trim($worksheetName, "'");
694+
$trimWorksheetName = StringHelper::strToUpper(trim($worksheetName, "'"));
695695
foreach ($this->workSheetCollection as $worksheet) {
696-
if (strcasecmp($worksheet->getTitle(), $trimWorksheetName) === 0) {
696+
if (StringHelper::strToUpper($worksheet->getTitle()) === $trimWorksheetName) {
697697
return $worksheet;
698698
}
699699
}
@@ -1017,13 +1017,34 @@ public function getDefinedName(string $definedName, ?Worksheet $worksheet = null
10171017
if ($definedName !== '') {
10181018
$definedName = StringHelper::strToUpper($definedName);
10191019
// first look for global defined name
1020-
if (isset($this->definedNames[$definedName])) {
1021-
$returnValue = $this->definedNames[$definedName];
1020+
foreach ($this->definedNames as $dn) {
1021+
$upper = StringHelper::strToUpper($dn->getName());
1022+
if (
1023+
!$dn->getLocalOnly()
1024+
&& $definedName === $upper
1025+
) {
1026+
$returnValue = $dn;
1027+
1028+
break;
1029+
}
10221030
}
10231031

10241032
// then look for local defined name (has priority over global defined name if both names exist)
1025-
if (($worksheet !== null) && isset($this->definedNames[$worksheet->getTitle() . '!' . $definedName])) {
1026-
$returnValue = $this->definedNames[$worksheet->getTitle() . '!' . $definedName];
1033+
if ($worksheet !== null) {
1034+
$wsTitle = StringHelper::strToUpper($worksheet->getTitle());
1035+
$definedName = (string) preg_replace('/^.*!/', '', $definedName);
1036+
foreach ($this->definedNames as $dn) {
1037+
$sheet = $dn->getScope() ?? $dn->getWorksheet();
1038+
$upper = StringHelper::strToUpper($dn->getName());
1039+
$upperTitle = StringHelper::strToUpper((string) $sheet?->getTitle());
1040+
if (
1041+
$dn->getLocalOnly()
1042+
&& $upper === $definedName
1043+
&& $upperTitle === $wsTitle
1044+
) {
1045+
return $dn;
1046+
}
1047+
}
10271048
}
10281049
}
10291050

‎tests/PhpSpreadsheetTests/Calculation/DefinedNamesCalculationTest.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,24 @@ public function testNamedRangeCalculations1(string $cellAddress, float $expected
2121

2222
$calculatedCellValue = $spreadsheet->getActiveSheet()->getCell($cellAddress)->getCalculatedValue();
2323
self::assertSame($expectedValue, $calculatedCellValue, "Failed calculation for cell {$cellAddress}");
24+
$spreadsheet->disconnectWorksheets();
25+
}
26+
27+
public function testNamedRangeCalculationsIfError(): void
28+
{
29+
$inputFileType = 'Xlsx';
30+
$inputFileName = __DIR__ . '/../../data/Calculation/DefinedNames/NamedRanges.xlsx';
31+
32+
$reader = IOFactory::createReader($inputFileType);
33+
$spreadsheet = $reader->load($inputFileName);
34+
$sheet = $spreadsheet->getActiveSheet();
35+
$sheet->getCell('E1')
36+
->setValue('=IFERROR(CHARGE_RATE, 999)');
37+
$sheet->getCell('F1')
38+
->setValue('=IFERROR(CHARGE_RATX, 999)');
39+
self::assertSame(7.5, $sheet->getCell('E1')->getCalculatedValue());
40+
self::assertSame(999, $sheet->getCell('F1')->getCalculatedValue());
41+
$spreadsheet->disconnectWorksheets();
2442
}
2543

2644
#[DataProvider('namedRangeCalculationProvider2')]
@@ -36,6 +54,7 @@ public function testNamedRangeCalculationsWithAdjustedRateValue(string $cellAddr
3654

3755
$calculatedCellValue = $spreadsheet->getActiveSheet()->getCell($cellAddress)->getCalculatedValue();
3856
self::assertSame($expectedValue, $calculatedCellValue, "Failed calculation for cell {$cellAddress}");
57+
$spreadsheet->disconnectWorksheets();
3958
}
4059

4160
#[DataProvider('namedRangeCalculationProvider1')]
@@ -49,6 +68,7 @@ public function testNamedFormulaCalculations1(string $cellAddress, float $expect
4968

5069
$calculatedCellValue = $spreadsheet->getActiveSheet()->getCell($cellAddress)->getCalculatedValue();
5170
self::assertSame($expectedValue, $calculatedCellValue, "Failed calculation for cell {$cellAddress}");
71+
$spreadsheet->disconnectWorksheets();
5272
}
5373

5474
#[DataProvider('namedRangeCalculationProvider2')]
@@ -64,6 +84,7 @@ public function testNamedFormulaeCalculationsWithAdjustedRateValue(string $cellA
6484

6585
$calculatedCellValue = $spreadsheet->getActiveSheet()->getCell($cellAddress)->getCalculatedValue();
6686
self::assertSame($expectedValue, $calculatedCellValue, "Failed calculation for cell {$cellAddress}");
87+
$spreadsheet->disconnectWorksheets();
6788
}
6889

6990
public static function namedRangeCalculationProvider1(): array

‎tests/PhpSpreadsheetTests/Calculation/Functions/Information/IsFormulaTest.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,9 @@ public function testIsFormula(): void
8181
$sheet1->getCell('G5')->setValue('=ISFORMULA(range2f)');
8282
$sheet1->getCell('G6')->setValue('=ISFORMULA(range2t)');
8383
$sheet1->getCell('G7')->setValue('=ISFORMULA(range2ft)');
84-
self::assertSame('#NAME?', $sheet1->getCell('G1')->getCalculatedValue());
84+
self::assertTrue(
85+
$sheet1->getCell('G1')->getCalculatedValue()
86+
);
8587
self::assertFalse($sheet1->getCell('G3')->getCalculatedValue());
8688
self::assertTrue($sheet1->getCell('G4')->getCalculatedValue());
8789
self::assertFalse($sheet1->getCell('G5')->getCalculatedValue());

‎tests/PhpSpreadsheetTests/Calculation/Functions/Information/IsRefTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public static function providerIsRef(): array
3636
'quoted sheet name' => [true, "'Worksheet2'!B1:B2"],
3737
'quoted sheet name with apostrophe' => [true, "'Work''sheet2'!B1:B2"],
3838
'named range' => [true, 'NAMED_RANGE'],
39-
'unknown named range' => ['#NAME?', 'xNAMED_RANGE'],
39+
'unknown named range' => [false, 'xNAMED_RANGE'],
4040
'indirect to a cell reference' => [true, 'INDIRECT("A1")'],
4141
'indirect to a worksheet/cell reference' => [true, 'INDIRECT("\'Worksheet\'!A1")'],
4242
'indirect to invalid worksheet/cell reference' => [false, 'INDIRECT("\'Invalid Worksheet\'!A1")'],

‎tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/ColumnOnSpreadsheetTest.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ public function testColumnOnSpreadsheet(mixed $expectedResult, string $cellRefer
2121

2222
$sheet1 = $this->getSpreadsheet()->createSheet();
2323
$sheet1->setTitle('OtherSheet');
24+
$this->getSpreadsheet()->addNamedRange(new NamedRange('localname', $sheet1, '$F$6:$H$6', true));
2425

2526
if ($cellReference === 'omitted') {
2627
$sheet->getCell('B3')->setValue('=COLUMN()');
@@ -61,7 +62,7 @@ public function testCOLUMNSheetWithApostrophe(): void
6162
$sheet = $this->getSheet();
6263

6364
$sheet1 = $this->getSpreadsheet()->createSheet();
64-
$sheet1->setTitle("apo''strophe");
65+
$sheet1->setTitle("apo'strophe");
6566
$this->getSpreadsheet()->addNamedRange(new NamedRange('newnr', $sheet1, '$F$5:$H$5', true)); // defined locally, only usable on sheet1
6667

6768
$sheet1->getCell('B3')->setValue('=COLUMN(newnr)');

‎tests/data/Calculation/LookupRef/COLUMNSonSpreadsheet.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,5 @@
1717
'unknown name' => ['#NAME?', 'namedrange2'],
1818
'unknown name as first part of range' => ['#NAME?', 'Invalid:A2'],
1919
'unknown name as second part of range' => ['#NAME?', 'A2:Invalid'],
20-
//'qualified out of scope $f$6:$h$6' => [3, 'OtherSheet!localname'], // needs investigation
20+
'qualified out of scope $f$6:$h$6' => [3, 'OtherSheet!localname'],
2121
];

‎tests/data/Calculation/LookupRef/COLUMNonSpreadsheet.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,5 @@
1515
'unknown name' => ['#NAME?', 'namedrange2'],
1616
'unknown name as first part of range' => ['#NAME?', 'Invalid:A2'],
1717
'unknown name as second part of range' => ['#NAME?', 'A2:Invalid'],
18-
//'qualified name' => [6, 'OtherSheet!localname'], // Never reaches function
18+
'qualified name' => [6, 'OtherSheet!localname'],
1919
];

0 commit comments

Comments
 (0)