Skip to content

Commit

Permalink
WIP Avoid a PHP8.4 Deprecation (#3789)
Browse files Browse the repository at this point in the history
Fix #3782. A signature of the ReflectionMethod constructor will be deprecated, and PhpSpreadsheet runs afoul of that change in one place. Php8.4 is not yet available in any form, and I am reluctant to make this change until we see that the issue is real (and that this PR fixes it), so am leaving this PR in draft status till then.

I note that one Excel function `PI` is implemented not as a class method in PhpSpreadsheet, but rather as a call to the native Php function `pi`. The ReflectionMethod call is subject to a TypeError in the changed code, but that is already the case. We haven't seen a TypeError because (a) it will arise only if the caller supplies an argument to the function (which must be called with zero arguments), and (b) there are no test cases for that function. The code is slightly cleaned up, and test cases are now added. This is not an important enough problem to rush this PR - the existing code (and the changed code), rather than failing with TypeError, will fail with a CalculationException (wrong number of arguments) before it gets to the TypeError; that is the correct behavior.
  • Loading branch information
oleibman authored Dec 6, 2023
1 parent 0fddcc1 commit 5bcbc7d
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/PhpSpreadsheet/Calculation/Calculation.php
Original file line number Diff line number Diff line change
Expand Up @@ -5072,7 +5072,7 @@ private function processTokenStack(mixed $tokens, $cellID = null, ?Cell $cell =
krsort($args);
krsort($emptyArguments);

if ($argCount > 0) {
if ($argCount > 0 && is_array($functionCall)) {
$args = $this->addDefaultArgumentValues($functionCall, $args, $emptyArguments);
}

Expand Down Expand Up @@ -5571,7 +5571,7 @@ public function getImplementedFunctionNames(): array

private function addDefaultArgumentValues(array $functionCall, array $args, array $emptyArguments): array
{
$reflector = new ReflectionMethod(implode('::', $functionCall));
$reflector = new ReflectionMethod($functionCall[0], $functionCall[1]);
$methodArguments = $reflector->getParameters();

if (count($methodArguments) > 0) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php

declare(strict_types=1);

namespace PhpOffice\PhpSpreadsheetTests\Calculation\Functions\MathTrig;

class PiTest extends AllSetupTeardown
{
/**
* @dataProvider providerPI
*/
public function testPI(mixed $expectedResult, mixed $number = 'omitted'): void
{
$this->mightHaveException($expectedResult);
$sheet = $this->getSheet();
if ($number !== null) {
$sheet->getCell('A1')->setValue($number);
}
if ($number === 'omitted') {
$sheet->getCell('B1')->setValue('=PI()');
} else {
$sheet->getCell('B1')->setValue('=PI(A1)');
}
$result = $sheet->getCell('B1')->getCalculatedValue();
self::assertEqualsWithDelta($expectedResult, $result, 1E-12);
}

public static function providerPI(): array
{
return require 'tests/data/Calculation/MathTrig/PI.php';
}
}
8 changes: 8 additions & 0 deletions tests/data/Calculation/MathTrig/PI.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?php

declare(strict_types=1);

return [
[3.141592653589793],
'should not supply any arguments' => ['exception', 1],
];

0 comments on commit 5bcbc7d

Please sign in to comment.