Fix#1425 which went stale 5 years ago, and which I've reopened. Style was not being set properly for last row. Also, Style was not being set at all for insertNewColumnBefore('B').
Fix#282, which went stale over 7 years ago, and is now reopened. This is certainly related to issue #4379. If merged cells are in the midst of deleted rows/columns, the merge may continue to exist after the deletion, leading to various problems including spreadsheet corruption. The problem is not with either Reader or Writer.
This PR will automatically unmerge all merged cells whose first cell is in the delete range before performing the deletion. This will address the corruption problem. Something more sophisticated may be required, so I won't merge this PR for a while to give me a chance to think about it some more.
Styles are a property of Spreadsheet, not Worksheet. There is a Styles property in Worksheet, initially set to an empty array and *never* updated. This PR removes the property and the getStyles method of Worksheet. This is, technically, a breaking change, so it will be installed with PhpSpreadsheet V4 (PR #4240).
Fix#4324. Serialization was explicity forbidden by PR #3199. This was in response to several issues, and concern that the Spreadsheet object contained non-serializable properties. This PR restores the ability to serialize a spreadsheet. Json serialization remains unsupported.
Fix#1757, closed in Nov. 2023 but just reopened. At the time, Cell property `formulaAttributes` was stored as a SimpleXmlElement. Dynamic arrays PR #3962 defined that property as `null|array<string, string>` in the doc block. However, it left the formal Php type for the property as `mixed`. This PR changes the formal type to `?array`.
Fix#1741, closed in Dec. 2020 but just reopened. Calculation property `referenceHelper` was defined as static, and, since static properties don't take part in serialization, this caused a problem after unserialization. There are at least 3 trivial ways to deal with this - make it an instance property, reinitialize it when unserialized using a wakeup method, or remove the property altogether. This PR uses the last of those 3.
Calculation does have other static properties. Almost all of these deal with locale. So serialize/unserialize might wind up using a default locale when non-default is desired (but not necessarily required). If that is a problem for end-users, it will be a new one, and I will work on a solution if and when the time comes.
Static property `returnArrayAsType` is potentially problematic. However, instance property `instanceArrayReturnType` is the preferred method of handling this, and using that will avoid any problems.
Issue #932 also dealt with serialization. I do not have the wherewithal to investigate that issue. If it is not solved by this and the earlier PR's, I will have to leave it to others to re-raise it.
Spreadsheet `copy` is now simplified to use serialize followed by unserialize. Formal tests are added. In addition, I have made a number of informal tests on very complicated spreadsheets, and it has performed correctly for all of them.
This is the second recent occasion where style for whole row or column is behaving unexpectedly (see issue #4285). For the earlier issue, a documentation update was made to show the preferred method of styling. Setting styles for row(s) or column(s) in that way will work just fine, however retrieving the style doesn't yield the expected result. Although that problem can be overcome with existing code, simpler methods are needed, and this PR adds methods getRowStyle and getColumnStyle to Worksheet. I will continue to investigate why this problem and the one from 4285 have unexpected results.
Fix#641 (marked stale in 2018, but now reopened). When a sheet's title is changed, PhpSpreadsheet updates references to the old sheet name found in formulas. Which is a good idea when the sheet is attached to the spreadsheet, but a bad idea when it isn't (often because it has been cloned without re-attaching to the spreadsheet). This PR continues to change formulas in the former case, but will no longer do so for the latter.
Fix#4241. Some security batches caused a minor break in Drawings, forcing `setWorksheet` to come after `setPath`. Although the problem is easily fixed in user code, this was not an intended change. Some slight recoding restores the earlier functionality where the order of calls was not important, without sacrificing the security gains. This change will be back-ported to the other active branches to which the security patch had been applied.
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.
Fix#4192. Although that issue can be dealt with by changing user code, it would be better to fix it within PhpSpreadsheet. A cloned worksheet may have a pointer to a spreadsheet to which it is not attached. Code can assume it does belong to the spreadsheet, and throw an exception when the spreadsheet cannot find the worksheet in question. It may also not throw an exception when it should.
In my comments to the issue, I was concerned that adding in the needed protection would add overhead to an extremely common situation (setting a cell's value) in order to avoid a pretty rare problem. However, there are problems with both the accuracy and efficiency of the existing code, and I think any performance losses caused by the additional checks will be offset by the performance gains and accuracy of the new code.
Spreadsheet `getIndex` attempts to find the index of a worksheet within its spreadsheet collection. It does so by comparing the hash codes of each sheet in its collection with the hash code of the sheet it is looking for. Its major problem problem is performance-related, namely that it recomputes the hash code of the target sheet with each iteration.
A more severe problem is the accuracy of the hash code. It generates this by hashing together the sheet title, the string range of its auto-filter, and a character representation of whether sheet protection is enabled. Title should definitely be part of the calculation (it must be unique for all sheets attached to a spreadsheet), but it is not clear why this subset of the other properties of Worksheet is used. It tries to save some cycles by using a `dirty` property to indicate whether re-hashing is necessary. It sets that property whenever the title changes, or when `setProtection` is called. So, it doesn't set it when auto-filter changes, and you can easily bypass `setProtection` when changing any of the `Protection` properties. Not to mention the many other properties of worksheet that can be changed. Additionally, if you clone a worksheet, the clone and the original will have the same hash code, which can lead to problems:
```php
$clone = clone $original;
$spreadsheet->getSheet($spreadsheet->getIndex($clone))
->setCellValue('A1', 100);
```
That code will change the value of A1 in the original, not the clone.
The `hash` property in Worksheet will now be calculated immediately when the object is constructed or cloned or unserialized. It will not be recalculated, and there is no longer a need for the `dirty` property, which is removed. Hash will be generated by spl_object_id, which was designed for this purpose. (So was spl_object_hash, but many online references suggest that \_id performs much better than \_hash.) Our problem example above will now throw an Exception, as it should, rather than changing the wrong cell. `setValueExplicit`, the problem in the original issue, will now test that the worksheet is attached to the spreadsheet before doing any style manipulation. In order that this not be a breaking change, `getHashCode` will continue to return string, but it is deprecated in favor of `getHashInt`, and Worksheet will no longer implement IComparable to facilitate the deprecation.
I had a vague hope that this change might help with issue #641. It doesn't.
Fix#4128. PR #4073 introduced applyStylesFromArray method, which allowed setting styles without affecting selectedCells or activeSheet. The first use of this method was in Cell setValueExplicit to set quotePrefix appropriately. The new method did not preserve activeCell. I'm not sure why that should matter, but this seems to have caused a problem for Excel 2016. This seems to be a bug in Excel, one which is fixed in newer releases. However, PhpSpreadsheet can avoid the problem by preserving activeCell as well as selectedCells and activeSheet. This PR makes that change.
The latest release seems to not want you to give a class element both a Php type and a doc-block type. I used the "fix" operand to delete the redundant doc-block declarations, with no other changes. So there should be no change to executable code.
Fix#4112. Direct cause is that `applyStylesFromArray` tries to save and restore `activeSheetIndex`. However, if activeSheetIndex is -1, indicating no active sheet, the restore should not be attempted. Code is changed to test before attempting to restore.
The actual problem, however, is that user specified a sheet number for `addSheet`. That method will set activeSheetIndex most of the time, but this was a gap - when the supplied sheet number (0 in this case) is greater than activeSheetIndex (-1 in this case), it was leaving activeSheetIndex as -1. It is changed to set activeSheetIndex to 0 when activeSheetIndex is negative.
Till now we have used a static variable/getter/setter to decide what type of result should be returned when a formula is evaluated and an array is the result. This is messy; it would be much better to use an instance variable instead. We cannot eliminate `setArrayReturnType` and `getArrayReturnType` because that would be a BC break. I am considering whether they should be deprecated. In the meantime, I have added a new instance property `instanceArrayReturnType` with getter and setter methods. The property is initially null, and, if it remains so when needed, the static property will be used instead. However, if it is set, its value will be used.
Provide a means to convert a range, possibly with unions and possibly with intersections, into something that both Excel and PhpSpreadsheet can handle. Intersections are changed into unions of the individual cells which they comprise. With this change, Xls Writer now handles intersections (previously it would have thrown an Exception or created a corrupt worksheet if this was attempted), and Xlsx Writer works correctly (it seemed to before, but Excel didn't understand what it wrote). Worksheet::getConditionalRange and ::getConditionalStyles would previously have thrown an Exception when presented with an intersection, and will no longer do so.
**NOTE:** Intersection support is limited to Conditional ranges. Use of intersections in other contexts will usually not achieve the desired result.
This has come up a number of times, most recently with issue #3901, and also issue #3659. It will certainly come up more often in days to come. Excel is changing formulas which PhpSpreadsheet has output as `=UNIQUE(A1:A19)`; Excel is processing the formula as it were `=@UNIQUE(A1:A19)`. This behavior is explained, in part, by https://github.com/PHPOffice/PhpSpreadsheet/pull/3659#issuecomment-1663040464. It is doing so in order to ensure that the function returns only a single value rather than an array of values, in case the spreadsheet is being processed (or possibly was created) by a less current version of Excel which cannot handle the array result.
PhpSpreadsheet follows Excel to a certain extent; it defaults to returning a single calculated value when an array would be returned. Further, its support for outputting an array even when that default is overridden is incomplete. I am not prepared to do everything that Excel does for the array functions (details below), but this PR is a start in that direction. If the default is changed via:
```php
use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
Calculation::setArrayReturnType(Calculation::RETURN_ARRAY_AS_ARRAY);
```
When that is done, `getCalculatedValue` will return an array (no code change necessary). However, Writer/Xlsx will now be updated to look at that value, and if an array is returned in that circumstance, will indicate in the Xml that the result is an array *and* will include a reference to the bounds of the array. This gets us close, although not completely there, to what Excel does, and may be good enough for now. Excel will still mess with the formula, but now it will treat it as `{=UNIQUE(A1:A19)}`. This means that the spreadsheet will now look correct; there will be superficial differences, but all cells will have the expected value.
Technically, the major difference between what PhpSpreadsheet will output now, and what Excel does on its own, is that Excel supplies values in the xml for all the cells in the range. That would be difficult for PhpSpreadsheet to do; that could be a project for another day. Excel will treat the output from PhpSpreadsheet as "Array Formulas" (a.k.a. CSE (control shift enter) formulas because you need to use that combination of keys to manually enter them in older versions of Excel). Current versions of Excel will instead use "Dynamic Array Formulas". Dynamic Array Formulas can be changed by the user; Array Formulas need to be deleted and re-entered if you want to change them. I don't know what else might have to change to get Excel to use the latter for PhpSpreadsheet formulas, and I will probably not even try to look now, saving it for a future date.
Unit testing of this change uncovered a bug in Calculation::calculateCellValue. That routine saves off ArrayReturnType, and may change it, and is supposed to restore it. But it does not do the restore if the calculation throws an exception. It is changed to do so.
Fix#3951. When an Excel sheet is protected, even when sorting is explicitly allowed without a password, sorts are permitted only on "protected ranges" within the sheet. PhpSpreadsheet already supports protected ranges, and only minor tinkering is necessary for that (e.g. the protected range can have, but does not require, a password). The more important part of this change is documenting the far-from-intuitive way that Excel handles this. To that end, documentation is updated, and a new sample is added.
A new class, `Worksheet\ProtectedRange` is added in place of the string array which had been used. `Worksheet::getProtectedCells` is deprecated in favor of the new `Worksheet::getProtectedCellRanges`.
Fix#3923. Cells which have not yet been allocated cause problems when they need to be moved due to a column/row insert/delete. Code had been added in ReferenceHelper earlier to create missing cells, but only in the last data column. It needs to change to create the missing cells:
- for columns, in rows 1 to "highest data row" for columns "before column" to "highest data column".
- for rows, in columns A to "highest data column" for rows "before row" to "highest data row".
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.
Fix#3687. Worksheet methods insertNewRowBefore and insertNewColumnBefore call ReferenceHelper insertNewBefore. That function fills in "missing" cells with null values. However, for boundaries, it uses getHighestRow and getHighestColumn. It should be sufficient to use getHighestDataRow and getHighestDataColumn. When there is a big gap between getHighest... and getHighestData..., this can result in a big increase in memory usage, and in file space when saving the spreadsheet. New test InsertTest demonstrates the problem by populating a worksheet with cells A1:D5 (so highestDataRow is 5), but also setting row 1000 to invisible (so highestRow is 1000).
The major part of the change is in ReferenceHelper::insertNewBefore, which will now use getHighestData... for its boundaries when filling in the missing cells. Changes of less impact are made to duplicateStylesByColumn and duplicateStylesByRow so that cells which don't yet exist are not created unless the style that will be applied is not the workbook default style.
As for reducing the file size, Writer/Xlsx/Worksheet is changed so that cells whose value is null or null-string and which use the workbook default style are not written to the output spreadsheet. This requires some changes to existing test ReadBlankCellsTest; I don't think the difference should matter to the end-user.
oleibman
Adrien Crivelli