Recalibrate Row/Column Dimensions After removeRow/Col

Fix PHPOffice#2442. Although data and styles are handled correctly after removing row(s) or column(s), the dimensions of the removed rows and columns remain behind to afflict their replacements. This PR will take care of removing the dimensions as well.

Dimensions has a _clone method for a deep clone, but all of its properties, as well as the properties of RowDimensions and ColumnDimensions, are scalars, and do not require a deep clone. The method is deleted.

src/PhpSpreadsheet/Worksheet/ColumnDimension.php

@@ -2,6 +2,7 @@
 
 namespace PhpOffice\PhpSpreadsheet\Worksheet;
 
+use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
 use PhpOffice\PhpSpreadsheet\Helper\Dimension as CssDimension;
 
 class ColumnDimension extends Dimension
@@ -53,16 +54,32 @@ public function getColumnIndex(): string
 
     /**
      * Set column index as string eg: 'A'.
-     *
-     * @return $this
      */
-    public function setColumnIndex(string $index)
+    public function setColumnIndex(string $index): self
     {
         $this->columnIndex = $index;
 
         return $this;
     }
 
+    /**
+     * Get column index as numeric.
+     */
+    public function getColumnNumeric(): int
+    {
+        return Coordinate::columnIndexFromString($this->columnIndex);
+    }
+
+    /**
+     * Set column index as numeric.
+     */
+    public function setColumnNumeric(int $index): self
+    {
+        $this->columnIndex = Coordinate::stringFromColumnIndex($index);
+
+        return $this;
+    }
+
     /**
      * Get Width.
      *

src/PhpSpreadsheet/Worksheet/Dimension.php

@@ -131,19 +131,4 @@ public function setXfIndex(int $XfIndex)
 
         return $this;
     }
-
-    /**
-     * 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/Worksheet/Worksheet.php

@@ -2076,6 +2076,7 @@ public function removeRow($row, $numberOfRows = 1)
             throw new Exception('Rows to be deleted should at least start from row 1.');
         }
 
+        $holdRowDimensions = $this->removeRowDimensions($row, $numberOfRows);
         $highestRow = $this->getHighestDataRow();
         $removedRowsCounter = 0;
 
@@ -2093,9 +2094,30 @@ public function removeRow($row, $numberOfRows = 1)
             --$highestRow;
         }
 
+        $this->rowDimensions = $holdRowDimensions;
+
         return $this;
     }
 
+    private function removeRowDimensions(int $row, int $numberOfRows): array
+    {
+        $highRow = $row + $numberOfRows - 1;
+        $holdRowDimensions = [];
+        foreach ($this->rowDimensions as $rowDimension) {
+            $num = $rowDimension->getRowIndex();
+            if ($num < $row) {
+                $holdRowDimensions[$num] = $rowDimension;
+            } elseif ($num > $highRow) {
+                $num -= $numberOfRows;
+                $cloneDimension = clone $rowDimension;
+                $cloneDimension->setRowIndex($num);
+                $holdRowDimensions[$num] = $cloneDimension;
+            }
+        }
+
+        return $holdRowDimensions;
+    }
+
     /**
      * Remove a column, updating all possible related data.
      *
@@ -2118,6 +2140,8 @@ public function removeColumn($column, $numberOfColumns = 1)
             return $this;
         }
 
+        $holdColumnDimensions = $this->removeColumnDimensions($pColumnIndex, $numberOfColumns);
+
         $column = Coordinate::stringFromColumnIndex($pColumnIndex + $numberOfColumns);
         $objReferenceHelper = ReferenceHelper::getInstance();
         $objReferenceHelper->insertNewBefore($column . '1', -$numberOfColumns, 0, $this);
@@ -2129,11 +2153,33 @@ public function removeColumn($column, $numberOfColumns = 1)
             $highestColumn = Coordinate::stringFromColumnIndex(Coordinate::columnIndexFromString($highestColumn) - 1);
         }
 
+        $this->columnDimensions = $holdColumnDimensions;
+
         $this->garbageCollect();
 
         return $this;
     }
 
+    private function removeColumnDimensions(int $pColumnIndex, int $numberOfColumns): array
+    {
+        $highCol = $pColumnIndex + $numberOfColumns - 1;
+        $holdColumnDimensions = [];
+        foreach ($this->columnDimensions as $columnDimension) {
+            $num = $columnDimension->getColumnNumeric();
+            if ($num < $pColumnIndex) {
+                $str = $columnDimension->getColumnIndex();
+                $holdColumnDimensions[$str] = $columnDimension;
+            } elseif ($num > $highCol) {
+                $cloneDimension = clone $columnDimension;
+                $cloneDimension->setColumnNumeric($num - $numberOfColumns);
+                $str = $cloneDimension->getColumnIndex();
+                $holdColumnDimensions[$str] = $cloneDimension;
+            }
+        }
+
+        return $holdColumnDimensions;
+    }
+
     /**
      * Remove a column, updating all possible related data.
      *

tests/PhpSpreadsheetTests/Worksheet/RemoveTest.php

@@ -0,0 +1,87 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests\Worksheet;
+
+use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
+use PhpOffice\PhpSpreadsheet\Style\Color;
+use PhpOffice\PhpSpreadsheet\Style\Fill;
+use PHPUnit\Framework\TestCase;
+
+class RemoveTest extends TestCase
+{
+    public function testRemoveRow(): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->getActiveSheet();
+        $fillColors = [
+            'FFFF0000',
+            'FF00FF00',
+            'FF0000FF',
+        ];
+        $rowHeights = [-1, -1, 1.2, 1.3, 1.4, 1.5, -1, -1, -1];
+        for ($row = 1; $row < 10; ++$row) {
+            $sheet->getCell("B$row")
+                ->getStyle()
+                ->getFill()
+                ->setFillType(Fill::FILL_SOLID)
+                ->setStartColor(new Color($fillColors[$row % 3]));
+            $sheet->getCell("B$row")->setValue("X$row");
+            $height = $rowHeights[$row - 1];
+            if ($height > 0) {
+                $sheet->getRowDimension($row)->setRowHeight($height);
+            }
+        }
+        //$mapRow = [1, 2, 3, 4, 5, 6, 7, 8, 9];
+        $sheet->removeRow(4, 2);
+        $mapRow = [1, 2, 3, 6, 7, 8, 9];
+        $rowCount = count($mapRow);
+        for ($row = 1; $row <= $rowCount; ++$row) {
+            $mappedRow = $mapRow[$row - 1];
+            self::assertSame("X$mappedRow", $sheet->getCell("B$row")->getValue(), "Row value $row mapped to $mappedRow");
+            self::assertSame($fillColors[$mappedRow % 3], $sheet->getCell("B$row")->getStyle()->getFill()->getStartColor()->getArgb(), "Row fill color $row mapped to $mappedRow");
+            self::assertSame($rowHeights[$mappedRow - 1], $sheet->getRowDimension($row)->getRowHeight(), "Row height $row mapped to $mappedRow");
+        }
+
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    public function testRemoveColumn(): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->getActiveSheet();
+        $fillColors = [
+            'FFFF0000',
+            'FF00FF00',
+            'FF0000FF',
+        ];
+        $colWidths = [-1, -1, 1.2, 1.3, 1.4, 1.5, -1, -1, -1];
+        for ($colNumber = 1; $colNumber < 10; ++$colNumber) {
+            $col = Coordinate::stringFromColumnIndex($colNumber);
+            $sheet->getCell("{$col}1")
+                ->getStyle()
+                ->getFill()
+                ->setFillType(Fill::FILL_SOLID)
+                ->setStartColor(new Color($fillColors[$colNumber % 3]));
+            $sheet->getCell("{$col}1")->setValue("100$col");
+            $width = $colWidths[$colNumber - 1];
+            if ($width > 0) {
+                $sheet->getColumnDimension($col)->setWidth($width);
+            }
+        }
+        //$mapCol = [1, 2, 3, 4, 5, 6, 7, 8, 9];
+        $sheet->removeColumn('D', 2);
+        $mapCol = [1, 2, 3, 6, 7, 8, 9];
+        $colCount = count($mapCol);
+        for ($colNumber = 1; $colNumber < $colCount; ++$colNumber) {
+            $col = Coordinate::stringFromColumnIndex($colNumber);
+            $mappedCol = $mapCol[$colNumber - 1];
+            $mappedColString = Coordinate::stringFromColumnIndex($mappedCol);
+            self::assertSame("100$mappedColString", $sheet->getCell("{$col}1")->getValue(), "Column value $colNumber mapped to $mappedCol");
+            self::assertSame($fillColors[$mappedCol % 3], $sheet->getCell("{$col}1")->getStyle()->getFill()->getStartColor()->getArgb(), "Col fill color $col mapped to $mappedColString");
+            self::assertEquals($colWidths[$mappedCol - 1], $sheet->getColumnDimension($col)->getWidth(), "Col width $col mapped to $mappedColString");
+        }
+
+        $spreadsheet->disconnectWorksheets();
+    }
+}