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.
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.
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+.
* Reader/Gnumeric vs. Scrutinizer
Just reviewing Scrutinizer's list of "bugs". There are 19 ascribed to me. For some, I will definitely take no action (e.g. use of bitwise operators in AND, OR, and XOR functions). However, where I can clean things up so that Scrutinizer is satisfied and the resulting code is not too contorted, I will make an attempt.
I believe this is the only one with which will involve more than 2 or 3 changes. It fixes 5 items ascribed to me, and 4 to others.
* Use Strict Checking for in_array
* Correct Some Problems Which Will Show Up for PHP8.1
PHP8.1 wants to issue a message when you use a float where it thinks you ought to be using an int (it wants its implicit casts made explicit). This is causing unit tests to fail. The following corrections are made in this PR:
- Calculation.php tests `isset(self::binaryOperators[$token])`, where token can be a float. No numeric values are members of that array, so we can test for numeric before isset.
- SharedOle.php packs a float, intending it as an int, in 2 places. I simplified the logic here, and added explicit casts to avoid the problem. This is used by Xls Reader and Writer; as added confirmation, I added some timestamps from before 1970 (i.e. negative values) to Document/EpochTest. Because of this, the test suite has been verified for 32-bit PHP as well as PHP 8.1.
- Writer/Xlsx/StringTable tests `isset($aFlippedStringTable[$cellValue])`. This is the same problem as in Calculation, but requires a different solution. The same if statement here also tests that the datatype is string, but it does so after the isset test. Changing the order of these tests avoids the problem.
* Update OLE.php
* 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.
oleibman
Adrien Crivelli
oleibman