From 7e4331e3ab175d162efa3a2058b2175baf1c1500 Mon Sep 17 00:00:00 2001 From: oleibman Date: Sat, 29 May 2021 03:02:36 -0700 Subject: [PATCH] Error in COUPNCD (#2119) See issue #2116. Code for handling end of month (method couponFirstPeriodDate) needed a fix. Fixed it, confirmed it covered the reported issue with no regression problems. Then added some extra similar tests to all the callers of couponFirstPeriodDate, and ... One new test, in COUPDAYSNC, does not agree with Excel. It also does not agree with LibreOffice. It does, however, agree with Gnumeric, and with my (hardly guaranteed) hand calculation of what the result should be. So, I'm going with it (and have added an appropriate comment to the test data). I'm glad to discuss the matter with anyone more familiar than I with how this is supposed to work - those 360-day years are killers. --- phpstan-baseline.neon | 40 ------------------- .../Calculation/Financial/Coupons.php | 32 +++++++++------ .../data/Calculation/Financial/COUPDAYBS.php | 14 +++++++ tests/data/Calculation/Financial/COUPDAYS.php | 14 +++++++ .../data/Calculation/Financial/COUPDAYSNC.php | 17 ++++++++ tests/data/Calculation/Financial/COUPNCD.php | 28 +++++++++++++ tests/data/Calculation/Financial/COUPPCD.php | 14 +++++++ 7 files changed, 106 insertions(+), 53 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 4eed9863..7fb5a0f6 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -635,46 +635,6 @@ parameters: count: 1 path: src/PhpSpreadsheet/Calculation/Financial/CashFlow/Variable/Periodic.php - - - message: "#^Binary operation \"\\*\" between float\\|string and int results in an error\\.$#" - count: 2 - path: src/PhpSpreadsheet/Calculation/Financial/Coupons.php - - - - message: "#^Binary operation \"\\*\" between float\\|string and int\\|string results in an error\\.$#" - count: 1 - path: src/PhpSpreadsheet/Calculation/Financial/Coupons.php - - - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Financial\\\\Coupons\\:\\:couponFirstPeriodDate\\(\\) has no return typehint specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Calculation/Financial/Coupons.php - - - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Financial\\\\Coupons\\:\\:couponFirstPeriodDate\\(\\) has parameter \\$maturity with no typehint specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Calculation/Financial/Coupons.php - - - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Financial\\\\Coupons\\:\\:couponFirstPeriodDate\\(\\) has parameter \\$next with no typehint specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Calculation/Financial/Coupons.php - - - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Financial\\\\Coupons\\:\\:couponFirstPeriodDate\\(\\) has parameter \\$settlement with no typehint specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Calculation/Financial/Coupons.php - - - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Financial\\\\Coupons\\:\\:validateCouponPeriod\\(\\) has parameter \\$maturity with no typehint specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Calculation/Financial/Coupons.php - - - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Financial\\\\Coupons\\:\\:validateCouponPeriod\\(\\) has parameter \\$settlement with no typehint specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Calculation/Financial/Coupons.php - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Financial\\\\Depreciation\\:\\:validateCost\\(\\) has parameter \\$cost with no typehint specified\\.$#" count: 1 diff --git a/src/PhpSpreadsheet/Calculation/Financial/Coupons.php b/src/PhpSpreadsheet/Calculation/Financial/Coupons.php index 5620c327..3fd6c1d2 100644 --- a/src/PhpSpreadsheet/Calculation/Financial/Coupons.php +++ b/src/PhpSpreadsheet/Calculation/Financial/Coupons.php @@ -2,6 +2,7 @@ namespace PhpOffice\PhpSpreadsheet\Calculation\Financial; +use DateTime; use PhpOffice\PhpSpreadsheet\Calculation\DateTimeExcel; use PhpOffice\PhpSpreadsheet\Calculation\Exception; use PhpOffice\PhpSpreadsheet\Calculation\Financial\Constants as FinancialConstants; @@ -73,7 +74,7 @@ class Coupons return abs((float) DateTimeExcel\Days::between($prev, $settlement)); } - return DateTimeExcel\YearFrac::fraction($prev, $settlement, $basis) * $daysPerYear; + return (float) DateTimeExcel\YearFrac::fraction($prev, $settlement, $basis) * $daysPerYear; } /** @@ -208,7 +209,7 @@ class Coupons } } - return DateTimeExcel\YearFrac::fraction($settlement, $next, $basis) * $daysPerYear; + return (float) DateTimeExcel\YearFrac::fraction($settlement, $next, $basis) * $daysPerYear; } /** @@ -322,7 +323,7 @@ class Coupons FinancialConstants::BASIS_DAYS_PER_YEAR_NASD ); - return (int) ceil($yearsBetweenSettlementAndMaturity * $frequency); + return (int) ceil((float) $yearsBetweenSettlementAndMaturity * $frequency); } /** @@ -379,28 +380,33 @@ class Coupons return self::couponFirstPeriodDate($settlement, $maturity, $frequency, self::PERIOD_DATE_PREVIOUS); } - private static function couponFirstPeriodDate($settlement, $maturity, int $frequency, $next) + private static function monthsDiff(DateTime $result, int $months, string $plusOrMinus, int $day, bool $lastDayFlag): void + { + $result->setDate((int) $result->format('Y'), (int) $result->format('m'), 1); + $result->modify("$plusOrMinus $months months"); + $daysInMonth = (int) $result->format('t'); + $result->setDate((int) $result->format('Y'), (int) $result->format('m'), $lastDayFlag ? $daysInMonth : min($day, $daysInMonth)); + } + + private static function couponFirstPeriodDate(float $settlement, float $maturity, int $frequency, bool $next): float { $months = 12 / $frequency; $result = Date::excelToDateTimeObject($maturity); - $maturityEoM = Helpers::isLastDayOfMonth($result); + $day = (int) $result->format('d'); + $lastDayFlag = Helpers::isLastDayOfMonth($result); while ($settlement < Date::PHPToExcel($result)) { - $result->modify('-' . $months . ' months'); + self::monthsDiff($result, $months, '-', $day, $lastDayFlag); } if ($next === true) { - $result->modify('+' . $months . ' months'); + self::monthsDiff($result, $months, '+', $day, $lastDayFlag); } - if ($maturityEoM === true) { - $result->modify('-1 day'); - } - - return Date::PHPToExcel($result); + return (float) Date::PHPToExcel($result); } - private static function validateCouponPeriod($settlement, $maturity): void + private static function validateCouponPeriod(float $settlement, float $maturity): void { if ($settlement >= $maturity) { throw new Exception(Functions::NAN()); diff --git a/tests/data/Calculation/Financial/COUPDAYBS.php b/tests/data/Calculation/Financial/COUPDAYBS.php index 260783fa..f41d3620 100644 --- a/tests/data/Calculation/Financial/COUPDAYBS.php +++ b/tests/data/Calculation/Financial/COUPDAYBS.php @@ -205,4 +205,18 @@ return [ 4, 4, ], + [ + 5, + '05-Apr-2019', + '30-Sep-2021', + 2, + 0, + ], + [ + 5, + '05-Oct-2019', + '31-Mar-2022', + 2, + 0, + ], ]; diff --git a/tests/data/Calculation/Financial/COUPDAYS.php b/tests/data/Calculation/Financial/COUPDAYS.php index c72d26c1..caba56e5 100644 --- a/tests/data/Calculation/Financial/COUPDAYS.php +++ b/tests/data/Calculation/Financial/COUPDAYS.php @@ -219,4 +219,18 @@ return [ 4, 4, ], + [ + 180, + '05-Apr-2019', + '30-Sep-2021', + 2, + 0, + ], + [ + 180, + '05-Oct-2019', + '31-Mar-2022', + 2, + 0, + ], ]; diff --git a/tests/data/Calculation/Financial/COUPDAYSNC.php b/tests/data/Calculation/Financial/COUPDAYSNC.php index ca611475..e8500263 100644 --- a/tests/data/Calculation/Financial/COUPDAYSNC.php +++ b/tests/data/Calculation/Financial/COUPDAYSNC.php @@ -219,4 +219,21 @@ return [ 4, 4, ], + [ + 175, + '05-Apr-2019', + '30-Sep-2021', + 2, + 0, + ], + // Excel and LibreOffice return 175 for the following calculation. + // Gnumeric returns 176. + // My hand calculation, hardly guaranteed, agrees with Gnumeric. + [ + 176, + '05-Oct-2019', + '31-Mar-2022', + 2, + 0, + ], ]; diff --git a/tests/data/Calculation/Financial/COUPNCD.php b/tests/data/Calculation/Financial/COUPNCD.php index 8dee4c0a..d690525c 100644 --- a/tests/data/Calculation/Financial/COUPNCD.php +++ b/tests/data/Calculation/Financial/COUPNCD.php @@ -219,4 +219,32 @@ return [ 4, 4, ], + [ + 44651, + '30-Sep-2021', + '31-Mar-2022', + 2, + 0, + ], + [ + 44834, + '31-Mar-2022', + '30-Sep-2022', + 2, + 0, + ], + [ + 43738, + '05-Apr-2019', + '30-Sep-2021', + 2, + 0, + ], + [ + 43921, + '05-Oct-2019', + '31-Mar-2022', + 2, + 0, + ], ]; diff --git a/tests/data/Calculation/Financial/COUPPCD.php b/tests/data/Calculation/Financial/COUPPCD.php index e906f147..88b93af5 100644 --- a/tests/data/Calculation/Financial/COUPPCD.php +++ b/tests/data/Calculation/Financial/COUPPCD.php @@ -219,4 +219,18 @@ return [ 4, 4, ], + [ + 43555, + '05-Apr-2019', + '30-Sep-2021', + 2, + 0, + ], + [ + 43738, + '05-Oct-2019', + '31-Mar-2022', + 2, + 0, + ], ];