From a40fcc1942ea9f31ce7ebb63185d81fd08e69a47 Mon Sep 17 00:00:00 2001 From: Owen Leibman Date: Tue, 25 May 2021 19:19:42 -0700 Subject: [PATCH] Error in COUPNCD 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 6390e0fc21..2f7379b330 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -770,46 +770,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 b95625cf9b..69cd68ab59 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 @@ public static function COUPDAYBS( return abs(DateTimeExcel\Days::funcDays($prev, $settlement)); } - return DateTimeExcel\YearFrac::funcYearFrac($prev, $settlement, $basis) * $daysPerYear; + return (float) DateTimeExcel\YearFrac::funcYearFrac($prev, $settlement, $basis) * $daysPerYear; } /** @@ -208,7 +209,7 @@ public static function COUPDAYSNC( } } - return DateTimeExcel\YearFrac::funcYearFrac($settlement, $next, $basis) * $daysPerYear; + return (float) DateTimeExcel\YearFrac::funcYearFrac($settlement, $next, $basis) * $daysPerYear; } /** @@ -322,7 +323,7 @@ public static function COUPNUM( FinancialConstants::BASIS_DAYS_PER_YEAR_NASD ); - return (int) ceil($yearsBetweenSettlementAndMaturity * $frequency); + return (int) ceil((float) $yearsBetweenSettlementAndMaturity * $frequency); } /** @@ -379,28 +380,33 @@ public static function COUPPCD( 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'); - } - - if ($maturityEoM === true) { - $result->modify('-1 day'); + self::monthsDiff($result, $months, '+', $day, $lastDayFlag); } - 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 260783faa5..f41d3620c0 100644 --- a/tests/data/Calculation/Financial/COUPDAYBS.php +++ b/tests/data/Calculation/Financial/COUPDAYBS.php @@ -205,4 +205,18 @@ 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 c72d26c136..caba56e55a 100644 --- a/tests/data/Calculation/Financial/COUPDAYS.php +++ b/tests/data/Calculation/Financial/COUPDAYS.php @@ -219,4 +219,18 @@ 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 ca61147529..e8500263c9 100644 --- a/tests/data/Calculation/Financial/COUPDAYSNC.php +++ b/tests/data/Calculation/Financial/COUPDAYSNC.php @@ -219,4 +219,21 @@ 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 8dee4c0a96..d690525c9b 100644 --- a/tests/data/Calculation/Financial/COUPNCD.php +++ b/tests/data/Calculation/Financial/COUPNCD.php @@ -219,4 +219,32 @@ 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 e906f147d5..88b93af5c6 100644 --- a/tests/data/Calculation/Financial/COUPPCD.php +++ b/tests/data/Calculation/Financial/COUPPCD.php @@ -219,4 +219,18 @@ 4, 4, ], + [ + 43555, + '05-Apr-2019', + '30-Sep-2021', + 2, + 0, + ], + [ + 43738, + '05-Oct-2019', + '31-Mar-2022', + 2, + 0, + ], ];