From f1d73d8dba8e05386e5b52a4109939785e8b01f6 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Tue, 18 Oct 2022 19:34:37 -0700 Subject: [PATCH] Eliminate Some Scrutinizer 'Major' Problems Part 2 (#3122) * Eliminate Some Scrutinizer 'Major' Problems Part 2 Mostly Scrutinizer bizarre false positives. From Calculation/Engineering/Bessell (and several others): ```php private static function besselI2(float $x, int $ord): float { if ($x === 0.0) { return 0.0; } ``` Scrutinizer complains that `$x` can never equal 0.0 here. Huh??? Another example repeated several times, from Calculation/Engineering/BesselJ: ```php $bool = false; foreach (whatever) { ... $bool = !$bool; ... } ``` Scrutinizer complains about the `!$bool` assignment because it says `$bool` is always false. Again, huh??? * Change Logical Not Handling From a suggestion from @MarkBaker, sidestep bug in Scrutinizer by recoding ```php $jsum = !$jsum ``` as: ```php $jsum = $jsum === false; ``` --- src/PhpSpreadsheet/Calculation/DateTimeExcel/Date.php | 4 ++-- .../Calculation/DateTimeExcel/Difference.php | 6 +++--- .../Calculation/DateTimeExcel/TimeValue.php | 2 +- .../Calculation/Engineering/BesselI.php | 9 ++++++++- .../Calculation/Engineering/BesselJ.php | 2 +- src/PhpSpreadsheet/Calculation/Engineering/ErfC.php | 2 +- .../Financial/CashFlow/Variable/Periodic.php | 11 +++++++++-- src/PhpSpreadsheet/Calculation/Financial/Coupons.php | 9 +++++++++ .../Calculation/Financial/Depreciation.php | 7 +++++-- .../Financial/Securities/AccruedInterest.php | 8 ++++++++ src/PhpSpreadsheet/Calculation/Functions.php | 2 +- src/PhpSpreadsheet/Chart/Chart.php | 8 ++++---- src/PhpSpreadsheet/Chart/DataSeriesValues.php | 2 +- src/PhpSpreadsheet/Chart/Properties.php | 2 +- src/PhpSpreadsheet/Settings.php | 2 +- 15 files changed, 55 insertions(+), 21 deletions(-) diff --git a/src/PhpSpreadsheet/Calculation/DateTimeExcel/Date.php b/src/PhpSpreadsheet/Calculation/DateTimeExcel/Date.php index 56982c5c..4b3b148c 100644 --- a/src/PhpSpreadsheet/Calculation/DateTimeExcel/Date.php +++ b/src/PhpSpreadsheet/Calculation/DateTimeExcel/Date.php @@ -72,10 +72,10 @@ class Date $baseYear = SharedDateHelper::getExcelCalendar(); try { - $year = self::getYear($year, $baseYear); + $year = self::getYear($year, $baseYear); // must be int - Scrutinizer is wrong $month = self::getMonth($month); $day = self::getDay($day); - self::adjustYearMonth($year, $month, $baseYear); + self::adjustYearMonth(/** @scrutinizer ignore-type */ $year, $month, $baseYear); } catch (Exception $e) { return $e->getMessage(); } diff --git a/src/PhpSpreadsheet/Calculation/DateTimeExcel/Difference.php b/src/PhpSpreadsheet/Calculation/DateTimeExcel/Difference.php index ecb1cf0a..12888848 100644 --- a/src/PhpSpreadsheet/Calculation/DateTimeExcel/Difference.php +++ b/src/PhpSpreadsheet/Calculation/DateTimeExcel/Difference.php @@ -47,12 +47,12 @@ class Difference // Execute function $PHPStartDateObject = SharedDateHelper::excelToDateTimeObject($startDate); $startDays = (int) $PHPStartDateObject->format('j'); - $startMonths = (int) $PHPStartDateObject->format('n'); + //$startMonths = (int) $PHPStartDateObject->format('n'); $startYears = (int) $PHPStartDateObject->format('Y'); $PHPEndDateObject = SharedDateHelper::excelToDateTimeObject($endDate); $endDays = (int) $PHPEndDateObject->format('j'); - $endMonths = (int) $PHPEndDateObject->format('n'); + //$endMonths = (int) $PHPEndDateObject->format('n'); $endYears = (int) $PHPEndDateObject->format('Y'); $PHPDiffDateObject = $PHPEndDateObject->diff($PHPStartDateObject); @@ -138,7 +138,7 @@ class Difference $PHPEndDateObject->modify('+1 year'); // Get the result - $retVal = $PHPEndDateObject->diff($PHPStartDateObject)->days; + $retVal = (int) $PHPEndDateObject->diff($PHPStartDateObject)->days; // Adjust for leap years cases $isLeapEndYear = $PHPEndDateObject->format('L'); diff --git a/src/PhpSpreadsheet/Calculation/DateTimeExcel/TimeValue.php b/src/PhpSpreadsheet/Calculation/DateTimeExcel/TimeValue.php index aa27206b..4abdd75d 100644 --- a/src/PhpSpreadsheet/Calculation/DateTimeExcel/TimeValue.php +++ b/src/PhpSpreadsheet/Calculation/DateTimeExcel/TimeValue.php @@ -67,7 +67,7 @@ class TimeValue if ($retType === Functions::RETURNDATE_EXCEL) { $retValue = (float) $excelDateValue; } elseif ($retType === Functions::RETURNDATE_UNIX_TIMESTAMP) { - $retValue = (int) $phpDateValue = SharedDateHelper::excelToTimestamp($excelDateValue + 25569) - 3600; + $retValue = (int) SharedDateHelper::excelToTimestamp($excelDateValue + 25569) - 3600; } else { $retValue = new DateTime('1900-01-01 ' . $PHPDateArray['hour'] . ':' . $PHPDateArray['minute'] . ':' . $PHPDateArray['second']); } diff --git a/src/PhpSpreadsheet/Calculation/Engineering/BesselI.php b/src/PhpSpreadsheet/Calculation/Engineering/BesselI.php index 2afb52bd..1134574e 100644 --- a/src/PhpSpreadsheet/Calculation/Engineering/BesselI.php +++ b/src/PhpSpreadsheet/Calculation/Engineering/BesselI.php @@ -111,9 +111,16 @@ class BesselI return ($x < 0.0) ? -$ans : $ans; } + /** + * Sop to Scrutinizer. + * + * @var float + */ + private static $zeroPointZero = 0.0; + private static function besselI2(float $x, int $ord): float { - if ($x === 0.0) { + if ($x === self::$zeroPointZero) { return 0.0; } diff --git a/src/PhpSpreadsheet/Calculation/Engineering/BesselJ.php b/src/PhpSpreadsheet/Calculation/Engineering/BesselJ.php index e23b0157..800a8a11 100644 --- a/src/PhpSpreadsheet/Calculation/Engineering/BesselJ.php +++ b/src/PhpSpreadsheet/Calculation/Engineering/BesselJ.php @@ -167,7 +167,7 @@ class BesselJ if ($jsum === true) { $sum += $bj; } - $jsum = !$jsum; + $jsum = $jsum === false; if ($j === $ord) { $ans = $bjp; } diff --git a/src/PhpSpreadsheet/Calculation/Engineering/ErfC.php b/src/PhpSpreadsheet/Calculation/Engineering/ErfC.php index 7b023bee..eb834b7b 100644 --- a/src/PhpSpreadsheet/Calculation/Engineering/ErfC.php +++ b/src/PhpSpreadsheet/Calculation/Engineering/ErfC.php @@ -59,7 +59,7 @@ class ErfC $a = $n = 1; $b = $c = $value; $d = ($value * $value) + 0.5; - $q1 = $q2 = $b / $d; + $q2 = $b / $d; do { $t = $a * $n + $b * $value; $a = $b; diff --git a/src/PhpSpreadsheet/Calculation/Financial/CashFlow/Variable/Periodic.php b/src/PhpSpreadsheet/Calculation/Financial/CashFlow/Variable/Periodic.php index 49d063c9..f02ef292 100644 --- a/src/PhpSpreadsheet/Calculation/Financial/CashFlow/Variable/Periodic.php +++ b/src/PhpSpreadsheet/Calculation/Financial/CashFlow/Variable/Periodic.php @@ -112,7 +112,7 @@ class Periodic $rr = 1.0 + $reinvestmentRate; $fr = 1.0 + $financeRate; - $npvPos = $npvNeg = 0.0; + $npvPos = $npvNeg = self::$zeroPointZero; foreach ($values as $i => $v) { if ($v >= 0) { $npvPos += $v / $rr ** $i; @@ -121,7 +121,7 @@ class Periodic } } - if (($npvNeg === 0.0) || ($npvPos === 0.0) || ($reinvestmentRate <= -1.0)) { + if (($npvNeg === self::$zeroPointZero) || ($npvPos === self::$zeroPointZero) || ($reinvestmentRate <= -1.0)) { return ExcelError::VALUE(); } @@ -131,6 +131,13 @@ class Periodic return is_finite($mirr) ? $mirr : ExcelError::VALUE(); } + /** + * Sop to Scrutinizer. + * + * @var float + */ + private static $zeroPointZero = 0.0; + /** * NPV. * diff --git a/src/PhpSpreadsheet/Calculation/Financial/Coupons.php b/src/PhpSpreadsheet/Calculation/Financial/Coupons.php index 6138075c..14c2d019 100644 --- a/src/PhpSpreadsheet/Calculation/Financial/Coupons.php +++ b/src/PhpSpreadsheet/Calculation/Financial/Coupons.php @@ -261,6 +261,7 @@ class Coupons self::validateCouponPeriod($settlement, $maturity); $frequency = FinancialValidations::validateFrequency($frequency); $basis = FinancialValidations::validateBasis($basis); + self::doNothing($basis); } catch (Exception $e) { return $e->getMessage(); } @@ -315,6 +316,7 @@ class Coupons self::validateCouponPeriod($settlement, $maturity); $frequency = FinancialValidations::validateFrequency($frequency); $basis = FinancialValidations::validateBasis($basis); + self::doNothing($basis); } catch (Exception $e) { return $e->getMessage(); } @@ -375,6 +377,7 @@ class Coupons self::validateCouponPeriod($settlement, $maturity); $frequency = FinancialValidations::validateFrequency($frequency); $basis = FinancialValidations::validateBasis($basis); + self::doNothing($basis); } catch (Exception $e) { return $e->getMessage(); } @@ -414,4 +417,10 @@ class Coupons throw new Exception(ExcelError::NAN()); } } + + /** @param mixed $basis */ + private static function doNothing($basis): bool + { + return $basis; + } } diff --git a/src/PhpSpreadsheet/Calculation/Financial/Depreciation.php b/src/PhpSpreadsheet/Calculation/Financial/Depreciation.php index 3d72757a..8e1a2fcc 100644 --- a/src/PhpSpreadsheet/Calculation/Financial/Depreciation.php +++ b/src/PhpSpreadsheet/Calculation/Financial/Depreciation.php @@ -8,6 +8,9 @@ use PhpOffice\PhpSpreadsheet\Calculation\Information\ExcelError; class Depreciation { + /** @var float */ + private static $zeroPointZero = 0.0; + /** * DB. * @@ -51,7 +54,7 @@ class Depreciation return $e->getMessage(); } - if ($cost === 0.0) { + if ($cost === self::$zeroPointZero) { return 0.0; } @@ -161,7 +164,7 @@ class Depreciation return $e->getMessage(); } - if ($life === 0.0) { + if ($life === self::$zeroPointZero) { return ExcelError::DIV0(); } diff --git a/src/PhpSpreadsheet/Calculation/Financial/Securities/AccruedInterest.php b/src/PhpSpreadsheet/Calculation/Financial/Securities/AccruedInterest.php index 95996b4e..e1bf04b9 100644 --- a/src/PhpSpreadsheet/Calculation/Financial/Securities/AccruedInterest.php +++ b/src/PhpSpreadsheet/Calculation/Financial/Securities/AccruedInterest.php @@ -54,6 +54,7 @@ class AccruedInterest $basis = FinancialConstants::BASIS_DAYS_PER_YEAR_NASD, $calcMethod = self::ACCRINT_CALCMODE_ISSUE_TO_SETTLEMENT ) { + self::doNothing($calcMethod); $issue = Functions::flattenSingleValue($issue); $firstInterest = Functions::flattenSingleValue($firstInterest); $settlement = Functions::flattenSingleValue($settlement); @@ -73,6 +74,7 @@ class AccruedInterest $rate = SecurityValidations::validateRate($rate); $parValue = SecurityValidations::validateParValue($parValue); $frequency = SecurityValidations::validateFrequency($frequency); + self::doNothing($frequency); $basis = SecurityValidations::validateBasis($basis); } catch (Exception $e) { return $e->getMessage(); @@ -148,4 +150,10 @@ class AccruedInterest return $parValue * $rate * $daysBetweenIssueAndSettlement; } + + /** @param mixed $arg */ + private static function doNothing($arg): bool + { + return (bool) $arg; + } } diff --git a/src/PhpSpreadsheet/Calculation/Functions.php b/src/PhpSpreadsheet/Calculation/Functions.php index 172f2022..87abe2b1 100644 --- a/src/PhpSpreadsheet/Calculation/Functions.php +++ b/src/PhpSpreadsheet/Calculation/Functions.php @@ -576,7 +576,7 @@ class Functions $flattened = []; $stack = array_values($array); - while ($stack) { + while (!empty($stack)) { $value = array_shift($stack); if (is_array($value)) { diff --git a/src/PhpSpreadsheet/Chart/Chart.php b/src/PhpSpreadsheet/Chart/Chart.php index f41d7883..e9051af2 100644 --- a/src/PhpSpreadsheet/Chart/Chart.php +++ b/src/PhpSpreadsheet/Chart/Chart.php @@ -463,8 +463,8 @@ class Chart /** * Set the offset position within the Top Left cell for the chart. * - * @param int $xOffset - * @param int $yOffset + * @param ?int $xOffset + * @param ?int $yOffset * * @return $this */ @@ -587,8 +587,8 @@ class Chart /** * Set the offset position within the Bottom Right cell for the chart. * - * @param int $xOffset - * @param int $yOffset + * @param ?int $xOffset + * @param ?int $yOffset * * @return $this */ diff --git a/src/PhpSpreadsheet/Chart/DataSeriesValues.php b/src/PhpSpreadsheet/Chart/DataSeriesValues.php index cd166b23..c86f5569 100644 --- a/src/PhpSpreadsheet/Chart/DataSeriesValues.php +++ b/src/PhpSpreadsheet/Chart/DataSeriesValues.php @@ -496,7 +496,7 @@ class DataSeriesValues extends Properties if (($dimensions[0] == 1) || ($dimensions[1] == 1)) { $this->dataValues = Functions::flattenArray($newDataValues); } else { - $newArray = array_values(array_shift($newDataValues)); + $newArray = array_values(array_shift(/** @scrutinizer ignore-type */ $newDataValues)); foreach ($newArray as $i => $newDataSet) { $newArray[$i] = [$newDataSet]; } diff --git a/src/PhpSpreadsheet/Chart/Properties.php b/src/PhpSpreadsheet/Chart/Properties.php index f737ca08..edb77545 100644 --- a/src/PhpSpreadsheet/Chart/Properties.php +++ b/src/PhpSpreadsheet/Chart/Properties.php @@ -539,7 +539,7 @@ abstract class Properties /** * Set Soft Edges Size. * - * @param float $size + * @param ?float $size */ public function setSoftEdges($size): void { diff --git a/src/PhpSpreadsheet/Settings.php b/src/PhpSpreadsheet/Settings.php index 3282a596..b1740d50 100644 --- a/src/PhpSpreadsheet/Settings.php +++ b/src/PhpSpreadsheet/Settings.php @@ -16,7 +16,7 @@ class Settings * Class name of the chart renderer used for rendering charts * eg: PhpOffice\PhpSpreadsheet\Chart\Renderer\JpGraph. * - * @var string + * @var ?string */ private static $chartRenderer;