Fix#2389. Hyperlinks referring to cells in the spreadsheet itself are not being handled properly. This is the first namespacing regression identified for release 19. Usual cause and fix - need to take greater care with attributes than was previously the case.
* Support Data Validations in More Versions of Excel
Attempt to deal with #2368, this time for good. Some deleted code was accidentally restored just before release 19, causing errors in spreadsheets with Data Validations. PR #2369 removed the duplicated code, and the fix was confirmed in current versions of Excel for Windows, Google sheets, and other versions of Excel. However, there were problems reported in earlier version of Excel for Windows, and some, versions of Excel for Mac, not all but including a recent one. This change, which is simpler than the original (no need for extLst) fix for DataValidations, is tested with Excel 2007 and Excel 2003 as well as more recent versions. I do not have a Mac on which to test.
* Multiple Identical Data Validation Lists
Using the same Data Validation List in multiple places on a worksheet caused them all to be merged into the same range. This was because sqref was not part of the hash code; it is now, avoiding this problem.
* Must Write Data Validations Before Hyperlinks
See discussion in #2389.
* ZipArchive and "Inconsistent" Zip File
Fix#2362. I added test for zip file inconsistency when dealing with a particularly nasty PHP/libzip bug affecting zero-length files. However, we also now verify that the file starts with a valid zip signature, so the consistency test is not really needed, and, from what I've read on the web, isn't particularly useful. The file with a problem, for example, opens just fine with Excel and zip, despite Php reporting it as inconsistent (when asked to check consistency). So, remove the consistency check.
* Update Issue2362Test.php
Latest Phpstan does not allow cast from 'mixed' to 'string'.
* Update Issue2362Test.php
See the discussion in PR #2232 which came about 3 months after it was merged. It caused a problem in an unusual situation which did not come to light until the change was part of the new release version. The original PR changed PhpSpreadsheet's behavior to match Excel's for (not case sensitive) strings `TRUE` and `FALSE`. Excel treats the values as boolean, and now so does PhpSpreadsheet.
When StringValueBinder is used, this becomes a tricky situation. The user wants the original strings preserved, including the case of all the letters. This PR changes the behavior of CSV reader as follows:
- If StringValueBinder is not in effect, convert to boolean.
- If StringValueBinder (actually any binder with method getBooleanConversion) is in effect, and the result of getBooleanConversion is true (which is the default in StringValueBinder), leave the value coming out of Csv Reader as the unchanged string.
- Otherwise, convert to boolean.
This should mean that there are no regression problems with StringValueBinder, while allowing PhpSpreadsheet to continue to match Excel in the default situation. No new settings are required.
* Update Some Doc Block Annotations
See PR #2010. That PR was never completed, and has gone stale. However, it was correct in identifying a situation where the doc block was not entirely accurate. It did not go far enough - several closely-related methods have similar problems. This PR attempts to fix the original problem and its close relations. Aside from the doc block changes, there are very minor changes to executable code. It also changes some of the unit tests targeted at the methods in question to eliminate mocking in favor of 'real' tests.
* Change Method to Static
Otherwise Scrutinizer will complain, even though Phpstan doesn't.
* Scrutinizer
Various clean-up activities.
* Scrutinizer
@#&$(*#&$ Got complexity down from 53 to (I think) 50. Don't really know what the target is.
* Code Changes Suggested By Review
Some improvements suggested in review by @PowerKiKi.
* Update Cells.php
* Merge Conflict
A change to a parameter name caused several problems when trying to fix it on Github. Fixing it locally should do the trick.
* Merge Conflicts in Phpstan Baseline
PR #2382 made a large number of changes to Phpstan Baseline, some of which conflicted with the Phpstan Baseline changes in this PR. This should resolve them all.
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.
Example: right shift shared formula: IF(A$1=0,0,A1/A$1)
Expected value: IF(B$1=0,0,B1/B$1)
Actual value: IF(B$1=0,0,A1/B$1)
Similar behavior is observed when copying formulas vertically.
This issue occurs because a fixed and a non-fixed cell hit the same element of the $newCellTokens array by index $cellIndex
* Update Doc Blocks to Discourage Use of Unix Timestamps
This was suggested by issue #2347. Unix timestamps have clear disadvantages compared with the alternate methods of supplying date and time to PhpSpreadsheet - DateTime objects, Excel date time fields, and strings. In particular, Unix timestamp is not Y2038-safe on a 32-bit system, and it reflects the time in UTC, which may come as a surprise to the end-user (as it did in the cited issue). The alternate methods do not come with such baggage. This change updates some doc blocks to note that Unix timestamps are discoburage (N.B. - not deprecated). No executable code is changed.
* Document in Code As Well as Commmit Message
Per suggestion from @PowerKiKi.
* Missed One DocBlock
Including it now.
Co-authored-by: Adrien Crivelli <adrien.crivelli@gmail.com>
Fix issue #2368. PR #2265 moved the place where data validations were written to the worksheet. PR #1694 was installed afterwards, and accidentally restored the original location, so validations are now being written twice.
See issue #2315. It is nominally solved by PR #2312, but that PR is completely unsuitable for merging. This one-line change is a replacement for that PR.
As with many problems of this type, it is not clear how how to create a spreadsheet with this sort of harmless corruption in the wild. An example was supplied with the issue, and I have tested manually against it. However, the file is huge and not suitable for a formal unit test. I do not understand BIFF well enough to try and craft a suitable example on my own.
Co-authored-by: Adrien Crivelli <adrien.crivelli@gmail.com>
PR #1844 fixes it, but changes were requested. It has been almost 3 months and those changes have not been made. This PR replaces that one; it should be suitable for all supported releases of PHP through 8.1, and includes a formal unit test.
Fixes#1685Closes#1844
Prevent calling clone and getHashCode when not needed
because these calls are very expensive.
When applying styles to a range of cells can we cache the
styles we encounter along the way so we don't need to look
them up with getHashCode later.
See issue #2331. Timestamp is expected in format yyyy-mm-dd (plus other information), with the expectation that month and day are 2 digits zero-filled on the left if needed. The user's file instead used a space rather than zero as filler. Although I don't know how the unexpected timestamp was created, it was easy enough to alter the timestamp in an otherwise normal spreadsheet, and use that file as a test case.
See issue #2123. HLOOKUP needs to do some conversions between column numbers and letters which it had not been doing.
HLOOKUP tests were performed using direct calls to the function in question rather than in the context of a spreadsheet. This contributed to keeping this error obscured even though there were, in theory, sufficient test cases. The tests are changed to perform in spreadsheet context. For the most part, the test cases are unchanged. One of the expected results was wrong; it has been changed, and a new case added to cover the case it was supposed to be testing.
After getting the HLOOKUP tests in order, it turned out that a test using literal arrays which had been succeeding now failed. The array constructed by the literals are considerably different than those constructed using spreadsheet cells; additional code was added to handle this situation.
This change was suggested by issue #2316. There was a problem reading Xlsx comments which appeared with release 18.0 but which was already fixed in master. So no source change was needed to fix the issue, but I thought we should at least add the test case to our unit tests.
In developing that case, I discovered that, although comment text was read correctly, there was a problem with comment author. In fact, there were two problems. One was new, with the namespacing changes - as in several other cases, the namespaced attribute `authorId` needed some special handling. However, another problem was much older - the code was checking `!empty($comment['authorId'])`, eliminating consideration of authorId=0, and should instead have been checking `isset`. Both problems are now fixed, and tested.
My bulletproofing of these tests was not yet sufficient. Although I have never had a failure in probably thousands of tests, one user submitted a PR which did fail testing NOW, fortunately not in a test that is required to pass. The problem is that it is not sufficient merely to set the cell value inside a do-while loop; it is necessary to calculate it in order to cache its result so that results based on that cell will be internally consistent.
No source code is changed for this PR, just some tests.
* isFormula Referencing Sheet With Space in Title
See issue #2304. User sets a cell to `ISFORMULA(cell)`, where `cell` exists on a sheet whose title contains a space, and receives an error. Coordinates are not being passed correctly to Functions::isFormula; in particular, the sheet name is not enclosed in apostrophes, e.g. `Sheet Name!A1` rather than `'Sheet Name'!A1`. (Note that sheet name was not specified in Cell; PhpSpreadsheet adds it before calling isFormula.) Sheets without embedded spaces (or other non-word characters) are handled correctly with or without apostrophes, but spaces require surrounding apostrophes.
As part of this investigation, I determined that Excel handles defined names and cell ranges in ISFORMULA (subject to spills), and that PhpSpreadsheet does not. It is changed to handle them. In the absence of spill support, it will use only the first cell in the range.
Existing tests for ISFORMULA used mocking unneccesarily. They are moved to a separate test member, and mocking is no longer used.
* PhpUnit and Jpgraph
35_Char_render.php had previously been a problem only for PHP8+. It is now a problem for PHP7.4, and will therefore be skipped all the time.
* Xlsx Reader Better Namespace Handling Phase 1 Second Bugfix
See issue #2301. The main problem in that issue had been introduced with 18.0 and had already ben fixed in master. However there was a subsequent problem that had been introduced in master, an undotted i uncrossed t with namespace handling. When using namespaces, need to call attributes() to access the attributes before trying to access them directly. Failure to do so in parseRichText caused fonts declared in Rich Text elements to be ignored.
* Add An Assertion
Addresses problem in 2301 that had already been fixed.
* Permit CSV Delimiter to be Set to Null
See issue #2287. A 1-character change. The delimiter variable is defined as nullable, and getDelimiter can return null; setDelimiter should follow suit.
* Scrutinizer Inanity
Are you sure the test always returns null?????
Yes, I'm sure, that's why it's part of the test.
Let's see if we can recode it and miss this "problem".
Fixes issue #2266. Writer/Xlsx fails when there is no longer a sheet which corresponds to the definition of a local defined name. The code is changed to drop such an orphaned name. Writer/Xls does not fail under the same cicrcumstances, so no correction is needed there. Writer/Ods fails in a different manner, and is corrected to no longer do so.
oleibman
Adrien Crivelli
dependabot[bot]
Zaytcev Ivan
Roland Eigelsreiter
Einar
Richard van Velzen
jomisacu
Sergiy Petrov
ayacoo
Georgiy