Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Explicit Array Return Type When Tests Require It #4328

Merged
merged 3 commits into from
Feb 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ and this project adheres to [Semantic Versioning](https://semver.org).
### Fixed

- Xls Writer Parser Mishandling True/False Argument. [Issue #4331](https://github.com/PHPOffice/PhpSpreadsheet/issues/4331) [PR #4333](https://github.com/PHPOffice/PhpSpreadsheet/pull/4333)
- Minor changes to dynamic array calculations exposed by using explicit array return types in some tests. [PR #4328](https://github.com/PHPOffice/PhpSpreadsheet/pull/4328)

## 2025-01-26 - 3.9.0

Expand Down
5 changes: 5 additions & 0 deletions samples/LookupRef/COLUMN.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php

use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
use PhpOffice\PhpSpreadsheet\Spreadsheet;

require __DIR__ . '/../Header.php';
Expand All @@ -8,6 +9,10 @@

// Create new PhpSpreadsheet object
$spreadsheet = new Spreadsheet();
$calculation = Calculation::getInstance($spreadsheet);
$calculation->setInstanceArrayReturnType(
Calculation::RETURN_ARRAY_AS_VALUE
);
$worksheet = $spreadsheet->getActiveSheet();

$worksheet->getCell('A1')->setValue('=COLUMN(C13)');
Expand Down
16 changes: 10 additions & 6 deletions src/PhpSpreadsheet/Calculation/Calculation.php
Original file line number Diff line number Diff line change
Expand Up @@ -4887,17 +4887,18 @@ private function processTokenStack(mixed $tokens, ?string $cellID = null, ?Cell
}
$result = $operand1;
} else {
// In theory, we should truncate here.
// But I can't figure out a formula
// using the concatenation operator
// with literals that fits in 32K,
// so I don't think we can overflow here.
if (Information\ErrorValue::isError($operand1)) {
$result = $operand1;
} elseif (Information\ErrorValue::isError($operand2)) {
$result = $operand2;
} else {
$result = self::FORMULA_STRING_QUOTE . str_replace('""', self::FORMULA_STRING_QUOTE, self::unwrapResult($operand1) . self::unwrapResult($operand2)) . self::FORMULA_STRING_QUOTE;
$result = str_replace('""', self::FORMULA_STRING_QUOTE, self::unwrapResult($operand1) . self::unwrapResult($operand2));
$result = Shared\StringHelper::substring(
$result,
0,
DataType::MAX_STRING_LENGTH
);
$result = self::FORMULA_STRING_QUOTE . $result . self::FORMULA_STRING_QUOTE;
}
}
$this->debugLog->writeDebugLog('Evaluation Result is %s', $this->showTypeDetails($result));
Expand Down Expand Up @@ -5046,6 +5047,9 @@ private function processTokenStack(mixed $tokens, ?string $cellID = null, ?Cell
while (is_array($cellValue)) {
$cellValue = array_shift($cellValue);
}
if (is_string($cellValue)) {
$cellValue = preg_replace('/"/', '""', $cellValue);
}
$this->debugLog->writeDebugLog('Scalar Result for cell %s is %s', $cellRef, $this->showTypeDetails($cellValue));
}
$this->processingAnchorArray = false;
Expand Down
8 changes: 4 additions & 4 deletions src/PhpSpreadsheet/Calculation/Financial/CashFlow/Single.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,13 @@ public static function periods(mixed $rate, mixed $presentValue, mixed $futureVa
*
* Calculates the interest rate required for an investment to grow to a specified future value .
*
* @param array|float $periods The number of periods over which the investment is made
* @param array|float $presentValue Present Value
* @param array|float $futureValue Future Value
* @param mixed $periods The number of periods over which the investment is made, expect array|float
* @param mixed $presentValue Present Value, expect array|float
* @param mixed $futureValue Future Value, expect array|float
*
* @return float|string Result, or a string containing an error
*/
public static function interestRate(array|float $periods = 0.0, array|float $presentValue = 0.0, array|float $futureValue = 0.0): string|float
public static function interestRate(mixed $periods = 0.0, mixed $presentValue = 0.0, mixed $futureValue = 0.0): string|float
{
$periods = Functions::flattenSingleValue($periods);
$presentValue = Functions::flattenSingleValue($presentValue);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@ class NonPeriodic
* Excel Function:
* =XIRR(values,dates,guess)
*
* @param float[] $values A series of cash flow payments
* @param mixed $values A series of cash flow payments, expecting float[]
* The series of values must contain at least one positive value & one negative value
* @param mixed[] $dates A series of payment dates
* The first payment date indicates the beginning of the schedule of payments
* All other dates must be later than this date, but they may occur in any order
* @param mixed $guess An optional guess at the expected answer
*/
public static function rate(array $values, array $dates, mixed $guess = self::DEFAULT_GUESS): float|string
public static function rate(mixed $values, mixed $dates, mixed $guess = self::DEFAULT_GUESS): float|string
{
$rslt = self::xirrPart1($values, $dates);
if ($rslt !== '') {
Expand Down Expand Up @@ -106,18 +106,18 @@ public static function rate(array $values, array $dates, mixed $guess = self::DE
* Excel Function:
* =XNPV(rate,values,dates)
*
* @param array|float $rate the discount rate to apply to the cash flows
* @param float[] $values A series of cash flows that corresponds to a schedule of payments in dates.
* @param mixed $rate the discount rate to apply to the cash flows, expect array|float
* @param mixed $values A series of cash flows that corresponds to a schedule of payments in dates, expecting floag[].
* The first payment is optional and corresponds to a cost or payment that occurs
* at the beginning of the investment.
* If the first value is a cost or payment, it must be a negative value.
* All succeeding payments are discounted based on a 365-day year.
* The series of values must contain at least one positive value and one negative value.
* @param mixed[] $dates A schedule of payment dates that corresponds to the cash flow payments.
* @param mixed $dates A schedule of payment dates that corresponds to the cash flow payments, expecting mixed[].
* The first payment date indicates the beginning of the schedule of payments.
* All other dates must be later than this date, but they may occur in any order.
*/
public static function presentValue(array|float $rate, array $values, array $dates): float|string
public static function presentValue(mixed $rate, mixed $values, mixed $dates): float|string
{
return self::xnpvOrdered($rate, $values, $dates, true);
}
Expand Down
9 changes: 9 additions & 0 deletions src/PhpSpreadsheet/Calculation/LookupRef/Matrix.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,15 @@ public static function index(mixed $matrix, mixed $rowNum = 0, mixed $columnNum

$rowNum = $rowNum ?? 0;
$columnNum = $columnNum ?? 0;
if (is_scalar($matrix)) {
if ($rowNum === 0 || $rowNum === 1) {
if ($columnNum === 0 || $columnNum === 1) {
if ($columnNum === 1 || $rowNum === 1) {
return $matrix;
}
}
}
}

try {
$rowNum = LookupRefValidations::validatePositiveInt($rowNum);
Expand Down
13 changes: 10 additions & 3 deletions src/PhpSpreadsheet/Cell/Cell.php
Original file line number Diff line number Diff line change
Expand Up @@ -408,9 +408,6 @@ public function getCalculatedValue(bool $resetLog = true): mixed
$oldAttributesT = $oldAttributes['t'] ?? '';
$coordinate = $this->getCoordinate();
$oldAttributesRef = $oldAttributes['ref'] ?? $coordinate;
if (!str_contains($oldAttributesRef, ':')) {
$oldAttributesRef .= ":$oldAttributesRef";
}
$originalValue = $this->value;
$originalDataType = $this->dataType;
$this->formulaAttributes = [];
Expand All @@ -434,6 +431,14 @@ public function getCalculatedValue(bool $resetLog = true): mixed
$result = array_shift($result);
}
}
if (
!is_array($result)
&& $calculation->getInstanceArrayReturnType() === Calculation::RETURN_ARRAY_AS_ARRAY
&& $oldAttributesT === 'array'
&& ($oldAttributesRef === $coordinate || $oldAttributesRef === "$coordinate:$coordinate")
) {
$result = [$result];
}
// if return_as_array for formula like '=sheet!cell'
if (is_array($result) && count($result) === 1) {
$resultKey = array_keys($result)[0];
Expand Down Expand Up @@ -560,6 +565,8 @@ public function getCalculatedValue(bool $resetLog = true): mixed
SharedDate::setExcelCalendar($currentCalendar);

if ($result === Functions::NOT_YET_IMPLEMENTED) {
$this->formulaAttributes = $oldAttributes;

return $this->calculatedValue; // Fallback if calculation engine does not support the formula.
}

Expand Down
3 changes: 3 additions & 0 deletions src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php
Original file line number Diff line number Diff line change
Expand Up @@ -1513,6 +1513,9 @@ private function writeCellFormula(XMLWriter $objWriter, string $cellValue, Cell

if (isset($attributes['ref'])) {
$ref = $this->parseRef($coordinate, $attributes['ref']);
if ($ref === "$coordinate:$coordinate") {
$ref = $coordinate;
}
} else {
$ref = $coordinate;
}
Expand Down
4 changes: 4 additions & 0 deletions tests/PhpSpreadsheetTests/Calculation/ArrayFormulaTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ public static function providerArrayFormulae(): array
public function testArrayFormulaUsingCells(): void
{
$spreadsheet = new Spreadsheet();
$calculation = Calculation::getInstance($spreadsheet);
$calculation->setInstanceArrayReturnType(
Calculation::RETURN_ARRAY_AS_VALUE
);
$sheet = $spreadsheet->getActiveSheet();
$sheet->getCell('A4')->setValue(-3);
$sheet->getCell('B4')->setValue(4);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ public function testFormulaWithLogging(): void
public function testFormulaWithMultipleCellLogging(): void
{
$spreadsheet = new Spreadsheet();
$calculation = Calculation::getInstance($spreadsheet);
$calculation->setInstanceArrayReturnType(
Calculation::RETURN_ARRAY_AS_VALUE
);
$sheet = $spreadsheet->getActiveSheet();

$sheet->fromArray(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,21 @@

namespace PhpOffice\PhpSpreadsheetTests\Calculation;

use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\TestCase;

class FormulaAsStringTest extends TestCase
{
#[\PHPUnit\Framework\Attributes\DataProvider('providerFunctionsAsString')]
#[DataProvider('providerFunctionsAsString')]
public function testFunctionsAsString(mixed $expectedResult, string $formula): void
{
$spreadsheet = new Spreadsheet();
$calculation = Calculation::getInstance($spreadsheet);
$calculation->setInstanceArrayReturnType(
Calculation::RETURN_ARRAY_AS_VALUE
);
$workSheet = $spreadsheet->getActiveSheet();
$workSheet->setCellValue('A1', 10);
$workSheet->setCellValue('A2', 20);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ class IsFormulaTest extends TestCase
public function testIsFormula(): void
{
$spreadsheet = new Spreadsheet();
$calculation = Calculation::getInstance($spreadsheet);
$calculation->setInstanceArrayReturnType(
Calculation::RETURN_ARRAY_AS_VALUE
);
$sheet1 = $spreadsheet->getActiveSheet();
$sheet1->setTitle('SheetOne'); // no space in sheet title
$sheet2 = $spreadsheet->createSheet();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace PhpOffice\PhpSpreadsheetTests\Calculation\Functions\Logical;

use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalcException;
use PhpOffice\PhpSpreadsheet\Calculation\Functions;
use PhpOffice\PhpSpreadsheet\Cell\DataType;
Expand Down Expand Up @@ -100,4 +101,13 @@ protected function runTestCase(string $functionName, mixed $expectedResult, mixe
$this->setCell('B1', $formula);
self::assertSame($expectedResult, $sheet->getCell('B1')->getCalculatedValue());
}

protected function setArrayAsValue(): void
{
$spreadsheet = $this->getSpreadsheet();
$calculation = Calculation::getInstance($spreadsheet);
$calculation->setInstanceArrayReturnType(
Calculation::RETURN_ARRAY_AS_VALUE
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@

namespace PhpOffice\PhpSpreadsheetTests\Calculation\Functions\Logical;

use PHPUnit\Framework\Attributes\DataProvider;

class AndTest extends AllSetupTeardown
{
#[\PHPUnit\Framework\Attributes\DataProvider('providerAND')]
#[DataProvider('providerAND')]
public function testAND(mixed $expectedResult, mixed ...$args): void
{
$this->setArrayAsValue();
$this->runTestCase('AND', $expectedResult, ...$args);
}

Expand All @@ -17,7 +20,7 @@ public static function providerAND(): array
return require 'tests/data/Calculation/Logical/AND.php';
}

#[\PHPUnit\Framework\Attributes\DataProvider('providerANDLiteral')]
#[DataProvider('providerANDLiteral')]
public function testANDLiteral(bool|string $expectedResult, float|int|string $formula): void
{
$sheet = $this->getSheet();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class XorTest extends AllSetupTeardown
#[\PHPUnit\Framework\Attributes\DataProvider('providerXOR')]
public function testXOR(mixed $expectedResult, mixed ...$args): void
{
$this->setArrayAsValue();
$this->runTestCase('XOR', $expectedResult, ...$args);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class AllSetupTeardown extends TestCase

protected string $arrayReturnType;

private ?Spreadsheet $spreadsheet = null;
protected ?Spreadsheet $spreadsheet = null;

private ?Worksheet $sheet = null;

Expand Down Expand Up @@ -86,4 +86,13 @@ protected function getSheet(): Worksheet

return $this->sheet;
}

protected function setArrayAsValue(): void
{
$spreadsheet = $this->getSpreadsheet();
$calculation = Calculation::getInstance($spreadsheet);
$calculation->setInstanceArrayReturnType(
Calculation::RETURN_ARRAY_AS_VALUE
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@
namespace PhpOffice\PhpSpreadsheetTests\Calculation\Functions\LookupRef;

use PhpOffice\PhpSpreadsheet\NamedRange;
use PHPUnit\Framework\Attributes\DataProvider;

class ColumnOnSpreadsheetTest extends AllSetupTeardown
{
#[\PHPUnit\Framework\Attributes\DataProvider('providerCOLUMNonSpreadsheet')]
#[DataProvider('providerCOLUMNonSpreadsheet')]
public function testColumnOnSpreadsheet(mixed $expectedResult, string $cellReference = 'omitted'): void
{
$this->mightHaveException($expectedResult);
$this->setArrayAsValue();
$sheet = $this->getSheet();
$this->getSpreadsheet()->addNamedRange(new NamedRange('namedrangex', $sheet, '$E$2:$E$6'));
$this->getSpreadsheet()->addNamedRange(new NamedRange('namedrangey', $sheet, '$F$2:$H$2'));
Expand All @@ -37,6 +39,7 @@ public static function providerCOLUMNonSpreadsheet(): array

public function testCOLUMNLocalDefinedName(): void
{
$this->setArrayAsValue();
$sheet = $this->getSheet();

$sheet1 = $this->getSpreadsheet()->createSheet();
Expand Down
Loading