Commits on Feb 21, 2021

1. 100% Coverage for Calculation/DateTime …

   The code in DateTime is now completely covered.
   Along the way, some errors were discovered and corrected.
   - The tests which have had to be changed at the start of every year are
   replaced by more robust equivalents which do not require annual changes.
   - Several places in the code where Gnumeric and OpenOffice were thought to differ
   from Excel do not appear to have had any justification.
   I have left a comment where such code has been removed.
   - Use DateTime when possible rather than date, time, or strftime functions to avoid
   potential Y2038 problems.
   - Some impossible code has been removed, replaced by an explanatory comment.
   - NETWORKDAYS had a bug when the start date was Sunday. There had been no tests
   of this condition.
   - Some functions allow boolean and null arguments where a number is expected.
   This is more complicated than the equivalent situations in MathTrig because
   the initial date for these calculations can be Day 1 rather than Day 0.
   - More testing for dates from 1900-01-01 through the fictitious
   everywhere-but-Excel 1900-01-29.
       - This showed that there is an additional Excel bug - Excel evaluates
   WEEKNUM(emptycell) as 0, which is not a valid result for
   WEEKNUM without a second argument.
   PhpSpreadsheet now duplicates this bug.
       - There is a similar and even worse bug for 1904-01-01 in 1904 calculations.
   Weeknum returns 0 for this,
   but returns the correct value for arguments of 0 or null.
       - DATEVALUE should accept 1900-02-29 (sigh) and relatives.
   PhpSpreadsheet now duplicates this bug.
   - Testing bootstrap sets default timezone. This appears to be a relic from
   the releases of PHP where the unwise decision, subsequenly reversed,
   was made to issue messages for
   "no default timezone is set" rather than just use a sensible default.
   This was a disruptive setting for some of the tests I added.
   There is only one test in the entire suite which is default-timezone-dependent.
   Setting and resetting of default timezone is moved to that test
   (Reader/ODS/ODSTest), and out of bootstrap.
   - There had been no testing of NOW() function.
   - DATEVALUE test had no tests for 1904 calendar and needs some.
   - DATE test changed 1900/1904 calendar in use without restoring it.
   - WEEKDAY test had no tests for 1904 calendar and needs some.
       - Which revealed a bug in Shared/Date (excelToDateTimeObject was not
   recognizing 1904-01-01 as valid when 1904 calendar is in use).
       - And an additional bug in that legal 1904-calendar values in the 0.0-1.0
   range yielded the same "wrong" answers as 1900-calendar (see "One note" below).
   Also the comment for one of the calendar-1904 tests was wrong in attempting
   to identify what time of day the fraction represented.
   
   I had wanted to break this up into a set of smaller modules, a process already
   started for Engineering and MathTrig.
   However the number of source code changes was sufficient that I wanted
   a clean delta for this request.
   If it is merged, I will work on breaking it up afterwards.
   
   One note - Shared/Date/excelToDateTimeObject, when calendar-1900 is in use,
   returns an unexpected result if its argument is between 0 and 1,
   which is nominally invalid for that calendar.
   It uses a base-1970 calendar in that instance. That check is not justifiable
   for calendar-1904, where values in that range are legal,
   so I made the check specific to calendar-1900,
   and adjusted 3 1904 unit test results accordingly. However, I have to admit that
   I don't understand why that check should be made even for calendar-1900.
   It certainly doesn't match anything that Excel does.
   I would recommend scrapping that code altogether.
   If agreed, I would do this as part of the break-up into smaller modules.
   
   Another note -
   more controversially, it is clear that PhpSpreadsheet needs to support
   the Excel and PHP date formats. Although it requires further study,
   I am not convinced that it needs to support Unix timestamp format.
   Since that is a potential source of Y2038 problems on 32-bit systems,
   I would like to open a PR to deprecate the use of that format.
   Please let me know if you are aware of a valid reason to continue to support it.

   

   oleibman committed Feb 21, 2021
   Configuration menu

   • View commit details
   • Copy full SHA for ff9ac57
   • Browse repository at this point

   Copy the full SHA

   ff9ac57 View commit details

   Browse the repository at this point in the history

2. Scrutinizer Recommendations …

   The usual set of stuff.

   

   oleibman committed Feb 21, 2021
   Configuration menu

   • View commit details
   • Copy full SHA for 264ad2b
   • Browse repository at this point

   Copy the full SHA

   264ad2b View commit details

   Browse the repository at this point in the history

3. More Scrutinizer …

   If at first you don't succeed ...

   

   oleibman committed Feb 21, 2021
   Configuration menu

   • View commit details
   • Copy full SHA for e0d5f3e
   • Browse repository at this point

   Copy the full SHA

   e0d5f3e View commit details

   Browse the repository at this point in the history