Phpstan level 10 reports an enormous number of errors. So does the excluded missingType.iterableValue. I do not plan to introduce either any time soon. But I will submit piecemeal changes from time to time. Changes will be mostly limited to phpdoc type declarations.
Its defaults are to unescape within single quotes, and escape within double quotes and here-docs. Right now, it leaves everything as-is, which means our code is inconsistent and need not be. Further, although dealing with complex regular expressions will never be easy, I find it much easier to figure out what's going on when superfluous back-slashes are removed.
These changes were all made automatically using the "fix" script, so should be reliable. They, of course, pass all unit tests.
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 am becoming concerned with the increasing run-time of php-cs-fixer, especially since it can time out. Some relief may come from PR #4118, but that won't be merged for some time, if ever. So, bump up the timeout period now. Also replace properties which php-cs-fixer has deprecated with their non-deprecated equivalents. No change to any source code.
Dependabot suggested some changes this month which required an unusual effort to implement successfully. With the elimination of Php 8.0 as a supported environment, it became possible to use Phpunit 10 rather than 9. Among other considerations, the configuration file for Phpunit is changed. I preserved the Phpunit 9 version under a different name. Aside from the configuration change, several other changes needed to be made to accommodate the change:
- Fix#3993. A peculiar problem indeed. One of the reporters said it had something to do with mocking, but I couldn't duplicate it. But Phpunit 10 revealed the problem in one test (Reader/Xlsx/AutoFilterTest), and that was sufficient for me to apply the trivial source code change to Worksheet/Worksheet.
- Some extra stringency on data providers required extra work in Calculation/CalculationFunctionListTest.
- There was a misplaced label in Calculation/ParseFormulaTest. Likewise in the data member CellGetRangeBoundaries.
- More stringency required changes to data members Shared/Trend/ExponentialBestFit and Shared/Trend/LinearBestFit.
- Issue3982Test testLoadAllRows seemed to go into a memory-acquiring loop in 10 that was not evident in 9. This particular test uses a lot of memory by design, but was included only to establish a base level for the other tests in that member. I feel it is acceptable to skip it for 10.
Phpstan errors with a new release are not unusual. One of the problems this time around was, however, unusual - it is fixed when Phpstan runs under Php8.3, but not for earlier Php releases. Our tools currently use Php 8.1. It is on my to-do list to get Phpstan and other test tools running under 8.3 before 8.1 goes EOL. There are also some configuration file changes needed for Phpstan.
Php-cs-fixer has been taking an increasingly long time to run. They've added an experimental option to permit it to run its checks in parallel. I've changed its configuration to use that option.
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.
* Automate Function-by-Name and Function-by-Category Docs
Examining whether it is possible to analyze the 2 Php scripts in the bin directory even though they lack a .php extension, I found that the docs generated by `generate-document` had not been updated in over a year even though support for a number of functions (e.g. ARRAYTOTEXT) had long ago been implemented. The files generated by the other script `generate-locales` are automatically generated as part of the unit test suite, and I can't see any reason not to do the same for `generate-document`.
It isn't totally clear that the 2 scripts in bin are needed any more now that each has an equivalent in the unit test suite. For now, I'm keeping them, and allowing them to be analyzed by phpcs, php-cs-fixer, and phpstan.
* Reflection Not Required
Remove it from bin scripts and DocumentGeneratorTest.
* Better Html/Css Display for Function Lists
* Redesign Features Cross Reference Doc
It was inaccurate, and pretty much impossible to read in entirety. This version should be easier to read, and to correct. That's important, because there are almost certainly new inaccuracies in this version.
* Add Html to Features Cross-Reference Readers
* Add iterateOnlyExistingCells to Constructors
Fix#3721. That issue can already be handled, but requires several statements when one ought to suffice. Adding an extra parameter to the RowCellIterator and ColumnCellIterator constructors is useful, easy, and becomes especially practical now that our supported Php releases all support named parameters (see new test Issue3721Test).
* Update Column.php
* Update .php-cs-fixer.dist.php
* Update Row.php
* Update Column.php
* Update ByColumnAndRowTest.php
* Check For and Eliminate Octal Literals
Php8.1 introduced a new form of octal literal (0o123); the old form (0123) continues to be supported, but can certainly be misleading. There are exactly 6 uses of the old form in the code base (5 in tests, 1 in infra). 4 of these are clearly unintended (02 or 03), and the leading 0 should just be dropped for those. The other 2 are for file permissions and are easily replaced.
* Minor Changes to LocaleGenerator
Testing the permissions change exposed some problems when the locale files don't exist. This never shows up as an issue when the files do exist, which is pretty much all the time, but correct the theoretical exposures anyhow.
* Populate bg and en_uk Locale Files from Spreadsheet
Also rename locale generator so that it is executed first. This is when it should happen because other tests depend on its results.
* Some Bulgarian Function Omissions
Figured out a reasonable way to compare old Bulgarian function file to new; this identified a handful of now-corrected omissions and errors. Note that JIS function, in old list, is not on spreadsheet, but the Bulgarian translation is the same as English (so the translation isn't needed), and the function has been replaced by DBCS, which is also not on the spreadsheet.
Zipstream's newest production release requires Php8.1. This is not necessarily a problem for us as we are locked to an earlier release. However, psr/simple-cache became a real mess for us as people wanted to install PhpSpreadsheet alongside other products which required a version of simple-cache that could not run in some of the versions of Php which we support, and I would like to avoid having to go through that again. This ticket is a contingency to put us ahead of the curve in case such a problem should arise with Zipstream. There is reason to believe that Zipstream is not so widely used as simple-cache, so we do not need to rush this change in. However, should that belief prove incorrect, the change should be ready to go when needed.
For a recent change, I removed some errors from Phpstan baseline and instead added annotations in the source members. I did this work incrementally, and was surprised when php-cs-fixer required a change from `// @phpstan-ignore-next-line` to `/** @phpstan-ignore-next-line */`. No problem thinks I, and continue to modify several members using the new convention until php-cs-fixer required a change from `/** @phpstan-ignore-next-line */` to `// @phpstan-ignore-next-line`??? I did as directed, and continued to be surprised for the rest of that ticket.
Having had time to research, the problem is due to two options in the php-cs-fixer config file `'comment_to_phpdoc' => true` and `'phpdoc_to_comment' => true`. It seems that php-cs-fixer is treating these annotations the same as doc-blocks, expecting `/**` before a `structural element`, and `//` otherwise. For the statements where I had questions, it expects `/**` before a statement which you might be able to precede with `/** @var`, and `//` where you would not be able to precede it with `/** @var`. However, in this case, what it is doing is forcing what appear to be inconsistencies between otherwise identical statements, whereas php-cs-fixer is supposed to be supporting consistent syntax throughout the project. This PR changes both options to false, allowing (but not requiring) a consistent syntax for these examples. It contains an example of a change from each format to the other, changes which php-cs-fixer would previously have flagged.
An added bonus for this change is that Scrutinizer annotations can now be added to the code; these were often rejected by php-cs-fixer. These should, of course, be used very conservatively, but there are cases where Scrutinizer's analysis is either faulty or not helpful. This PR takes advantage of the change by adding annotations to eliminate the two existing problems which Scrutinizer classifies as 'Security', problems for which there is no sensible way to satisfy Scrutinizer's complaint.
No executable code is changed by this PR.
* Allow single-cell checks on conditional styles, even when the style is configured for a range of cells
* Work on the CellMatcher logic to evaluate Conditionals for a cell based on its value, and identify which conditional styles should be applied
* Refactor style merging and cell matching for conditional formatting into separate classes; this should make it easier to test, and easier to extend for other CF expressions subsequently
* Added support for containsErrors and notContainsErrors
* Initial work on a wizard to help simplify created Conditional Formatting rules, to ensure that the correct expressions are set
* Further work on extending the Conditional Formatting rules to cover more of the options that are available in MS Excel
* Prevent phpcs-fixer from removing class @method annotations, used to identify the signature for magic methods used in Wizard classes
* Implement `fromConditional()`` method to allow the creation of a CF Wizard from an existing Conditional
* Ensure that xlsx Reader picks up the timePeriod attribute for DatesOccurring CF Rules
* Allow Duplicates/Uniques CF Rules to be recognised in the Xlsx Reader
* Basic Xlsx reading of CF Rules/Styles from <extLst><ext><ConditinalFormattings> element, and not just the <ConditinalFormatting> element of the worksheet
* Add some validation for operands passed to the CF Wizards
- remove any leading ``=` from formulae, because they'll be embedded into other formulae
- unwrap any string literals from quotes, because that's also handled internally
Handle cross-worksheet cell references in cellReferences and Formulae/Expressions
* re-baseline phpstan
* Update Change Log with details of the CF Improvements
Dependabot opened PR #2365 to upgrade php-cs-fixer from 2.19.2 to 3.2.1. Changes are required before that can be merged successfully. I believe all the necessary changes are in this PR.
One of the changes is to replace .php_cs.dist with .php-cs-fixer.dist.php. I have made those two identical for this PR so that there will be a meaningful delta listing. After this change is merged, master can be merged into 2365, which will hopefully pass all tests and be mergeable at that point. We can delete the unneeded file after that merge.
Spacing is changed in a handful of source members because of extra stringency in 3.2.1.
oleibman
Adrien Crivelli
Vincent Langlet
MarkBaker
Owen Leibman