From b53a4b760246e07254a95103b6e11f1d6fb7619b Mon Sep 17 00:00:00 2001
From: oleibman <10341515+oleibman@users.noreply.github.com>
Date: Tue, 16 Apr 2024 19:45:13 -0700
Subject: [PATCH] Xlsx Reader Use Dimensions from Functions With Array Results
Thinking about #3958 - user wondered if unsupported formulas with array results could be handled better. I said that the answer was "no", but I think Xlsx Reader can make use of the dimensions of the result after all, so the answer is actually "sometimes". This is an initial attempt to do that. Implementing it revealed a bug in how Xlsx Reader handles array formula attributes, and that is now corrected. Likewise, Xlsx Writer did not indicate a value for the first cell in the array, and does now.
---
src/PhpSpreadsheet/Reader/Xlsx.php | 13 ++++--
src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php | 15 ++++++-
.../Writer/Xlsx/ArrayFunctionsTest.php | 40 +++++++++++++++++-
tests/data/Reader/XLSX/atsign.choosecols.xlsx | Bin 0 -> 9555 bytes
4 files changed, 61 insertions(+), 7 deletions(-)
create mode 100644 tests/data/Reader/XLSX/atsign.choosecols.xlsx
diff --git a/src/PhpSpreadsheet/Reader/Xlsx.php b/src/PhpSpreadsheet/Reader/Xlsx.php
index c8b2de2a9..2daa2dc07 100644
--- a/src/PhpSpreadsheet/Reader/Xlsx.php
+++ b/src/PhpSpreadsheet/Reader/Xlsx.php
@@ -891,9 +891,16 @@ class Xlsx extends BaseReader
} else {
// Formula
$this->castToFormula($c, $r, $cellDataType, $value, $calculatedValue, 'castToString');
- if (isset($c->f['t'])) {
- $attributes = $c->f['t'];
- $docSheet->getCell($r)->setFormulaAttributes(['t' => (string) $attributes]);
+ $formulaAttributes = [];
+ $attributes = $c->f->attributes();
+ if (isset($attributes['t'])) {
+ $formulaAttributes['t'] = (string) $attributes['t'];
+ }
+ if (isset($attributes['ref'])) {
+ $formulaAttributes['ref'] = (string) $attributes['ref'];
+ }
+ if (!empty($formulaAttributes)) {
+ $docSheet->getCell($r)->setFormulaAttributes($formulaAttributes);
}
}
diff --git a/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php b/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php
index 301d988ab..1b149c833 100644
--- a/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php
+++ b/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php
@@ -1401,8 +1401,8 @@ class Worksheet extends WriterPart
$calculatedValue = (int) $calculatedValue;
}
- $attributes = $cell->getFormulaAttributes();
- $ref = $cell->getCoordinate();
+ $attributes = $cell->getFormulaAttributes() ?? [];
+ $ref = array_key_exists('ref', $attributes) ? $attributes['ref'] : $cell->getCoordinate();
if (is_array($calculatedValue)) {
$attributes['t'] = 'array';
$rows = max(1, count($calculatedValue));
@@ -1424,6 +1424,17 @@ class Worksheet extends WriterPart
$objWriter->writeAttribute('ca', '1');
$objWriter->text(FunctionPrefix::addFunctionPrefixStripEquals($cellValue));
$objWriter->endElement();
+ $result = $calculatedValue;
+ while (is_array($result)) {
+ $result = array_shift($result);
+ }
+ if (
+ is_scalar($result)
+ && $this->getParentWriter()->getOffice2003Compatibility() === false
+ && $this->getParentWriter()->getPreCalculateFormulas()
+ ) {
+ $objWriter->writeElement('v', (string) $result);
+ }
} else {
$objWriter->writeElement('f', FunctionPrefix::addFunctionPrefixStripEquals($cellValue));
self::writeElementIf(
diff --git a/tests/PhpSpreadsheetTests/Writer/Xlsx/ArrayFunctionsTest.php b/tests/PhpSpreadsheetTests/Writer/Xlsx/ArrayFunctionsTest.php
index 39cc72ff7..fc26beb54 100644
--- a/tests/PhpSpreadsheetTests/Writer/Xlsx/ArrayFunctionsTest.php
+++ b/tests/PhpSpreadsheetTests/Writer/Xlsx/ArrayFunctionsTest.php
@@ -120,8 +120,44 @@ class ArrayFunctionsTest extends TestCase
if ($data === false) {
self::fail('Unable to read file');
} else {
- self::assertStringContainsString('_xlfn.UNIQUE(A1:A19)', $data, '15 results for UNIQUE');
- self::assertStringContainsString('_xlfn._xlws.SORT(A1:A19)', $data, '19 results for SORT');
+ self::assertStringContainsString('_xlfn.UNIQUE(A1:A19)41', $data, '15 results for UNIQUE');
+ self::assertStringContainsString('_xlfn._xlws.SORT(A1:A19)26', $data, '19 results for SORT');
+ }
+ }
+
+ public function testUnimplementedArrayOutput(): void
+ {
+ //Calculation::setArrayReturnType(Calculation::RETURN_ARRAY_AS_ARRAY); // not required for this test
+ $reader = new XlsxReader();
+ $spreadsheet = $reader->load('tests/data/Reader/XLSX/atsign.choosecols.xlsx');
+ $writer = new XlsxWriter($spreadsheet);
+ $this->outputFile = File::temporaryFilename();
+ $writer->save($this->outputFile);
+ $spreadsheet->disconnectWorksheets();
+
+ $reader = new XlsxReader();
+ $spreadsheet2 = $reader->load($this->outputFile);
+ $sheet2 = $spreadsheet2->getActiveSheet();
+ self::assertSame('=_xlfn.CHOOSECOLS(A1:C5,3,1)', $sheet2->getCell('F1')->getValue());
+ $expectedFG = [
+ ['11', '1'],
+ ['12', '2'],
+ ['13', '3'],
+ ['14', '4'],
+ ['15', '5'],
+ ];
+ $actualFG = $sheet2->rangeToArray('F1:G5');
+ self::assertSame($expectedFG, $actualFG);
+ $spreadsheet2->disconnectWorksheets();
+
+ $file = 'zip://';
+ $file .= $this->outputFile;
+ $file .= '#xl/worksheets/sheet1.xml';
+ $data = file_get_contents($file);
+ if ($data === false) {
+ self::fail('Unable to read file');
+ } else {
+ self::assertStringContainsString('_xlfn.CHOOSECOLS(A1:C5,3,1)11', $data);
}
}
}
diff --git a/tests/data/Reader/XLSX/atsign.choosecols.xlsx b/tests/data/Reader/XLSX/atsign.choosecols.xlsx
new file mode 100644
index 0000000000000000000000000000000000000000..45ea6ca2ae5a7613923705d5105b70c8a030a635
GIT binary patch
literal 9555
zcmeHt1y>yF()9!h9z1xEpuru22X}%8f-}Jx+=EL7OCV@)C%C)I;10pvB}kCL`6jvd
zeCM2;`~HG^d#&kSv$}V!?tW@l?W$@Od3Xdoz+(V1005u_NXRAXT08;(&=3ItTmUkh
zt~kin325tNpzdx5bkt*Uv$3YgM1Z4D2f)Fu|KIjs{06FGMwH;#u^>mZEuFbqmAc1<
z;YO>z9%}q6@ZsN?=0|;Y@d9gWbw34&@jPy_uKYd-y_*|$;+iGYD`NlTQ1hg?{Q2a3%Dm-VMzX&~y^=@jp^YE>6hmcGaIR{+&X6K^
zxoqWG7tYgO5K@2uOLBy{36TH>&od!oct*0YJjE|dQ3#V=p}Q<)rv*fArLn#VGFNl`
z+lNq{Ki-u{?xJX)ha5wrZFT~Gmmr|$71aaiU%!LzF()4;0
z^nT;uSXnxiC`{yNoi1~y*q(mhisc2mTSvANL3CF1@$al?P`fUt<=#`MS0K}biHv<=
z6wCXAH&s2eh6pIw_v`%z7jr_$a8(I4Uth8e+KmcrrpdWHP0pKadi3z*criuKW%n~y
zaQ^XVTF>6?N&@l20htN3N+w3S$=$mjVgPflhX;6o%HJ4Tt-<#E40c8VMnE(eLk%2&
z){d+!KllG*=6^9A|MKWX(Td8QZ0Ny9GPfbU(5b~wSg+(=#bg>O)xCXX7ci?MvZzTG
zTj@x#)QJP&rG1*cZ~NyL1ta!)DKFNzio$U41fJKp76->)+dCt@pmm6ovM*Zg!giTD
zpSnntmUE|fY5w@4ur5DCzHj-t)WnHIIo1e^7BLEL4r%aHp+r9&h~k?i79!c_T-|J{>He2G=YfX86MXaG_&Tmm>oa*qB%H6=!
zy6sdVsTKFmi&-hTSCxtj=aOSYx`!tH+()O1>trCsy^9y6v#|HL&p(Vje+ibX{z;Mo
z>bAE)cmSXfmenv}Z^q4<)fMDmWefsY{Y+s+8aALTKCFB1@jIk0SAs!ZKx_#GmR^pF
zMoztpvW~^3KZ@p~?QjcKjhkfHrw35xWjuhR+fL^&SsVw(k0#0=dbv~zcfFpP?YsRGyV(Dmb(@)$;Lf@>TQu&Zcs
zc3ko2op5zNr@>8Qoy>q#hLbQv1EWz$_i7*ZE|kprkiACOtYbtJCQaDKZs_>j(ob*(zY#CixjVf
zd3RX|7mwxX4w`XL)CQ&dj_rq{D*eeQVa(QM^7BkPYp#2Qe_U)+!O!a<7rsJ-cQ)
zRfnt%ud4a6SE9Zc1k{JSL0<6opmdvtdgkF`O$UsdEgQVRWdCV>Mi|vtMEt9niGY&z
z!P<&|1Z5N3c6R(!OD-zsI2rv))b5u9S~Q-{RQ%HI)`S~O)l9{#*#Obow3Ji=h
z*=xL4Ps4opXv>?=pL{QhHxPlQHzD%
z%|C3pp^Z(SNo;p7f+J5yrVICR-1Ms?h?
zruX0b`moW4I9%)>X&C0qqI*rtnj*Yk@UG5o?$S1QNs|+5o@EHOmJN0#cUi#d8!QKv
zr_$0HmBa^@&&=LD*5&Oy(9OfC*Eyqa8F!;<$3Zmvbf0%Rkb%h}{DAPEoXA6LeHjhr
zKsrbO0O6lu-O(HfbaG_&yhsOlDx=ZpK2rLGI*=z$@c0E)~5&>w48AG<>8p+sLR0mbMF>Uu?k)d@V%9Q}Ov;
z@D;=BOt(c#ldD|uj^tJ)9|7~$=@PI
zsgSQYnrY-=th*`Bts{WZID=uAgQZ1@UU#tP_BP7SiQIDSYKzW^LNHxQp$}^$g1s^0
z1V5gNx}n^N!eMpuk$(EVTm!@w%42B=hC9eTE^jH&FiCIKRTb}qraTx$E{odgeyn)I
zCzd6y8hYIXN5_^)PnO%c^6)EP!`ANMA?EX#Tqhf0kO9p`f4er6w|FA@@jJ7%pwTcP
z-@t?lwBqQvhUR7qwV2zpT-VigyB}mSZm#7uBD?x%hb`V87#UQ?-|yF)=B0p6>{z8#49P!G9`$+t3Q${Q}8wjQj6b;)cYVhsP}Df
z$J2;^G>`Y<&|@`LZ$m~lCRl&iPE@s#5$_S|_N2TYoW+>D-;xJ?=wox=$MqW@*$tm7
zK70^#kovM38rpm>8g1;#$~k!?bidWy)5H7G5&F3>Ap?zydQ)~9?eOv$RbTjsMa6-K
zGVQwDytwMBX(W-d%(O1Z@c2;)6rYqg9vyl_sqTtZzc6D9-y}|B#TN*TF&AxG=dhJ;IkL)dWLqcuf&6TkiL1%J$FOaay%~Ls&>6@rgUduc{
zMo{bp7tLaHT!eBt+i48w&xj=7E2PjSj()|L2^yu0XlJz|5VNZWCave?k0kDY4cX|B
z>ZoWN6!U(S0EFDQ<=7pl^0%`ylH4K}_DcS+W_|};ka=-@7R?qZU>
zeBkn#yeotz+n_`BHe)9hx2|{Fz!j9BADC<17BSK|PBH$TRm%GofyK@8%kr*GB|)35
zyl+g(Ske}q^!(B5%h9PXcx#3bIIaAt3~3WGpKCQBVh`K}eOS_pnCWMzEhDg5eM}!-
zu#Bbsd-+9-zwi+$y!RPU+8eL>K#-)gjE`?1~=blyVk&LV=GT`
z^w3f@TN?*L`!espRg-St?En6ge{Q}}dJ}_=Q0@ut(f1oBdJ{9tq24I2VseyH?o)R3
zFR3vW3br@C76|dz?uKw^wRkhV6zx->{Lo>lJEyv
z7Q)4ACc-0gDY>m1X0$Qd`0L*A#LBaLRR!w7wlzP*hCt$rWqMyU_k(@ono+>xraa&;
z0TNc6m&7;?ju+nAPNYn8I(HBafn0#4ZfTn*uYc`%$#lM1c=~fB?(4TV436iJ#yS3@
zsfnBLzJ6>#akrkbE7x@M$GkKav})etD#;E$8Z^eFz>`>pV^aiBr@K?!&7las9q+kw
zp50eTFuIDn3SsBv;ZV-S{qI&2VeIUi6>ZQ$29l6gh*r6SfXoU)6!Sk%*1iH4LM
zA+c~9Z@9B2vFJmqm4I66U9orx6}d^A6yLIH|Eyp@5(ZGdXTiosrC}hA2))6oV^{U4
z?5OE`i+Aa_IL3<}rE&`$3rSGYq4o8u%N0(cF+sI9u>Qtl_`7v98A5?$k{h(SRiZdy
zFf*6vXv-+2>hyKeoM#cTT%)q%$HkhYzzG~s<9WcsdH$3@S8f@XALz;OfZ4p)HF|Up
zmJ+rWlfd_PC<}?>976bzS~X#V?RbTEVA|7q7P(6G`vfMe6a(kxQ4RjBkREsLHXPBU
zF9e!1nxk*N&-!~*4oFraHsTNW&sd!oFgL}{kXG6TDO!z5>gw4jO*$9bzWerMAayR=
z$b_M8u9~W<4>5S0Utn;^fpsI1F;;8NE(q3;Y+k;O(0`A`^BBK%kx*l=y1~GL*P~jl
zbSxh+OKJu4W_4RsS<-&Ax5bOF(*jF(ds!smBn3pVHsm**LiDk4!E0TGdx>{h6k+uq
zM9~%{%1DZMUPkcbMS#(0(PXDl<+$yXN37TkX|T{tRnq#>_Ok}Im2?r($exI@aZ*6Z
zA(S{)zm+GSYd*`H?}^lT2Ih*_O}&3DVTz-pZd=Z(^O5v_O!UeZ^y6P)9YU?g0073n
z61}67yEV}9r##Nqu(6tF!@3uq@xi)2>~~pbLaC9Tm{i&6PbWJlG#&uw>xXA&4@2%f
zm0k2nU)GFygO&3p97h(P>x~y`qE-PR4#ga#dVz%O8hfFI8o{k1`0WVK_wx(#M?-;b
zJxneAB0h4(^C%Z}?*=J-Rd_nBw!aWj*1YAE{T^hS;Gd#HB1^_J6gmT8(|O|~gNhTf
znjq7Ue~A6A;+7mscIY%0MUI(d?=l+g(1q-&R7LV)aSV?WgRdn?ggJQ5+}e2Zi+J?#
z4eiH`oH5PmLpVEdiIsjuty7+t+Ah%5m0$W8I}$>CLZq;2L#bx(;e(dFCpyC)okR&d(%i_MexK-tlw%b#U%F_6_5qs0MPF
z4qWNU(I_W7X>A{`(Ho1S=6YW>G`{FOPVmBgLW@_y>)GVJdJ$gukdR~?VK|fk?<^3|Eu|dW7)Sw-8%haNs>;^+kKaNU_%}Anjy|GUHeWnO*>a2
ztG}rervX!_1z`wpE0Wp@JZCzIjyr0p)jLE5)Jj$tgSGX(_|^8zyyeB$=|UUA5k0HU
zI6-3eL{jbM3UuwJUWo{jINf0<=H5woeZHPQhTmXE)9>X)(xnZO#w*x4kM5<9j-pYu
z|6rB;6o9jF_VH6I%ya)KUa5W%^3=d|?F(21`0u8k4baKR)X2%`-$kD-%&R4#h}W11
zP>#(yMN)B9L+axfi2k$~4P>9i!|2K3WOH~z?>(hezrXA&VYIlcb37u_SLi&<5Tf-}-mgVJQ@)n1!aG!%lmag!<
zD};Ee_%4`JvC)@3QH4}jKZKf<3E?QrIT5;M%lLe)qy;G`m>4|m9GN#H*Uie)(Fv?e
zYJ;U?Z>$LO6G
z(rT;1%Xxy{Z^#vgd~Bv%8e`<15^rgHHMDfW#if;o2EE;pWEcpus=rjkf}(2r_?059
zU`FXH4F;?1H(Fq2ffN&&oltfoVzTu1WOF~JbQ_8{T`Z{gd@%9nP0cdJZHv!Zqqq0o
zM3*~-RU!Xe;!Tn0w<)l21%dsr{uQ=NjI2$>&5bNvO@@I6T@wxsw8~ds*@J3C0n#4U`VeqETWfwZ?oc9Ksve
z7^zMFsA|48zDBor7>Rr}`DQ(tq+(M{vz?BR(K0rRI`lOlbn0zBhqUs%MxCgiv%0;`
z6sm(0LReh)xdw#GsM-%n&5BLQ7ei3?O@~wD#ju5cIneXdq|{PIQ(a+1_At33m>YOh
zg^&z6A(gj_QIobuLq8XlprbZ}SM^CKfVT_IfJ5njlPrO9fANxS{GNZ5;dvh3VgCC(
zwFUO1N`2A=B*w=k6I`!3ca1M^QM_l1$QuhXRKAxGrcT_O1%(MN_pgO_jlyao2g3{KnS#m)>P7vB_bfyETRe6vP#1fY(ppw;fz*E?YDd3?=5*~Cj`0%_)mJ-PL%lO{Xj1r
zhk5lAm9ObM$r?>zeG9eo&LA@{nMOA&Wc_J(VfFcMrukQhO8deXlR_Yl8%oo~FJ&Ru
zQ>U$Lg7@oZo##J1UVF1#>6vlTEZFmK{;#i4{#dR*j{o7C6czcu8~FQ>
z{14#IVsebGpW$^ZZW
literal 0
HcmV?d00001