Skip to content

Commit c936f1d

Browse files
oleibmanoleibman
authored
Coverage Improvements (#2859)
Mostly new tests, some code annotations, some minor code changes: - RichText clone logic is wrong - TextElement doesn't have object properties, doesn't need clone
1 parent 0aedef4 commit c936f1d

File tree

18 files changed

+349
-63
lines changed

18 files changed

+349
-63
lines changed

‎src/PhpSpreadsheet/Chart/Renderer/JpGraph.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,14 @@
2121
use Spline;
2222
use StockPlot;
2323

24+
/**
25+
* Jpgraph is not maintained in Composer, and the version there
26+
* is extremely out of date. For that reason, all unit test
27+
* requiring Jpgraph are skipped. So, do not measure
28+
* code coverage for this class till that is fixed.
29+
*
30+
* @codeCoverageIgnore
31+
*/
2432
class JpGraph implements IRenderer
2533
{
2634
private static $width = 640;

‎src/PhpSpreadsheet/Reader/Gnumeric/PageSetup.php

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,8 @@ public function __construct(Spreadsheet $spreadsheet)
2222

2323
public function printInformation(SimpleXMLElement $sheet): self
2424
{
25-
if (isset($sheet->PrintInformation)) {
25+
if (isset($sheet->PrintInformation, $sheet->PrintInformation[0])) {
2626
$printInformation = $sheet->PrintInformation[0];
27-
if (!$printInformation) {
28-
return $this;
29-
}
3027

3128
$scale = (string) $printInformation->Scale->attributes()['percentage'];
3229
$pageOrder = (string) $printInformation->order;

‎src/PhpSpreadsheet/Reader/Xlsx/Theme.php

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -41,19 +41,23 @@ public function __construct($themeName, $colourSchemeName, $colourMap)
4141
}
4242

4343
/**
44-
* Get Theme Name.
44+
* Not called by Reader, never accessible any other time.
4545
*
4646
* @return string
47+
*
48+
* @codeCoverageIgnore
4749
*/
4850
public function getThemeName()
4951
{
5052
return $this->themeName;
5153
}
5254

5355
/**
54-
* Get colour Scheme Name.
56+
* Not called by Reader, never accessible any other time.
5557
*
5658
* @return string
59+
*
60+
* @codeCoverageIgnore
5761
*/
5862
public function getColourSchemeName()
5963
{
@@ -69,25 +73,6 @@ public function getColourSchemeName()
6973
*/
7074
public function getColourByIndex($index)
7175
{
72-
if (isset($this->colourMap[$index])) {
73-
return $this->colourMap[$index];
74-
}
75-
76-
return null;
77-
}
78-
79-
/**
80-
* Implement PHP __clone to create a deep clone, not just a shallow copy.
81-
*/
82-
public function __clone()
83-
{
84-
$vars = get_object_vars($this);
85-
foreach ($vars as $key => $value) {
86-
if ((is_object($value)) && ($key != '_parent')) {
87-
$this->$key = clone $value;
88-
} else {
89-
$this->$key = $value;
90-
}
91-
}
76+
return $this->colourMap[$index] ?? null;
9277
}
9378
}

‎src/PhpSpreadsheet/RichText/RichText.php

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -158,11 +158,14 @@ public function __clone()
158158
{
159159
$vars = get_object_vars($this);
160160
foreach ($vars as $key => $value) {
161-
if (is_object($value)) {
162-
$this->$key = clone $value;
163-
} else {
164-
$this->$key = $value;
161+
$newValue = is_object($value) ? (clone $value) : $value;
162+
if (is_array($value)) {
163+
$newValue = [];
164+
foreach ($value as $key2 => $value2) {
165+
$newValue[$key2] = is_object($value2) ? (clone $value2) : $value2;
166+
}
165167
}
168+
$this->$key = $newValue;
166169
}
167170
}
168171
}

‎src/PhpSpreadsheet/RichText/TextElement.php

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -68,19 +68,4 @@ public function getHashCode()
6868
__CLASS__
6969
);
7070
}
71-
72-
/**
73-
* Implement PHP __clone to create a deep clone, not just a shallow copy.
74-
*/
75-
public function __clone()
76-
{
77-
$vars = get_object_vars($this);
78-
foreach ($vars as $key => $value) {
79-
if (is_object($value)) {
80-
$this->$key = clone $value;
81-
} else {
82-
$this->$key = $value;
83-
}
84-
}
85-
}
8671
}

‎src/PhpSpreadsheet/Style/Font.php

Lines changed: 37 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -299,11 +299,14 @@ public function setLatin(string $fontname): self
299299
if ($fontname == '') {
300300
$fontname = 'Calibri';
301301
}
302-
if ($this->isSupervisor) {
302+
if (!$this->isSupervisor) {
303+
$this->latin = $fontname;
304+
} else {
305+
// should never be true
306+
// @codeCoverageIgnoreStart
303307
$styleArray = $this->getStyleArray(['latin' => $fontname]);
304308
$this->getActiveSheet()->getStyle($this->getSelectedCells())->applyFromArray($styleArray);
305-
} else {
306-
$this->latin = $fontname;
309+
// @codeCoverageIgnoreEnd
307310
}
308311

309312
return $this;
@@ -314,11 +317,14 @@ public function setEastAsian(string $fontname): self
314317
if ($fontname == '') {
315318
$fontname = 'Calibri';
316319
}
317-
if ($this->isSupervisor) {
320+
if (!$this->isSupervisor) {
321+
$this->eastAsian = $fontname;
322+
} else {
323+
// should never be true
324+
// @codeCoverageIgnoreStart
318325
$styleArray = $this->getStyleArray(['eastAsian' => $fontname]);
319326
$this->getActiveSheet()->getStyle($this->getSelectedCells())->applyFromArray($styleArray);
320-
} else {
321-
$this->eastAsian = $fontname;
327+
// @codeCoverageIgnoreEnd
322328
}
323329

324330
return $this;
@@ -329,11 +335,14 @@ public function setComplexScript(string $fontname): self
329335
if ($fontname == '') {
330336
$fontname = 'Calibri';
331337
}
332-
if ($this->isSupervisor) {
338+
if (!$this->isSupervisor) {
339+
$this->complexScript = $fontname;
340+
} else {
341+
// should never be true
342+
// @codeCoverageIgnoreStart
333343
$styleArray = $this->getStyleArray(['complexScript' => $fontname]);
334344
$this->getActiveSheet()->getStyle($this->getSelectedCells())->applyFromArray($styleArray);
335-
} else {
336-
$this->complexScript = $fontname;
345+
// @codeCoverageIgnoreEnd
337346
}
338347

339348
return $this;
@@ -533,11 +542,14 @@ public function getBaseLine(): int
533542

534543
public function setBaseLine(int $baseLine): self
535544
{
536-
if ($this->isSupervisor) {
545+
if (!$this->isSupervisor) {
546+
$this->baseLine = $baseLine;
547+
} else {
548+
// should never be true
549+
// @codeCoverageIgnoreStart
537550
$styleArray = $this->getStyleArray(['baseLine' => $baseLine]);
538551
$this->getActiveSheet()->getStyle($this->getSelectedCells())->applyFromArray($styleArray);
539-
} else {
540-
$this->baseLine = $baseLine;
552+
// @codeCoverageIgnoreEnd
541553
}
542554

543555
return $this;
@@ -554,11 +566,14 @@ public function getStrikeType(): string
554566

555567
public function setStrikeType(string $strikeType): self
556568
{
557-
if ($this->isSupervisor) {
569+
if (!$this->isSupervisor) {
570+
$this->strikeType = $strikeType;
571+
} else {
572+
// should never be true
573+
// @codeCoverageIgnoreStart
558574
$styleArray = $this->getStyleArray(['strikeType' => $strikeType]);
559575
$this->getActiveSheet()->getStyle($this->getSelectedCells())->applyFromArray($styleArray);
560-
} else {
561-
$this->strikeType = $strikeType;
576+
// @codeCoverageIgnoreEnd
562577
}
563578

564579
return $this;
@@ -575,11 +590,14 @@ public function getUSchemeClr(): string
575590

576591
public function setUSchemeClr(string $uSchemeClr): self
577592
{
578-
if ($this->isSupervisor) {
593+
if (!$this->isSupervisor) {
594+
$this->uSchemeClr = $uSchemeClr;
595+
} else {
596+
// should never be true
597+
// @codeCoverageIgnoreStart
579598
$styleArray = $this->getStyleArray(['uSchemeClr' => $uSchemeClr]);
580599
$this->getActiveSheet()->getStyle($this->getSelectedCells())->applyFromArray($styleArray);
581-
} else {
582-
$this->uSchemeClr = $uSchemeClr;
600+
// @codeCoverageIgnoreEnd
583601
}
584602

585603
return $this;
@@ -743,6 +761,7 @@ protected function exportArray1(): array
743761
$this->exportArray2($exportedArray, 'subscript', $this->getSubscript());
744762
$this->exportArray2($exportedArray, 'superscript', $this->getSuperscript());
745763
$this->exportArray2($exportedArray, 'underline', $this->getUnderline());
764+
$this->exportArray2($exportedArray, 'uSchemeClr', $this->getUSchemeClr());
746765

747766
return $exportedArray;
748767
}

‎src/PhpSpreadsheet/Style/Style.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,8 +421,10 @@ public function applyFromArray(array $styleArray, $advancedBorders = true)
421421
// Handle bug in PHPStan, see https://github.com/phpstan/phpstan/issues/5805
422422
// $newStyle should always be defined.
423423
// This block might not be needed in the future
424+
// @codeCoverageIgnoreStart
424425
$newStyle = clone $style;
425426
$newStyle->applyFromArray($styleArray);
427+
// @codeCoverageIgnoreEnd
426428
}
427429

428430
// we don't have such a cell Xf, need to add

‎src/PhpSpreadsheet/Writer/BaseWriter.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,9 @@ public function openFileHandle($filename): void
118118
$mode = 'wb';
119119
$scheme = parse_url($filename, PHP_URL_SCHEME);
120120
if ($scheme === 's3') {
121+
// @codeCoverageIgnoreStart
121122
$mode = 'w';
123+
// @codeCoverageIgnoreEnd
122124
}
123125
$fileHandle = $filename ? fopen($filename, $mode) : false;
124126
if ($fileHandle === false) {

‎tests/PhpSpreadsheetTests/Reader/BaseNoLoad.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?php
2+
3+
namespace PhpOffice\PhpSpreadsheetTests\Reader;
4+
5+
use PhpOffice\PhpSpreadsheet\Reader\BaseReader;
6+
7+
class BaseNoLoad extends BaseReader
8+
{
9+
public function canRead(string $filename): bool
10+
{
11+
return $filename !== '';
12+
}
13+
14+
public function loadxxx(string $filename): void
15+
{
16+
$this->loadSpreadsheetFromFile($filename);
17+
}
18+
}

‎tests/PhpSpreadsheetTests/Reader/BaseNoLoadTest.php

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php
2+
3+
namespace PhpOffice\PhpSpreadsheetTests\Reader;
4+
5+
use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException;
6+
use PHPUnit\Framework\TestCase;
7+
8+
class BaseNoLoadTest extends TestCase
9+
{
10+
public function testBaseNoLoad(): void
11+
{
12+
$this->expectException(SpreadsheetException::class);
13+
$this->expectExceptionMessage('Reader classes must implement their own loadSpreadsheetFromFile() method');
14+
$reader = new BaseNoLoad();
15+
$reader->loadxxx('unknown.file');
16+
}
17+
}

0 commit comments

Comments
 (0)