Many problems with Dependabot this month. Phpstan introduced a lot of new "errors". These are now fixed or annotated. I combined this with a change to require comments for `phpstan-ignore`. These won't always be useful, but I think requiring them makes sense.
Tcpdf is more of a non-update. Our composer.json specified `^6.5`. For some reason, Dependabot decided it was okay to change that to `^6.5||^7.0`, which seems presumptuous. (One of the triggers was probably the elimination of Php8.1, since the new product requires 8.2+.) Tcpdf is nominally deprecated, replaced by tc-lib-pdf. Tcpdf 7 passes control to the new product. However, the upgrade is not straightforward. The user needs to supply font files which were formerly distributed with the product, and a code change to define a (shudder) global constant is required. Consequently, Dependabot's upgrade failed its unit tests. While I may evaluate what might be needed at some point in the future, for now I am just updating composer.json to reject Tcpdf 7+.
Writer/Xlsx/Chart hard-codes a lot of output values. While nobody has reported a problem involving these, it makes sense to me to copy those values over from what Xlsx/Reader/Chart read, rather than hard-coding them. This involves adding some new properties to Chart and Layout, all, naturally, initialized to the values that we have been hard-coding.
- Chart\date1904
- Chart\lang
- Chart\pageMargins
- Chart\pageSetup
- Layout\bodyPr
With one exception, I have not investigated any of the new properties in depth. That may come in time. The one I did look at is `date1904`. Chances are that it should match the equivalent spreadsheet setting, but it won't matter if there are no dates on your chart, and it often won't matter even if you do. It may well matter if you are using a `date axis`. While looking into that, it became apparent that some Shared Date conversions need to be a bit more flexible, specifying an optional `calendar` parameter rather than relying on the Spreadsheet `calendar` (which is not accessible while processing the chart) or the Shared\Date `calendar` (which is accessible).
# This is the 1st commit message:
Integrated unit test to capture the bug.
# This is the commit message #2:
Added Date.safeModify() method for secure DateTime modifications and integrated exception handling - Fixed#4840.
# This is the commit message #3:
PHPCS Fixes.
# This is the commit message #4:
PHPCS Fixes.
# This is the commit message #5:
Codestyle fix.
# This is the commit message #6:
Added description to CHANGELOG.md.
# This is the commit message #7:
Enhanced Date.safeModify() to ensure consistent exception handling across PHP versions, addressing changes in DateTime::modify() behavior in PHP 8.3 - #4840.
# This is the commit message #8:
Confusion Checking for Union Arguments
Fix#4832. PR #4657 added support for passing union arguments to functions. User reports a problem with a peculiar formula afterwards. See issue for details - the very loose restrictions placed on worksheet names can lead to an ambiguous situation for the parser.
This problem arose with a function whose first argument was a cell address *including sheet name*. Parser is changed to try to avoid this situation. If the regular expression which tells us we have a potential need for union has a left parenthesis in the "sheet name" without a right parenthesis, then it will no longer try to treat the formula as containing union arguments except in the unlikely event that "sheet name" truly is a worksheet title in the spreadsheet. This feels pretty kludgey, but it solves the problem at hand, and seems unlikely to cause problems. There may still be edge cases more subtle than the one in the issue; I am satisfied to wait for reports of such.
# This is the commit message #9:
Weird Regexp Difference between Php8.4+ and Php8.3-
Specify codeCoverageIgnore for now, remove later.
# This is the commit message #10:
Move Test to More Sensible Location
# This is the commit message #11:
Ods Reader/Writer Support for Integer Styles with Leading Zero
Fix#1606, which went stale and is now reopened. PR #4806 supplied support for some Number Formats for Ods, but did not directly address this issue. This PR does. In addition to the original issue, this could be useful for zip codes. As I stated in 4806, I may be amenable to adding some unsupported styles to the built-in list, but the custom style option will always be around in case I am being slow or unreasonable.
# This is the commit message #12:
A Bit More Flexibility
And some more tests.
# This is the commit message #13:
Update CHANGELOG.md
Xlsx Writer Support Data URI for Images
Fix#4823. When writing a spreadsheet to Html, a data Uri can be used if `embedImages` is true. Reading such an Html spreadsheet and attempting to write it to Xlsx results in an Exception. It should be noted that Excel itself cannot open the Html properly; none of the images are present.
The PhpSpreadsheet problem arises not with the inclusion of the image, but rather with attempting to include the appropriate entry in `[ContentTypes].xml`. This PR corrects that problem.
For the record, Xls Writer does not have a problem with this situation. Just to demonstrate that, a parallel test for Xls Writer is added in addition to the new Xlsx Writer test.
Update CHANGELOG.md
Update CHANGELOG.md
Added phpcs and php-cs-fixer corrections #4840.
Updated DateTest to handle DateMalformedStringException for PHP 8.3 in excelToDateTimeObject exception tests.
Fixed phpcs issues.
Fixed php-cs-fixer issue.
Updated DateTest so to not directly reference DateMalformedStringException class which is not existing prior to PHP 8.3.
Refactored DateTest to dynamically set the expected exception class, ensuring compatibility across PHP versions.
Removed unused Throwable import and updated expected exception type in DateTest for consistency with PHP version handling.
Added DateMalformedStringException polyfill to solve the phpstan issue.
Fixed phpcs issue.
In issue #4557, the user complains, with some justification, about the way Excel handles certain calculations. We are not able to help with that problem. However, the user also notes a problem in Shared/Date when `isDateTime` has to evaluate a cell whose calculated value is an array. This is solved by flattening the calculated result to a single value.
It became obvious while working on this change that the code to set `instanceArrayReturnType` was kind of awkward. Simpler methods `returnArrayAsArray` and `returnArrayAsValue` are added to `Spreadsheet`. Even these started out a bit awkward because `Spreadsheet::calculationEngine` was defined as nullable, which really isn't true. It is allocated by the constructor, and never freed except in the destructor. It is no longer nullable.
With PhpUnit 10 came the ability to use Php attributes rather than doc-block annotations for things like "data provider". PhpUnit 11 deprecates the use of annotations, and PhpUnit 12 will not not permit their use. Since PhpUnit 11 requires Php8.2+, we cannot adopt it as long as we support Php8.1, which will continue to be the case for some time. However, there is no penalty for early adoption.
Php-cs-fixer can use:
```
'php_unit_attributes' => ['keep_annotations' => false],
```
This allows us to run `composer fix` to automate all the needed changes. No manual changes were needed for any of the test members.
With this change, PhpUnit 9 can no longer be used with the test suite. File composer.json is updated to reflect that reality, and phpunit9.xml.dist, which has been supplied in case anyone needed to use PhpUnit 9, is no longer required, and is thus deleted. For now, PhpUnit 11 is not being added as a possibility.
No source code is changed in this PR.
I check from time to time. There are a number of problems now, mostly due to the elimination of Php 7.4 and replacement of doc-block typing with explicit Php typing.
- Bitwise functions were particularly affected by PR #3718 and PR #3793.
- Chart/Axis and Writer/Xlsx were amusingly affected by PR #3836, which added a scaling option which included an array indexed by the known allowable factors, one of which is 1 trillion, which cannot be represented as an integer on a 32-bit system. Issue3833Test, introduced by the same PR (and not suffering any errors) was expanded to test this value.
- Some minor changes to Reader/Xls and Shared/OLE/PPS to accommodate hex values which are negative in 32-bit but which Php-32 may wind up casting to large floating point numbers; it is not clear to me why these hadn't shown up as problems previously. Possibly this is the result of changes in the most recent Php versions.
- BitAndTest, BitOrTest, BitXorTest and Shared/DateTest were adversely affected by PR #3859 when arguments and/or expected results too large for a 32-bit integer were supplied.
- ImExpTest required a slightly reduced precision for 32-bit. No idea why this hadn't shown up earlier.
Change "mixed" declarations to more accurate types in test members; in particular, change those that would be flagged if we were to run Phpstan at level 9 (we currently run level 8). I may or may not follow up with source code (over 700 level-9 problems remain for src), but, as with strict typing, there is no reason to avoid the effort for test members.
It was necessary to update some doc blocks in src to accommodate this change. However, no executable code is touched.
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.
And quite a bit more manual changes. The idea is that typing of our
tests can be a bit more loose, so we assume PHPDoc is mostly correct. If
that happens to be wrong, it should be caught by the tests themselves.
Successor to PR #3523. There are 494 single-line changes (`public function provider` to `public static function provider`) in this PR. None of these were made manually; they were all created with the following script (adapted from
https://stackoverflow.com/questions/25909820/how-to-recursively-iterate-through-files-in-php):
```php
$dir = 'C:/git/unit10prep2/tests/PhpSpreadsheetTests';
$it = new RecursiveDirectoryIterator($dir);
// Loop through files
foreach(new RecursiveIteratorIterator($it) as $file) {
if ($file->getExtension() === 'php') {
$contents = file_get_contents($file);
$new = preg_replace('/public function (\\w*)([Pp])rovider/', 'public static function $1$2rovider', $contents);
if ($new !== $contents) {
echo "changing $file\n";
file_put_contents($file, $new);
}
}
}
```
After this PR, there will be one more, with a small number of test changes, and enabling PhpUnit 10 for Php 8.1+.
* Eliminate Most Scrutinizer Problems in Test Suite
Mostly minor code changes, with some annotations.
* Missed 2 php-cs-fixer Problems
They should be fixed now.
* Document Properties - Coverage and 32-bit-safe Timestamps
While researching an issue, I noticed that coverage of Document/Properties was poor. Further, the use of int timestamps will eventually lead to problems for 32-bit PHP (see issue #1826).
Coverage Changes:
- Many property types with no special handling are enumerated but not tested. These are removed, but will continue to function as before.
- Existing code theoretically allows property to be set to an object, but there is no means to read or write such a property, and, even if there were, I don't believe Excel supports it. Setting a property to an object will now be changed to a no-op (can throw an exception if preferred).
- Since the Properties object now has no members which are themselves objects, there is no need for a deep clone. The untested __clone method is removed.
- Large switch statements are replaced with associative arrays. Scrutinizer will like that.
- Coverage is now 100%.
<!-- end of coverage changes list -->
Timestamp Changes:
- Timestamps will be stored as int if possible, or float if not. This is, or will soon be, needed for 32-bit systems. Tests have been added for beyond-epoch dates, and run successfully with 32-bit.
- LibreOffice doesn't quite get the Created/Modified properties correct. These are written to the file as a string which includes offset from UTC, but LibreOffice ignores the offset portion when displaying them. Code had been generating these in UTC, but now generates them in default timezone, which should meet user's expectations.
<!-- end of timestamp changes list -->
Other Changes:
- Custom properties added to ODS Writer.
- Samples had not been generating any ODS files. One is now generated.
- Ods uses a single 'keywords' property rather than multiple 'keyword' properties.
- Breaking change - default company is changed to null string from Microsoft Corporation.
- Breaking change of sorts - PropertiesTest incorrectly tested a custom date property against a string, Reader/XlsxTest correctly tested against a timestamp converted to a string. PropertiesTest was defective, and will no longer work as coded; anyone using it as a model will likewise have a problem.
- PHP8.1 has been complaining for weeks about a time zone conversion test. I have now downloaded a version, and changed the code so that it will work in 8.1 as well as prior releases. (It is still likely that the existing code should work in 8.1, but I haven't yet figured out how to file a bug report.) In the course of testing, 3 additional 8.1 problems were reported (all along the lines of "can't pass null to strpos"), and are fixed with null coercion.
- Two Calculation tests failed because of large results on 32-bit system. These are corrected by allowing the functions involved to return float|int rather than int. I suspect that there are other functions with this problem, and will investigate as a follow-up activity.
- See issue #2090. I believe that changes between 17.1 and master will merely cause the problematic spreadsheet to fail in a different way. I believe that enclosing in quotes some variables passed to Document/Properties by Reader/Xlsx will eliminate the problem, but, in the absence of an example file, cannot say for sure.
- Properties tests are now separated out from Reader/XlsxTest and Reader/OdsTest, and now test both Read and Write (via reload).
<!-- end of other changes list -->
Miscellaneous Notes:
- There remains no support for Custom Properties in Xls Reader or Writer.
- We now have default timezones for all of PHP itself, Shared/Date, and Shared/Timezone. That is least one too many. I was unable to disentangle the latter two for this change, but will look into deprecating one or the other in future.
* Phpstan
6 baseline deletions, 2 docblock changes
* Scrutinizer's Turn
3 minor errors that hadn't blocked the request.
I ran the test suite using 32-bit PHP. There were 2 places where changes
were needed due to 32-bit timestamps.
Reader\\Xml.php was using strtotime as an intermediate step in converting
a string timestamp to an Excel timestamp. The XML file type stores pure timestamps
(i.e. no date portion) as, e.g., 1899-12-31T02:30:00.000, and that value
causes an error using strtotime on a 32-bit system. However, it is sufficient
to use that value in a DateTime constructor, and that will work for 32- and 64-bit.
There was no test for that particular cell, so I added one to the XML read test.
And that's when I discovered the getFormattedValue bug. The cell's format
is `hh":"mm":"ss`. The quotes around the colons are disrupting the formatting.
PhpSpreadsheet formats the cell by converting the Excel format
to a Php Date format, in this case `H\:m\:s`.
That's a problem,
since Excel thinks 'm' means *minutes*, but PHP thinks it means *months*.
This is not a problem when the colon is not quoted; there are ample tests for that.
I added my best guess as to how to recognize this situation,
changing `\:m` to `:i`. The XML read test
now succeeds, and no other tests were broken by this change.
Test Shared\\DateTest had one test where the expected result of converting to a
Unix timestamp exceeds 2**32. Since a Unix timestamp is strictly an int,
that test fails on a 32-bit system. In the discussion regarding recently merged
PR #1870, it was felt that the user base might still be using the functions
that convert to and from a timestamp. So, we should not drop this test, but,
since it cannot succeed on a 32-bit system, I changed it to be skipped
whenever the expected result exceeded PHP_INT_MAX. There are 3 "toTimestamp"
functions within that test. Only one of these had been affected, but I thought
it was a good idea to add additional tests to the others to demonstrate this
condition.
In the course of testing, I also discovered some 32-bit problems with
bitwise and base-conversion functions. I am preparing separate PRs to
deal with those.
* Fix Issue 1441 (isDateTime and Formulas)
When you have a date-field which is a formula, isDateTime returns false.
https://github.com/PHPOffice/PhpSpreadsheet/issues/1441
Report makes sense; fixed as suggested. Also fixed a few minor
related issues, and added tests so that Shared/Date and Shared/TimeZone
are now completely covered.
Date/setDefaultTimeZone and TimeZone/setTimeZone were not consistent
about what to do in event of failure - return false or throw.
They will now both return false, which is what Date's function
said it would do in its doc block anyhow. Date/validateTimeZone will
continue to throw; it was protected, but was never called outside
Date, so I changed it to private.
TimeZone/getTimeZoneAdjustment checked for 'UST' when it probably
meant 'UTC', and, as it turns out, the check is not even needed.
The most serious problem was that TimeZone/validateTimeZone does not
check the backwards-compatible time zones. The timezone project
aggressively, and very controversially, "demotes" timezones;
such timezones eventually wind up in the PHP backwards-compatible list.
We want to make sure to check that list so that our applications do not
break when this happens.
Because even if it doesn't make a difference in practice, it is
technically more correct to call static methods statically. It
also better advertise that those methods can be used from any context.
oleibman
Adriano Adriano
Marc Bennewitz
Adrien Crivelli
MarkBaker
MarkBaker
oleibman
Gabriel Caruso
Zharikov Viktor