From 5bcbc7db0c00c1351d4f2a9625e2e6f6240a58a9 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Wed, 6 Dec 2023 07:08:14 -0800 Subject: [PATCH] WIP Avoid a PHP8.4 Deprecation (#3789) 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. --- .../Calculation/Calculation.php | 4 +-- .../Calculation/Functions/MathTrig/PiTest.php | 32 +++++++++++++++++++ tests/data/Calculation/MathTrig/PI.php | 8 +++++ 3 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Calculation/Functions/MathTrig/PiTest.php create mode 100644 tests/data/Calculation/MathTrig/PI.php diff --git a/src/PhpSpreadsheet/Calculation/Calculation.php b/src/PhpSpreadsheet/Calculation/Calculation.php index 34062a531..960db939c 100644 --- a/src/PhpSpreadsheet/Calculation/Calculation.php +++ b/src/PhpSpreadsheet/Calculation/Calculation.php @@ -5072,7 +5072,7 @@ class Calculation krsort($args); krsort($emptyArguments); - if ($argCount > 0) { + if ($argCount > 0 && is_array($functionCall)) { $args = $this->addDefaultArgumentValues($functionCall, $args, $emptyArguments); } @@ -5571,7 +5571,7 @@ class Calculation 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) { diff --git a/tests/PhpSpreadsheetTests/Calculation/Functions/MathTrig/PiTest.php b/tests/PhpSpreadsheetTests/Calculation/Functions/MathTrig/PiTest.php new file mode 100644 index 000000000..130f02f98 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Calculation/Functions/MathTrig/PiTest.php @@ -0,0 +1,32 @@ +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'; + } +} diff --git a/tests/data/Calculation/MathTrig/PI.php b/tests/data/Calculation/MathTrig/PI.php new file mode 100644 index 000000000..317cd1786 --- /dev/null +++ b/tests/data/Calculation/MathTrig/PI.php @@ -0,0 +1,8 @@ + ['exception', 1], +];