From 7b1957f996be566766aec370197dfacdd898592b Mon Sep 17 00:00:00 2001 From: Gianni Genovesi Date: Sat, 23 May 2020 12:49:54 +0200 Subject: [PATCH 01/12] fix: issue #1476 crash with numeric string value terminating with new line (#1481) * fix: issue #1476 crash with numeric string value terminating with new line * test: provided tests for issue #1476 --- CHANGELOG.md | 1 + src/PhpSpreadsheet/Cell/DefaultValueBinder.php | 2 ++ tests/PhpSpreadsheetTests/Cell/DefaultValueBinderTest.php | 1 + tests/data/Cell/DefaultValueBinder.php | 4 ++++ 4 files changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 27d72197..eb9b3810 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). - Fix RATE, PRICE, XIRR, and XNPV Functions [#1456](https://github.com/PHPOffice/PhpSpreadsheet/pull/1456) - Save Excel 2010+ functions properly in XLSX [#1461](https://github.com/PHPOffice/PhpSpreadsheet/pull/1461) - Several improvements in HTML writer [#1464](https://github.com/PHPOffice/PhpSpreadsheet/pull/1464) +- Fix Crash while trying setting a cell the value "123456\n" [#1476](https://github.com/PHPOffice/PhpSpreadsheet/pull/1481) ### Changed diff --git a/src/PhpSpreadsheet/Cell/DefaultValueBinder.php b/src/PhpSpreadsheet/Cell/DefaultValueBinder.php index 3a676c4f..693446e6 100644 --- a/src/PhpSpreadsheet/Cell/DefaultValueBinder.php +++ b/src/PhpSpreadsheet/Cell/DefaultValueBinder.php @@ -65,6 +65,8 @@ class DefaultValueBinder implements IValueBinder return DataType::TYPE_STRING; } elseif ((strpos($pValue, '.') === false) && ($pValue > PHP_INT_MAX)) { return DataType::TYPE_STRING; + } elseif (!is_numeric($pValue)) { + return DataType::TYPE_STRING; } return DataType::TYPE_NUMERIC; diff --git a/tests/PhpSpreadsheetTests/Cell/DefaultValueBinderTest.php b/tests/PhpSpreadsheetTests/Cell/DefaultValueBinderTest.php index 90dabce3..e13cd942 100644 --- a/tests/PhpSpreadsheetTests/Cell/DefaultValueBinderTest.php +++ b/tests/PhpSpreadsheetTests/Cell/DefaultValueBinderTest.php @@ -57,6 +57,7 @@ class DefaultValueBinderTest extends TestCase ['#REF!'], [new DateTime()], [new DateTimeImmutable()], + ['123456\n'], ]; } diff --git a/tests/data/Cell/DefaultValueBinder.php b/tests/data/Cell/DefaultValueBinder.php index 53ba6281..1f0c40e0 100644 --- a/tests/data/Cell/DefaultValueBinder.php +++ b/tests/data/Cell/DefaultValueBinder.php @@ -73,4 +73,8 @@ return [ 'e', '#DIV/0!', ], + [ + 's', + '123456\n', + ], ]; From 0945e87b6da1db95cf94b0cdd92db5456657de61 Mon Sep 17 00:00:00 2001 From: F0Rt04ka Date: Sat, 23 May 2020 17:51:48 +0700 Subject: [PATCH 02/12] Fix "Undefined offset" if border style is "none" (#1423) --- src/PhpSpreadsheet/Reader/Html.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/PhpSpreadsheet/Reader/Html.php b/src/PhpSpreadsheet/Reader/Html.php index 5e12a2fb..2fe85b6f 100644 --- a/src/PhpSpreadsheet/Reader/Html.php +++ b/src/PhpSpreadsheet/Reader/Html.php @@ -933,7 +933,12 @@ class Html extends BaseReader */ private function setBorderStyle(Style $cellStyle, $styleValue, $type): void { - [, $borderStyle, $color] = explode(' ', $styleValue); + if (trim($styleValue) === Border::BORDER_NONE) { + $borderStyle = Border::BORDER_NONE; + $color = null; + } else { + [, $borderStyle, $color] = explode(' ', $styleValue); + } $cellStyle->applyFromArray([ 'borders' => [ From 6c23de78d8a147557aa2b3cc0c154c51bb35df83 Mon Sep 17 00:00:00 2001 From: Collie-IT <40590185+Collie-IT@users.noreply.github.com> Date: Sat, 23 May 2020 12:56:26 +0200 Subject: [PATCH 03/12] Fix Warning if $relsVML->Relationship is NULL (#1420) Fix Warning if $relsVML->Relationship is NULL (#1420) --- src/PhpSpreadsheet/Reader/Xlsx.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/PhpSpreadsheet/Reader/Xlsx.php b/src/PhpSpreadsheet/Reader/Xlsx.php index 77035a39..797e59ea 100644 --- a/src/PhpSpreadsheet/Reader/Xlsx.php +++ b/src/PhpSpreadsheet/Reader/Xlsx.php @@ -1000,12 +1000,13 @@ class Xlsx extends BaseReader Settings::getLibXmlLoaderOptions() ); $drawings = []; - foreach ($relsVML->Relationship as $ele) { - if ($ele['Type'] == 'http://schemas.openxmlformats.org/officeDocument/2006/relationships/image') { - $drawings[(string) $ele['Id']] = self::dirAdd($vmlRelationship, $ele['Target']); + if (isset($relsVML->Relationship)) { + foreach ($relsVML->Relationship as $ele) { + if ($ele['Type'] == 'http://schemas.openxmlformats.org/officeDocument/2006/relationships/image') { + $drawings[(string) $ele['Id']] = self::dirAdd($vmlRelationship, $ele['Target']); + } } } - // Fetch VML document $vmlDrawing = simplexml_load_string( $this->securityScanner->scan($this->getFromZipArchive($zip, $vmlRelationship)), From 3446bb0ef781465f12e073b918dace0cffd03a18 Mon Sep 17 00:00:00 2001 From: Vagir Date: Sat, 23 May 2020 14:09:10 +0300 Subject: [PATCH 04/12] Fix saving XLSX with drawings (#1462) * Fix incorrect behaviour when saving XLSX file with drawings --- CHANGELOG.md | 1 + src/PhpSpreadsheet/Writer/Xlsx/Rels.php | 11 +++-- src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php | 1 + .../Writer/Xlsx/DrawingsTest.php | 45 ++++++++++++++++++ .../data/Writer/XLSX/drawing_on_2nd_page.xlsx | Bin 0 -> 16265 bytes 5 files changed, 55 insertions(+), 3 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Writer/Xlsx/DrawingsTest.php create mode 100644 tests/data/Writer/XLSX/drawing_on_2nd_page.xlsx diff --git a/CHANGELOG.md b/CHANGELOG.md index eb9b3810..22fd8c3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). - Fix RATE, PRICE, XIRR, and XNPV Functions [#1456](https://github.com/PHPOffice/PhpSpreadsheet/pull/1456) - Save Excel 2010+ functions properly in XLSX [#1461](https://github.com/PHPOffice/PhpSpreadsheet/pull/1461) - Several improvements in HTML writer [#1464](https://github.com/PHPOffice/PhpSpreadsheet/pull/1464) +- Fix incorrect behaviour when saving XLSX file with drawings [#1462](https://github.com/PHPOffice/PhpSpreadsheet/pull/1462), - Fix Crash while trying setting a cell the value "123456\n" [#1476](https://github.com/PHPOffice/PhpSpreadsheet/pull/1481) ### Changed diff --git a/src/PhpSpreadsheet/Writer/Xlsx/Rels.php b/src/PhpSpreadsheet/Writer/Xlsx/Rels.php index c2876065..a4e5769e 100644 --- a/src/PhpSpreadsheet/Writer/Xlsx/Rels.php +++ b/src/PhpSpreadsheet/Writer/Xlsx/Rels.php @@ -183,7 +183,6 @@ class Rels extends WriterPart $objWriter->writeAttribute('xmlns', 'http://schemas.openxmlformats.org/package/2006/relationships'); // Write drawing relationships? - $d = 0; $drawingOriginalIds = []; $unparsedLoadedData = $pWorksheet->getParent()->getUnparsedLoadedData(); if (isset($unparsedLoadedData['sheets'][$pWorksheet->getCodeName()]['drawingOriginalIds'])) { @@ -197,13 +196,19 @@ class Rels extends WriterPart } if (($pWorksheet->getDrawingCollection()->count() > 0) || (count($charts) > 0) || $drawingOriginalIds) { - $relPath = '../drawings/drawing' . $pWorksheetId . '.xml'; - $rId = ++$d; + $rId = 1; + // Use original $relPath to get original $rId. + // Take first. In future can be overwritten. + // (! synchronize with \PhpOffice\PhpSpreadsheet\Writer\Xlsx\Worksheet::writeDrawings) + reset($drawingOriginalIds); + $relPath = key($drawingOriginalIds); if (isset($drawingOriginalIds[$relPath])) { $rId = (int) (substr($drawingOriginalIds[$relPath], 3)); } + // Generate new $relPath to write drawing relationship + $relPath = '../drawings/drawing' . $pWorksheetId . '.xml'; $this->writeRelationship( $objWriter, $rId, diff --git a/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php b/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php index 15359a4c..3d47eeaa 100644 --- a/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php +++ b/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php @@ -1215,6 +1215,7 @@ class Worksheet extends WriterPart if (isset($unparsedLoadedData['sheets'][$pSheet->getCodeName()]['drawingOriginalIds'])) { $drawingOriginalIds = $unparsedLoadedData['sheets'][$pSheet->getCodeName()]['drawingOriginalIds']; // take first. In future can be overriten + // (! synchronize with \PhpOffice\PhpSpreadsheet\Writer\Xlsx\Rels::writeWorksheetRelationships) $rId = reset($drawingOriginalIds); } diff --git a/tests/PhpSpreadsheetTests/Writer/Xlsx/DrawingsTest.php b/tests/PhpSpreadsheetTests/Writer/Xlsx/DrawingsTest.php new file mode 100644 index 00000000..d6ad77c6 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Writer/Xlsx/DrawingsTest.php @@ -0,0 +1,45 @@ +prevValue = Settings::getLibXmlLoaderOptions(); + + // Disable validating XML with the DTD + Settings::setLibXmlLoaderOptions($this->prevValue & ~LIBXML_DTDVALID & ~LIBXML_DTDATTR & ~LIBXML_DTDLOAD); + } + + protected function tearDown(): void + { + Settings::setLibXmlLoaderOptions($this->prevValue); + } + + /** + * Test save and load XLSX file with drawing on 2nd worksheet. + */ + public function testSaveLoadWithDrawingOn2ndWorksheet(): void + { + // Read spreadsheet from file + $inputFilename = 'tests/data/Writer/XLSX/drawing_on_2nd_page.xlsx'; + $reader = new Xlsx(); + $spreadsheet = $reader->load($inputFilename); + + // Save spreadsheet to file and read it back + $reloadedSpreadsheet = $this->writeAndReload($spreadsheet, 'Xlsx'); + + // Fake assert. The only thing we need is to ensure the file is loaded without exception + self::assertNotNull($reloadedSpreadsheet); + } +} diff --git a/tests/data/Writer/XLSX/drawing_on_2nd_page.xlsx b/tests/data/Writer/XLSX/drawing_on_2nd_page.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..fa51215ac8fe09ce4b7e2e8f0473cc85ffb364b0 GIT binary patch literal 16265 zcmeHuWmFx@w(bIg1$T#_!QFzpyGwwuaCdiy;1)c%y9S5g?h+h=yNBR!?d*L{_RiVo z-TTIPJZ;f8bHk83~2_6ba(1LmDkk3&89r@RJafJKi?Z>R_dWsLkE|-Pu3VH;CRva4y z$y9J!-xbsHDQcNurzTzs);X;?`pZd%Qgq!iC@R18R6eh_3r}GR-(r@SXB94yq)S*m zZ0{suCq?@7lo7=G1a2!%5a2VlQOxTba(JxxBAKRd(uStfgUAQa)(lKA=-S5?{cjae1&qdgaio1UR%=cPzm2il`2om~2W5(z8ncjj^Ne)6nqAFVpp z^WhA)9uC;<(t)!<|8SzBRZti9uSODA;QpiuG7v#f=Y|Ao8COdN7aMyEppA{iZyj5n z%9{KVBeHj9-Sg**>2EOS%2F5D9HBq8dez>>Bg=eaXDvrR=3PZOE?8VBh)Ek>$ z~qU?8wmwMw5P<+<0;9uUO|r9A@T*!AVq3O>U9&) z2=foVKdIUyj1YN_6w9h#)RlbYmMMaF)Y$9l*d7zqxYj@qRJ{&QY*&I5gRHfVlYCu+ zp4p{V+3VLiGpkAmH_^uwdC!S|yy#|)+{vZCBUXXE)|t{ti3X>Jb%LnX;s9eWHU8D_ zlaeZID*{qUFp ziNBj6dPLVXhGGI4QgId@5~XG2av%8w(y@Ql!n@R#(LKGjm|N($`Ql%;Ph=&jC>aSD-ZqHC5vc zRiBz8^OQ!ATHx{>WqCgIV`anytuCcNnwB9%zbiFnN~nMPiNfl%KGsw)m&nsOXOaXF z_cmS1*Gwa8>z(b7%stbucAcI+DnG3TwKp~%fIjcP>WSSKOXVDgACUHywXk392OR??RuZ{>5MH3lR;hn;{uyzJ_b z_otng(|4O8_TeXEE&xB2>>G?zp(@c-_XR%`K0!F%>$<%Nm>I{k8ExnsoYy!R^fgDE zY(m$b@|4Tk*k2l?x?XO-%R=%9g`fl9zsaca&cV6GOh-Afu7XC5B=6BO!*C`|U?B}J zm5r7wW7~d=VF)rrit>hQMk+Mw`(BzD5r@dzKum1kJpaJ;7=J~EVNcOE_AHz1nZnhm zZU>R65Pl;$6m2~B71sH?+?R9s@NN6vtkn0FG1D^Goq5hTwY&us|2fO`3|GycRGQAzv?^MCDzf3Zyl$}=vr(^mgF&KUwA+&=ss1t+dwQY zga@l5Cd`l};)pM`aLjzSc|U3?8CG3JqY05QuqTM^p6K(Ga3aW`ss$q*Gm#Q8H&@bg zgHS0t904um$f@jSozk1VdMjr3d1#}s;XL=1#&^1S0$x;9FGPqEDf?=$)lWIK;-Q!b zwMA<)N-42(7Y?>`)6s3IcBU944K=af7zL=z-0WKp1!}DV-1tM7Hn%i4(Kfh=KH+?Q z&0Qa7!Kk2#H29==X?92Bv(3U6D|g8cht}7QDL+N9PD6c{oOcEUidI(LLhFaa?hNI} z>wds_RPIT;rVib$by68Iv9qg9rVCeP(yq*L^VFdY7p;1dzH~er%otPNvm)iTD-7G$ z=Bet|Vyzmd)Ve0OWpJr?0^^r|-0x7z5q+x}im56V&UlO3jM43%sdO}&X7lx-71Gra z?NiYu!qM?!+UN12d~}RMJP(9X2F*p>Q@NvFJ8bCiJ^!fMo2qB(sIOgcr>L)8ZD{M1 zkq^H{BqoGJGU5|N;(5Se8(AkCxs~Y4PY@7OlqI)V==)P9T|6sZCUDDb2q1>E!24a_ zcY^+Qya3W~m^^1VFrO$jsx!X#UCB8Iv*zH|H0OzQq{oM>VI~aE&KkmDS};eMAEAJ& zLJ>9>fLCn-X1PuRs@h22Q)Lrv*|et8NC`-ntxUXNE)mck&C9Q-)Ev~TRkW`PmrIAZ zwtADNYu?b$E?!{D`rX9b@|OQ^JtOL&AD1UI0C4;2w@Hd0p3%|N$jXS}$L|lPc%(UM zjn9GF4n4rlddK&%5buoAWv;TX#x9uQj4X)Rv$AD_glHGMIIpBO^aU z1`im27!3wgTJ|sM3LiiFMn+yDQ`LABNFwxqAb4{|PX?yMabF6oF1a&sI=>J{0v-?m z>0zX`yl<5TZ)}-?25~alPZ=9o6KA}{ip*jOB_vqd+S6O~#2EdJcw_QxvFy@|*XQ%;@pv&cWbI@kK(1mwbYQg?V z_}XHFjdGT#Yy&!g-gUCeLg78q{F#2?!v4Y?8)fhO+}X8K3)T$XBQCmP9DZ50w0`-b zPK7gM3S;weAV>E*g-XLA2Xw=3>ZKWUv=0NdJGv1kEPMbka*0^7u-8~DJc6VWSM)iC znP@g4GNckcI8w>J8%p`Cl&Tw*cL-OBQH+aS12JmO{zgD6iG|%Tl-Dc^7nOc$YrZ_s z=lv$pnS34(-L>O<9^by-TUwV7#Vv^q&Lnu;-c))&9u}@LJb%C2j_1!;&g_Fjd;Z!o zuJ>@rU#WydGp2L?Fy7Dbk&yp+80-1*TbQs%TCNuw9~l$oDuUPwxU~YDf?PBDhL>|fj7YhFebVwmd&8R- zWf{z#O?C)Pgg3=OoVF5Z+z}1OCHSS6*@{s6XmO_}g&^a0M(UU#f{IJXAfk<%h?{n= zDrNg}dCvl+YKYyLe2&dzU%~^s~m5Gbpdsj@#FqE~tTPTqg zDV;Cd=_>B=QHV0@2;dkbTjA4G9FTjAaV;H^hvhJu9!bquC!RXCG~Om|{p)S5&jqLC z1M3_gz<~U7A`0&s_b^4u&UQ)6^=5$%h08tOW|xct*@m;+Op?0V59!n% zUHk<{c36zN3TKTN&mWac(DE%ascqReo}h^+H$-uIw@0Ypxq~?osL=cx3;h&k1rqfvh=s5-%UXS(aHm9+ zGkl%dLO`>TN?faNQ^s4!unsF5r|$hw^XE4hDF?-8#xerXl1PUqG<^BmH`zPq^mv6C z6ZV=;yvj*0$u7^88gWA^5@jaq-B(w%i{~Gwicf~<5fo}{fPqXYRwA?dB3^G`6x|Y0 z+>LgDqSN}K!o_;Q<~Y5HG@W&-D(h&=0Z2C=h@i|ua8exr;IO|o2fZ>arb4% z@{FMb2HQq_d8TBI4*Rx?XY-4lgC(@Zu=kDe^z-yz5cO#~r_$YCMmVt`6&roD@4c`Z z7%k#h6)nRkPX7v(nk*xTc3QMOUqX!E6ERV?EuR$tpL7myFJ4s1C{k#v#1hBjwXqhg zY5aD85LNNOR+ftjuSW!UqKNv*v;IMqeL}<`w4wYhn2@VYqy>0+t4*Ka#PO@m!57ha zIL-zlZd`4U!SYRAw<_Tsyc&`2;{m zjN{~`oTgxw7U@jO7MxngbCeBnC=Qn-G+%Qx3^Y|VZE;Dc?9!j7>fFHN`Ri-(r5L5w zxMl=nn3&`?nC)sHV}C5NI$JW2wsrq%<>DQ~ol$QdpfQ)ib$(6N;`Vav+bM+G)jE%z zT>eCMAd6L0)}FLHrShV$`j+2_xk%&1PI}!#NPZQ8BaQwi1@pj1Aq51fAh9nMn2Y)v zbT{4h#q-5H{*j_$c5h_lzeC&R31Th0=^)hOK8#K(WKR;KrDygE)SC%DtbMZYQusmyOezKkb7dIW1-*J?g-?Qc8l#iLia%ODQ9 z4Fv!o{N7nRIJ#LHIsBM%QI)m^O}VUK?GeD7V0ZzW8i=VvH(_96z#-$nSEf+&kvGJ% zOx3L?ov@mOPbZA9+5#MM5y@%PQuC~o7>+h#JTHw`ZSe`%{Bzl6*_4_4p8P^k5>tj! zXbm2on4KuFXM+dQZOP3dIO_Vu2M^U1t=Q9b-zu6T3k-1esrRX{%$L{FP)m4aQB&&| zDrg}?Nb58^q>cu3lEoV8Zdz{EESCx;#B3A~t`FC7|8C<@h=?n_yy)4&e1bXJqf zAB=;T?IVhXXM56sE}Ha;nY(`#^)5xNo_Yjqy}5)^LyTxDqggr0(1T3C8z+vaS=W}~ zMyvzbY|FgaSdA}TBq**xEJaHaZ4(f>h&ob{e@Cg@0{QY(j$H8(wj35ZI%I`x2wfd_ z-yUa^^?EVdNt(perxwJp~O*| z|*N`8|N*J0ryRLM7MVjO~0<#1p zWFm#90wV_IFZ zF&k3@UmZW0;cieX2ns!4<@-di8Ey4xvM+u9jB*{ToHhJ>cltJM|0^p(dKgD0XRa_V z*R*ZBZP($o1*%|eui+3sud%%GxV@3G{XwT+F(LP#aJP!gE7o(d+5U+gc%O(2KS!xN(AbS;D+7{XU}7d?MC6o^utNH>0z` z4s8dtMdRYnq}sB>P@`5R?yr?}Rl|d=0v%m-4}zqSW!}}b$rFpNxUndXo{zAQ;!GL? zHmnqR6w{LGC93ge#PA*ETLrD!tojUa=%ThC_>;mth0dxI<5TnCZtDy9KXdL0%e_R} zUW{{cN;@_hq&HgYNjuJW9O4lKv&Yqf_!PnrQi+sF$Vb#p;r7`aLv0nx+XoN0`{=Fs4h(uv1yRGwgfV@tk|+AvUU7nIE-cbP4pLjnm!kJpPUBs z6mlj^WM)26zK=WCSAVDM`Bl@}s>Ou{HSQgAYU8B2pH5u9G=~&cHh1ucE@Xyo#ne?& z8@@}+o9=nJNjpn1{+tPYA%o8S`gcT$)1i*SX%_7YbSAhJT@VYpy;j zMi!lx@KBi*DqVS4itNlg-$?!79*+0jt#w-U+oWt;nZ=)Z*|fFDVfdjvpu_yc%m&$( zu&V)et6*IOs?ktz$0)J^E#?UNtgBe4av%D9lWIA()0f(d@)z~9u1IWYVVRYYp_x8|nU%hY5fi<+t;zG3=M4b1sH>SV03azz z{bSMjcX5!-905>7oD3}7Kog!XKqn1Br%nKff`&FgBYvm zXgDx%D2P8E`j2-7?ZLqyUO*xP08kJB&m@M> zHZz$ElOib##k@vxBTf@87s`((0baa-1UZu*1q>X8mi^bH|W z7S`uQ06eHl;3yC%0Diz&IA5usZ|6HkYKhpemg~?daJyte6DvLWZFEpmg@ce z!R2b`r0e6O#rgQcGr+~;8KB>1s3&>nvSyWuH0r_k4Cv^v*!(u~9r&17SlU~S#!a*I z3^3z=sdp|+{R~Ko3Va4Qr+{u#?`^fLrN_OX8}t?@W|2mUE#siUB-a~K?k1;~Q!r~q zHzkuwRpvtv%wzcgb9bG!WZpHhkoSSvsoIkttGE16U3%_3wFYS5xIU!s+qw9GuZE+s zuC~(ETy%I2jcJv;?*m>3P`1^ z!W3`nZNl~3M({w1z>|`0E#Xp|KxDyyZP~PjgqeJ2Z6jV0ZKSOJ;0t z55HIGjn9Pl?j1+_tB1mQzIp50X!+|*3iZlw4s=>evp6hFwUtqYo4OjVkJSXO*HfVt z>h|aCIukRh*x7-8^S;)=uT9JQ8Q2;P3??ERGI#@aG`bx_MOo1g&ExoL9Rr)u3|EdL zXXD|tV~dCM5RYy}hLn5W29d97xwy2-*u+_tOy7smH0;ubE~Ly2WX=@`d^cbrr9vFt zN7PV)92$QkB(1RkdUGFcfPShEs6w zQZC`y+f3OZM;@UCCmg!|;&izG@nB>`WGrr*)}%GujjXuXhxi_8@9*{w5QojLmRo1j{b3S5B zUJ$zeu*6)us}N?}%lY26jAe#C0~wUJeX0j(-v&C0qmqKVJo@_*?i2Q!_V?eSlq9^a z{c@7L3^{5xp!j8fGk%X&79+JU^lO{Shs9W^5dgxifgER5jHM3=@JpK=!bU;d5=BQt zhv3W&oegpGgMmiEDeltnm}8kSCi3e^3*hGxDW#R{g(|iw$`&>Yb~bLBDJ+O3+)=f_ zY8om=oj$ae8V0v$XytaAoZjv=zQ<3^EvBpbp{wau!wf6a%+-bp>;p!2@QDQI3+I~m zECEwA#LC5XW&%Mt7E&3E38tdtwB)6*!IZD6;N4FxWYpOrhJb9d(UD2_*lsU2q{T`l zw@G`0Z|~wheZ(L&N}d1a?`Ci>tZX}Bbq&-Z;Zwy(YVTKSig7#Ip?jxgM*8LR{(jbJ z#Eow4lhjx8w?OB_OE=_;__0-P&hyuL8PA$NEwBN)1Y(up35arIWxTl>iiswHR8f> zFiJ?2&9T?WUes<>c7~1ceI31hjUcWi*z8DyE2;t>Fll$wu_rZNhPVY3^L zx5^dqWI{q-so9EwF>1l@t`Da&(u>Cyg^U~1g$hOP0r|Z) zpPG+a$+F9=>N6(3jF;FZ%QuzCLI}%UvQc^+u!Osf5D6jYW?V`bk87Q~+S*S&KTyHc z;AE(?88RUHdTTlfxbjBA%EQ9`hI|rd3+HU6Uy_D~Jd)IW5T4*+#%D#9FcGhdtr!C< z3(UcMX2=ozaU}t9rd78!n*}@lb+Gauy`$5CQpaN{&odxg{q%|d$n!${k)J=e@EZrn z1WsCOHof{cFSD0TYMU3MxnS0G*%r>6)j8C)bcX}9$CSeizwxjq4J>Al^&)?$<76d) zh}X(55EH)`60C`24Otes2zNzaLZqtUv2h4_t8UwKV{<&xI}tNGF3SvK<=|A|R=uXZ zZ7qS`P+uV}tKqeL%osiF9JG9;^?hNwf@ViXjv>St25&Im3iq2aKT%3bG~|Mt_PJmu z0fO+^_^YdE524%B>ZYZ1;B_L7vlfG00lB)P8W{HbZX@&j37ouzq9!V(*I!oC{gtpD zTiD8!T)WI`oZU>Ejt=vB)TYBlC@37#P0NU&`2<^rqAsRP!j`xTCtRKZe8)y9nDXXK zGIQEx6Mml$`&%$d++gc<&?@5baOi|Jnda3*PhPXLGboXuM6bvIN_S^H`b|EuBwOV$ zWyCf}I?pag-UrH7=td^7&$0#TE@dD}GRJ%L)K-(#efK+Hf^l%LDmISkwNw0CVVSKW2lpt(&I|#PnT`U||eLU+Z&HjT^fi_$cWARj#dmn{x$6gxj|0icBbGazcq9GK0M(~A9le#f&wy)+c%F>R zEA{K>m8td=x63CpZb!r%0p`zGi{vlCB~rF#1k;Omp|Om4o)PAm5`pp7kJd53tj zN03Z+c&~l!HFth##G|BrgD8>E4z?ZuNa2K+b_63)^o{Fc9hA*a4>SitX6=a-GD^Pu z;!7lOY6LN_H(V)oA1&gY&td!N!x|ZZUe78f52ec64(d1+pP3E>W!x(3Mz^6w3d^$D zaZUxuSAjzL#ccVCSdvV&@rfn2BIfEqDJxnkIe?5}>)7yOo9g@h>A$`AICx_zRWW z=LHo5q>$q&aXedSG(vl8%~XvY8p7!=kChftZ6FDutg$z&OU+AiQz(m8(!<&sw?miK z$_X`duC<3UF;KD}$?IEfU7TyiS&8M?qD9cE_ogMml9tepl{`uKA}ULil?OP|knlL< zeL1v%<@Go!ZyQvhrKcizzNBP~G>!VuhZhHUIidSCe#<*9w=+f6NX>hI={-_*?EB3=XKe{?6lIVc%UB~)`1nz>Xp=-8J0FYtG13$aFo1s_4C#Q%OGPjQ$#Q#p|G4=+(7Wx~smAJ%Gm=kT? zV+gn0@6SVT!=;G#9a2^h88`2xJknG96DR@wla> z^zoq9T<_k=7t*LVdt@E#>3r502uz+geS02>cgS=wk#_p2={1$Y3k!#CVDwd(S^*CA zRLa9PHkc27acCXTDVUo6&E2f4N`!O(vuLlZ^jusSO>C{i3B6x}h!W9pvSv2An4o!M z6&D%5@sYMnYmiwuR$+q%EArvn$B$*Z2-}8j#~o0KBneKJoQJmuxH>zX&cQF}A?{+N`ee4UwXQsjeQX3C@#`#1{Kq`+_$f!~~J zKA#=C@5{@ev2eE&HSln|9$a8C8dbaJW~3H;qd@~Yvrg|l)HuBzR%gt;jU_RP;hwM2 zn5;40f>7fvfD2BDWT@ zHgZ_{aE)~1B0p1cU3%8eUC($kL-g-@R!oPgd6O$PkdPYg*F9|e6D zu)V^F{9C*b)wbL#Pe((w6q=eT-^7f>0b!gsdfM9YJ`N|&F7lh_R0sLXj(m&s-A8bcK%%y<(dUppUR zAe@+9zt%v9vKi2q+OW1SxRn29t}t{)1TID4=X{)&QR*11 zq{m@*BuHZdT`5R3^fU!(6_~KZ77fn!0pU?|OS|{_eB9ZE@w>}8oqo5wPoegN(6+Kc z@%P-G1GX3JQp03P)icK!TVp#(oH$f?xXzyoq_jzxm8o$$03(z+LEH8FO#Kz@W^YA$ zc1*;~ykC5e^Xc+tq`c!hFoYneCsAgrE60xAl&iJ-lt%uj9quVRY=oUn990%mlZN^R z6AV6!f%c+{IFJjwjaM~QiNpoua2BjO=H@cDHt46b0D@~*B&cA;r#chlYk2XDPhiHg`+T=`gB zLTdgNv$lHyqm@A{#iIHdz`EDiJANW}a;W}>W_6Tja|YL}FWdmRf!M~Qd5#9VVkBkk z#BKEN=Q8|qq!yP0x6M&ri9`zfHS@adE5)R57ao4SdZU?-30BOZNB9g#_P~4wL_Gt> z{mOqCbN^3GkN7+<{56uB+lpEGGiXvr2sDvI3fliQ?CbYP!JoljKjH)fqd&jzVni3c zfp`*j^@yJff)`fiAZb+Hg#g|yLMt)Z!a;Zg1O;Z?mV}^AzJ5BgWFsG=m3i z8DN=Kop~(MbqH2-9xS87((%OTPt&)jLdeNUh4sg1V8B%eDUv;6cYF6NR7b%U$MEee zNQSvF4&H|2O_Y_GTV3cu*D=V|pVDr5sRwZJDrr6)MR^hNSm$<(NnDHvO1_DC$Co4V zW>xB|%loCYQI;{e17vxo+7fgiT)c!gYw%-WxIqT9 z(lLB7t(>`*SbT<$3u$VF*@^7!sIPag%Kf|SBVp(R+aagRwIf#e$xY>pen`oRGREx2 zad%D8X677?iMzL(4ig6@@H@@2_nGWXBB-d$MAb_CS=hY|7T=#o06p<_$f0E=5^-@B+9r0 z$GXc`gYb7ZP|FdFjKj@I9*AuCC*COh>EYpmf(+cq_>Qdf8jwksyE_mf#pv*KOjZ8S zSey}uvuC|9m^@03M=;U$@ihCa=8qYljXS^h zp})?3l_`Vt^+96vfuCD0e%dqQZpop@hjQX}$68?MLn%1bWUbxRkcXQrrY-*2@u%~@ zRgF{$qt|Sp*Ix#0uR$BgIBf0BKp9H*%0`ZkKNeGd`Jag%Xl4z3mj%yQ!&tQphD+?3 z>E{>g7q5@`30C}NOz#--=N_0m(&S{nJkkKzpl>m`1R&ghZTSE9Co}(lda_=>LnABb zb8~`Lk5EAI!iF{m3idX(4h#l1_C~)gqJk1U{u3q)`e+e9oB(p*4=3OO|Em&O+yL7ga^sfYCIn+W1WHDX61>Fy{sH1E$Y5O7<) zy@Xih(yC8(h04oJ+V68Y8(nk}`Wmciou_(3Z>hYsdD5QG90c8J^s2Z{58-JAANCFxsa(3|-T?H-?a^gLNwj+=Cm8F00T}sQ{UM7K5jQ|u4iu`9))3>$#kEVfC?2nQjC-_^~ zg6IvHJG5V^BZ{)~%UniED8c2rL}Uc=WK2Xs6P$S|Xbk@nimjvE$h!RA4%zqg(ZGo@mgB^-v z#rB0_t3t&Z^0svMttsM0b&K;Ge59$vgw;k4v1r)<8k;^lJd` zuK>S>5&Z;E0wqU)0DcA){VMuv=*3S_O}rnX|71{q=1#w({QK4IpC~+pKTv*M>;APG zzY_bO0RBur0DjB=`Qvl^EzSQG@K>Jv6EKkZ{{r}vHUA3uE4lm$n8otfhWa;D^H;!M zNz+fjOx7QOzmTwBW{@0H4 h&*D!Ue-i(r8&!~o1bKnq*c=L=2o<%=O!U{|Du@0A~OI literal 0 HcmV?d00001 From 86e9d669c60edb57bec05b02ff457e4d959c9e4e Mon Sep 17 00:00:00 2001 From: Mark Baker Date: Sat, 23 May 2020 22:07:45 +0200 Subject: [PATCH 05/12] Range Operator Tests (#1486) * Range Operator Tests --- .../Calculation/Engine/RangeTest.php | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 tests/PhpSpreadsheetTests/Calculation/Engine/RangeTest.php diff --git a/tests/PhpSpreadsheetTests/Calculation/Engine/RangeTest.php b/tests/PhpSpreadsheetTests/Calculation/Engine/RangeTest.php new file mode 100644 index 00000000..7dfda9ca --- /dev/null +++ b/tests/PhpSpreadsheetTests/Calculation/Engine/RangeTest.php @@ -0,0 +1,53 @@ +spreadSheet = new Spreadsheet(); + $this->spreadSheet->getActiveSheet() + ->setCellValue('A1', 1) + ->setCellValue('B1', 2) + ->setCellValue('C1', 3) + ->setCellValue('A2', 4) + ->setCellValue('B2', 5) + ->setCellValue('C2', 6) + ->setCellValue('A3', 7) + ->setCellValue('B3', 8) + ->setCellValue('C3', 9); + } + + /** + * @dataProvider providerRangeEvaluation + * + * @param mixed $formula + * @param int $expectedResult + */ + public function testRangeEvaluation($formula, $expectedResult): void + { + $workSheet = $this->spreadSheet->getActiveSheet(); + $workSheet->setCellValue('E1', $formula); + + $actualRresult = $workSheet->getCell('E1')->getCalculatedValue(); + self::assertSame($expectedResult, $actualRresult); + } + + public function providerRangeEvaluation() + { + return[ + ['=SUM(A1:B3,A1:C2)', 48], + ['=SUM(A1:B3 A1:C2)', 12], + ['=SUM(A1:A3,C1:C3)', 30], + ['=SUM(A1:A3 C1:C3)', 0], + ['=SUM(A1:B2,B2:C3)', 40], + ['=SUM(A1:B2 B2:C3)', 5], + ]; + } +} From 8ca7bfe53c093ea2a5f443081ffa20953422ebbb Mon Sep 17 00:00:00 2001 From: Mark Baker Date: Sun, 24 May 2020 00:25:54 +0200 Subject: [PATCH 06/12] Range operator tests (#1487) * Range Operator Tests * Correct handling for range intersections that result in an empty array --- .../Calculation/Calculation.php | 70 +++++++++++-------- src/PhpSpreadsheet/Calculation/MathTrig.php | 2 + .../Calculation/Engine/RangeTest.php | 3 +- 3 files changed, 44 insertions(+), 31 deletions(-) diff --git a/src/PhpSpreadsheet/Calculation/Calculation.php b/src/PhpSpreadsheet/Calculation/Calculation.php index 0be9ab6f..cdfe7b53 100644 --- a/src/PhpSpreadsheet/Calculation/Calculation.php +++ b/src/PhpSpreadsheet/Calculation/Calculation.php @@ -37,6 +37,10 @@ class Calculation const RETURN_ARRAY_AS_VALUE = 'value'; const RETURN_ARRAY_AS_ARRAY = 'array'; + const FORMULA_OPEN_FUNCTION_BRACE = '{'; + const FORMULA_CLOSE_FUNCTION_BRACE = '}'; + const FORMULA_STRING_QUOTE = '"'; + private static $returnArrayAsType = self::RETURN_ARRAY_AS_VALUE; /** @@ -2593,11 +2597,11 @@ class Calculation for ($i = 0; $i < $strlen; ++$i) { $chr = mb_substr($formula, $i, 1); switch ($chr) { - case '{': + case self::FORMULA_OPEN_FUNCTION_BRACE: $inBraces = true; break; - case '}': + case self::FORMULA_CLOSE_FUNCTION_BRACE: $inBraces = false; break; @@ -2626,10 +2630,10 @@ class Calculation if (self::$localeLanguage !== 'en_us') { $inBraces = false; // If there is the possibility of braces within a quoted string, then we don't treat those as matrix indicators - if (strpos($formula, '"') !== false) { + if (strpos($formula, self::FORMULA_STRING_QUOTE) !== false) { // So instead we skip replacing in any quoted strings by only replacing in every other array element after we've exploded // the formula - $temp = explode('"', $formula); + $temp = explode(self::FORMULA_STRING_QUOTE, $formula); $i = false; foreach ($temp as &$value) { // Only count/replace in alternating array entries @@ -2640,7 +2644,7 @@ class Calculation } unset($value); // Then rebuild the formula string - $formula = implode('"', $temp); + $formula = implode(self::FORMULA_STRING_QUOTE, $temp); } else { // If there's no quoted strings, then we do a simple count/replace $formula = preg_replace($from, $to, $formula); @@ -2741,7 +2745,7 @@ class Calculation return $value; } // Return strings wrapped in quotes - return '"' . $value . '"'; + return self::FORMULA_STRING_QUOTE . $value . self::FORMULA_STRING_QUOTE; // Convert numeric errors to NaN error } elseif ((is_float($value)) && ((is_nan($value)) || (is_infinite($value)))) { return Functions::NAN(); @@ -2760,7 +2764,7 @@ class Calculation public static function unwrapResult($value) { if (is_string($value)) { - if ((isset($value[0])) && ($value[0] == '"') && (substr($value, -1) == '"')) { + if ((isset($value[0])) && ($value[0] == self::FORMULA_STRING_QUOTE) && (substr($value, -1) == self::FORMULA_STRING_QUOTE)) { return substr($value, 1, -1); } // Convert numeric errors to NAN error @@ -3227,8 +3231,8 @@ class Calculation } return '{ ' . implode($rpad, $returnMatrix) . ' }'; - } elseif (is_string($value) && (trim($value, '"') == $value)) { - return '"' . $value . '"'; + } elseif (is_string($value) && (trim($value, self::FORMULA_STRING_QUOTE) == $value)) { + return self::FORMULA_STRING_QUOTE . $value . self::FORMULA_STRING_QUOTE; } elseif (is_bool($value)) { return ($value) ? self::$localeBoolean['TRUE'] : self::$localeBoolean['FALSE']; } @@ -3282,34 +3286,34 @@ class Calculation */ private function convertMatrixReferences($formula) { - static $matrixReplaceFrom = ['{', ';', '}']; + static $matrixReplaceFrom = [self::FORMULA_OPEN_FUNCTION_BRACE, ';', self::FORMULA_CLOSE_FUNCTION_BRACE]; static $matrixReplaceTo = ['MKMATRIX(MKMATRIX(', '),MKMATRIX(', '))']; // Convert any Excel matrix references to the MKMATRIX() function - if (strpos($formula, '{') !== false) { + if (strpos($formula, self::FORMULA_OPEN_FUNCTION_BRACE) !== false) { // If there is the possibility of braces within a quoted string, then we don't treat those as matrix indicators - if (strpos($formula, '"') !== false) { + if (strpos($formula, self::FORMULA_STRING_QUOTE) !== false) { // So instead we skip replacing in any quoted strings by only replacing in every other array element after we've exploded // the formula - $temp = explode('"', $formula); + $temp = explode(self::FORMULA_STRING_QUOTE, $formula); // Open and Closed counts used for trapping mismatched braces in the formula $openCount = $closeCount = 0; $i = false; foreach ($temp as &$value) { // Only count/replace in alternating array entries if ($i = !$i) { - $openCount += substr_count($value, '{'); - $closeCount += substr_count($value, '}'); + $openCount += substr_count($value, self::FORMULA_OPEN_FUNCTION_BRACE); + $closeCount += substr_count($value, self::FORMULA_CLOSE_FUNCTION_BRACE); $value = str_replace($matrixReplaceFrom, $matrixReplaceTo, $value); } } unset($value); // Then rebuild the formula string - $formula = implode('"', $temp); + $formula = implode(self::FORMULA_STRING_QUOTE, $temp); } else { // If there's no quoted strings, then we do a simple count/replace - $openCount = substr_count($formula, '{'); - $closeCount = substr_count($formula, '}'); + $openCount = substr_count($formula, self::FORMULA_OPEN_FUNCTION_BRACE); + $closeCount = substr_count($formula, self::FORMULA_CLOSE_FUNCTION_BRACE); $formula = str_replace($matrixReplaceFrom, $matrixReplaceTo, $formula); } // Trap for mismatched braces and trigger an appropriate error @@ -3715,9 +3719,9 @@ class Calculation } $localeConstant = false; - if ($opCharacter == '"') { + if ($opCharacter == self::FORMULA_STRING_QUOTE) { // UnEscape any quotes within the string - $val = self::wrapResult(str_replace('""', '"', self::unwrapResult($val))); + $val = self::wrapResult(str_replace('""', self::FORMULA_STRING_QUOTE, self::unwrapResult($val))); } elseif (is_numeric($val)) { if ((strpos($val, '.') !== false) || (stripos($val, 'e') !== false) || ($val > PHP_INT_MAX) || ($val < -PHP_INT_MAX)) { $val = (float) $val; @@ -4058,7 +4062,7 @@ class Calculation $result = '#VALUE!'; } } else { - $result = '"' . str_replace('""', '"', self::unwrapResult($operand1) . self::unwrapResult($operand2)) . '"'; + $result = self::FORMULA_STRING_QUOTE . str_replace('""', self::FORMULA_STRING_QUOTE, self::unwrapResult($operand1) . self::unwrapResult($operand2)) . self::FORMULA_STRING_QUOTE; } $this->debugLog->writeDebugLog('Evaluation Result is ', $this->showTypeDetails($result)); $stack->push('Value', $result); @@ -4078,9 +4082,15 @@ class Calculation $cellIntersect[$row] = array_intersect_key($operand1[$row], $operand2[$row]); } } - $cellRef = Coordinate::stringFromColumnIndex(min($oCol) + 1) . min($oRow) . ':' . Coordinate::stringFromColumnIndex(max($oCol) + 1) . max($oRow); - $this->debugLog->writeDebugLog('Evaluation Result is ', $this->showTypeDetails($cellIntersect)); - $stack->push('Value', $cellIntersect, $cellRef); + if (count(Functions::flattenArray($cellIntersect)) === 0) { + $this->debugLog->writeDebugLog('Evaluation Result is ', $this->showTypeDetails($cellIntersect)); + $stack->push('Error', Functions::null(), null); + } else { + $cellRef = Coordinate::stringFromColumnIndex(min($oCol) + 1) . min($oRow) . ':' . + Coordinate::stringFromColumnIndex(max($oCol) + 1) . max($oRow); + $this->debugLog->writeDebugLog('Evaluation Result is ', $this->showTypeDetails($cellIntersect)); + $stack->push('Value', $cellIntersect, $cellRef); + } break; } @@ -4284,7 +4294,7 @@ class Calculation $branchStore[$storeKey] = self::$excelConstants[$excelConstant]; } $this->debugLog->writeDebugLog('Evaluating Constant ', $excelConstant, ' as ', $this->showTypeDetails(self::$excelConstants[$excelConstant])); - } elseif ((is_numeric($token)) || ($token === null) || (is_bool($token)) || ($token == '') || ($token[0] == '"') || ($token[0] == '#')) { + } elseif ((is_numeric($token)) || ($token === null) || (is_bool($token)) || ($token == '') || ($token[0] == self::FORMULA_STRING_QUOTE) || ($token[0] == '#')) { $stack->push('Value', $token); if (isset($storeKey)) { $branchStore[$storeKey] = $token; @@ -4329,7 +4339,7 @@ class Calculation if (is_string($operand)) { // We only need special validations for the operand if it is a string // Start by stripping off the quotation marks we use to identify true excel string values internally - if ($operand > '' && $operand[0] == '"') { + if ($operand > '' && $operand[0] == self::FORMULA_STRING_QUOTE) { $operand = self::unwrapResult($operand); } // If the string is a numeric value, we treat it as a numeric, so no further testing @@ -4342,7 +4352,7 @@ class Calculation return false; } elseif (!Shared\StringHelper::convertToNumberIfFraction($operand)) { // If not a numeric or a fraction, then it's a text string, and so can't be used in mathematical binary operations - $stack->push('Value', '#VALUE!'); + $stack->push('Error', '#VALUE!'); $this->debugLog->writeDebugLog('Evaluation Result is a ', $this->showTypeDetails('#VALUE!')); return false; @@ -4402,10 +4412,10 @@ class Calculation } // Simple validate the two operands if they are string values - if (is_string($operand1) && $operand1 > '' && $operand1[0] == '"') { + if (is_string($operand1) && $operand1 > '' && $operand1[0] == self::FORMULA_STRING_QUOTE) { $operand1 = self::unwrapResult($operand1); } - if (is_string($operand2) && $operand2 > '' && $operand2[0] == '"') { + if (is_string($operand2) && $operand2 > '' && $operand2[0] == self::FORMULA_STRING_QUOTE) { $operand2 = self::unwrapResult($operand2); } @@ -4570,7 +4580,7 @@ class Calculation case '/': if ($operand2 == 0) { // Trap for Divide by Zero error - $stack->push('Value', '#DIV/0!'); + $stack->push('Error', '#DIV/0!'); $this->debugLog->writeDebugLog('Evaluation Result is ', $this->showTypeDetails('#DIV/0!')); return false; diff --git a/src/PhpSpreadsheet/Calculation/MathTrig.php b/src/PhpSpreadsheet/Calculation/MathTrig.php index d92ba404..85f94cc2 100644 --- a/src/PhpSpreadsheet/Calculation/MathTrig.php +++ b/src/PhpSpreadsheet/Calculation/MathTrig.php @@ -1339,6 +1339,8 @@ class MathTrig // Is it a numeric value? if ((is_numeric($arg)) && (!is_string($arg))) { $returnValue += $arg; + } elseif (Functions::isError($arg)) { + return $arg; } } diff --git a/tests/PhpSpreadsheetTests/Calculation/Engine/RangeTest.php b/tests/PhpSpreadsheetTests/Calculation/Engine/RangeTest.php index 7dfda9ca..84cac747 100644 --- a/tests/PhpSpreadsheetTests/Calculation/Engine/RangeTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/Engine/RangeTest.php @@ -2,6 +2,7 @@ namespace PhpOffice\PhpSpreadsheetTests\Calculation\Engine; +use PhpOffice\PhpSpreadsheet\Calculation\Functions; use PhpOffice\PhpSpreadsheet\Spreadsheet; use PHPUnit\Framework\TestCase; @@ -45,7 +46,7 @@ class RangeTest extends TestCase ['=SUM(A1:B3,A1:C2)', 48], ['=SUM(A1:B3 A1:C2)', 12], ['=SUM(A1:A3,C1:C3)', 30], - ['=SUM(A1:A3 C1:C3)', 0], + ['=SUM(A1:A3 C1:C3)', Functions::null()], ['=SUM(A1:B2,B2:C3)', 40], ['=SUM(A1:B2 B2:C3)', 5], ]; From 84e03da5c7faba00034d5f4189ba7c16d58ca721 Mon Sep 17 00:00:00 2001 From: oleibman Date: Sun, 24 May 2020 03:51:28 -0700 Subject: [PATCH 07/12] Code Coverage for Shared\CodePage (#1491) While investigating something else in Shared, I noticed that CodePage had poor test coverage and a high complexity rating. This change addresses both; Scrutinizer would love it, although its interface on GitHub seems broken at the moment (all PRs show "Waiting for External Code Coverage"). --- src/PhpSpreadsheet/Shared/CodePage.php | 184 +++++++----------- .../Shared/CodePageTest.php | 16 ++ tests/data/Shared/CodePage.php | 65 ++++++- 3 files changed, 146 insertions(+), 119 deletions(-) diff --git a/src/PhpSpreadsheet/Shared/CodePage.php b/src/PhpSpreadsheet/Shared/CodePage.php index b395293c..97cbfbbe 100644 --- a/src/PhpSpreadsheet/Shared/CodePage.php +++ b/src/PhpSpreadsheet/Shared/CodePage.php @@ -6,6 +6,65 @@ use PhpOffice\PhpSpreadsheet\Exception as PhpSpreadsheetException; class CodePage { + private static $pageArray = [ + 0 => 'CP1252', // CodePage is not always correctly set when the xls file was saved by Apple's Numbers program + 367 => 'ASCII', // ASCII + 437 => 'CP437', // OEM US + //720 => 'notsupported', // OEM Arabic + 737 => 'CP737', // OEM Greek + 775 => 'CP775', // OEM Baltic + 850 => 'CP850', // OEM Latin I + 852 => 'CP852', // OEM Latin II (Central European) + 855 => 'CP855', // OEM Cyrillic + 857 => 'CP857', // OEM Turkish + 858 => 'CP858', // OEM Multilingual Latin I with Euro + 860 => 'CP860', // OEM Portugese + 861 => 'CP861', // OEM Icelandic + 862 => 'CP862', // OEM Hebrew + 863 => 'CP863', // OEM Canadian (French) + 864 => 'CP864', // OEM Arabic + 865 => 'CP865', // OEM Nordic + 866 => 'CP866', // OEM Cyrillic (Russian) + 869 => 'CP869', // OEM Greek (Modern) + 874 => 'CP874', // ANSI Thai + 932 => 'CP932', // ANSI Japanese Shift-JIS + 936 => 'CP936', // ANSI Chinese Simplified GBK + 949 => 'CP949', // ANSI Korean (Wansung) + 950 => 'CP950', // ANSI Chinese Traditional BIG5 + 1200 => 'UTF-16LE', // UTF-16 (BIFF8) + 1250 => 'CP1250', // ANSI Latin II (Central European) + 1251 => 'CP1251', // ANSI Cyrillic + 1252 => 'CP1252', // ANSI Latin I (BIFF4-BIFF7) + 1253 => 'CP1253', // ANSI Greek + 1254 => 'CP1254', // ANSI Turkish + 1255 => 'CP1255', // ANSI Hebrew + 1256 => 'CP1256', // ANSI Arabic + 1257 => 'CP1257', // ANSI Baltic + 1258 => 'CP1258', // ANSI Vietnamese + 1361 => 'CP1361', // ANSI Korean (Johab) + 10000 => 'MAC', // Apple Roman + 10001 => 'CP932', // Macintosh Japanese + 10002 => 'CP950', // Macintosh Chinese Traditional + 10003 => 'CP1361', // Macintosh Korean + 10004 => 'MACARABIC', // Apple Arabic + 10005 => 'MACHEBREW', // Apple Hebrew + 10006 => 'MACGREEK', // Macintosh Greek + 10007 => 'MACCYRILLIC', // Macintosh Cyrillic + 10008 => 'CP936', // Macintosh - Simplified Chinese (GB 2312) + 10010 => 'MACROMANIA', // Macintosh Romania + 10017 => 'MACUKRAINE', // Macintosh Ukraine + 10021 => 'MACTHAI', // Macintosh Thai + 10029 => 'MACCENTRALEUROPE', // Macintosh Central Europe + 10079 => 'MACICELAND', // Macintosh Icelandic + 10081 => 'MACTURKISH', // Macintosh Turkish + 10082 => 'MACCROATIAN', // Macintosh Croatian + 21010 => 'UTF-16LE', // UTF-16 (BIFF8) This isn't correct, but some Excel writer libraries erroneously use Codepage 21010 for UTF-16LE + 32768 => 'MAC', // Apple Roman + //32769 => 'unsupported', // ANSI Latin I (BIFF2-BIFF3) + 65000 => 'UTF-7', // Unicode (UTF-7) + 65001 => 'UTF-8', // Unicode (UTF-8) + ]; + /** * Convert Microsoft Code Page Identifier to Code Page Name which iconv * and mbstring understands. @@ -14,123 +73,20 @@ class CodePage * * @return string Code Page Name */ - public static function numberToName($codePage) + public static function numberToName(int $codePage): string { - switch ($codePage) { - case 367: - return 'ASCII'; // ASCII - case 437: - return 'CP437'; // OEM US - case 720: - throw new PhpSpreadsheetException('Code page 720 not supported.'); // OEM Arabic - case 737: - return 'CP737'; // OEM Greek - case 775: - return 'CP775'; // OEM Baltic - case 850: - return 'CP850'; // OEM Latin I - case 852: - return 'CP852'; // OEM Latin II (Central European) - case 855: - return 'CP855'; // OEM Cyrillic - case 857: - return 'CP857'; // OEM Turkish - case 858: - return 'CP858'; // OEM Multilingual Latin I with Euro - case 860: - return 'CP860'; // OEM Portugese - case 861: - return 'CP861'; // OEM Icelandic - case 862: - return 'CP862'; // OEM Hebrew - case 863: - return 'CP863'; // OEM Canadian (French) - case 864: - return 'CP864'; // OEM Arabic - case 865: - return 'CP865'; // OEM Nordic - case 866: - return 'CP866'; // OEM Cyrillic (Russian) - case 869: - return 'CP869'; // OEM Greek (Modern) - case 874: - return 'CP874'; // ANSI Thai - case 932: - return 'CP932'; // ANSI Japanese Shift-JIS - case 936: - return 'CP936'; // ANSI Chinese Simplified GBK - case 949: - return 'CP949'; // ANSI Korean (Wansung) - case 950: - return 'CP950'; // ANSI Chinese Traditional BIG5 - case 1200: - return 'UTF-16LE'; // UTF-16 (BIFF8) - case 1250: - return 'CP1250'; // ANSI Latin II (Central European) - case 1251: - return 'CP1251'; // ANSI Cyrillic - case 0: - // CodePage is not always correctly set when the xls file was saved by Apple's Numbers program - case 1252: - return 'CP1252'; // ANSI Latin I (BIFF4-BIFF7) - case 1253: - return 'CP1253'; // ANSI Greek - case 1254: - return 'CP1254'; // ANSI Turkish - case 1255: - return 'CP1255'; // ANSI Hebrew - case 1256: - return 'CP1256'; // ANSI Arabic - case 1257: - return 'CP1257'; // ANSI Baltic - case 1258: - return 'CP1258'; // ANSI Vietnamese - case 1361: - return 'CP1361'; // ANSI Korean (Johab) - case 10000: - return 'MAC'; // Apple Roman - case 10001: - return 'CP932'; // Macintosh Japanese - case 10002: - return 'CP950'; // Macintosh Chinese Traditional - case 10003: - return 'CP1361'; // Macintosh Korean - case 10004: - return 'MACARABIC'; // Apple Arabic - case 10005: - return 'MACHEBREW'; // Apple Hebrew - case 10006: - return 'MACGREEK'; // Macintosh Greek - case 10007: - return 'MACCYRILLIC'; // Macintosh Cyrillic - case 10008: - return 'CP936'; // Macintosh - Simplified Chinese (GB 2312) - case 10010: - return 'MACROMANIA'; // Macintosh Romania - case 10017: - return 'MACUKRAINE'; // Macintosh Ukraine - case 10021: - return 'MACTHAI'; // Macintosh Thai - case 10029: - return 'MACCENTRALEUROPE'; // Macintosh Central Europe - case 10079: - return 'MACICELAND'; // Macintosh Icelandic - case 10081: - return 'MACTURKISH'; // Macintosh Turkish - case 10082: - return 'MACCROATIAN'; // Macintosh Croatian - case 21010: - return 'UTF-16LE'; // UTF-16 (BIFF8) This isn't correct, but some Excel writer libraries erroneously use Codepage 21010 for UTF-16LE - case 32768: - return 'MAC'; // Apple Roman - case 32769: - throw new PhpSpreadsheetException('Code page 32769 not supported.'); // ANSI Latin I (BIFF2-BIFF3) - case 65000: - return 'UTF-7'; // Unicode (UTF-7) - case 65001: - return 'UTF-8'; // Unicode (UTF-8) + if (array_key_exists($codePage, self::$pageArray)) { + return self::$pageArray[$codePage]; + } + if ($codePage == 720 || $codePage == 32769) { + throw new PhpSpreadsheetException("Code page $codePage not supported."); // OEM Arabic } throw new PhpSpreadsheetException('Unknown codepage: ' . $codePage); } + + public static function getEncodings(): array + { + return self::$pageArray; + } } diff --git a/tests/PhpSpreadsheetTests/Shared/CodePageTest.php b/tests/PhpSpreadsheetTests/Shared/CodePageTest.php index b86f9015..2bdbda72 100644 --- a/tests/PhpSpreadsheetTests/Shared/CodePageTest.php +++ b/tests/PhpSpreadsheetTests/Shared/CodePageTest.php @@ -24,6 +24,22 @@ class CodePageTest extends TestCase return require 'tests/data/Shared/CodePage.php'; } + public function testCoverage(): void + { + $covered = []; + $expected = CodePage::getEncodings(); + foreach ($expected as $key => $val) { + $covered[$key] = 0; + } + $tests = $this->providerCodePage(); + foreach ($tests as $test) { + $covered[$test[1]] = 1; + } + foreach ($covered as $key => $val) { + self::assertEquals(1, $val, "Codepage $key not tested"); + } + } + public function testNumberToNameWithInvalidCodePage(): void { $invalidCodePage = 12345; diff --git a/tests/data/Shared/CodePage.php b/tests/data/Shared/CodePage.php index 1cf09d88..82bb23e4 100644 --- a/tests/data/Shared/CodePage.php +++ b/tests/data/Shared/CodePage.php @@ -1,6 +1,11 @@ Date: Sun, 24 May 2020 03:54:59 -0700 Subject: [PATCH 08/12] Restore working directory if test fails (#1490) This test changes directory then performs an assertion. No problem if the assertion succeeds. I was a little concerned about what would happen if the assertion fails, leaving us in the new directory. So I have changed test to use setUp/tearDown to ensure that we end up where we started. --- .../Writer/Html/ImagesRootTest.php | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tests/PhpSpreadsheetTests/Writer/Html/ImagesRootTest.php b/tests/PhpSpreadsheetTests/Writer/Html/ImagesRootTest.php index 5c887d53..6cc7f18f 100644 --- a/tests/PhpSpreadsheetTests/Writer/Html/ImagesRootTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Html/ImagesRootTest.php @@ -9,6 +9,18 @@ use PhpOffice\PhpSpreadsheetTests\Functional; class ImagesRootTest extends Functional\AbstractFunctional { + private $curdir; + + protected function setUp(): void + { + $this->curdir = getcwd(); + } + + protected function tearDown(): void + { + chdir($this->curdir); + } + public function testImagesRoot(): void { $spreadsheet = new Spreadsheet(); @@ -20,7 +32,6 @@ class ImagesRootTest extends Functional\AbstractFunctional $newdir = __DIR__ . '/../../../data/Reader/HTML'; $stub = 'image.jpg'; $imagePath = "./$stub"; - $curdir = getcwd(); chdir($newdir); self::assertFileExists($imagePath); $drawing->setPath($imagePath); @@ -34,7 +45,6 @@ class ImagesRootTest extends Functional\AbstractFunctional $writer = new Html($spreadsheet); $writer->setImagesRoot($root); $html = $writer->generateHTMLAll(); - chdir($curdir); $dom = new DOMDocument(); $dom->loadHTML($html); $body = $dom->getElementsByTagName('body')[0]; From 41b95c15422a3a2cc4dc35dd25214c1b07353bb1 Mon Sep 17 00:00:00 2001 From: oleibman Date: Sun, 24 May 2020 03:57:39 -0700 Subject: [PATCH 09/12] CSV Sample File Was Miscoded (#1489) File author erroneously assumed that backslash was used to escape quotes in CSV; in fact, doubling the quote is used for escape. The test still worked, but mainly because the content of the cell with the escape wasn't tested. The file is now fixed, and a new test added. --- tests/PhpSpreadsheetTests/Reader/CsvTest.php | 31 +++++++++++++++++ .../Reader/CSV/line_break_escaped_32le.csv | Bin 0 -> 2120 bytes ...break_in_enclosure_with_escaped_quotes.csv | 32 +++++++++--------- 3 files changed, 47 insertions(+), 16 deletions(-) create mode 100644 tests/data/Reader/CSV/line_break_escaped_32le.csv diff --git a/tests/PhpSpreadsheetTests/Reader/CsvTest.php b/tests/PhpSpreadsheetTests/Reader/CsvTest.php index cb2b6196..e4ccd931 100644 --- a/tests/PhpSpreadsheetTests/Reader/CsvTest.php +++ b/tests/PhpSpreadsheetTests/Reader/CsvTest.php @@ -200,6 +200,37 @@ EOF; self::assertEquals($expected, $sheet->getCell('B3')->getValue()); } + public function testLineBreakEscape(): void + { + $reader = new Csv(); + $spreadsheet = $reader->load('tests/data/Reader/CSV/line_break_in_enclosure_with_escaped_quotes.csv'); + $sheet = $spreadsheet->getActiveSheet(); + $expected = <<getCell('B3')->getValue()); + } + + public function testUtf32LineBreakEscape(): void + { + $reader = new Csv(); + $reader->setInputEncoding('UTF-32LE'); + $spreadsheet = $reader->load('tests/data/Reader/CSV/line_break_escaped_32le.csv'); + $sheet = $spreadsheet->getActiveSheet(); + $expected = <<getCell('B3')->getValue()); + } + public function testSeparatorLine(): void { $reader = new Csv(); diff --git a/tests/data/Reader/CSV/line_break_escaped_32le.csv b/tests/data/Reader/CSV/line_break_escaped_32le.csv new file mode 100644 index 0000000000000000000000000000000000000000..8e0f0243ecadcb1773684cf9c78913d7dbcc310a GIT binary patch literal 2120 zcmeH_NeTin5Jj{06v4IV-mP14A?*R$S)6CQz3+hqjo=B);?YUv*H2ZtMk%El_OO5s z4B!YSc!9AAj9~~Zs6hP{ZlIfX@CGy6u+9Jg literal 0 HcmV?d00001 diff --git a/tests/data/Reader/CSV/line_break_in_enclosure_with_escaped_quotes.csv b/tests/data/Reader/CSV/line_break_in_enclosure_with_escaped_quotes.csv index e84db1b5..01ce36a6 100644 --- a/tests/data/Reader/CSV/line_break_in_enclosure_with_escaped_quotes.csv +++ b/tests/data/Reader/CSV/line_break_in_enclosure_with_escaped_quotes.csv @@ -1,21 +1,21 @@ Name,Copy,URL -Test,"This is a \"test csv file\" -with both \"line breaks\" -and \"escaped -quotes\" that breaks +Test,"This is a ""test csv file"" +with both ""line breaks"" +and ""escaped +quotes"" that breaks the delimiters",http://google.com -Test,"This is a \"test csv file\" -with both \"line breaks\" -and \"escaped -quotes\" that breaks +Test,"This is a ""test csv file"" +with both ""line breaks"" +and ""escaped +quotes"" that breaks the delimiters",http://google.com -Test,"This is a \"test csv file\" -with both \"line breaks\" -and \"escaped -quotes\" that breaks +Test,"This is a ""test csv file"" +with both ""line breaks"" +and ""escaped +quotes"" that breaks the delimiters",http://google.com -Test,"This is a \"test csv file\" -with both \"line breaks\" -and \"escaped -quotes\" that breaks +Test,"This is a ""test csv file"" +with both ""line breaks"" +and ""escaped +quotes"" that breaks the delimiters",http://google.com From 585409a9496912983f6722c58f19fbd7527b4227 Mon Sep 17 00:00:00 2001 From: oleibman Date: Sun, 24 May 2020 04:03:07 -0700 Subject: [PATCH 10/12] Testing - Delete Temp Files When No Longer Needed (#1488) No code changes. The tests in all of these scripts write to at least one temporary file, which is then read and not used again. The file should be deleted to avoid filling up the disk system. --- samples/Basic/07_Reader.php | 1 + samples/Basic/16_Csv.php | 12 ++++++++---- samples/Basic/20_Read_Xls.php | 1 + samples/Basic/24_Readfilter.php | 4 +++- samples/Basic/28_Iterator.php | 9 +++++---- samples/Basic/44_Worksheet_info.php | 2 ++ samples/Chart/34_Chart_update.php | 12 +++++++----- .../Calculation/DefinedNameConfusedForCellTest.php | 1 + tests/PhpSpreadsheetTests/Reader/Xlsx2Test.php | 10 ++-------- tests/PhpSpreadsheetTests/Reader/XlsxTest.php | 1 + .../Writer/Html/InvalidFileNameTest.php | 13 +++++++++++-- .../Writer/Xls/FormulaErrTest.php | 9 +-------- .../Writer/Xlsx/FloatsRetainedTest.php | 1 + 13 files changed, 44 insertions(+), 32 deletions(-) diff --git a/samples/Basic/07_Reader.php b/samples/Basic/07_Reader.php index 4d9bd79e..67b3ae56 100644 --- a/samples/Basic/07_Reader.php +++ b/samples/Basic/07_Reader.php @@ -17,3 +17,4 @@ $helper->logRead('Xlsx', $filename, $callStartTime); // Save $helper->write($spreadsheet, __FILE__); +unlink($filename); diff --git a/samples/Basic/16_Csv.php b/samples/Basic/16_Csv.php index de753d56..15bbf0d4 100644 --- a/samples/Basic/16_Csv.php +++ b/samples/Basic/16_Csv.php @@ -1,13 +1,15 @@ log('Write to CSV format'); /** @var \PhpOffice\PhpSpreadsheet\Writer\Csv $writer */ -$writer = IOFactory::createWriter($spreadsheet, 'Csv')->setDelimiter(',') +$writer = new CsvWriter($spreadsheet); +$writer->setDelimiter(',') ->setEnclosure('"') ->setSheetIndex(0); @@ -19,13 +21,15 @@ $helper->logWrite($writer, $filename, $callStartTime); $helper->log('Read from CSV format'); /** @var \PhpOffice\PhpSpreadsheet\Reader\Csv $reader */ -$reader = IOFactory::createReader('Csv')->setDelimiter(',') +$reader = new CsvReader(); +$reader->setDelimiter(',') ->setEnclosure('"') ->setSheetIndex(0); $callStartTime = microtime(true); $spreadsheetFromCSV = $reader->load($filename); $helper->logRead('Csv', $filename, $callStartTime); +unlink($filename); // Write Xlsx $helper->write($spreadsheetFromCSV, __FILE__, ['Xlsx']); @@ -33,7 +37,7 @@ $helper->write($spreadsheetFromCSV, __FILE__, ['Xlsx']); // Write CSV $filenameCSV = $helper->getFilename(__FILE__, 'csv'); /** @var \PhpOffice\PhpSpreadsheet\Writer\Csv $writerCSV */ -$writerCSV = IOFactory::createWriter($spreadsheetFromCSV, 'Csv'); +$writerCSV = new CsvWriter($spreadsheetFromCSV); $writerCSV->setExcelCompatibility(true); $callStartTime = microtime(true); diff --git a/samples/Basic/20_Read_Xls.php b/samples/Basic/20_Read_Xls.php index 9e5fa014..daeaf664 100644 --- a/samples/Basic/20_Read_Xls.php +++ b/samples/Basic/20_Read_Xls.php @@ -17,6 +17,7 @@ $helper->logWrite($writer, $filename, $callStartTime); $callStartTime = microtime(true); $spreadsheet = IOFactory::load($filename); $helper->logRead('Xls', $filename, $callStartTime); +unlink($filename); // Save $helper->write($spreadsheet, __FILE__); diff --git a/samples/Basic/24_Readfilter.php b/samples/Basic/24_Readfilter.php index 844996f2..ab1c2e41 100644 --- a/samples/Basic/24_Readfilter.php +++ b/samples/Basic/24_Readfilter.php @@ -3,6 +3,7 @@ namespace PhpOffice\PhpSpreadsheet; use PhpOffice\PhpSpreadsheet\Reader\IReadFilter; +use PhpOffice\PhpSpreadsheet\Reader\Xlsx as XlsxReader; use PhpOffice\PhpSpreadsheet\Writer\Xlsx; require __DIR__ . '/../Header.php'; @@ -29,10 +30,11 @@ class MyReadFilter implements IReadFilter } $helper->log('Load from Xlsx file'); -$reader = IOFactory::createReader('Xlsx'); +$reader = new XlsxReader(); $reader->setReadFilter(new MyReadFilter()); $callStartTime = microtime(true); $spreadsheet = $reader->load($filename); +unlink($filename); $helper->logRead('Xlsx', $filename, $callStartTime); $helper->log('Remove unnecessary rows'); $spreadsheet->getActiveSheet()->removeRow(2, 18); diff --git a/samples/Basic/28_Iterator.php b/samples/Basic/28_Iterator.php index 4aec7a92..104dc47f 100644 --- a/samples/Basic/28_Iterator.php +++ b/samples/Basic/28_Iterator.php @@ -1,21 +1,22 @@ getTemporaryFilename(); -$writer = new Xlsx($sampleSpreadsheet); +$writer = new XlsxWriter($sampleSpreadsheet); $callStartTime = microtime(true); $writer->save($filename); $helper->logWrite($writer, $filename, $callStartTime); $callStartTime = microtime(true); -$reader = IOFactory::createReader('Xlsx'); +$reader = new XlsxReader(); $spreadsheet = $reader->load($filename); $helper->logRead('Xlsx', $filename, $callStartTime); +unlink($filename); $helper->log('Iterate worksheets'); foreach ($spreadsheet->getWorksheetIterator() as $worksheet) { $helper->log('Worksheet - ' . $worksheet->getTitle()); diff --git a/samples/Basic/44_Worksheet_info.php b/samples/Basic/44_Worksheet_info.php index 33c0cd05..57822369 100644 --- a/samples/Basic/44_Worksheet_info.php +++ b/samples/Basic/44_Worksheet_info.php @@ -24,3 +24,5 @@ var_dump($sheetList); $helper->log('Worksheet Names:'); var_dump($sheetInfo); + +unlink($filename); diff --git a/samples/Chart/34_Chart_update.php b/samples/Chart/34_Chart_update.php index a4287927..5d725c49 100644 --- a/samples/Chart/34_Chart_update.php +++ b/samples/Chart/34_Chart_update.php @@ -1,20 +1,22 @@ getTemporaryFilename(); -$writer = new Xlsx($sampleSpreadsheet); +$writer = new XlsxWriter($sampleSpreadsheet); +$writer->setIncludeCharts(true); $writer->save($filename); $helper->log('Load from Xlsx file'); -$reader = IOFactory::createReader('Xlsx'); +$reader = new XlsxReader(); $reader->setIncludeCharts(true); $spreadsheet = $reader->load($filename); +unlink($filename); $helper->log('Update cell data values that are displayed in the chart'); $worksheet = $spreadsheet->getActiveSheet(); @@ -31,7 +33,7 @@ $worksheet->fromArray( // Save Excel 2007 file $filename = $helper->getFilename(__FILE__); -$writer = IOFactory::createWriter($spreadsheet, 'Xlsx'); +$writer = new XlsxWriter($spreadsheet); $writer->setIncludeCharts(true); $callStartTime = microtime(true); $writer->save($filename); diff --git a/tests/PhpSpreadsheetTests/Calculation/DefinedNameConfusedForCellTest.php b/tests/PhpSpreadsheetTests/Calculation/DefinedNameConfusedForCellTest.php index 12d73356..76886c23 100644 --- a/tests/PhpSpreadsheetTests/Calculation/DefinedNameConfusedForCellTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/DefinedNameConfusedForCellTest.php @@ -18,5 +18,6 @@ class DefinedNameConfusedForCellTest extends TestCase $filename = tempnam(File::sysGetTempDir(), 'phpspreadsheet-test'); $writer->save($filename); self::assertTrue(true); + unlink($filename); } } diff --git a/tests/PhpSpreadsheetTests/Reader/Xlsx2Test.php b/tests/PhpSpreadsheetTests/Reader/Xlsx2Test.php index 4d0b6a8a..1220c378 100644 --- a/tests/PhpSpreadsheetTests/Reader/Xlsx2Test.php +++ b/tests/PhpSpreadsheetTests/Reader/Xlsx2Test.php @@ -11,14 +11,6 @@ use PHPUnit\Framework\TestCase; class Xlsx2Test extends TestCase { - protected function tearDown(): void - { - $outfile = tempnam(File::sysGetTempDir(), 'phpspreadsheet-test'); - if (file_exists($outfile)) { - unlink($outfile); - } - } - public function testLoadXlsxConditionalFormatting2(): void { // Make sure Conditionals are read correctly from existing file @@ -63,6 +55,7 @@ class Xlsx2Test extends TestCase $writer = IOFactory::createWriter($spreadshee1, 'Xlsx'); $writer->save($outfile); $spreadsheet = $reader->load($outfile); + unlink($outfile); $worksheet = $spreadsheet->getActiveSheet(); $conditionalStyle = $worksheet->getConditionalStyles('A2:A8'); @@ -110,6 +103,7 @@ class Xlsx2Test extends TestCase $writer->save($outfile); $reader = IOFactory::createReader('Xlsx'); $spreadsheet = $reader->load($outfile); + unlink($outfile); $worksheet = $spreadsheet->getActiveSheet(); $conditionalStyle = $worksheet->getConditionalStyles('A1:A6'); diff --git a/tests/PhpSpreadsheetTests/Reader/XlsxTest.php b/tests/PhpSpreadsheetTests/Reader/XlsxTest.php index 9e0b5f66..b326c142 100644 --- a/tests/PhpSpreadsheetTests/Reader/XlsxTest.php +++ b/tests/PhpSpreadsheetTests/Reader/XlsxTest.php @@ -222,6 +222,7 @@ class XlsxTest extends TestCase $writer = new \PhpOffice\PhpSpreadsheet\Writer\Xlsx($excel); $writer->save($resultFilename); $excel = $reader->load($resultFilename); + unlink($resultFilename); // Fake assert. The only thing we need is to ensure the file is loaded without exception self::assertNotNull($excel); } diff --git a/tests/PhpSpreadsheetTests/Writer/Html/InvalidFileNameTest.php b/tests/PhpSpreadsheetTests/Writer/Html/InvalidFileNameTest.php index 7baa2338..36a40e96 100644 --- a/tests/PhpSpreadsheetTests/Writer/Html/InvalidFileNameTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Html/InvalidFileNameTest.php @@ -30,9 +30,8 @@ class InvalidFileNameTest extends Functional\AbstractFunctional $writer->save(''); } - public function testEmptyTempdirNamePdf(): void + public function testNotEmptyTempdirNamePdf(): void { - $this->expectException(WriterException::class); $spreadsheet = new Spreadsheet(); $spreadsheet->getActiveSheet()->getCell('A1')->setValue('Cell 1'); $writer = new Mpdf($spreadsheet); @@ -41,6 +40,16 @@ class InvalidFileNameTest extends Functional\AbstractFunctional $writer->setPaperSize(PageSetup::PAPERSIZE_LEDGER); self::assertEquals($writer->getPaperSize(), PageSetup::PAPERSIZE_LEDGER); self::assertEquals(File::sysGetTempDir() . '/phpsppdf', $writer->getTempDir()); + $writer->setTempDir(File::sysGetTempDir()); + self::assertEquals(File::sysGetTempDir(), $writer->getTempDir()); + } + + public function testEmptyTempdirNamePdf(): void + { + $this->expectException(WriterException::class); + $spreadsheet = new Spreadsheet(); + $spreadsheet->getActiveSheet()->getCell('A1')->setValue('Cell 1'); + $writer = new Mpdf($spreadsheet); $writer->setTempDir(''); } diff --git a/tests/PhpSpreadsheetTests/Writer/Xls/FormulaErrTest.php b/tests/PhpSpreadsheetTests/Writer/Xls/FormulaErrTest.php index 61c70cb6..21a7c928 100644 --- a/tests/PhpSpreadsheetTests/Writer/Xls/FormulaErrTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Xls/FormulaErrTest.php @@ -9,14 +9,6 @@ use PHPUnit\Framework\TestCase; class FormulaErrTest extends TestCase { - protected function tearDown(): void - { - $filename = tempnam(File::sysGetTempDir(), 'phpspreadsheet-test'); - if (file_exists($filename)) { - unlink($filename); - } - } - public function testFormulaError(): void { $obj = new \PhpOffice\PhpSpreadsheet\Spreadsheet(); @@ -31,6 +23,7 @@ class FormulaErrTest extends TestCase $writer->save($filename); $reader = IOFactory::createReader('Xls'); $robj = $reader->load($filename); + unlink($filename); $sheet0 = $robj->setActiveSheetIndex(0); $a1 = $sheet0->getCell('A1')->getCalculatedValue(); self::assertEquals(2, $a1); diff --git a/tests/PhpSpreadsheetTests/Writer/Xlsx/FloatsRetainedTest.php b/tests/PhpSpreadsheetTests/Writer/Xlsx/FloatsRetainedTest.php index aad074de..746b9846 100644 --- a/tests/PhpSpreadsheetTests/Writer/Xlsx/FloatsRetainedTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Xlsx/FloatsRetainedTest.php @@ -28,6 +28,7 @@ class FloatsRetainedTest extends TestCase $reader = new Reader(); $sheet = $reader->load($outputFilename); + unlink($outputFilename); self::assertSame($value, $sheet->getActiveSheet()->getCell('A1')->getValue()); } From 5dd7e883c6bd409063882de2cde1a07a556f25be Mon Sep 17 00:00:00 2001 From: oleibman Date: Sun, 24 May 2020 11:02:39 -0700 Subject: [PATCH 11/12] Fix Issue 1441 (isDateTime and Formulas) (#1480) * Fix Issue 1441 (isDateTime and Formulas) When you have a date-field which is a formula, isDateTime returns false. https://github.com/PHPOffice/PhpSpreadsheet/issues/1441 Report makes sense; fixed as suggested. Also fixed a few minor related issues, and added tests so that Shared/Date and Shared/TimeZone are now completely covered. Date/setDefaultTimeZone and TimeZone/setTimeZone were not consistent about what to do in event of failure - return false or throw. They will now both return false, which is what Date's function said it would do in its doc block anyhow. Date/validateTimeZone will continue to throw; it was protected, but was never called outside Date, so I changed it to private. TimeZone/getTimeZoneAdjustment checked for 'UST' when it probably meant 'UTC', and, as it turns out, the check is not even needed. The most serious problem was that TimeZone/validateTimeZone does not check the backwards-compatible time zones. The timezone project aggressively, and very controversially, "demotes" timezones; such timezones eventually wind up in the PHP backwards-compatible list. We want to make sure to check that list so that our applications do not break when this happens. --- src/PhpSpreadsheet/Shared/Date.php | 25 ++++---- src/PhpSpreadsheet/Shared/TimeZone.php | 6 +- tests/PhpSpreadsheetTests/Shared/DateTest.php | 50 ++++++++++++++++ .../Shared/TimeZoneTest.php | 58 ++++++++++++++++++- tests/data/Shared/Date/FormatCodes.php | 4 ++ 5 files changed, 125 insertions(+), 18 deletions(-) diff --git a/src/PhpSpreadsheet/Shared/Date.php b/src/PhpSpreadsheet/Shared/Date.php index fd49c1ec..9dc99292 100644 --- a/src/PhpSpreadsheet/Shared/Date.php +++ b/src/PhpSpreadsheet/Shared/Date.php @@ -4,10 +4,10 @@ namespace PhpOffice\PhpSpreadsheet\Shared; use DateTimeInterface; use DateTimeZone; -use Exception; use PhpOffice\PhpSpreadsheet\Calculation\DateTime; use PhpOffice\PhpSpreadsheet\Calculation\Functions; use PhpOffice\PhpSpreadsheet\Cell\Cell; +use PhpOffice\PhpSpreadsheet\Exception as PhpSpreadsheetException; use PhpOffice\PhpSpreadsheet\Style\NumberFormat; class Date @@ -97,17 +97,18 @@ class Date * @param DateTimeZone|string $timeZone The timezone to set for all Excel datetimestamp to PHP DateTime Object conversions * * @return bool Success or failure - * @return bool Success or failure */ public static function setDefaultTimezone($timeZone) { - if ($timeZone = self::validateTimeZone($timeZone)) { + try { + $timeZone = self::validateTimeZone($timeZone); self::$defaultTimeZone = $timeZone; - - return true; + $retval = true; + } catch (PhpSpreadsheetException $e) { + $retval = false; } - return false; + return $retval; } /** @@ -130,17 +131,17 @@ class Date * @param DateTimeZone|string $timeZone The timezone to validate, either as a timezone string or object * * @return DateTimeZone The timezone as a timezone object - * @return DateTimeZone The timezone as a timezone object */ - protected static function validateTimeZone($timeZone) + private static function validateTimeZone($timeZone) { - if (is_object($timeZone) && $timeZone instanceof DateTimeZone) { + if ($timeZone instanceof DateTimeZone) { return $timeZone; - } elseif (is_string($timeZone)) { + } + if (in_array($timeZone, DateTimeZone::listIdentifiers(DateTimeZone::ALL_WITH_BC))) { return new DateTimeZone($timeZone); } - throw new Exception('Invalid timezone'); + throw new PhpSpreadsheetException('Invalid timezone'); } /** @@ -316,7 +317,7 @@ class Date */ public static function isDateTime(Cell $pCell) { - return is_numeric($pCell->getValue()) && + return is_numeric($pCell->getCalculatedValue()) && self::isDateTimeFormat( $pCell->getWorksheet()->getStyle( $pCell->getCoordinate() diff --git a/src/PhpSpreadsheet/Shared/TimeZone.php b/src/PhpSpreadsheet/Shared/TimeZone.php index a87987df..43fd3653 100644 --- a/src/PhpSpreadsheet/Shared/TimeZone.php +++ b/src/PhpSpreadsheet/Shared/TimeZone.php @@ -23,7 +23,7 @@ class TimeZone */ private static function validateTimeZone($timezone) { - return in_array($timezone, DateTimeZone::listIdentifiers()); + return in_array($timezone, DateTimeZone::listIdentifiers(DateTimeZone::ALL_WITH_BC)); } /** @@ -73,10 +73,6 @@ class TimeZone $timezone = self::$timezone; } - if ($timezone == 'UST') { - return 0; - } - $objTimezone = new DateTimeZone($timezone); $transitions = $objTimezone->getTransitions($timestamp, $timestamp); diff --git a/tests/PhpSpreadsheetTests/Shared/DateTest.php b/tests/PhpSpreadsheetTests/Shared/DateTest.php index 23b31076..7254635e 100644 --- a/tests/PhpSpreadsheetTests/Shared/DateTest.php +++ b/tests/PhpSpreadsheetTests/Shared/DateTest.php @@ -3,10 +3,23 @@ namespace PhpOffice\PhpSpreadsheetTests\Shared; use PhpOffice\PhpSpreadsheet\Shared\Date; +use PhpOffice\PhpSpreadsheet\Style\NumberFormat; use PHPUnit\Framework\TestCase; class DateTest extends TestCase { + private $dttimezone; + + protected function setUp(): void + { + $this->dttimezone = Date::getDefaultTimeZone(); + } + + protected function tearDown(): void + { + Date::setDefaultTimeZone($this->dttimezone); + } + public function testSetExcelCalendar(): void { $calendarValues = [ @@ -168,4 +181,41 @@ class DateTest extends TestCase { return require 'tests/data/Shared/Date/ExcelToTimestamp1900Timezone.php'; } + + public function testVarious(): void + { + Date::setDefaultTimeZone('UTC'); + self::assertFalse(Date::stringToExcel('2019-02-29')); + self::assertTrue((bool) Date::stringToExcel('2019-02-28')); + self::assertTrue((bool) Date::stringToExcel('2019-02-28 11:18')); + self::assertFalse(Date::stringToExcel('2019-02-28 11:71')); + $date = Date::PHPToExcel('2020-01-01'); + self::assertEquals(43831.0, $date); + $spreadsheet = new \PhpOffice\PhpSpreadsheet\Spreadsheet(); + $sheet = $spreadsheet->getActiveSheet(); + $sheet->setCellValue('B1', 'x'); + $val = $sheet->getCell('B1')->getValue(); + self::assertFalse(Date::timestampToExcel($val)); + $cell = $sheet->getCell('A1'); + self::assertNotNull($cell); + $cell->setValue($date); + $sheet->getStyle('A1') + ->getNumberFormat() + ->setFormatCode(NumberFormat::FORMAT_DATE_DATETIME); + self::assertTrue(null !== $cell && Date::isDateTime($cell)); + $cella2 = $sheet->getCell('A2'); + self::assertNotNull($cella2); + $cella2->setValue('=A1+2'); + $sheet->getStyle('A2') + ->getNumberFormat() + ->setFormatCode(NumberFormat::FORMAT_DATE_DATETIME); + self::assertTrue(null !== $cella2 && Date::isDateTime($cella2)); + $cella3 = $sheet->getCell('A3'); + self::assertNotNull($cella3); + $cella3->setValue('=A1+4'); + $sheet->getStyle('A3') + ->getNumberFormat() + ->setFormatCode('0.00E+00'); + self::assertFalse(null !== $cella3 && Date::isDateTime($cella3)); + } } diff --git a/tests/PhpSpreadsheetTests/Shared/TimeZoneTest.php b/tests/PhpSpreadsheetTests/Shared/TimeZoneTest.php index f6e2f5d5..ff38badf 100644 --- a/tests/PhpSpreadsheetTests/Shared/TimeZoneTest.php +++ b/tests/PhpSpreadsheetTests/Shared/TimeZoneTest.php @@ -2,11 +2,29 @@ namespace PhpOffice\PhpSpreadsheetTests\Shared; +use DateTime; +use PhpOffice\PhpSpreadsheet\Shared\Date; use PhpOffice\PhpSpreadsheet\Shared\TimeZone; use PHPUnit\Framework\TestCase; class TimeZoneTest extends TestCase { + private $tztimezone; + + private $dttimezone; + + protected function setUp(): void + { + $this->tztimezone = TimeZone::getTimeZone(); + $this->dttimezone = Date::getDefaultTimeZone(); + } + + protected function tearDown(): void + { + TimeZone::setTimeZone($this->tztimezone); + Date::setDefaultTimeZone($this->dttimezone); + } + public function testSetTimezone(): void { $timezoneValues = [ @@ -20,13 +38,51 @@ class TimeZoneTest extends TestCase foreach ($timezoneValues as $timezoneValue) { $result = TimeZone::setTimezone($timezoneValue); self::assertTrue($result); + $result = Date::setDefaultTimezone($timezoneValue); + self::assertTrue($result); } } + public function testSetTimezoneBackwardCompatible(): void + { + $bcTimezone = 'Etc/GMT+10'; + $result = TimeZone::setTimezone($bcTimezone); + self::assertTrue($result); + $result = Date::setDefaultTimezone($bcTimezone); + self::assertTrue($result); + } + public function testSetTimezoneWithInvalidValue(): void { - $unsupportedTimezone = 'Etc/GMT+10'; + $unsupportedTimezone = 'XEtc/GMT+10'; $result = TimeZone::setTimezone($unsupportedTimezone); self::assertFalse($result); + $result = Date::setDefaultTimezone($unsupportedTimezone); + self::assertFalse($result); + } + + public function testTimeZoneAdjustmentsInvalidTz(): void + { + $this->expectException(\PhpOffice\PhpSpreadsheet\Exception::class); + $dtobj = DateTime::createFromFormat('Y-m-d H:i:s', '2008-09-22 00:00:00'); + $tstmp = $dtobj->getTimestamp(); + $unsupportedTimeZone = 'XEtc/GMT+10'; + TimeZone::getTimeZoneAdjustment($unsupportedTimeZone, $tstmp); + } + + public function testTimeZoneAdjustments(): void + { + $dtobj = DateTime::createFromFormat('Y-m-d H:i:s', '2008-01-01 00:00:00'); + $tstmp = $dtobj->getTimestamp(); + $supportedTimeZone = 'UTC'; + $adj = TimeZone::getTimeZoneAdjustment($supportedTimeZone, $tstmp); + self::assertEquals(0, $adj); + $supportedTimeZone = 'America/Toronto'; + $adj = TimeZone::getTimeZoneAdjustment($supportedTimeZone, $tstmp); + self::assertEquals(-18000, $adj); + $supportedTimeZone = 'America/Chicago'; + TimeZone::setTimeZone($supportedTimeZone); + $adj = TimeZone::getTimeZoneAdjustment(null, $tstmp); + self::assertEquals(-21600, $adj); } } diff --git a/tests/data/Shared/Date/FormatCodes.php b/tests/data/Shared/Date/FormatCodes.php index 7245000a..64810de3 100644 --- a/tests/data/Shared/Date/FormatCodes.php +++ b/tests/data/Shared/Date/FormatCodes.php @@ -146,4 +146,8 @@ return [ false, '#,##0.00 "dollars"', ], + [ + true, + '"date " y-m-d', + ], ]; From 5a92a5f6b42e985da6345d996197e9cb27a1d5f9 Mon Sep 17 00:00:00 2001 From: Adrien Crivelli Date: Mon, 25 May 2020 11:19:13 +0900 Subject: [PATCH 12/12] Scrutinizer wait shorter for coverage Since switching to pcov Travis build are much faster and we don't need to wait that long --- .scrutinizer.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.scrutinizer.yml b/.scrutinizer.yml index 748f3ac3..08c61255 100644 --- a/.scrutinizer.yml +++ b/.scrutinizer.yml @@ -18,7 +18,7 @@ build: tools: external_code_coverage: - timeout: 3600 + timeout: 600 build_failure_conditions: - 'elements.rating(<= C).new.exists' # No new classes/methods with a rating of C or worse allowed