Skip to content

Commit e637713

Browse files
oleibmanoleibman
committed
Make Explicit Array Return Type When Tests Require It
When the Dynamic Array PR PHPOffice#3962 was introduced, it left the default as Return Array as Value. At some point, the default should be changed to Return Array as Array. This would, of course, be a breaking change, one which will not be part of Release 4. However, it will possibly be part of Release 5. Rather than relying on the default setting, this PR explicitly sets Return Array as Value when tests require that setting. This will make it easier to identify potential breaks when the default is changed. The entire test suite will now succeed with either setting as default. In making these changes, a few minor problems were discovered with how Array as Array is handled. These are fixed with this PR.
1 parent fb757cf commit e637713

36 files changed

+237
-78
lines changed

‎samples/LookupRef/COLUMN.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
<?php
22

3+
use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
34
use PhpOffice\PhpSpreadsheet\Spreadsheet;
45

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

910
// Create new PhpSpreadsheet object
1011
$spreadsheet = new Spreadsheet();
12+
$calculation = Calculation::getInstance($spreadsheet);
13+
$calculation->setInstanceArrayReturnType(
14+
Calculation::RETURN_ARRAY_AS_VALUE
15+
);
1116
$worksheet = $spreadsheet->getActiveSheet();
1217

1318
$worksheet->getCell('A1')->setValue('=COLUMN(C13)');

‎src/PhpSpreadsheet/Calculation/Calculation.php

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4882,17 +4882,18 @@ private function processTokenStack(mixed $tokens, ?string $cellID = null, ?Cell
48824882
}
48834883
$result = $operand1;
48844884
} else {
4885-
// In theory, we should truncate here.
4886-
// But I can't figure out a formula
4887-
// using the concatenation operator
4888-
// with literals that fits in 32K,
4889-
// so I don't think we can overflow here.
48904885
if (Information\ErrorValue::isError($operand1)) {
48914886
$result = $operand1;
48924887
} elseif (Information\ErrorValue::isError($operand2)) {
48934888
$result = $operand2;
48944889
} else {
4895-
$result = self::FORMULA_STRING_QUOTE . str_replace('""', self::FORMULA_STRING_QUOTE, self::unwrapResult($operand1) . self::unwrapResult($operand2)) . self::FORMULA_STRING_QUOTE;
4890+
$result = str_replace('""', self::FORMULA_STRING_QUOTE, self::unwrapResult($operand1) . self::unwrapResult($operand2));
4891+
$result = Shared\StringHelper::substring(
4892+
$result,
4893+
0,
4894+
DataType::MAX_STRING_LENGTH
4895+
);
4896+
$result = self::FORMULA_STRING_QUOTE . $result . self::FORMULA_STRING_QUOTE;
48964897
}
48974898
}
48984899
$this->debugLog->writeDebugLog('Evaluation Result is %s', $this->showTypeDetails($result));
@@ -5041,6 +5042,9 @@ private function processTokenStack(mixed $tokens, ?string $cellID = null, ?Cell
50415042
while (is_array($cellValue)) {
50425043
$cellValue = array_shift($cellValue);
50435044
}
5045+
if (is_string($cellValue)) {
5046+
$cellValue = preg_replace('/"/', '""', $cellValue);
5047+
}
50445048
$this->debugLog->writeDebugLog('Scalar Result for cell %s is %s', $cellRef, $this->showTypeDetails($cellValue));
50455049
}
50465050
$this->processingAnchorArray = false;

