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.
Phpstan hasn't identified any errors in Samples in a long time. But, as it turns out, one particular error is suppressed:
```
Variable $helper might not be defined.
```
This error would be generated by almost all samples, and the errors that it suppresses will become problematic if we move to Level 10. We are not yet committed to doing that, but it is pretty easy to write a script to change the samples so that error no longer happens, and there isn't really any reason to delay doing so. The results of that script constitute this PR.
The samples have become unwieldy when running them from a browser. In particular, the drop-down lists are fixed size with no scrolling, and many of them are now just too large. I have moved all the Calculation samples up a level, and broken several categories (Basic, Chart, DateTime, Engineering, Financial, and Reader) into several pieces.
Convert-Online (now found in the Engineering category) had a number of different problems which are now resolved. It is the only member with any significant code change.
* Let Phpstan Run on Samples
Phpstan currently analyzes all source and test members. We already run phpcs and php-cs-fixer on samples as well. I would expect that samples are often used as templates for code in userland; it behooves us to be at least as careful with those members as for the others which are already being analyzed. Aside from 1300+ messages `Variable $helper might not be defined.`, which will be suppressed in phpstan.neon.dist, there are really only a few changes needed for sample members, so that part of the code base was already in good shape, and is now even better. No annotations were needed.
* Scrutinizer 2 out of 3
1 false positive, now suppressed; fix other 2.
* Remove Dead Code
* Very Minor Changes
* Add infra
Because they tend to clutter our code a lot and unfortunately, there are
lots of false positives. Instead, it would probably be better to deal
with false positives out of band, via the Scrutinizer web UI.
* WIP Limited Support for Form Controls V2 (ListBox, Buttons, etc.)
This is a replacement for draft PR #2455 and draft PR #3127. There is some useful commentary in those PRs which I have mostly, but not entirely, duplicated below. Fix#2396. Fix#1770. Fix#2388.
A related problem is that the vml files used for the form controls sometimes contain invalid xml. Fix#3125 (rejected previous PR #1181 as too risky, issue was also reported as #170). Vml file should be valid Xml, but Excel can generate unclosed `<br>` tags, preventing Xlsx reader from reading file correctly. I believe a very narrowly targeted fix, changing `<br>` to `<br/>`, and only when reading vml files, probably mitigates the risk. The sample file formscomments.xlsx which is part of this change shows this problem with `Button 1` on sheet `Forms`; the spreadsheet was created with Excel 365, so the problem is not restricted to Excel 2013 as originally reported. A comment on PR 3127 indicates that other tags might be involved, but, without a file demonstrating that, I will restrict this change to br tags for now.
I am starting this out in draft status, and will probably leave it that way for some time. I'm not sure where we want to go with this. It fixes some problems, but in a limited manner, and creates some others. I'm not sure the pain of the others is balanced considering the limitations of the fix. If enough interest is generated as a result of this ticket being out there, we can proceed; if not, it probably isn't worth it.
This fix allows form control elements to be read in and written out. It does not allow you to add such elements, nor even to locate them or determine their properties (so you can't modify or delete them). Although it handles reading and writing of sheets containing both form controls and comments, it will probably create a corrupt spreadsheet if you try adding a new comment to a sheet with form controls - probably quite difficult to solve. Cloning the sheet probably won't work either - probably easier than the other. It is conceivable that we want to add a new property to the Xlsx Reader which turns the reading of form elements on or off (default=off), so that negative effects will be limited to those who have explictly opted in. The change in its current form does not implement such a property.
Because of its limitations, the change isn't really testable. As in some other recent installs, I have added a sample to demonstrate that it works correctly.
As it turns out, if we have a worksheet which contains both form controls and comments (see formscomments.xlsx which is part of this PR), PhpSpreadsheet already creates a corrupt file when it tries to load and save the spreadsheet with such a worksheet. With this change, the file is saved without corruption. This tilts things in favor of proceeding. I'm still not ready, but this will be an important consideration.
A sample file for issue #2621 illustrated a problem with shape files. Since they are involved here, I took a look at how the sample worked with this code. In master, and with this change, a corrupt file results. Fixing that is probably easier than the general problem of handling shape files, but it's an argument against moving this forward until the corruption problem can be addressed.
Fix#2661. A template including checkboxes was leading to file corruption solved by this PR. Another argument for moving forward.
* Improved Sample File, and Documentation
Add more realistic worksheet to spreadsheet. Document new feature, adding caveats to how it can be used.
* Name Clashes Between Parsed and Unparsed Drawings
This is at least a partial fix for #2396 and #1767 (which has been around for a long time). PhpSpreadsheet renames drawing XML files when it reads them from a spreadsheet. However, when it writes unparsed drawing files, it uses the original names, which can result in a clash with the renamed files. The solution in this PR is to write the unparsed files using the same renaming convention as the the others.
This is an incredibly simple fix, basically a one-line change, for such a long-lived problem. It is conceivable that this PR breaks a more sophisticated file than I have come across, e.g. with multiple unparsed files associated with a single worksheet. However, this PR does fix at least part of the problem for both issues, and causes no regression issues. The changed code was covered in only 2 tests - Reader/XlsxTest testLoadSaveWithEmptyDrawings and Writer/Xlsx/UnparsedDataTest testLoadSaveXlsxWithUnparsedData.
2396 is covered by a new test Unparsed2396Test. I had trouble figuring out what to test for 1767. Since it is a problem that becomes evident only when the output file is opened in Excel, I added a new sample to cover it.
* Sloppy Errors
I neglected to run php-cs-fixer and phpstan, and it bit me.
* Scrutinizer
It's not as good as Phpstan at recognizing problems that can't happen due to previous assertions.
* Scrutinizer Again
It can be really stupid sometimes.
* Tweaks to Input File Validation
This started as a response to issue #1718, for which it is a partial (not complete) solution. The following changes are made:
- canRead can currently throw an exception. This seems wrong. It should just return true/false.
- Breaking change of sorts. When AssertFile encounters a non-existent or unreadable file, it throws InvalidArgumentException. This does not make sense. I have changed it to throw PhpSpreadsheet/Reader/Exception.
- Since the previous bullet item required changing of most of the Reader files anyhow, this is a good time to add explicit typing for canRead in the function signature rather than the DocBlock. Since all the canRead functions inherit from an abstract version in IReader, they all have to be changed simulatneously. Except for Xlsx and Ods, most of the Reader files are otherwise unchanged.
- AssertFile is changed to add an optional "zip member" parameter. It will check for the existence of an appropriate member in what is supposed to be a zip file. It is used by Xlsx and Ods.
- Verifying that a given file is a valid zip ought to be a feature of ZipArchive. Thanks to a particularly nasty bug in php/libzip (see https://bugs.php.net/bug.php?id=81222), it is unsafe to attempt to open a zero-length file as a zip archive. There is a solution, but it does not apply to all the PHP releases which we support, and isn't even necessarily supported on all the point versions of the PHP versions which we do support. I have coded up a manual test for "valid zip", with a comment pointing to the spec.
- In theory, tests now cover 100% of the code in Shared/File. In practice ... One of the tests require that chmod works properly, which is not quite true on Windows systems, so that test is skipped on Windows. Another test requires that php.ini uses a non-default value for upload_temp_dir (can't be overridden in application code), which is probably not the case when Github runs the unit tests, so that test is skipped when appropriate. I have run tests for both on systems where they are not skipped.
* Update File.php
* Scrutinizer Timeout
It's not actually timing out, it's just waiting for something to finish that finished ages ago. Making a meaningless comment change in hopes that will clear the jam. Not particularly hopeful.
There are no changes to code. Additional tests are added,
so that the following 6 items now have 100% test coverage:
- Comment
- DefinedName
- DocumentGenerator
- IOFactory
- NamedFormula
- NamedRange
* Document calculation caching; and how to disable it and how to flush the cache
* Quoted text for string values beginning with `=`, so that they are still treated as strings and not as formulae
* Warning about assigning cells to variables
* Further warning about assigning cells to variables
* getCell() with a second argument
* Added String Value Binder, and a Reader example demonstrating how to use it
* Ensure value is a string before binding
* Sample file for String Value Binder
* PHPCS moaning about order or use statements
* Order of annotations, that PHPStorm determined, isn't what phpcs says it should be
oleibman
Adrien Crivelli
MarkBaker
oleibman
MarkBaker
Jon Dufresne
yuzhakov