Fix#4249. Technically speaking, only the 1.29 branch needs fixing, and only for TEXT. It was fixed for the other branches by PR #3898. However, in adding test cases for the fix, it became apparent that PhpSpreadsheet's parsing in TIMEVALUE (which is called from TEXT in the original issue) did not really match Excel's. There are probably still edge cases where it doesn't, but, in the absence of a spec for how it operates, this will do for now.
We do not usually backport fixes from the master branch. Because this is more of a forward port from the earlier branch, there is an equivalent PR for each active branch.
These are 5 closely related functions for manipulating arrays. Now that dynamic arrays are part of PhpSpreadsheet, this PR implements those previously-unimplemented functions. Documentation undergoes a very minor change, since they are re-categorized as "Lookup and Reference", which is how Excel classifies them, rather than "Math and Trig".
Fix#4213. Early versions of Php 8.4 caused problems for some Excel functions, among them ROUNDDOWN and ROUNDUP. New code was added which seemed to work for 8.4 and all prior releases (back to 7.4). However, as the issue shows, there is a problem with the new logic that had not been a problem for prior releases (PhpSpreadsheet 1.29.1 with Php 8.3 or earlier).
As it happens, some time after 8.4 broke the existing functionality, new features were added to it which enabled much cleaner logic. The code will now take this cleaner path when available, and revert to the older logic when not. This appears to work for all relevant Php releases, at least till the next non-match between PhpSpreadsheet and Excel is reported.
And, having made the change to ROUNDDOWN, it occurred to me that ROUNDDOWN always returns the same result as TRUNC (they differ only in number of required arguments). So I changed TRUNC to call ROUNDDOWN, and that didn't break anything. So I will keep that change. This probably means I should change Calculation so that ROUNDDOWN is called automatically, and deprecate TRUNC. That can happen at a later time.
Fix#4113. TRUNC isn't always producing the expected result. There was a promising algorithm at https://stackoverflow.com/questions/4668628/truncate-float-numbers-with-php from user Juan. It works through Php8.3, but failed in Php8.4 (more on this later). User Savageman on the same page has a solution that needs work, but, once the work had taken place, it works on Php8.1-8.4.
The ROUNDUP and ROUNDDOWN functions were adversely affected by Php8.4, probably for the same reasons as Juan's TRUNC suggestion. I put a kludge in place for them some time ago, but I wasn't happy with it. The solution used for TRUNC here suggested a change to the ROUNDUP and ROUNDDOWN code that would no longer require the kludge. The change to those functions now works more cleanly on Php8.1-8.4.
Suggested by PR #848 from @markkimsal. The RATE calculation had already been corrected, so that part of the PR was unnecessary, however one of the tests included a floating point value for Number of Periods, which Excel permits. PhpSpreadsheet till now expected that parameter to be an integer. This is trivially changed with some tests added.
PR #4073 adds parsing of formulas for `$cell->setValue()`, but the spill operator won't parse correctly, breaking some of this PR. Add code to avoid that problem.
Fix#64 (really!), closed as stale in December 2017, another in our "better late than never" series. Excel's INDEX function doesn't really behave quite as described. If a single row is used as an argument, either in literal form `{item1, item2, item3}` or expressed as a range `A1:A6`, INDEX is happy to evaluate the array as if each entry were a row rather than a single item. PhpSpreadsheet is changed to do likewise.
INDEX also returned `#REF!` when it would normally return an array (which would often be reduced to its leftmost topmost entry later). This code is deleted, invalidating one existing test, and INDEX will now operate like other functions which can return arrays.
The CONCATENATE function has been treated as equivalent to CONCAT. This is not how it is treated in Excel; it is closer to (and probably identical to) the ampersand concatenate operator. The difference manifests itself when any of the arguments is an array (typically a cell range). Code is added to support this difference.
Support for array results is added to Csv Writer, Html Writer, and Ods Reader and Writer. I have not figured out how to get it to work with Xls.
Investigating issue #1622, I found that its extremely complicated formula, which had been leading to an incorrect result, now led to an exception. I am able to fix the exception; unfortunately, I am no closer to resolving the original issue. So I'll apply the baby step while continuing to investigate. Function POWER had changed from untyped args to a defined set of types. The set of types was determined according to the doc block, but that was incomplete - it had neglected to include `null` and `bool`. This PR corrects the function prototype and the doc block, and adds the missing tests for those conditions.
Excel supports the following notations for cell styles:
- `[$-F800]` and `[$-x-sysdate]` will format the date according to what appears to be the user's system long date format.
- `[$-F400]` and `[$-x-systime]`, will format the time according to what appears to be the user's system long time format.
- Builtin style 14 will format date according to what appears to be the user's system short date format.
- Builtin style 22 will format date and time according to the user's preference. It appears that the date portion is formatted according to the user's system short date format, but the time portion is formatted according to a format which is neither system short time format nor system long time format, so I'm not sure how this preference is set.
For F800, sysdate, F400, and systime, any other characters in the style are ignored, except that, if you have more than one of this type of block in the style, Excel will treat it as corrupt (error message on open and style changed to General).
Support is added for the new codes. In addition, note that the value displayed in the cell may differ in different environments. To give the PhpSpreadsheet programmer an opportunity to emulate what the intended audience will most often see, properties `shortDateFormat` (default value is builtin 14), `longDateFormat` (default value is `dddd, mmmm d, yyyy`), `dateTimeFormat` (defaults to builtin 22), and `timeFormat` (default is `FORMAT_DATE_TIME2`), with corresponding setters and getters, are added to Style/NumberFormat. Note that, if these properties are set to some other value in PhpSpreadsheet, it will not affect the values in the cell or the style - it is merely a convenience for the programmer. It will, however, affect column width if autosize is specified for the column. If the programmer does not alter any of the new properties, the output should be unchanged from before for builtins 14 and 22.
The new styles are also recognized by the `TEXT` function. In this case, the cell's calculated value may differ from user to user.
Note that this is a small subset of adding locale information to styles. No attempt is made to support any of the other possibilities - locale data will continue to be passed through to the spreadsheet, but PhpSpreadsheet will discard it before attempting to generate the formatted value of a cell.
Fix#3819. Excel can add these prefixes (basically invisible to end-user). Formula translation in PhpSpreadsheet fails when dealing with these unexpected prefixes, and, even if it handled it correctly, the unexpected prefixes confuse the users. I have changed to strip those prefixes when translating to a locale. This is probably not perfect, but is almost certainly good enough. I could easily add the same change when translating from a locale to English, but I don't think there's a good use case for that, so am opting not to do so for now.
The documentation mentions `translateFormulaToLocale` and `translateFormulaToEnglish`. Neither of these exist; both names are preceded by an underscore. I have changed the code to match the documentation rather than vice versa, retaining deprecated versions of the underscored routines which merely invoke the non-underscored routines.
Fix#3782. A signature of the ReflectionMethod constructor will be deprecated, and PhpSpreadsheet runs afoul of that change in one place. Php8.4 is not yet available in any form, and I am reluctant to make this change until we see that the issue is real (and that this PR fixes it), so am leaving this PR in draft status till then.
I note that one Excel function `PI` is implemented not as a class method in PhpSpreadsheet, but rather as a call to the native Php function `pi`. The ReflectionMethod call is subject to a TypeError in the changed code, but that is already the case. We haven't seen a TypeError because (a) it will arise only if the caller supplies an argument to the function (which must be called with zero arguments), and (b) there are no test cases for that function. The code is slightly cleaned up, and test cases are now added. This is not an important enough problem to rush this PR - the existing code (and the changed code), rather than failing with TypeError, will fail with a CalculationException (wrong number of arguments) before it gets to the TypeError; that is the correct behavior.
While we might never be able to have 100% of our code strict, we can at
the very least do it for all of our tests. This ensures that our tests
are using our API with the types as intended by the test author, and not
silently be cast to what our API requires.
* Fixed inconsistent string handling in SUM() implementations
* Use assertEqual instead of assertSame to allow ints and floats with the
same value
* Removewd special handling of empty strings and use default casts
* Eliminate Phpstan errors
* More Test Cases
* More New Tests
---------
Co-authored-by: oleibman <10341515+oleibman@users.noreply.github.com>
PR #3513, developed by @SaidkhojaIftikhor, has been stuck for some time awaiting tests. This is the first of three PRs to replace that one. This accomodates the use of slash as a delimiter in functions TEXTAFTER, TEXTBEFORE, TEXTSPLIT, and NUMBERVALUE. The source changes are very simple. Additional tests exercise all the source changes.
* Changes to NUMBERVALUE, VALUE, DATEVALUE, TIMEVALUE
Fix#3574. Reporter received deprecation notice for NUMBERVALUE function with invalid arguments. In fact, the arguments turn out to be valid after all; NUMBERVALUE treats a null-string or an all-blank-string in the first argument as if it were 0. Fixed this, and added several test cases suggested by it.
VALUE had been parsing its argument the same way as NUMBERVALUE. However, VALUE does not substitute 0 for null-string or all-blank-string. Coded up the difference between the two, and added the same tests for VALUE as for NUMBERVALUE.
VALUE can also pass its argument to DATEVALUE or TIMEVALUE. It is currently over-permissive about that, because Php is over-permissive, e.g. `new DateTime('q')` will return a DateTime object with the current date and time with a timezone of 'q'. Excel will, naturally, return `#VALUE!` for `DATEVALUE('q')`. I don't know that we can ever match Excel's (AFAIK not formally documented) decisions here 100%, but we can get a lot closer by parsing the date string if and only if it contains at least one digit. Code to enforce that is added to DATEVALUE and TIMEVALUE, and appropriate tests are added.
* Failure for Php 7.4 Linux
After not seeing any such problem for many months, this is the second day in a row where result is different on Windows than Linux with no apparent reason to think why that should be the case. At any rate, easily solved.
Fix#3561. PhpSpreadsheet VLOOKUP/HLOOKUP allow the specification of index_number (third parameter) as an array when lookup_value (first parameter) is supplied as an array. Change to permit index_number as an array even when lookup_value is not an array, as Excel does.
Also enforce more rigorous Excel Function implementation by testing the underlying implementation, call via the Calc Engine, and execution from in a worksheet.
Separate out unhappy path (exception) checks into a separate test, so that a single test isn't made overcomplex checking for every potentiality.
Scrutinizer may dislike variadics, for variable number of arguments of mixed type; but tough. It's 100% valid PHP, accepted by phpstan, and makes life a lot easier.
Initial work here covers all the database and datetime unit tests for Excel function implementations.
* Additional Unit Tests for XIRR and XNPV
See discussion in issue #3297. Excel result disagreed with PhpSpreadsheet, but it seems apparent that the error is Excel's. Although we have other test cases where Excel and PhpSpreadsheet disagree, those have so far been cases where Excel cannot give an answer because its algorithm does not converge, which is possible for any algorithm. In the issue in question, Excel seems to converge to a different answer; this is different than the other exceptions, and so deserves documentation with a formal test case. The discussion also mentions some cases where XNPV seems to give the wrong result in Excel, and those cases are added for XNPV.
* Scrutinizer
Whatever.
* Minor Fix for AND/OR/XOR
These 3 fall into the set of functions where Excel treats string literals differently depending on whether they are passed to the function directly or as a cell reference. PhpSpreadsheet is updated to try to duplicate that logic. New tests are added. Some existing test results had to change as a result of this code change.
* Adopt A Suggestion From Mark Baker
Reduce if statements by adding functions.
* Eliminate Shared\JAMA
The code under `Shared\JAMA` is called from exactly 3 places in Calculation, and exactly 1 other place (see next paragraph). These places are inadequately covered in the test suite, so it is not clear that the existing code works. In addition, see PR #2964 for commentary decrying the continued used of JAMA. I will also note that it has a pitiful 8.20% coverage in the test suite. There seems to already be a perfectly adequate equivalent in Calculation for those parts of JAMA which are used (e.g. we don't use any decompositions, so the fact that no decomposition code is present in Calculation is not a problem). This PR replaces the uses of JAMA within Calculation with `checkMatrixOperands`, and deletes Shared/JAMA entirely (which, among other thing, makes many Phpstan reported problems go away). The test suite is enhanced to ensure coverage of all the changed statements. A side benefit of deleting Shared/Jama is that the only function which PhpSpreadsheet has added to the global namespace (hypo in Maths.php) goes away.
There is one additional use in Shared/Trend/PolynomialBestFit. That use is best replaced with Matrix/Matrix, which is already a requirement for PhpSpreadsheet. PolynomialBestFit has zero test coverage, and I didn't add any for this change. There seem to be existing errors which both Phpstan and Scrutinizer complain about, and I am quite convinced that they are not false positives. Fixing those problems will be a project for another day.
Calculation has many calls to `Information\ErrorValue` and `Information\ExcelError`. It is inconsistent in how it does so - most invoke `Information\E...` but a small number take advantage of additional `use` statements to just invoke `E...`. I have made the usage consistent by changing the small number to act like the majority and eliminating the `use` statements.
Some of the test cases exposed the fact that MAX, MAXA, MIN, and MINA do not properly handle error strings in their input. For example, if cell A1 contains 2, and A2 contains `=5/0`, `=MAX(A1, A2)` should return `#DIV/0!`. They are changed to handle them properly.
`Shared\JAMA` was normally omitted from code coverage. Since it is being deleted, there is no longer a reason to explicitly exclude it. `Writer\PDF` was also on the exclude list, probably for historical reasons, but there is no reason to exclude it now (it is, in fact, 100% covered), so it will no longer be excluded.
* Scrutinizer
Eliminate some newly dead code.
Fix#689. XIRR is calculated by making guesses which are hopefully better with each iteration. It is not guaranteed to succeed for Excel, PhpSpreadsheet, or any other implementation. PhpSpreadsheet uses the Newton-Raphson method for its guesses. So does Python package xirr (https://github.com/tarioch/xirr/), but, if Newton-Raphson fails to converge, Python tries Brent's method as an alternative. Two sets of non-converging data are noted in 689. For both, a solution does converge in Excel. For the first of the problems, a solution converges in Python with Newton-Raphson; but, for the second, a solution converges which requires Brent. For the Java package https://github.com/RayDeCampo/java-xirr on which Python was based, and which uses only Newton-Raphson, a solution converges for the first, and does not converge for the second.
To try to match the good results of the others, I added an alternate algorithm if Newton-Raphson fails. Brent's algorithm seems difficult to implement to me. I might have gone there regardless, but I first tried a slightly simpler alternative, bisection. This solved the problem for both of the cases in 689. Perhaps someone will one day report a problem that doesn't converge for Newton-Raphson or bisection, but does for Brent. We can review this decision then.
The new code causes 3 changes in the unit test. In all 3 tests, Excel and PhpSpreadsheet had not converged, but Python and/or Java had. I now believe that Python/Java is correct in those cases, and Excel is not. The new code aligns PhpSpreadsheet with Python/Java for those tests. It is, of course, impossible to know when Excel's implementation doesn't converge, so we aren't guaranteed to match its results in those hopefully rare situations.
* Change Additional Statistical Tests to Use Spreadsheet Context
With an earlier change, I made all but 18 Statistical tests run in spreadsheet context. This PR changes 12 of those 18. The remaining 6 usually return array results, so it is a tougher task to handle them. I will continue to think on it.
AVERAGEIF, AVERAGEIFS, and COUNTBLANK are changed to throw an Exception when a range is specified as a literal. They previously accepted array (enclosed in braces) literals, and bumbled along till they threw an error for non-array literals. Throwing an exception appears to be analogous to how Excel operates, rather than something more friendly like a VALUE error. There may be other functions which require similar treatment.
There also remains a TODO for COUNTIFS, and possibly other functions. It appears that PhpSpreadsheet counts booleans for both integer and string compares and probably shouldn't. Again, this is a problem for another day.
* Scrutinizer
Fix one problem.
* Scrutinizer Ignores Its Own Suggested Remedy
Try another approach.
* Fix Unintential Deprecated Calls in Tests - STATISTICAL
I think it's best to install these before PR #3166. There are no changes to source code, only to doc-blocks and to test members which continue to inadvertently use calls to deprecated functions.
* Missed One Deprecation
Fix it now.
* Run Tests in Spreadsheet Context
This is quite a bit more difficult for Statistical than for the other Calculation categories. This is partly because of the use of multi-dimensional matrices, and also because some arguments are interpreted differently when they come from a cell rather than entered directly in a formula. This push leaves 18 out of 89 test members unchanged, except that they are marked with a TODO to show that the work isn't finished. I will not revisit them as part of this PR, but probably will take a look in a subsequent ticket.
* Fix Unintential Deprecated Calls in Tests - FINANCIAL
I think it's best to install these before PR #3166. There are no changes to source code, only to doc-blocks and to test members which continue to inadvertently use calls to deprecated functions.
* Change Tests to Run in Spreadsheet Context
Found and fixed some problems with how MIRR handles errors.
* Fix Unintential Deprecated Calls in Tests - ENGINEERING
I think it's best to install these before PR #3166. There are no changes to source code, only to test members which continue to inadvertently use calls to deprecated functions.
* Fix deprecation DocBlocks
Deprecated->deprecated, adjust see and comments
* Fix Deliberate Deprecated Tests
Add annotations.
* Run Unit Tests in Spreadsheet Context
This turned up only one error, in IMSUB. The only group that still needs this is Statistical. Not sure if I will get to that quickly.
* Fix Unintential Deprecated Calls in Tests - LOGICAL
I think it's best to install these before PR #3166. There are no changes to source code, only to doc-blocks and to test members which continue to inadvertently use calls to deprecated functions.
* Change Unit Tests to Run in Spreadsheet Context
They had been run as direct calls, which is not how most users would use them. Making this change exposed some minor coding errors - SWITCH needs to flatten its arguments, and IFERROR and IFNA were not handling a null testValue in the same manner as Excel.
* Fix Unintential Deprecated Calls in Tests - DATABASE
I think it's best to install these before PR #3166. There are no changes to source code, only to test members which continue to inadvertently use calls to deprecated functions.
* Fix deprecation Blocks
Deprecated->deprecated, adjust see and comments
* Fix Deliberate Deprecated Tests
Add annotations.
* Change Unit Tests to Run in Spreadsheet Context
... rather than as direct calls. The major difference is that specifying an invalid column should result in an Excel error, not null. Minor code changes were needed, including to Statistical/Conditional which sometimes calls Database.
* Correct Some DocBlocks
Null is no longer a possible output for most of these functions.
* 2 Overlooked Tests
Change to run in spreadsheet context.
* T Function Should Return Null-String, not Null
Fix it.
Allow default values for TEXTJOIN() delimiter and ignore_empty... even though the official MS documentation lists them as required arguments, they are actually optional
* MATCH Problems with Int/Float Compare and Wildcards
Fix#3141. Function matchSmallestValue did not recognize that an integer could match a float. Adding test cases, it seems that matchFirstValue had the same problem. However, matchLargestValue seemed to handle things correctly - but see below.
In addition, the wildcard logic in matchFirstValue is faulty. It ignored tilde as a wildcard character. Although it would have been easy to just add that, I think it was wrong to determine on its own if a wildcard was in use. Just using the already available wildcard functions whenever comparing two strings is sufficient.
I note that Excel doesn't seem to follow its own rules for MATCH (https://support.microsoft.com/en-us/office/match-function-e8dffd45-c762-47d6-bf89-533f4a37673a?ns=excel&version=90&syslcid=1033&uilcid=1033&appver=zxl900&helpid=xlmain11.chm60112&ui=en-us&rs=en-us&ad=us). PhpSpreadsheet's results match Excel's, so no problem. However, when match_type is not zero, the match array is supposed to be sorted, so I would expect `#N/A` when it isn't; but that's not how Excel operates. I have no idea what Excel is doing. If `MATCH(2,{2,0,4,3},1)` isn't `#N/A` because of the unsorted array, then surely it should be `1` (item 1 of the array is the largest number less than or equal to the lookup value); but Excel and PhpSpreadsheet (before and after changes) return `2`. I have moved this example to be the first of the test cases.
One would think strings would behave similarly. But, no - see the second test case. This time Excel does look for an exact match. But the existing logic doesn't get the matching result in PhpSpreadsheet. It requires a whole new block of code, one which doesn't work correctly for numeric lookup value. Ugh.
LibreOffice doesn't always agree with Excel. It seems that it will use wildcard matching even when the match type is not zero (Excel documentation says wildcards are only for type zero, which is just as well because I don't really know what greater/less mean when wildcards are involved). I have not attempted to duplicate this behavior. For the record, Gnumeric agrees with Excel here.
* More Changes - LibreOffice
Add support for LibreOffice matching wildcard strings when type is not zero. Add support for type to be specified as integer other than 0/1/-1, or as float, or as numeric string; non-numeric string should case `#VALUE!` error.
I have found an example of undefined behavior (unsorted array where type is non-zero) where PhpSpreadsheet does not produce the same result as Excel. It is present as a new `incomplete` test case. I can fix it, but not without breaking other tests where the proper behavior is undefined. IMO, this is not a problem we should be concerned about.
Many test cases are added. Chances are I will add some more before merging this change.
Scrutinizer had previously suggested annotations for 3 LookupRef tests, but it no longer accepts its own annotation for those cases. This PR cleans them up. ColumnsTest and RowsTest are extremely straightforward.
IndexTest is a bit more complicated, but only because, unlike the other two, it had no test which executed in the context of a spreadsheet. And, when I added those, I discovered a couple of bugs. INDEX always requires at least 2 parameters (row# is always required), but its entry in the function table specified 1-4 parameters, now changed to 2-4. And, omitting col# is not handled the same way as specifying 0 for col#, though the code had treated them identically. (The same would have been true for row# but, because it is now required, ...)
oleibman
Adrien Crivelli
Jacek
MarkBaker