Skip to content

Commit

Permalink
Improved Support for INDIRECT, ROW, and COLUMN Functions (PHPOffice#2004
Browse files Browse the repository at this point in the history
)

* Improved Support for INDIRECT, ROW, and COLUMN Functions

This should address issues PHPOffice#1913 and PHPOffice#1993. INDIRECT had heretofore not supported an optional parameter intended to support addresses in R1C1 format which was introduced with Excel 2010. It also had not supported the use of defined names as an argument. This PR is a replacement for PHPOffice#1995, which is currently in draft status and which I will close in a day or two.

The ROW and COLUMN functions also should support defined names. I have added that, and test cases, with the latest push. ROWS and COLUMNS already supported it correctly, but there had been no test cases. Because ROW and COLUMN can return arrays, and PhpSpreadsheet does not support dynamic arrays, I left the existing direct-call tests unchanged to demonstrate those capabilities.

The unit tests for INDIRECT had used mocking, and were sorely lacking (tested only error conditions). They have been replaced with normal, and hopefully adequate, tests. This includes testing globally defined names, as well as locally defined names, both in and out of scope.

The test case in 1913 was too complicated for me to add as a unit test. The main impediments to it are now removed, and its complex situation will, I hope, be corrected with this fix.

INDIRECT can also support a reference of the form Sheetname!localName when localName on its own would be out of scope. That functionality is added. It is also added, in theory, for ROW and COLUMN, however such a construction is rejected by the Calculation engine before passing control to ROW or COLUMN. It might be possible to change the engine to allow this, and I may want to look into that later, but it seems much too risky, and not nearly useful enough, to attempt to address that as part of this change.

Several unusual test cases (leading equals sign, not-quite-as-expected name definition in file, complex indirection involving concatenation and a dropdown list) were suggested by @MarkBaker and are included in this request.
  • Loading branch information
oleibman authored Apr 20, 2021
1 parent aeccdb3 commit c79a9a8
Show file tree
Hide file tree
Showing 22 changed files with 724 additions and 113 deletions.
15 changes: 0 additions & 15 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -1090,16 +1090,6 @@ parameters:
count: 1
path: src/PhpSpreadsheet/Calculation/LookupRef/HLookup.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\LookupRef\\\\Indirect\\:\\:extractRequiredCells\\(\\) has no return typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Calculation/LookupRef/Indirect.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\LookupRef\\\\Indirect\\:\\:extractWorksheet\\(\\) has parameter \\$cellAddress with no typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Calculation/LookupRef/Indirect.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\LookupRef\\\\Lookup\\:\\:verifyResultVector\\(\\) has no return typehint specified\\.$#"
count: 1
Expand Down Expand Up @@ -1185,11 +1175,6 @@ parameters:
count: 3
path: src/PhpSpreadsheet/Calculation/LookupRef/RowColumnInformation.php

-
message: "#^Cannot cast array\\|string to string\\.$#"
count: 2
path: src/PhpSpreadsheet/Calculation/LookupRef/RowColumnInformation.php

