Financial functions more rationalization

[MarkBaker]

This is:

- [ ] a bugfix
- [ ] a new feature
- [X] refactoring

Checklist:

• Changes are covered by unit tests
• Code style is respected
• Commit message explains why the change is made (see https://github.com/erlang/otp/wiki/Writing-good-commit-messages)
• CHANGELOG.md contains a short summary of the change
• Documentation is updated as necessary

Why this change is needed?

[MarkBaker]

I wish I could figure out what it is that phpstan is complaining about

Error: Ignored error pattern #^Method PhpOffice\\PhpSpreadsheet\\Calculation\\Financial\\Amortization\:\:validateBasis\(\) has parameter \$basis with no typehint specified\.$# in path /home/runner/work/PhpSpreadsheet/PhpSpreadsheet/src/PhpSpreadsheet/Calculation/Financial/Amortization.php was not matched in reported errors.

    /**
     * @param mixed $basis
     */
    public static function validateBasis($basis): int
    {
        if (!is_numeric($basis)) {
            throw new Exception(Functions::VALUE());
        }

        $basis = (int) $basis;
        if (($basis < 0) || ($basis > 4)) {
            throw new Exception(Functions::NAN());
        }

        return $basis;
    }

Input argument for validateBasis() is type mixed defined in the docblock annotation validateBasis... output value is type int, defined in the function definition: public static function validateBasis($basis): int {...}

TYpehint for $basis argument passed to the Amortization functions is defined as mixed in the docblocks

So what exactly is phpstan's compaint?

[oleibman]

I have a wild guess about phpstan. If nothing else, it shouldn't take long to test. Your code includes calls to getDateValue, which phpstan doesn't object to, and calls to, among others, validateBasis, which it doesn't like. Here's the docBlock for getDateValue:

    /**
     * getDateValue.
     *
     * @param mixed $dateValue
     *
     * @return float Excel date/time serial value
     */

And here's the docBlock for validateBasis:

    /**
     * @param mixed $basis
     */

Is it possible that adding the equivalent of the first 2 lines in the getDateValue docBlock to the validateBasis docBlock will make the problem go away? It shouldn't matter, but it's easy to test.

[PowerKiKi]

PHPStan is complaining that your code is perfect 😎 and that he cannot find the error that used to be there.

I rebased your branch on master (without any modifications), then I double checked that all PHPStan errors started with "Ignored error pattern", and finally I ran vendor/bin/phpstan analyse --generate-baseline to automatically update phpstan-baseline.neon. That last step was made in dbe1d2a.

That file can be edited either manually or automatically, like I did. In both cases we should be careful to only remove things, and never add anything, in order to (one day) reach zero error at PHPStan max level and delete the file entirely.

You can read more about baseline over there: https://phpstan.org/user-guide/baseline

[PowerKiKi]

As a side note, I bought a license for PHPStan Pro, it is absolutely not required, and it's not even a "game changer", but it's slightly more friendly to fix huge batch of issues. You might want to consider it, or not. For me the main reason was to support the developer, and I appreciate the slightly improved feedback loop (though it can be reproduced with CLI tools).

[PowerKiKi]

And one last thing, I plan to refactor Worksheet::getCell(), in order to remove a lot of PHPStan issues, like so:

-    public function getCell($pCoordinate, $createIfNotExists = true): ?Cell
+    public function getCell($pCoordinate): Cell