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

src/PhpSpreadsheet/Chart/Renderer/JpGraph.php

@@ -21,6 +21,14 @@ use ScatterPlot;
 use Spline;
 use StockPlot;
 
+/**
+ * Jpgraph is not maintained in Composer, and the version there
+ * is extremely out of date. For that reason, all unit test
+ * requiring Jpgraph are skipped. So, do not measure
+ * code coverage for this class till that is fixed.
+ *
+ * @codeCoverageIgnore
+ */
 class JpGraph implements IRenderer
 {
     private static $width = 640;

src/PhpSpreadsheet/Reader/Gnumeric/PageSetup.php

@@ -22,11 +22,8 @@ class PageSetup
 
     public function printInformation(SimpleXMLElement $sheet): self
     {
-        if (isset($sheet->PrintInformation)) {
+        if (isset($sheet->PrintInformation, $sheet->PrintInformation[0])) {
             $printInformation = $sheet->PrintInformation[0];
-            if (!$printInformation) {
-                return $this;
-            }
 
             $scale = (string) $printInformation->Scale->attributes()['percentage'];
             $pageOrder = (string) $printInformation->order;

src/PhpSpreadsheet/Reader/Xlsx/Theme.php

@@ -41,9 +41,11 @@ class Theme
     }
 
     /**
-     * Get Theme Name.
+     * Not called by Reader, never accessible any other time.
      *
      * @return string
+     *
+     * @codeCoverageIgnore
      */
     public function getThemeName()
     {
@@ -51,9 +53,11 @@ class Theme
     }
 
     /**
-     * Get colour Scheme Name.
+     * Not called by Reader, never accessible any other time.
      *
      * @return string
+     *
+     * @codeCoverageIgnore
      */
     public function getColourSchemeName()
     {
@@ -69,25 +73,6 @@ class Theme
      */
     public function getColourByIndex($index)
     {
-        if (isset($this->colourMap[$index])) {
+        return $this->colourMap[$index] ?? null;
-            return $this->colourMap[$index];
-        }
-
-        return null;
-    }
-
-    /**
-     * Implement PHP __clone to create a deep clone, not just a shallow copy.
-     */
-    public function __clone()
-    {
-        $vars = get_object_vars($this);
-        foreach ($vars as $key => $value) {
-            if ((is_object($value)) && ($key != '_parent')) {
-                $this->$key = clone $value;
-            } else {
-                $this->$key = $value;
-            }
-        }
     }
 }

src/PhpSpreadsheet/RichText/RichText.php

@@ -158,11 +158,14 @@ class RichText implements IComparable
     {
         $vars = get_object_vars($this);
         foreach ($vars as $key => $value) {
-            if (is_object($value)) {
+            $newValue = is_object($value) ? (clone $value) : $value;
-                $this->$key = clone $value;
+            if (is_array($value)) {
-            } else {
+                $newValue = [];
-                $this->$key = $value;
+                foreach ($value as $key2 => $value2) {
-            }
+                    $newValue[$key2] = is_object($value2) ? (clone $value2) : $value2;
+                }
+            }
+            $this->$key = $newValue;
         }
     }
 }

src/PhpSpreadsheet/RichText/TextElement.php

@@ -68,19 +68,4 @@ class TextElement implements ITextElement
             __CLASS__
         );
     }
-
-    /**
-     * Implement PHP __clone to create a deep clone, not just a shallow copy.
-     */
-    public function __clone()
-    {
-        $vars = get_object_vars($this);
-        foreach ($vars as $key => $value) {
-            if (is_object($value)) {
-                $this->$key = clone $value;
-            } else {
-                $this->$key = $value;
-            }
-        }
-    }
 }

src/PhpSpreadsheet/Style/Font.php

@@ -299,11 +299,14 @@ class Font extends Supervisor
         if ($fontname == '') {
             $fontname = 'Calibri';
         }
