Merge pull request #4968 from sapientpro/feature/table-header-row-needs-a-data-row

Xlsx writer: a header-row table takes the empty row below it

Author: oleibman

CHANGELOG.md

@@ -16,6 +16,10 @@ Some earlier branches remain supported and security fixes are applied to them; i
 - Support for Excel sparklines (line, column, and win/loss) in Xlsx reader and writer. [Issue #4941](https://github.com/PHPOffice/PhpSpreadsheet/issues/4941)
 - Read-only object model for Pivot Tables. Existing pivot tables in an Xlsx file are now parsed into `Worksheet\PivotTable\PivotTable` objects (name, location, source cache definition, and row/column/page/data field layout), accessible via `Worksheet::getPivotTableCollection()` / `getPivotTableByName()`. Pivot tables (their tables, caches and records) are now also preserved through an Xlsx load/save round-trip instead of being silently dropped. [Issue #4534](https://github.com/PHPOffice/PhpSpreadsheet/issues/4534)
 
+### Fixed
+
+- Xlsx writer: a table showing a header row is written over the row below it when its range covers the header row alone. Excel reports such a workbook as unreadable and repairs it by dropping the table; Excel itself writes the extra row, leaving it without a cell, and the writer now does the same. The row is taken only when it is empty — a table that would swallow a row holding something else throws instead.
+
 ### Removed
 
 - Nothing yet.

src/PhpSpreadsheet/Writer/Xlsx/Table.php

@@ -3,6 +3,7 @@
 namespace PhpOffice\PhpSpreadsheet\Writer\Xlsx;
 
 use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
+use PhpOffice\PhpSpreadsheet\Exception as PhpSpreadsheetException;
 use PhpOffice\PhpSpreadsheet\Reader\Xlsx\Namespaces;
 use PhpOffice\PhpSpreadsheet\Shared\XMLWriter;
 use PhpOffice\PhpSpreadsheet\Worksheet\Table as WorksheetTable;
@@ -31,7 +32,7 @@ public function writeTable(WorksheetTable $table, int $tableRef): string
 
         // Table
         $name = 'Table' . $tableRef;
-        $range = $table->getRange();
+        $range = $this->rangeWithRoomForData($table);
 
         $objWriter->startElement('table');
         $objWriter->writeAttribute('xmlns', Namespaces::MAIN);
@@ -111,4 +112,47 @@ public function writeTable(WorksheetTable $table, int $tableRef): string
         // Return
         return $objWriter->getData();
     }
+
+    /**
+     * The range to write, given that a table showing a header row needs a row of data under it.
+     *
+     * Excel reports a workbook whose table covers its header row alone as unreadable, and repairs
+     * it by dropping the table — the markup is lost without a word. Excel itself never writes such
+     * a table: asked to make one over a single row of headings, it writes the table over the row
+     * below as well, and leaves that row empty (its `sheetData` holds no cell for it). This does
+     * the same, so what is written is what Excel would have written.
+     *
+     * The row below is taken only when it holds nothing: a table silently swallowing a row that
+     * belongs to something else would change what the sheet says, so that case is refused instead.
+     *
+     * @throws PhpSpreadsheetException
+     */
+    private function rangeWithRoomForData(WorksheetTable $table): string
+    {
+        $range = $table->getRange();
+
+        if (!$table->getShowHeaderRow()) {
+            return $range;
+        }
+
+        [$rangeStart, $rangeEnd] = Coordinate::rangeBoundaries($range);
+
+        if ($rangeEnd[1] > $rangeStart[1]) {
+            return $range;
+        }
+
+        $worksheet = $table->getWorksheet();
+        $rowBelow = $rangeEnd[1] + 1;
+
+        if ($worksheet !== null && $worksheet->getHighestDataRow() >= $rowBelow) {
+            throw new PhpSpreadsheetException(
+                'Table ' . $table->getName() . ' shows a header row over ' . $range
+                . ', which leaves no row for its data, and row ' . $rowBelow
+                . ' below it is not empty; a table with a header row needs at least 2 rows'
+            );
+        }
+
+        return Coordinate::stringFromColumnIndex($rangeStart[0]) . $rangeStart[1]
+            . ':' . Coordinate::stringFromColumnIndex($rangeEnd[0]) . $rowBelow;
+    }
 }

