It flagged a statement as dead code. It was correct - there was a typo in the variable name. But no tests had failed. The explanation was, of course, that this particular path was not adequately tested.
PhpSpreadsheet extends Excel (dating back to PHPExcel) by allowing the ordinal form of days in the DATE function, implemented as "take the numeric portion if the field is a string consisting of a numeric portion followed by some alphabetics". Whether or not this is a good idea, it would be a breaking change to eliminate it, so that's not going to happen. However, the same logic has been applied to month, and I don't see a use case for that, so I'm eliminating it - any non-numeric string used as the month parameter will now result in a VALUE error. It also turns out that Excel accepts null, false, and true for the month, and PhpSpreadsheet will now do likewise.
While we might never be able to have 100% of our code strict, we can at
the very least do it for all of our tests. This ensures that our tests
are using our API with the types as intended by the test author, and not
silently be cast to what our API requires.
Also enforce more rigorous Excel Function implementation by testing the underlying implementation, call via the Calc Engine, and execution from in a worksheet.
Separate out unhappy path (exception) checks into a separate test, so that a single test isn't made overcomplex checking for every potentiality.
Scrutinizer may dislike variadics, for variable number of arguments of mixed type; but tough. It's 100% valid PHP, accepted by phpstan, and makes life a lot easier.
Initial work here covers all the database and datetime unit tests for Excel function implementations.
* Complete Breakup Of Calculation/DateTime Functions
In conjunction with parallel breakups happening in other areas of Calculation, this change breaks up all the DateTime functions into their own classes. All methods remaining in DateTime itself have a doc block deprecation notice, and consist only of stub code to call the replacement methods. Coverage of DateTime itself and all the replacement methods is 100%.
There is only one substantive change to the code (see next paragraph). Among the non-substantive changes, it now adopts the same parsing technique (throwing and catching exceptions) already in use in Engineering and MathTrig. Boolean parameters are allowed in lieu of numbers when Excel allows them. Most of the code changes involve refactoring due to the need to avoid Scrutinizer "complexity" failures in what it will consider to be new methods.
Issue #1936 was opened just as I was staging this. It is now fixed. One existing WORKDAY test was wrong (noted in a comment in the test data file), and a bunch of new tests are added.
I found it confusing to use DateTime as a node of the the class name since most of the methods invoke native DateTime methods. So, everything is moved to directory DateTimeExcel, and that is what is used in the class names.
There are several follow-up activities that I am planning to undertake if this PR is merged.
- ODS supports dates well before 1900. There are exactly 2 assertions for this functionality. More are needed (and some functions might have to change to accept this).
- WEEKDAY has some poorly documented extra options for "style" which are not yet implemented.
- Most tests have been changed to use a formula as entered on a spreadsheet rather than a direct call to the method which implements the formula. There are 3 exceptions at this time. WORKDAY and NETWORKDAYS, which include arrays as part of their parameters, are more complicated than most. YEARFRAC was just too large to deal with now.
- There are direct calls to the now-deprecated methods in both source code and tests, mostly in Financial code, but possibly in others as well. These need to be changed.
- Some constants, none "officially" documented, remain in the original class. These should be either deleted or marked deprecated. I wasn't sure if deprecation was even possible (or desirable), and did not want that to be something which would cause Scrutinizer to fail the change.
* Deprecate Now-unused Constants, Fix Yearfrac bug, Change 3 Tests
Add new DateTime/Constants class, initially populated with constants used in Weeknum.
MS has another inconsistency with how it handles null cells in Yearfrac. Change PhpSpreadsheet to behave compatibly with this bug.
I have modified YearFrac, WorkDay, and NetworkDays tests to be more to my liking. Many tests added to YearFrac because of the bug above. Only minor modifications to the existing tests for the others.
* Merge branch 'master' of C:\Projects\PHPOffice\PHPSpreadsheet\develop with conflicts.
* Separate out date/time tests into individual tests
* Need to update the version of phpunit at some point to deal with the new assertions and deprecated assertions
* Appease the CS Gods
* More refactoring of Date/Time tests
* Replace self assertions with instance assertions (looking forward to upgrading phpunit)
* Finish refactoring of date/time tests as individual tests
* Test for DateTimeInterface rather than for DateTime
* A few strict comparisons
* Fix to test names
oleibman
Adrien Crivelli
MarkBaker
oleibman
MarkBaker