-        if ($this->isSupervisor) {
+        if (!$this->isSupervisor) {
+            $this->latin = $fontname;
+        } else {
+            // should never be true
+            // @codeCoverageIgnoreStart
             $styleArray = $this->getStyleArray(['latin' => $fontname]);
             $this->getActiveSheet()->getStyle($this->getSelectedCells())->applyFromArray($styleArray);
-        } else {
+            // @codeCoverageIgnoreEnd
-            $this->latin = $fontname;
         }
 
         return $this;
@@ -314,11 +317,14 @@ class Font extends Supervisor
         if ($fontname == '') {
             $fontname = 'Calibri';
         }
-        if ($this->isSupervisor) {
+        if (!$this->isSupervisor) {
+            $this->eastAsian = $fontname;
+        } else {
+            // should never be true
+            // @codeCoverageIgnoreStart
             $styleArray = $this->getStyleArray(['eastAsian' => $fontname]);
             $this->getActiveSheet()->getStyle($this->getSelectedCells())->applyFromArray($styleArray);
-        } else {
+            // @codeCoverageIgnoreEnd
-            $this->eastAsian = $fontname;
         }
 
         return $this;
@@ -329,11 +335,14 @@ class Font extends Supervisor
         if ($fontname == '') {
             $fontname = 'Calibri';
         }
-        if ($this->isSupervisor) {
+        if (!$this->isSupervisor) {
+            $this->complexScript = $fontname;
+        } else {
+            // should never be true
+            // @codeCoverageIgnoreStart
             $styleArray = $this->getStyleArray(['complexScript' => $fontname]);
             $this->getActiveSheet()->getStyle($this->getSelectedCells())->applyFromArray($styleArray);
-        } else {
+            // @codeCoverageIgnoreEnd
-            $this->complexScript = $fontname;
         }
 
         return $this;
@@ -533,11 +542,14 @@ class Font extends Supervisor
 
     public function setBaseLine(int $baseLine): self
     {
-        if ($this->isSupervisor) {
+        if (!$this->isSupervisor) {
+            $this->baseLine = $baseLine;
+        } else {
+            // should never be true
+            // @codeCoverageIgnoreStart
             $styleArray = $this->getStyleArray(['baseLine' => $baseLine]);
             $this->getActiveSheet()->getStyle($this->getSelectedCells())->applyFromArray($styleArray);
-        } else {
+            // @codeCoverageIgnoreEnd
-            $this->baseLine = $baseLine;
         }
 
         return $this;
@@ -554,11 +566,14 @@ class Font extends Supervisor
 
     public function setStrikeType(string $strikeType): self
     {
-        if ($this->isSupervisor) {
+        if (!$this->isSupervisor) {
+            $this->strikeType = $strikeType;
+        } else {
+            // should never be true
+            // @codeCoverageIgnoreStart
             $styleArray = $this->getStyleArray(['strikeType' => $strikeType]);
             $this->getActiveSheet()->getStyle($this->getSelectedCells())->applyFromArray($styleArray);
-        } else {
+            // @codeCoverageIgnoreEnd
-            $this->strikeType = $strikeType;
         }
 
         return $this;
@@ -575,11 +590,14 @@ class Font extends Supervisor
 
     public function setUSchemeClr(string $uSchemeClr): self
     {
-        if ($this->isSupervisor) {
+        if (!$this->isSupervisor) {
+            $this->uSchemeClr = $uSchemeClr;
+        } else {
+            // should never be true
+            // @codeCoverageIgnoreStart
             $styleArray = $this->getStyleArray(['uSchemeClr' => $uSchemeClr]);
             $this->getActiveSheet()->getStyle($this->getSelectedCells())->applyFromArray($styleArray);
-        } else {
+            // @codeCoverageIgnoreEnd
-            $this->uSchemeClr = $uSchemeClr;
         }
 
         return $this;
@@ -743,6 +761,7 @@ class Font extends Supervisor
         $this->exportArray2($exportedArray, 'subscript', $this->getSubscript());
         $this->exportArray2($exportedArray, 'superscript', $this->getSuperscript());
         $this->exportArray2($exportedArray, 'underline', $this->getUnderline());
+        $this->exportArray2($exportedArray, 'uSchemeClr', $this->getUSchemeClr());
 
         return $exportedArray;
     }

src/PhpSpreadsheet/Style/Style.php

@@ -421,8 +421,10 @@ class Style extends Supervisor
                         // Handle bug in PHPStan, see https://github.com/phpstan/phpstan/issues/5805
                         // $newStyle should always be defined.
                         // This block might not be needed in the future
+                        // @codeCoverageIgnoreStart
                         $newStyle = clone $style;
                         $newStyle->applyFromArray($styleArray);
+                        // @codeCoverageIgnoreEnd
                     }
 
                     // we don't have such a cell Xf, need to add

src/PhpSpreadsheet/Writer/BaseWriter.php

@@ -118,7 +118,9 @@ abstract class BaseWriter implements IWriter
         $mode = 'wb';
         $scheme = parse_url($filename, PHP_URL_SCHEME);
         if ($scheme === 's3') {
+            // @codeCoverageIgnoreStart
             $mode = 'w';
+            // @codeCoverageIgnoreEnd
         }
         $fileHandle = $filename ? fopen($filename, $mode) : false;
         if ($fileHandle === false) {

tests/PhpSpreadsheetTests/Reader/BaseNoLoad.php

@@ -0,0 +1,18 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests\Reader;
+
+use PhpOffice\PhpSpreadsheet\Reader\BaseReader;
+
+class BaseNoLoad extends BaseReader
+{
+    public function canRead(string $filename): bool
+    {
+        return $filename !== '';
+    }
+
+    public function loadxxx(string $filename): void
+    {
+        $this->loadSpreadsheetFromFile($filename);
+    }
+}

tests/PhpSpreadsheetTests/Reader/BaseNoLoadTest.php

@@ -0,0 +1,17 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests\Reader;
+
+use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException;
+use PHPUnit\Framework\TestCase;
+
+class BaseNoLoadTest extends TestCase
+{
+    public function testBaseNoLoad(): void
+    {
+        $this->expectException(SpreadsheetException::class);
+        $this->expectExceptionMessage('Reader classes must implement their own loadSpreadsheetFromFile() method');
+        $reader = new BaseNoLoad();
+        $reader->loadxxx('unknown.file');
+    }
+}

tests/PhpSpreadsheetTests/Reader/Html/HtmlBorderTest.php

@@ -18,6 +18,8 @@ class HtmlBorderTest extends TestCase
                         <td style="border-left: 1px solid green;">Border left</td>
                         <td style="border-right: 1px solid #333333;">Border right</td>
                         <td style="border: none"></td>
+                        <td style="border: dashed;"></td>
+                        <td style="border: dotted #333333;"></td>
                     </tr>
                 </table>';
         $filename = HtmlHelper::createHtml($html);
@@ -61,6 +63,17 @@ class HtmlBorderTest extends TestCase
         foreach ([$borders->getTop(), $borders->getBottom(), $borders->getLeft(), $borders->getRight()] as $border) {
             self::assertEquals(Border::BORDER_NONE, $border->getBorderStyle());
         }
+
+        $style = $firstSheet->getCell('G1')->getStyle();
+        $borders = $style->getBorders();
+        $border = $borders->getRight();
+        self::assertEquals(Border::BORDER_DASHED, $border->getBorderStyle());
+
+        $style = $firstSheet->getCell('H1')->getStyle();
+        $borders = $style->getBorders();
+        $border = $borders->getRight();
+        self::assertEquals(Border::BORDER_DOTTED, $border->getBorderStyle());
+        self::assertEquals('333333', $border->getColor()->getRGB());
     }
 
     /**

tests/PhpSpreadsheetTests/Reader/Html/HtmlTest.php

@@ -327,4 +327,35 @@ class HtmlTest extends TestCase
             self::assertEquals(Border::BORDER_THIN, $border->getBorderStyle());
         }
     }
+
+    public function testBorderWithColspan(): void
+    {
+        $html = '<table>
+                    <tr>
+                        <td style="border: 1px solid black;">NOT SPANNED</td>
+                        <td colspan="2" style="border: 1px solid black;">SPANNED</td>
+                    </tr>
+                    <tr>
+                        <td style="border: 1px solid black;">NOT SPANNED</td>
+                    </tr>
+                </table>';
+
+        $reader = new Html();
+        $spreadsheet = $reader->loadFromString($html);
+        $firstSheet = $spreadsheet->getSheet(0);
+        $style = $firstSheet->getStyle('B1:B2');
+
+        $borders = $style->getBorders();
+
+        $totalBorders = [
+            $borders->getTop(),
+            $borders->getLeft(),
+            $borders->getBottom(),
+            $borders->getRight(),
+        ];
+
+        foreach ($totalBorders as $border) {
+            self::assertEquals(Border::BORDER_THIN, $border->getBorderStyle());
+        }
+    }
 }

tests/PhpSpreadsheetTests/Reader/Xlsx/AutoFilter2Test.php

@@ -0,0 +1,106 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;
+
+use PhpOffice\PhpSpreadsheet\IOFactory;
+use PhpOffice\PhpSpreadsheet\Worksheet\AutoFilter\Column\Rule;
+use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
+use PHPUnit\Framework\TestCase;
+
+class AutoFilter2Test extends TestCase
+{
+    private const TESTBOOK = 'tests/data/Reader/XLSX/autofilter2.xlsx';
+
+    public function getVisibleSheet(Worksheet $sheet, int $maxRow): array
+    {
+        $actualVisible = [];
+        for ($row = 2; $row <= $maxRow; ++$row) {
+            if ($sheet->getRowDimension($row)->getVisible()) {
+                $actualVisible[] = $row;
+            }
+        }
+
+        return $actualVisible;
+    }
+
+    public function testReadDateRange(): void
+    {
+        $spreadsheet = IOFactory::load(self::TESTBOOK);
+        $sheet = $spreadsheet->getSheetByName('daterange');
+        self::assertNotNull($sheet);
+        $filter = $sheet->getAutoFilter();
+        $maxRow = 30;
+        self::assertSame("A1:A$maxRow", $filter->getRange());
+        $columns = $filter->getColumns();
+        self::assertCount(1, $columns);
+        $column = $columns['A'] ?? null;
+        self::assertNotNull($column);
+        $ruleset = $column->getRules();
+        self::assertCount(1, $ruleset);
+        $rule = $ruleset[0];
+        self::assertSame(Rule::AUTOFILTER_RULETYPE_DATEGROUP, $rule->getRuleType());
+        $value = $rule->getValue();
+        self::assertIsArray($value);
+        self::assertCount(6, $value);
+        self::assertSame('2002', $value['year']);
+        self::assertSame('', $value['month']);
+        self::assertSame('', $value['day']);
+        self::assertSame('', $value['hour']);
+        self::assertSame('', $value['minute']);
+        self::assertSame('', $value['second']);
+        self::assertSame(
+            [25, 26, 27, 28, 29, 30],
+            $this->getVisibleSheet($sheet, $maxRow)
+        );
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    public function testReadTopTen(): void
+    {
+        $spreadsheet = IOFactory::load(self::TESTBOOK);
+        $sheet = $spreadsheet->getSheetByName('top10');
+        self::assertNotNull($sheet);
+        $filter = $sheet->getAutoFilter();
+        $maxRow = 65;
+        self::assertSame("A1:A$maxRow", $filter->getRange());
+        $columns = $filter->getColumns();
+        self::assertCount(1, $columns);
+        $column = $columns['A'] ?? null;
+        self::assertNotNull($column);
+        $ruleset = $column->getRules();
+        self::assertCount(1, $ruleset);
+        $rule = $ruleset[0];
+        self::assertSame(Rule::AUTOFILTER_RULETYPE_TOPTENFILTER, $rule->getRuleType());
+        $value = $rule->getValue();
+        self::assertSame('10', $value);
+        self::assertSame(
+            [56, 57, 58, 59, 60, 61, 62, 63, 64, 65],
+            $this->getVisibleSheet($sheet, $maxRow)
+        );
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    public function testReadDynamic(): void
+    {
+        $spreadsheet = IOFactory::load(self::TESTBOOK);
+        $sheet = $spreadsheet->getSheetByName('dynamic');
+        self::assertNotNull($sheet);
+        $filter = $sheet->getAutoFilter();
+        $maxRow = 30;
+        self::assertSame("A1:A$maxRow", $filter->getRange());
+        $columns = $filter->getColumns();
+        self::assertCount(1, $columns);
+        $column = $columns['A'] ?? null;
+        self::assertNotNull($column);
+        $ruleset = $column->getRules();
+        self::assertCount(1, $ruleset);
+        $rule = $ruleset[0];
+        self::assertSame(Rule::AUTOFILTER_RULETYPE_DYNAMICFILTER, $rule->getRuleType());
+        self::assertSame('M4', $rule->getGrouping());
+        self::assertSame(
+            [5, 17, 28],
+            $this->getVisibleSheet($sheet, $maxRow)
+        );
+        $spreadsheet->disconnectWorksheets();
+    }
+}

tests/PhpSpreadsheetTests/Reader/Xlsx/PageSetup2Test.php

@@ -0,0 +1,41 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;
+
+use PhpOffice\PhpSpreadsheet\IOFactory;
+use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
+use PHPUnit\Framework\TestCase;
+
+class PageSetup2Test extends TestCase
+{
+    private const TESTBOOK = 'tests/data/Reader/XLSX/autofilter2.xlsx';
+
+    public function testHeaderFooter(): void
+    {
+        $spreadsheet = IOFactory::load(self::TESTBOOK);
+        $sheets = 0;
+        foreach ($spreadsheet->getAllSheets() as $worksheet) {
+            ++$sheets;
+            $hf = $worksheet->getHeaderFooter();
+            self::assertTrue($hf->getDifferentOddEven());
+            self::assertTrue($hf->getDifferentFirst());
+        }
+        self::assertSame(4, $sheets);
+
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    public function testColumnBreak(): void
+    {
+        $spreadsheet = IOFactory::load(self::TESTBOOK);
+        $sheet = $spreadsheet->getSheetByName('colbreak');
+        self::assertNotNull($sheet);
+        $breaks = $sheet->getBreaks();
+        self::assertCount(1, $breaks);
+        $break = $breaks['D1'] ?? null;
+        self::assertNotNull($break);
+        self::assertSame($break, Worksheet::BREAK_COLUMN);
+
+        $spreadsheet->disconnectWorksheets();
+    }
+}

tests/PhpSpreadsheetTests/RichTextTest.php

@@ -0,0 +1,41 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests;
+
+use PhpOffice\PhpSpreadsheet\RichText\RichText;
+use PhpOffice\PhpSpreadsheet\RichText\TextElement;
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
+use PHPUnit\Framework\TestCase;
+
+class RichTextTest extends TestCase
+{
+    public function testConstructorSpecifyingCell(): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->getActiveSheet();
+        $cell = $sheet->getCell('A1');
+        $cell->setValue(2);
+        self::assertSame(2, $cell->getCalculatedValue());
+        $cell->getStyle()->getFont()->setName('whatever');
+        $richText = new RichText($cell);
+        self::assertSame('whatever', $sheet->getCell('A1')->getStyle()->getFont()->getName());
+        self::assertEquals($richText, $cell->getValue());
+        self::assertSame('2', $cell->getCalculatedValue());
+
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    public function testTextElements(): void
+    {
+        $element1 = new TextElement('A');
+        self::assertNull($element1->getFont());
+        $element2 = new TextElement('B');
+        $element3 = new TextElement('C');
+        $richText = new RichText();
+        $richText->setRichTextElements([$element1, $element2, $element3]);
+        self::assertSame('ABC', $richText->getPlainText());
+        $cloneText = clone $richText;
+        self::assertEquals($richText, $cloneText);
+        self::assertNotSame($richText, $cloneText);
+    }
+}

tests/PhpSpreadsheetTests/Style/FontTest.php

@@ -38,6 +38,7 @@ class FontTest extends TestCase
         $font->setSuperscript(true);
         self::assertTrue($font->getSuperscript());
         self::assertFalse($font->getSubscript(), 'False remains unchanged');
+        $spreadsheet->disconnectWorksheets();
     }
 
     public function testSize(): void
@@ -70,5 +71,21 @@ class FontTest extends TestCase
             $font->setSize($invalidFontSizeValue);
             self::assertEquals(10, $font->getSize(), 'Set to 10 after trying to set an invalid value.');
         }
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    public function testName(): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->getActiveSheet();
+        $cell = $sheet->getCell('A1');
+        $cell->setValue('Cell A1');
+        $font = $cell->getStyle()->getFont();
+        self::assertEquals('Calibri', $font->getName(), 'The default is Calibri');
+        $font->setName('whatever');
+        self::assertEquals('whatever', $font->getName(), 'The default is Calibri');
+        $font->setName('');
+        self::assertEquals('Calibri', $font->getName(), 'Null string changed to default');
+        $spreadsheet->disconnectWorksheets();
     }
 }

tests/data/Style/Color/ColorGetBlue.php

@@ -32,4 +32,5 @@ return [
         'FFFF00',
         false,
     ],
+    'invalid hex' => ['00', 'AABBDX'],
 ];