-
message: "#^Parameter \\#1 \\$low of function range expects float\\|int\\|string, string\\|null given\\.$#"
count: 1
Expand Down
10 changes: 5 additions & 5 deletions src/PhpSpreadsheet/Calculation/LookupRef.php
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,8 @@ public static function ROWS($cellAddress = null)
* Excel Function:
* =HYPERLINK(linkURL,displayName)
*
* @param mixed $linkURL URL Value to check, is also the value returned when no error
* @param mixed $displayName String Value to return when testValue is an error condition
* @param mixed $linkURL Expect string. Value to check, is also the value returned when no error
* @param mixed $displayName Expect string. Value to return when testValue is an error condition
* @param Cell $pCell The cell to set the hyperlink in
*
* @return mixed The value of $displayName (or $linkURL if $displayName was blank)
Expand Down Expand Up @@ -188,16 +188,16 @@ public static function HYPERLINK($linkURL = '', $displayName = null, ?Cell $pCel
*
* NOTE - INDIRECT() does not yet support the optional a1 parameter introduced in Excel 2010
*
* @param null|array|string $cellAddress $cellAddress The cell address of the current cell (containing this formula)
* @param array|string $cellAddress $cellAddress The cell address of the current cell (containing this formula)
* @param Cell $pCell The current cell (containing this formula)
*
* @return array|string An array containing a cell or range of cells, or a string on error
*
* @TODO Support for the optional a1 parameter introduced in Excel 2010
*/
public static function INDIRECT($cellAddress = null, ?Cell $pCell = null)
public static function INDIRECT($cellAddress, Cell $pCell)
{
return Indirect::INDIRECT($cellAddress, $pCell);
return Indirect::INDIRECT($cellAddress, true, $pCell);
}

/**
Expand Down
74 changes: 74 additions & 0 deletions src/PhpSpreadsheet/Calculation/LookupRef/Helpers.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
<?php

namespace PhpOffice\PhpSpreadsheet\Calculation\LookupRef;

use PhpOffice\PhpSpreadsheet\Cell\AddressHelper;
use PhpOffice\PhpSpreadsheet\Cell\Cell;
use PhpOffice\PhpSpreadsheet\DefinedName;
use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;

class Helpers
{
public const CELLADDRESS_USE_A1 = true;

public const CELLADDRESS_USE_R1C1 = false;

private static function convertR1C1(string &$cellAddress1, ?string &$cellAddress2, bool $a1): string
{
if ($a1 === self::CELLADDRESS_USE_R1C1) {
$cellAddress1 = AddressHelper::convertToA1($cellAddress1);
if ($cellAddress2) {
$cellAddress2 = AddressHelper::convertToA1($cellAddress2);
}
}

return $cellAddress1 . ($cellAddress2 ? ":$cellAddress2" : '');
}

private static function adjustSheetTitle(string &$sheetTitle, ?string $value): void
{
if ($sheetTitle) {
$sheetTitle .= '!';
if (stripos($value ?? '', $sheetTitle) === 0) {
$sheetTitle = '';
}
}
}

public static function extractCellAddresses(string $cellAddress, bool $a1, Worksheet $sheet, string $sheetName = ''): array
{
$cellAddress1 = $cellAddress;
$cellAddress2 = null;
$namedRange = DefinedName::resolveName($cellAddress1, $sheet, $sheetName);
if ($namedRange !== null) {
$workSheet = $namedRange->getWorkSheet();
$sheetTitle = ($workSheet === null) ? '' : $workSheet->getTitle();
$value = preg_replace('/^=/', '', $namedRange->getValue());
self::adjustSheetTitle($sheetTitle, $value);
$cellAddress1 = $sheetTitle . $value;
$cellAddress = $cellAddress1;
$a1 = self::CELLADDRESS_USE_A1;
}
if (strpos($cellAddress, ':') !== false) {
[$cellAddress1, $cellAddress2] = explode(':', $cellAddress);
}
$cellAddress = self::convertR1C1($cellAddress1, $cellAddress2, $a1);

return [$cellAddress1, $cellAddress2, $cellAddress];
}

public static function extractWorksheet(string $cellAddress, Cell $pCell): array
{
$sheetName = '';
if (strpos($cellAddress, '!') !== false) {
[$sheetName, $cellAddress] = Worksheet::extractSheetTitle($cellAddress, true);
$sheetName = trim($sheetName, "'");
}

$pSheet = ($sheetName !== '')
? $pCell->getWorksheet()->getParent()->getSheetByName($sheetName)
: $pCell->getWorksheet();

return [$cellAddress, $pSheet, $sheetName];
}
}
83 changes: 51 additions & 32 deletions src/PhpSpreadsheet/Calculation/LookupRef/Indirect.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,45 +2,74 @@

namespace PhpOffice\PhpSpreadsheet\Calculation\LookupRef;

use Exception;
use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
use PhpOffice\PhpSpreadsheet\Calculation\Functions;
use PhpOffice\PhpSpreadsheet\Cell\Cell;
use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;

class Indirect
{
/**
* Determine whether cell address is in A1 (true) or R1C1 (false) format.
*
* @param mixed $a1fmt Expect bool Helpers::CELLADDRESS_USE_A1 or CELLADDRESS_USE_R1C1, but can be provided as numeric which is cast to bool
*/
private static function a1Format($a1fmt): bool
{
$a1fmt = Functions::flattenSingleValue($a1fmt);
if ($a1fmt === null) {
return Helpers::CELLADDRESS_USE_A1;
}
if (is_string($a1fmt)) {
throw new Exception(Functions::VALUE());
}

return (bool) $a1fmt;
}

/**
* Convert cellAddress to string, verify not null string.
*
* @param array|string $cellAddress
*/
private static function validateAddress($cellAddress): string
{
$cellAddress = Functions::flattenSingleValue($cellAddress);
if (!is_string($cellAddress) || !$cellAddress) {
throw new Exception(Functions::REF());
}

return $cellAddress;
}

/**
* INDIRECT.
*
* Returns the reference specified by a text string.
* References are immediately evaluated to display their contents.
*
* Excel Function:
* =INDIRECT(cellAddress)
* =INDIRECT(cellAddress, bool) where the bool argument is optional
*
* NOTE - INDIRECT() does not yet support the optional a1 parameter introduced in Excel 2010
*
* @param null|array|string $cellAddress $cellAddress The cell address of the current cell (containing this formula)
* @param null|Cell $pCell The current cell (containing this formula)
* @param array|string $cellAddress $cellAddress The cell address of the current cell (containing this formula)
* @param mixed $a1fmt Expect bool Helpers::CELLADDRESS_USE_A1 or CELLADDRESS_USE_R1C1, but can be provided as numeric which is cast to bool
* @param Cell $pCell The current cell (containing this formula)
*
* @return array|string An array containing a cell or range of cells, or a string on error
*
* @TODO Support for the optional a1 parameter introduced in Excel 2010
*/
public static function INDIRECT($cellAddress = null, ?Cell $pCell = null)
public static function INDIRECT($cellAddress, $a1fmt, Cell $pCell)
{
$cellAddress = Functions::flattenSingleValue($cellAddress);
if ($cellAddress === null || $cellAddress === '' || !is_object($pCell)) {
return Functions::REF();
try {
$a1 = self::a1Format($a1fmt);
$cellAddress = self::validateAddress($cellAddress);
} catch (Exception $e) {
return $e->getMessage();
}

[$cellAddress, $pSheet] = self::extractWorksheet($cellAddress, $pCell);
[$cellAddress, $pSheet, $sheetName] = Helpers::extractWorksheet($cellAddress, $pCell);

$cellAddress1 = $cellAddress;
$cellAddress2 = null;
if (strpos($cellAddress, ':') !== false) {
[$cellAddress1, $cellAddress2] = explode(':', $cellAddress);
}
[$cellAddress1, $cellAddress2, $cellAddress] = Helpers::extractCellAddresses($cellAddress, $a1, $pCell->getWorkSheet(), $sheetName);

if (
(!preg_match('/^' . Calculation::CALCULATION_REGEXP_CELLREF . '$/i', $cellAddress1, $matches)) ||
Expand All @@ -52,24 +81,14 @@ public static function INDIRECT($cellAddress = null, ?Cell $pCell = null)
return self::extractRequiredCells($pSheet, $cellAddress);
}

/**
* Extract range values.
*
* @return mixed Array of values in range if range contains more than one element. Otherwise, a single value is returned.
*/
private static function extractRequiredCells(?Worksheet $pSheet, string $cellAddress)
{
return Calculation::getInstance($pSheet !== null ? $pSheet->getParent() : null)
->extractCellRange($cellAddress, $pSheet, false);
}

private static function extractWorksheet($cellAddress, Cell $pCell): array
{
$sheetName = '';
if (strpos($cellAddress, '!') !== false) {
[$sheetName, $cellAddress] = Worksheet::extractSheetTitle($cellAddress, true);
$sheetName = trim($sheetName, "'");
}

$pSheet = ($sheetName !== '')
? $pCell->getWorksheet()->getParent()->getSheetByName($sheetName)
: $pCell->getWorksheet();

return [$cellAddress, $pSheet];
}
}
58 changes: 47 additions & 11 deletions src/PhpSpreadsheet/Calculation/LookupRef/RowColumnInformation.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,21 @@

class RowColumnInformation
{
/**
* Test if cellAddress is null or whitespace string.
*
* @param null|array|string $cellAddress A reference to a range of cells
*/
private static function cellAddressNullOrWhitespace($cellAddress): bool
{
return $cellAddress === null || (!is_array($cellAddress) && trim($cellAddress) === '');
}

private static function cellColumn(?Cell $pCell): int
{
return ($pCell !== null) ? (int) Coordinate::columnIndexFromString($pCell->getColumn()) : 1;
}

/**
* COLUMN.
*
Expand All @@ -27,10 +42,10 @@ class RowColumnInformation
*
* @return int|int[]
*/
public static function COLUMN($cellAddress = null, ?Cell $cell = null)
public static function COLUMN($cellAddress = null, ?Cell $pCell = null)
{
if ($cellAddress === null || (!is_array($cellAddress) && trim($cellAddress) === '')) {
return ($cell !== null) ? (int) Coordinate::columnIndexFromString($cell->getColumn()) : 1;
if (self::cellAddressNullOrWhitespace($cellAddress)) {
return self::cellColumn($pCell);
}

if (is_array($cellAddress)) {
Expand All @@ -39,9 +54,16 @@ public static function COLUMN($cellAddress = null, ?Cell $cell = null)

return (int) Coordinate::columnIndexFromString($columnKey);
}

return self::cellColumn($pCell);
}

[, $cellAddress] = Worksheet::extractSheetTitle((string) $cellAddress, true);
$cellAddress = $cellAddress ?? '';
if ($pCell != null) {
[,, $sheetName] = Helpers::extractWorksheet($cellAddress, $pCell);
[,, $cellAddress] = Helpers::extractCellAddresses($cellAddress, true, $pCell->getWorksheet(), $sheetName);
}
[, $cellAddress] = Worksheet::extractSheetTitle($cellAddress, true);
if (strpos($cellAddress, ':') !== false) {
[$startAddress, $endAddress] = explode(':', $cellAddress);
$startAddress = preg_replace('/[^a-z]/i', '', $startAddress);
Expand Down Expand Up @@ -73,9 +95,10 @@ public static function COLUMN($cellAddress = null, ?Cell $cell = null)
*/
public static function COLUMNS($cellAddress = null)
{
if ($cellAddress === null || (is_string($cellAddress) && trim($cellAddress) === '')) {
if (self::cellAddressNullOrWhitespace($cellAddress)) {
return 1;
} elseif (!is_array($cellAddress)) {
}
if (!is_array($cellAddress)) {
return Functions::VALUE();
}

Expand All @@ -90,6 +113,11 @@ public static function COLUMNS($cellAddress = null)
return $columns;
}

private static function cellRow(?Cell $pCell): int
{
return ($pCell !== null) ? $pCell->getRow() : 1;
}

/**
* ROW.
*
Expand All @@ -109,8 +137,8 @@ public static function COLUMNS($cellAddress = null)
*/
public static function ROW($cellAddress = null, ?Cell $pCell = null)
{
if ($cellAddress === null || (!is_array($cellAddress) && trim($cellAddress) === '')) {
return ($pCell !== null) ? $pCell->getRow() : 1;
if (self::cellAddressNullOrWhitespace($cellAddress)) {
return self::cellRow($pCell);
}

if (is_array($cellAddress)) {
Expand All @@ -119,9 +147,16 @@ public static function ROW($cellAddress = null, ?Cell $pCell = null)
return (int) preg_replace('/\D/', '', $rowKey);
}
}

return self::cellRow($pCell);
}

[, $cellAddress] = Worksheet::extractSheetTitle((string) $cellAddress, true);
$cellAddress = $cellAddress ?? '';
if ($pCell !== null) {
[,, $sheetName] = Helpers::extractWorksheet($cellAddress, $pCell);
[,, $cellAddress] = Helpers::extractCellAddresses($cellAddress, true, $pCell->getWorksheet(), $sheetName);
}
[, $cellAddress] = Worksheet::extractSheetTitle($cellAddress, true);
if (strpos($cellAddress, ':') !== false) {
[$startAddress, $endAddress] = explode(':', $cellAddress);
$startAddress = preg_replace('/\D/', '', $startAddress);
Expand Down Expand Up @@ -154,9 +189,10 @@ function ($value) {
*/
public static function ROWS($cellAddress = null)
{
if ($cellAddress === null || (is_string($cellAddress) && trim($cellAddress) === '')) {
if (self::cellAddressNullOrWhitespace($cellAddress)) {
return 1;
} elseif (!is_array($cellAddress)) {
}
if (!is_array($cellAddress)) {
return Functions::VALUE();
}

Expand Down
2 changes: 1 addition & 1 deletion src/PhpSpreadsheet/Cell/Cell.php
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ public function getCalculatedValue($resetLog = true)
} catch (Exception $ex) {
if (($ex->getMessage() === 'Unable to access External Workbook') && ($this->calculatedValue !== null)) {
return $this->calculatedValue; // Fallback for calculations referencing external files.
} elseif (strpos($ex->getMessage(), 'undefined name') !== false) {
} elseif (preg_match('/[Uu]ndefined (name|offset: 2|array key 2)/', $ex->getMessage()) === 1) {
return \PhpOffice\PhpSpreadsheet\Calculation\Functions::NAME();
}

Expand Down
Loading

0 comments on commit c79a9a8

Please sign in to comment.