It requires an index to an array which needs to be supplied as a string, even though the array is indexed exclusively by int. It is called only once in the code base, by some inefficient code which becomes much simpler when restructured and eliminates the call altogether. It is not independently tested. The use case for its existence is not clear. It should probably just be deprecated. However, for now, I'm just allowing the parameter to be int|string, rather than just string, and adding tests.
Php8.5 *may* deprecate the use of `__wakeup` (it is planned, but not yet implemented, and it is convtroversial). We use it only twice. In the first instance, it just throws an exception to prevent a security exploit. This can be trivially replaced with `__unserialize`.
The other instance is merely to initialize a Worksheet instance variable. Converting this use to `__unserialize` is *not* trivial (one of the reasons for the controversy). However, I see no useful purpose for that variable. Since it has no use, there is no real need for `__wakeup`, so we will just remove the routine altogether, and deprecate the variable's getter (there is no setter).
A similar instance variable in Spreadsheet also serves no useful purpose, so we will deprecate its getter as well (again no setter).
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.
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.
oleibman