Add some additional support for Intersection and Union in the Calculation Engine. This allows me to reinstate 2 tests which were formerly skipped, without breaking any existing tests. There are almost certainly edge cases which I haven't thought of yet; I will leave this PR as a draft for several weeks before moving it formward.
This change gives some opportunities for users to go wrong. If you place the following formula in a cell:
```
=B1:B8 A7:D7
```
Excel will treat the space as an intersection operator, and will return the value in B7, which is where the ranges intersect. PhpSpreadsheet will do so as well. But, if you use the following formula:
```
=B1:B8,A7:D7
```
Excel will return `#VALUE!`. This seems like something they forgot to take care of when adding dynamic arrays. The comma should be interpreted as a union operator, and PhpSpreadsheet will now return the union of the ranges. It seems very difficult for PhpSpreadsheet to return an error when the formula seems easily evaluated. Furthermore, if you use the following formula:
```
=SUM(B1:B8,A7:D7)
```
Excel evaluates it as you would expect, summing the union of the ranges. So does PhpSpreadsheet. I guess there's no rule requiring Excel to be consistent, but ...
Another way that users might go wrong is by actually entering the union or intersection symbols in a formula rather than comma or space. Excel will not allow this; PhpSpreadsheet needs to change comma or space to the appropriate symbol in order for the rest of this PR to work properly. By the time the parser gets to it, it can't tell whether the symbol was part of the cell's original formula or if PhpSpreadsheet substituted it, so it just has to permit it. I do not expect many users to fall afoul of this problem, at least not more than once.
Just by way of explanation, the reason why PhpSpreadsheet has to make the substitution is because intersection has a higher priority than union, just as multiplication has a higher priority than addition. So, if you don't change the symbols beforehand, you may wind up figuring out that you need to perform an intersection too late - a lower-priority union may have already taken place.
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.
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.
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.
* fix: Allow to read data from a table located in a different sheet
* Add Test Case, Correct Calculation
Calculate correct result, at least for calculations with a single answer. For an array of answers, result will be similar to other functions which return an array (dynamic arrays not yet fully supported).
* Rename test class
* Allow Appropriate Use of Table as DefinedName in Calculations
See Issue3569Test, which behaves like Excel.
* Remove Dead Code
Scrutinizer is correct here.
* Memory Leak
Worksheet points to Table, Table points back to Worksheet. Circular reference prevents garbage collection. Remove Table collection in Worksheet at destruct time to avoid this problem.
* PhpUnit10 Failure
Avoiding memory leak triggered segfault in Phpunit 10. Research and fix later.
* Add toString to StructuredReference
Absence of such a method seems to cause problems for untaken IF branches in Calculation.
* Remove Dead Assignments
Correctly detected by Scrutinizer.
---------
Co-authored-by: Kevin Verschaeve <kevin.verschaeve@exotec.com>
Co-authored-by: oleibman <10341515+oleibman@users.noreply.github.com>
oleibman
Adrien Crivelli
Kevin Verschaeve