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 <victor.sonza@nexvia.lu>

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
 

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

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,
+    ],
 ];

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,
+    ],
 ];

tests/data/Calculation/Financial/CUMIPMT.php

@@ -84,4 +84,13 @@ return [
         13,
         2,
     ],
+    [
+        '#NUM!',
+        0.0074999999999999997,
+        10,
+        125000,
+        13,
+        24,
+        0,
+    ],
 ];

tests/data/Calculation/Financial/CUMPRINC.php

@@ -84,4 +84,13 @@ return [
         13,
         2,
     ],
+    [
+        '#NUM!',
+        0.0074999999999999997,
+        10,
+        125000,
+        13,
+        24,
+        0,
+    ],
 ];

tests/data/Calculation/Financial/IPMT.php

@@ -59,7 +59,7 @@ return [
         6,
     ],
     [
-        '#VALUE!',
+        '#NUM!',
         0.0050000000000000001,
         8,
         2,