‎src/PhpSpreadsheet/Calculation/Financial/CashFlow/Single.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,13 @@ public static function periods(mixed $rate, mixed $presentValue, mixed $futureVa
7777
*
7878
* Calculates the interest rate required for an investment to grow to a specified future value .
7979
*
80-
* @param array|float $periods The number of periods over which the investment is made
81-
* @param array|float $presentValue Present Value
82-
* @param array|float $futureValue Future Value
80+
* @param mixed $periods The number of periods over which the investment is made, expect array|float
81+
* @param mixed $presentValue Present Value, expect array|float
82+
* @param mixed $futureValue Future Value, expect array|float
8383
*
8484
* @return float|string Result, or a string containing an error
8585
*/
86-
public static function interestRate(array|float $periods = 0.0, array|float $presentValue = 0.0, array|float $futureValue = 0.0): string|float
86+
public static function interestRate(mixed $periods = 0.0, mixed $presentValue = 0.0, mixed $futureValue = 0.0): string|float
8787
{
8888
$periods = Functions::flattenSingleValue($periods);
8989
$presentValue = Functions::flattenSingleValue($presentValue);

‎src/PhpSpreadsheet/Calculation/Financial/CashFlow/Variable/NonPeriodic.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,14 @@ class NonPeriodic
2323
* Excel Function:
2424
* =XIRR(values,dates,guess)
2525
*
26-
* @param float[] $values A series of cash flow payments
26+
* @param mixed $values A series of cash flow payments, expecting float[]
2727
* The series of values must contain at least one positive value & one negative value
2828
* @param mixed[] $dates A series of payment dates
2929
* The first payment date indicates the beginning of the schedule of payments
3030
* All other dates must be later than this date, but they may occur in any order
3131
* @param mixed $guess An optional guess at the expected answer
3232
*/
33-
public static function rate(array $values, array $dates, mixed $guess = self::DEFAULT_GUESS): float|string
33+
public static function rate(mixed $values, mixed $dates, mixed $guess = self::DEFAULT_GUESS): float|string
3434
{
3535
$rslt = self::xirrPart1($values, $dates);
3636
if ($rslt !== '') {
@@ -106,18 +106,18 @@ public static function rate(array $values, array $dates, mixed $guess = self::DE
106106
* Excel Function:
107107
* =XNPV(rate,values,dates)
108108
*
109-
* @param array|float $rate the discount rate to apply to the cash flows
110-
* @param float[] $values A series of cash flows that corresponds to a schedule of payments in dates.
109+
* @param mixed $rate the discount rate to apply to the cash flows, expect array|float
110+
* @param mixed $values A series of cash flows that corresponds to a schedule of payments in dates, expecting floag[].
111111
* The first payment is optional and corresponds to a cost or payment that occurs
112112
* at the beginning of the investment.
113113
* If the first value is a cost or payment, it must be a negative value.
114114
* All succeeding payments are discounted based on a 365-day year.
115115
* The series of values must contain at least one positive value and one negative value.
116-
* @param mixed[] $dates A schedule of payment dates that corresponds to the cash flow payments.
116+
* @param mixed $dates A schedule of payment dates that corresponds to the cash flow payments, expecting mixed[].
117117
* The first payment date indicates the beginning of the schedule of payments.
118118
* All other dates must be later than this date, but they may occur in any order.
119119
*/
120-
public static function presentValue(array|float $rate, array $values, array $dates): float|string
120+
public static function presentValue(mixed $rate, mixed $values, mixed $dates): float|string
121121
{
122122
return self::xnpvOrdered($rate, $values, $dates, true);
123123
}

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,15 @@ public static function index(mixed $matrix, mixed $rowNum = 0, mixed $columnNum
8282

8383
$rowNum = $rowNum ?? 0;
8484
$columnNum = $columnNum ?? 0;
85+
if (is_scalar($matrix)) {
86+
if ($rowNum === 0 || $rowNum === 1) {
87+
if ($columnNum === 0 || $columnNum === 1) {
88+
if ($columnNum === 1 || $rowNum === 1) {
89+
return $matrix;
90+
}
91+
}
92+
}
93+
}
8594

8695
try {
8796
$rowNum = LookupRefValidations::validatePositiveInt($rowNum);

‎src/PhpSpreadsheet/Cell/Cell.php

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -408,9 +408,6 @@ public function getCalculatedValue(bool $resetLog = true): mixed
408408
$oldAttributesT = $oldAttributes['t'] ?? '';
409409
$coordinate = $this->getCoordinate();
410410
$oldAttributesRef = $oldAttributes['ref'] ?? $coordinate;
411-
if (!str_contains($oldAttributesRef, ':')) {
412-
$oldAttributesRef .= ":$oldAttributesRef";
413-
}
414411
$originalValue = $this->value;
415412
$originalDataType = $this->dataType;
416413
$this->formulaAttributes = [];
@@ -434,6 +431,14 @@ public function getCalculatedValue(bool $resetLog = true): mixed
434431
$result = array_shift($result);
435432
}
436433
}
434+
if (
435+
!is_array($result)
436+
&& $calculation->getInstanceArrayReturnType() === Calculation::RETURN_ARRAY_AS_ARRAY
437+
&& $oldAttributesT === 'array'
438+
&& ($oldAttributesRef === $coordinate || $oldAttributesRef === "$coordinate:$coordinate")
439+
) {
440+
$result = [$result];
441+
}
437442
// if return_as_array for formula like '=sheet!cell'
438443
if (is_array($result) && count($result) === 1) {
439444
$resultKey = array_keys($result)[0];
@@ -560,6 +565,8 @@ public function getCalculatedValue(bool $resetLog = true): mixed
560565
SharedDate::setExcelCalendar($currentCalendar);
561566

562567
if ($result === Functions::NOT_YET_IMPLEMENTED) {
568+
$this->formulaAttributes = $oldAttributes;
569+
563570
return $this->calculatedValue; // Fallback if calculation engine does not support the formula.
564571
}
565572

‎src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1498,6 +1498,9 @@ private function writeCellFormula(XMLWriter $objWriter, string $cellValue, Cell
14981498

14991499
if (isset($attributes['ref'])) {
15001500
$ref = $this->parseRef($coordinate, $attributes['ref']);
1501+
if ($ref === "$coordinate:$coordinate") {
1502+
$ref = $coordinate;
1503+
}
15011504
} else {
15021505
$ref = $coordinate;
15031506
}

‎tests/PhpSpreadsheetTests/Calculation/ArrayFormulaTest.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,10 @@ public static function providerArrayFormulae(): array
6262
public function testArrayFormulaUsingCells(): void
6363
{
6464
$spreadsheet = new Spreadsheet();
65+
$calculation = Calculation::getInstance($spreadsheet);
66+
$calculation->setInstanceArrayReturnType(
67+
Calculation::RETURN_ARRAY_AS_VALUE
68+
);
6569
$sheet = $spreadsheet->getActiveSheet();
6670
$sheet->getCell('A4')->setValue(-3);
6771
$sheet->getCell('B4')->setValue(4);

‎tests/PhpSpreadsheetTests/Calculation/CalculationLoggingTest.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ public function testFormulaWithLogging(): void
4242
public function testFormulaWithMultipleCellLogging(): void
4343
{
4444
$spreadsheet = new Spreadsheet();
45+
$calculation = Calculation::getInstance($spreadsheet);
46+
$calculation->setInstanceArrayReturnType(
47+
Calculation::RETURN_ARRAY_AS_VALUE
48+
);
4549
$sheet = $spreadsheet->getActiveSheet();
4650

4751
$sheet->fromArray(

‎tests/PhpSpreadsheetTests/Calculation/FormulaAsStringTest.php

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

55
namespace PhpOffice\PhpSpreadsheetTests\Calculation;
66

7+
use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
78
use PhpOffice\PhpSpreadsheet\Spreadsheet;
9+
use PHPUnit\Framework\Attributes\DataProvider;
810
use PHPUnit\Framework\TestCase;
911

1012
class FormulaAsStringTest extends TestCase
1113
{
12-
#[\PHPUnit\Framework\Attributes\DataProvider('providerFunctionsAsString')]
14+
#[DataProvider('providerFunctionsAsString')]
1315
public function testFunctionsAsString(mixed $expectedResult, string $formula): void
1416
{
1517
$spreadsheet = new Spreadsheet();
18+
$calculation = Calculation::getInstance($spreadsheet);
19+
$calculation->setInstanceArrayReturnType(
20+
Calculation::RETURN_ARRAY_AS_VALUE
21+
);
1622
$workSheet = $spreadsheet->getActiveSheet();
1723
$workSheet->setCellValue('A1', 10);
1824
$workSheet->setCellValue('A2', 20);

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ class IsFormulaTest extends TestCase
1515
public function testIsFormula(): void
1616
{
1717
$spreadsheet = new Spreadsheet();
18+
$calculation = Calculation::getInstance($spreadsheet);
19+
$calculation->setInstanceArrayReturnType(
20+
Calculation::RETURN_ARRAY_AS_VALUE
21+
);
1822
$sheet1 = $spreadsheet->getActiveSheet();
1923
$sheet1->setTitle('SheetOne'); // no space in sheet title
2024
$sheet2 = $spreadsheet->createSheet();

‎tests/PhpSpreadsheetTests/Calculation/Functions/Logical/AllSetupTeardown.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
namespace PhpOffice\PhpSpreadsheetTests\Calculation\Functions\Logical;
66

7+
use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
78
use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalcException;
89
use PhpOffice\PhpSpreadsheet\Calculation\Functions;
910
use PhpOffice\PhpSpreadsheet\Cell\DataType;
@@ -100,4 +101,13 @@ protected function runTestCase(string $functionName, mixed $expectedResult, mixe
100101
$this->setCell('B1', $formula);
101102
self::assertSame($expectedResult, $sheet->getCell('B1')->getCalculatedValue());
102103
}
104+
105+
protected function setArrayAsValue(): void
106+
{
107+
$spreadsheet = $this->getSpreadsheet();
108+
$calculation = Calculation::getInstance($spreadsheet);
109+
$calculation->setInstanceArrayReturnType(
110+
Calculation::RETURN_ARRAY_AS_VALUE
111+
);
112+
}
103113
}

‎tests/PhpSpreadsheetTests/Calculation/Functions/Logical/AndTest.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,14 @@
44

55
namespace PhpOffice\PhpSpreadsheetTests\Calculation\Functions\Logical;
66

7+
use PHPUnit\Framework\Attributes\DataProvider;
8+
79
class AndTest extends AllSetupTeardown
810
{
9-
#[\PHPUnit\Framework\Attributes\DataProvider('providerAND')]
11+
#[DataProvider('providerAND')]
1012
public function testAND(mixed $expectedResult, mixed ...$args): void
1113
{
14+
$this->setArrayAsValue();
1215
$this->runTestCase('AND', $expectedResult, ...$args);
1316
}
1417

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

20-
#[\PHPUnit\Framework\Attributes\DataProvider('providerANDLiteral')]
23+
#[DataProvider('providerANDLiteral')]
2124
public function testANDLiteral(bool|string $expectedResult, float|int|string $formula): void
2225
{
2326
$sheet = $this->getSheet();

‎tests/PhpSpreadsheetTests/Calculation/Functions/Logical/XorTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ class XorTest extends AllSetupTeardown
99
#[\PHPUnit\Framework\Attributes\DataProvider('providerXOR')]
1010
public function testXOR(mixed $expectedResult, mixed ...$args): void
1111
{
12+
$this->setArrayAsValue();
1213
$this->runTestCase('XOR', $expectedResult, ...$args);
1314
}
1415

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ class AllSetupTeardown extends TestCase
1818

1919
protected string $arrayReturnType;
2020

21-
private ?Spreadsheet $spreadsheet = null;
21+
protected ?Spreadsheet $spreadsheet = null;
2222

2323
private ?Worksheet $sheet = null;
2424

@@ -86,4 +86,13 @@ protected function getSheet(): Worksheet
8686

8787
return $this->sheet;
8888
}
89+
90+
protected function setArrayAsValue(): void
91+
{
92+
$spreadsheet = $this->getSpreadsheet();
93+
$calculation = Calculation::getInstance($spreadsheet);
94+
$calculation->setInstanceArrayReturnType(
95+
Calculation::RETURN_ARRAY_AS_VALUE
96+
);
97+
}
8998
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,15 @@
55
namespace PhpOffice\PhpSpreadsheetTests\Calculation\Functions\LookupRef;
66

77
use PhpOffice\PhpSpreadsheet\NamedRange;
8+
use PHPUnit\Framework\Attributes\DataProvider;
89

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

3840
public function testCOLUMNLocalDefinedName(): void
3941
{
42+
$this->setArrayAsValue();
4043
$sheet = $this->getSheet();
4144

4245
$sheet1 = $this->getSpreadsheet()->createSheet();

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

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,15 @@
77
use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
88
use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
99
use PhpOffice\PhpSpreadsheet\NamedRange;
10-
use PhpOffice\PhpSpreadsheet\Spreadsheet;
11-
use PHPUnit\Framework\TestCase;
10+
use PHPUnit\Framework\Attributes\DataProvider;
1211

13-
class HLookupTest extends TestCase
12+
class HLookupTest extends AllSetupTeardown
1413
{
15-
#[\PHPUnit\Framework\Attributes\DataProvider('providerHLOOKUP')]
14+
#[DataProvider('providerHLOOKUP')]
1615
public function testHLOOKUP(mixed $expectedResult, mixed $lookup, array $values, mixed $rowIndex, ?bool $rangeLookup = null): void
1716
{
18-
$spreadsheet = new Spreadsheet();
19-
$sheet = $spreadsheet->getActiveSheet();
17+
$this->setArrayAsValue();
18+
$sheet = $this->getSheet();
2019
$maxRow = 0;
2120
$maxCol = 0;
2221
$maxColLetter = 'A';
@@ -52,8 +51,6 @@ public function testHLOOKUP(mixed $expectedResult, mixed $lookup, array $values,
5251
}
5352
$sheet->getCell('ZZ1')->setValue("=HLOOKUP(ZZ8, A1:$maxColLetter$maxRow, $indexarg$boolArg)");
5453
self::assertEquals($expectedResult, $sheet->getCell('ZZ1')->getCalculatedValue());
55-
56-
$spreadsheet->disconnectWorksheets();
5754
}
5855

5956
private static function parseRangeLookup(?bool $rangeLookup): string
@@ -70,7 +67,7 @@ public static function providerHLOOKUP(): array
7067
return require 'tests/data/Calculation/LookupRef/HLOOKUP.php';
7168
}
7269

73-
#[\PHPUnit\Framework\Attributes\DataProvider('providerHLookupNamedRange')]
70+
#[DataProvider('providerHLookupNamedRange')]
7471
public function testHLookupNamedRange(string $expectedResult, string $cellAddress): void
7572
{
7673
$lookupData = [
@@ -85,12 +82,11 @@ public function testHLookupNamedRange(string $expectedResult, string $cellAddres
8582
['Cleanliness', 3, '=HLOOKUP(C8,Lookup_Table,2,FALSE)'],
8683
];
8784

88-
$spreadsheet = new Spreadsheet();
89-
$worksheet = $spreadsheet->getActiveSheet();
85+
$worksheet = $this->getSheet();
9086
$worksheet->fromArray($lookupData, null, 'F4');
9187
$worksheet->fromArray($formData, null, 'B4');
9288

93-
$spreadsheet->addNamedRange(new NamedRange('Lookup_Table', $worksheet, '=$G$4:$J$5'));
89+
$this->getSpreadsheet()->addNamedRange(new NamedRange('Lookup_Table', $worksheet, '=$G$4:$J$5'));
9490

9591
$result = $worksheet->getCell($cellAddress)->getCalculatedValue();
9692
self::assertEquals($expectedResult, $result);
@@ -106,7 +102,7 @@ public static function providerHLookupNamedRange(): array
106102
];
107103
}
108104

109-
#[\PHPUnit\Framework\Attributes\DataProvider('providerHLookupArray')]
105+
#[DataProvider('providerHLookupArray')]
110106
public function testHLookupArray(array $expectedResult, string $values, string $database, string $index): void
111107
{
112108
$calculation = Calculation::getInstance();

0 commit comments

Comments
 (0)