tests/PhpSpreadsheetTests/Writer/Xlsx/TableHeaderRowTest.php

@@ -0,0 +1,96 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpOffice\PhpSpreadsheetTests\Writer\Xlsx;
+
+use PhpOffice\PhpSpreadsheet\Exception as PhpSpreadsheetException;
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
+use PhpOffice\PhpSpreadsheet\Worksheet\Table;
+use PhpOffice\PhpSpreadsheetTests\Functional\AbstractFunctional;
+
+/**
+ * Excel reports a workbook whose table covers its header row alone as unreadable, and repairs it
+ * by dropping the table. Excel itself never writes one: asked to make a table over a single row of
+ * headings it writes the table over the row below as well, leaving that row without a cell. The
+ * writer does the same — unless that row already holds something.
+ */
+class TableHeaderRowTest extends AbstractFunctional
+{
+    private ?Spreadsheet $spreadsheet = null;
+
+    protected function tearDown(): void
+    {
+        if ($this->spreadsheet !== null) {
+            $this->spreadsheet->disconnectWorksheets();
+            $this->spreadsheet = null;
+        }
+    }
+
+    public function testHeaderRowTakesTheEmptyRowBelowIt(): void
+    {
+        $this->spreadsheet = new Spreadsheet();
+        $sheet = $this->spreadsheet->getActiveSheet();
+        $sheet->fromArray([['Year', 'Country']], null, 'A1');
+        $sheet->addTable(new Table('A1:B1', 'SalesData'));
+
+        $reloaded = $this->writeAndReload($this->spreadsheet, 'Xlsx');
+        $worksheet = $reloaded->getActiveSheet();
+        $table = $worksheet->getTableByName('SalesData');
+
+        // exactly what Excel writes for a table made over a single row of headings
+        self::assertInstanceOf(Table::class, $table);
+        self::assertSame('A1:B2', $table->getRange());
+        self::assertSame(1, $worksheet->getHighestDataRow(), 'The row taken is left without a cell');
+
+        $reloaded->disconnectWorksheets();
+    }
+
+    public function testHeaderRowWillNotSwallowARowThatHoldsSomething(): void
+    {
+        $this->spreadsheet = new Spreadsheet();
+        $sheet = $this->spreadsheet->getActiveSheet();
+        $sheet->fromArray([['Year', 'Country']], null, 'A1');
+        $sheet->getCell('A2')->setValue('Total');
+        $sheet->addTable(new Table('A1:B1', 'SalesData'));
+
+        $this->expectException(PhpSpreadsheetException::class);
+        $this->expectExceptionMessage('needs at least 2 rows');
+
+        $this->writeAndReload($this->spreadsheet, 'Xlsx')->disconnectWorksheets();
+    }
+
+    public function testHeaderRowWithOneRowOfDataIsWritten(): void
+    {
+        $this->spreadsheet = new Spreadsheet();
+        $sheet = $this->spreadsheet->getActiveSheet();
+        $sheet->fromArray([['Year', 'Country'], [2010, 'Belgium']], null, 'A1');
+        $sheet->addTable(new Table('A1:B2', 'SalesData'));
+
+        $reloaded = $this->writeAndReload($this->spreadsheet, 'Xlsx');
+        $table = $reloaded->getActiveSheet()->getTableByName('SalesData');
+
+        self::assertInstanceOf(Table::class, $table);
+        self::assertSame('A1:B2', $table->getRange());
+        self::assertTrue($table->getShowHeaderRow());
+
+        $reloaded->disconnectWorksheets();
+    }
+
+    public function testASingleRowTableWithoutAHeaderRowIsWritten(): void
+    {
+        $this->spreadsheet = new Spreadsheet();
+        $sheet = $this->spreadsheet->getActiveSheet();
+        $sheet->fromArray([[2010, 'Belgium']], null, 'A1');
+        $sheet->addTable((new Table('A1:B1', 'SalesData'))->setShowHeaderRow(false));
+
+        $reloaded = $this->writeAndReload($this->spreadsheet, 'Xlsx');
+        $table = $reloaded->getActiveSheet()->getTableByName('SalesData');
+
+        self::assertInstanceOf(Table::class, $table);
+        self::assertSame('A1:B1', $table->getRange());
+        self::assertFalse($table->getShowHeaderRow());
+
+        $reloaded->disconnectWorksheets();
+    }
+}