Handling Unions as Function Arguments#4657
Merged
oleibman merged 14 commits intoPHPOffice:masterfrom Jan 22, 2026
Merged
Conversation
Fix PHPOffice#4656. (Also fix PHPOffice#503, which went stale years ago, and which I reopened, and which I re-closed in a state of confusion.) Continuing the work of PR PHPOffice#4596. Calculation engine was unable to parse a formula which used union arguments. (I find it very difficult to parse as well.) This PR will, I hope, fix that. More tests are needed.
Collaborator
Author
|
Back to draft status. Example from #1950 does not work. |
Collaborator
Author
|
An argument in favor of this change is that it fixes issue #316 which went stale in 2018. |
See if we can identify where comma needs to be replaced by union operator.
Collaborator
Author
Collaborator
Author
|
I am not going to rush into merging this, but it now addresses several problems, does not cause problems for any existing unit tests, and I am unaware of further manifestations of this problem. The solution of using a regexp to identify cases where comma means union seems kludgey; then again, so do the problems. Removing from draft status. |
Unused in this project. I checked to see if they might be of use for this PR. They aren't.
oleibman
added a commit
to oleibman/PhpSpreadsheet
that referenced
this pull request
Mar 12, 2026
Fix PHPOffice#4832. PR PHPOffice#4657 added support for passing union arguments to functions. User reports a problem with a peculiar formula afterwards. See issue for details - the very loose restrictions placed on worksheet names can lead to an ambiguous situation for the parser. This problem arose with a function whose first argument was a cell address *including sheet name*. Parser is changed to try to avoid this situation. If the regular expression which tells us we have a potential need for union has a left parenthesis in the "sheet name" without a right parenthesis, then it will no longer try to treat the formula as containing union arguments except in the unlikely event that "sheet name" truly is a worksheet title in the spreadsheet. This feels pretty kludgey, but it solves the problem at hand, and seems unlikely to cause problems. There may still be edge cases more subtle than the one in the issue; I am satisfied to wait for reports of such.
11 tasks
acataluddi
pushed a commit
to acataluddi/PhpSpreadsheet
that referenced
this pull request
Mar 21, 2026
# This is the 1st commit message: Integrated unit test to capture the bug. # This is the commit message PHPOffice#2: Added Date.safeModify() method for secure DateTime modifications and integrated exception handling - Fixed PHPOffice#4840. # This is the commit message PHPOffice#3: PHPCS Fixes. # This is the commit message PHPOffice#4: PHPCS Fixes. # This is the commit message PHPOffice#5: Codestyle fix. # This is the commit message PHPOffice#6: Added description to CHANGELOG.md. # This is the commit message PHPOffice#7: Enhanced Date.safeModify() to ensure consistent exception handling across PHP versions, addressing changes in DateTime::modify() behavior in PHP 8.3 - PHPOffice#4840. # This is the commit message PHPOffice#8: Confusion Checking for Union Arguments Fix PHPOffice#4832. PR PHPOffice#4657 added support for passing union arguments to functions. User reports a problem with a peculiar formula afterwards. See issue for details - the very loose restrictions placed on worksheet names can lead to an ambiguous situation for the parser. This problem arose with a function whose first argument was a cell address *including sheet name*. Parser is changed to try to avoid this situation. If the regular expression which tells us we have a potential need for union has a left parenthesis in the "sheet name" without a right parenthesis, then it will no longer try to treat the formula as containing union arguments except in the unlikely event that "sheet name" truly is a worksheet title in the spreadsheet. This feels pretty kludgey, but it solves the problem at hand, and seems unlikely to cause problems. There may still be edge cases more subtle than the one in the issue; I am satisfied to wait for reports of such. # This is the commit message PHPOffice#9: Weird Regexp Difference between Php8.4+ and Php8.3- Specify codeCoverageIgnore for now, remove later. # This is the commit message PHPOffice#10: Move Test to More Sensible Location # This is the commit message PHPOffice#11: Ods Reader/Writer Support for Integer Styles with Leading Zero Fix PHPOffice#1606, which went stale and is now reopened. PR PHPOffice#4806 supplied support for some Number Formats for Ods, but did not directly address this issue. This PR does. In addition to the original issue, this could be useful for zip codes. As I stated in 4806, I may be amenable to adding some unsupported styles to the built-in list, but the custom style option will always be around in case I am being slow or unreasonable. # This is the commit message PHPOffice#12: A Bit More Flexibility And some more tests. # This is the commit message PHPOffice#13: Update CHANGELOG.md Xlsx Writer Support Data URI for Images Fix PHPOffice#4823. When writing a spreadsheet to Html, a data Uri can be used if `embedImages` is true. Reading such an Html spreadsheet and attempting to write it to Xlsx results in an Exception. It should be noted that Excel itself cannot open the Html properly; none of the images are present. The PhpSpreadsheet problem arises not with the inclusion of the image, but rather with attempting to include the appropriate entry in `[ContentTypes].xml`. This PR corrects that problem. For the record, Xls Writer does not have a problem with this situation. Just to demonstrate that, a parallel test for Xls Writer is added in addition to the new Xlsx Writer test. Update CHANGELOG.md Update CHANGELOG.md Added phpcs and php-cs-fixer corrections PHPOffice#4840. Updated DateTest to handle DateMalformedStringException for PHP 8.3 in excelToDateTimeObject exception tests. Fixed phpcs issues. Fixed php-cs-fixer issue. Updated DateTest so to not directly reference DateMalformedStringException class which is not existing prior to PHP 8.3. Refactored DateTest to dynamically set the expected exception class, ensuring compatibility across PHP versions. Removed unused Throwable import and updated expected exception type in DateTest for consistency with PHP version handling. Added DateMalformedStringException polyfill to solve the phpstan issue. Fixed phpcs issue.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix #4656. Fix #316. (Also fix #503, which went stale years ago, and which I reopened, and which I re-closed in a state of confusion.) Continuing the work of PR #4596. Calculation engine was unable to parse a formula which used union arguments. (I find it very difficult to parse as well.) This PR will, I hope, fix that. More tests are needed.
This is:
Checklist: