From b975fb7ddd03a7a360fe661e76f7362a944bc503 Mon Sep 17 00:00:00 2001 From: ElPopcorn <33669386+ElPopcorn@users.noreply.github.com> Date: Fri, 12 Feb 2021 18:04:52 +0100 Subject: [PATCH] Update PPMT & IPMT implementation to better reflect excel behaviour. Update CUMPRINC & CUMIPMT implementation to prevent a crash while trying to add a string to a number. Update AMORLINC & AMORDEGRC to prevent crash when trying to multiply a string by a number. Update related unit tests. Update changelog to describe what we fixed. (#1840) Co-authored-by: Obmecha --- CHANGELOG.md | 11 ++++++++ src/PhpSpreadsheet/Calculation/Financial.php | 27 +++++++++++++++---- .../data/Calculation/Financial/AMORDEGRC.php | 4 +++ tests/data/Calculation/Financial/AMORLINC.php | 4 +++ tests/data/Calculation/Financial/CUMIPMT.php | 9 +++++++ tests/data/Calculation/Financial/CUMPRINC.php | 9 +++++++ tests/data/Calculation/Financial/IPMT.php | 2 +- 7 files changed, 60 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b8f4930..fe2f4ec5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,15 @@ and this project adheres to [Semantic Versioning](https://semver.org). One TextData function is also affected: `REPT()` (str_repeat). - `formatAsDate` correctly matches language metadata, reverting c55272e +- Formulae that previously crashed on sub function call returning excel error value now return said value. + The following functions are affected `CUMPRINC()`, `CUMIPMT()`, `AMORLINC()`, + `AMORDEGRC()`. + +- Adapt some function error return value to match excel's error. + The following functions are affected `PPMT()`, `IPMT()`. + +- All related unit tests have been updated to test for those modifications. + ### Deprecated - Nothing. @@ -42,6 +51,8 @@ and this project adheres to [Semantic Versioning](https://semver.org). - Fix Xlsx reader cell alignment. [PR #1710](https://github.com/PHPOffice/PhpSpreadsheet/pull/1710) - Fix for not yet implemented data-types in Open Document writer [Issue #1674](https://github.com/PHPOffice/PhpSpreadsheet/issues/1674) - Fix XLSX reader when having a corrupt numeric cell data type [PR #1664](https://github.com/phpoffice/phpspreadsheet/pull/1664) +- Fix on `CUMPRINC()`, `CUMIPMT()`, `AMORLINC()`, `AMORDEGRC()` usage. When those functions called one of `YEARFRAC()`, `PPMT()`, `IPMT()` and they would get back an error +value (represented as a string), trying to use numeral operands (`+`, `/`, `-`, `*`) on said return value and a number (`float or `int`) would fail. ## 1.16.0 - 2020-12-31 diff --git a/src/PhpSpreadsheet/Calculation/Financial.php b/src/PhpSpreadsheet/Calculation/Financial.php index 728167f4..7be0b594 100644 --- a/src/PhpSpreadsheet/Calculation/Financial.php +++ b/src/PhpSpreadsheet/Calculation/Financial.php @@ -253,6 +253,10 @@ class Financial $period = floor(Functions::flattenSingleValue($period)); $rate = Functions::flattenSingleValue($rate); $basis = ($basis === null) ? 0 : (int) Functions::flattenSingleValue($basis); + $yearFrac = DateTime::YEARFRAC($purchased, $firstPeriod, $basis); + if (is_string($yearFrac)) { + return $yearFrac; + } // The depreciation coefficients are: // Life of assets (1/rate) Depreciation coefficient @@ -272,7 +276,7 @@ class Financial } $rate *= $amortiseCoeff; - $fNRate = round(DateTime::YEARFRAC($purchased, $firstPeriod, $basis) * $rate * $cost, 0); + $fNRate = round($yearFrac * $rate * $cost, 0); $cost -= $fNRate; $fRest = $cost - $salvage; @@ -335,6 +339,9 @@ class Financial // Note, quirky variation for leap years on the YEARFRAC for this function $purchasedYear = DateTime::YEAR($purchased); $yearFrac = DateTime::YEARFRAC($purchased, $firstPeriod, $basis); + if (is_string($yearFrac)) { + return $yearFrac; + } if (($basis == 1) && ($yearFrac < 1) && (DateTime::isLeapYear($purchasedYear))) { $yearFrac *= 365 / 366; @@ -739,7 +746,12 @@ class Financial // Calculate $interest = 0; for ($per = $start; $per <= $end; ++$per) { - $interest += self::IPMT($rate, $per, $nper, $pv, 0, $type); + $ipmt = self::IPMT($rate, $per, $nper, $pv, 0, $type); + if (is_string($ipmt)) { + return $ipmt; + } + + $interest += $ipmt; } return $interest; @@ -785,7 +797,12 @@ class Financial // Calculate $principal = 0; for ($per = $start; $per <= $end; ++$per) { - $principal += self::PPMT($rate, $per, $nper, $pv, 0, $type); + $ppmt = self::PPMT($rate, $per, $nper, $pv, 0, $type); + if (is_string($ppmt)) { + return $ppmt; + } + + $principal += $ppmt; } return $principal; @@ -1219,7 +1236,7 @@ class Financial return Functions::NAN(); } if ($per <= 0 || $per > $nper) { - return Functions::VALUE(); + return Functions::NAN(); } // Calculate @@ -1573,7 +1590,7 @@ class Financial return Functions::NAN(); } if ($per <= 0 || $per > $nper) { - return Functions::VALUE(); + return Functions::NAN(); } // Calculate diff --git a/tests/data/Calculation/Financial/AMORDEGRC.php b/tests/data/Calculation/Financial/AMORDEGRC.php index 6d75feaf..59549e78 100644 --- a/tests/data/Calculation/Financial/AMORDEGRC.php +++ b/tests/data/Calculation/Financial/AMORDEGRC.php @@ -23,4 +23,8 @@ return [ 42, 150, '2011-01-01', '2011-09-30', 20, 1, 0.4, 4, ], + [ + '#VALUE!', + 550, 'notADate', '2020-12-25', 20, 1, 0.2, 4, + ], ]; diff --git a/tests/data/Calculation/Financial/AMORLINC.php b/tests/data/Calculation/Financial/AMORLINC.php index 00ab2ae4..46f19332 100644 --- a/tests/data/Calculation/Financial/AMORLINC.php +++ b/tests/data/Calculation/Financial/AMORLINC.php @@ -23,4 +23,8 @@ return [ 0.0, 150, '2011-01-01', '2011-09-30', 20, 5, 0.20000000000000001, 4, ], + [ + '#VALUE!', + 150, 'notADate', '2011-09-30', 20, 1, 0.20000000000000001, 4, + ], ]; diff --git a/tests/data/Calculation/Financial/CUMIPMT.php b/tests/data/Calculation/Financial/CUMIPMT.php index bc14d92a..f3b27c92 100644 --- a/tests/data/Calculation/Financial/CUMIPMT.php +++ b/tests/data/Calculation/Financial/CUMIPMT.php @@ -84,4 +84,13 @@ return [ 13, 2, ], + [ + '#NUM!', + 0.0074999999999999997, + 10, + 125000, + 13, + 24, + 0, + ], ]; diff --git a/tests/data/Calculation/Financial/CUMPRINC.php b/tests/data/Calculation/Financial/CUMPRINC.php index 63392c52..0bcd8318 100644 --- a/tests/data/Calculation/Financial/CUMPRINC.php +++ b/tests/data/Calculation/Financial/CUMPRINC.php @@ -84,4 +84,13 @@ return [ 13, 2, ], + [ + '#NUM!', + 0.0074999999999999997, + 10, + 125000, + 13, + 24, + 0, + ], ]; diff --git a/tests/data/Calculation/Financial/IPMT.php b/tests/data/Calculation/Financial/IPMT.php index aaea844b..5ae1b1b4 100644 --- a/tests/data/Calculation/Financial/IPMT.php +++ b/tests/data/Calculation/Financial/IPMT.php @@ -59,7 +59,7 @@ return [ 6, ], [ - '#VALUE!', + '#NUM!', 0.0050000000000000001, 8, 2,