Recalibrate Row/Column Dimensions After removeRow/Col (#2486)

Fix #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 @@ class ColumnDimension extends Dimension
 
     /**
      * 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 @@ abstract class Dimension
 
         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

@@ -2101,6 +2101,7 @@ class Worksheet implements IComparable
             throw new Exception('Rows to be deleted should at least start from row 1.');
         }
 
+        $holdRowDimensions = $this->removeRowDimensions($row, $numberOfRows);
         $highestRow = $this->getHighestDataRow();
         $removedRowsCounter = 0;
 
@@ -2118,9 +2119,30 @@ class Worksheet implements IComparable
             --$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.
      *
@@ -2143,6 +2165,8 @@ class Worksheet implements IComparable
             return $this;
         }
 
+        $holdColumnDimensions = $this->removeColumnDimensions($pColumnIndex, $numberOfColumns);
+
         $column = Coordinate::stringFromColumnIndex($pColumnIndex + $numberOfColumns);
         $objReferenceHelper = ReferenceHelper::getInstance();
         $objReferenceHelper->insertNewBefore($column . '1', -$numberOfColumns, 0, $this);
@@ -2154,11 +2178,33 @@ class Worksheet implements IComparable
             $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();
+    }
+}