Repository: PHPOffice/PhpSpreadsheet (php)

Issue:
Removing columns containing merged columns
This is:

- [X] a bug report
- [ ] a feature request
- [ ] **not** a usage question (ask them on https://stackoverflow.com/questions/tagged/phpspreadsheet or https://gitter.im/PHPOffice/PhpSpreadsheet)

### What is the expected behavior?
The columns are not correctly removed and the file need to be repaired in Excel
<img width="292" alt="expected" src="https://user-images.githubusercontent.com/1458015/33539106-22930f68-d8de-11e7-8357-6a205974e4ef.png">


### What is the current behavior?
After the repair
<img width="350" alt="result" src="https://user-images.githubusercontent.com/1458015/33539105-225a3260-d8de-11e7-8427-1807ff830a05.png">

### What are the steps to reproduce?
Initial template:
<img width="617" alt="template" src="https://user-images.githubusercontent.com/1458015/33539149-599bf024-d8de-11e7-8df7-8aa55147a4c5.png">
The excel file: [TestMergedColumns.xlsx](https://github.com/PHPOffice/PhpSpreadsheet/files/1525901/TestMergedColumns.xlsx)

```php
<?php

require __DIR__ . '/vendor/autoload.php';

// Create new Spreadsheet object
$reader = \PhpOffice\PhpSpreadsheet\IOFactory::createReader('Xlsx');
$spreadsheet = $reader->load('/tmp/TestMergedColumns.xlsx');
$worksheet = $spreadsheet->getSheetByName('Sheet1');

// Remove 5 columns (from B to F)
$worksheet->removeColumnByIndex(2, 5);
$writer = \PhpOffice\PhpSpreadsheet\IOFactory::createWriter($spreadsheet, 'Xlsx');
$writer->save('/tmp/result.xlsx');
```


### Which versions of PhpSpreadsheet and PHP are affected?
- Commit: 59326f1064d61a6ff77822b64124a8046e06a46d


Pull request description:
Removing Columns/Rows Containing Merged Cells
Fix #282, which went stale over 7 years ago, and is now reopened. This is certainly related to issue #4379. If merged cells are in the midst of deleted rows/columns, the merge may continue to exist after the deletion, leading to various problems including spreadsheet corruption. The problem is not with either Reader or Writer.

This PR will automatically unmerge all merged cells whose first cell is in the delete range before performing the deletion. This will address the corruption problem. Something more sophisticated may be required, so I won't merge this PR for a while to give me a chance to think about it some more.

This is:

- [x] a bugfix
- [ ] a new feature
- [ ] refactoring
- [ ] additional unit tests

Checklist:

- [x] Changes are covered by unit tests
  - [x] Changes are covered by existing unit tests
  - [x] New unit tests have been added
- [x] Code style is respected
- [x] Commit message explains **why** the change is made (see https://github.com/erlang/otp/wiki/Writing-good-commit-messages)
- [ ] CHANGELOG.md contains a short summary of the change and a link to the pull request if applicable
- [ ] Documentation is updated as necessary


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7900e663f..395ba1af7 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -30,6 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org).
 ### Fixed
 
 - Micro-optimization for excelToDateTimeObject. [Issue #4438](https://github.com/PHPOffice/PhpSpreadsheet/issues/4438) [PR #4442](https://github.com/PHPOffice/PhpSpreadsheet/pull/4442)
+- Removing Columns/Rows Containing Merged Cells. [Issue #282](https://github.com/PHPOffice/PhpSpreadsheet/issues/282) [PR #4465](https://github.com/PHPOffice/PhpSpreadsheet/pull/4465)
 - Print Area and Row Break. [Issue #1275](https://github.com/PHPOffice/PhpSpreadsheet/issues/1275) [PR #4450](https://github.com/PHPOffice/PhpSpreadsheet/pull/4450)
 - Xls Writer Treat Hyperlink Starting with # as Internal. [Issue #56](https://github.com/PHPOffice/PhpSpreadsheet/issues/56) [PR #4453](https://github.com/PHPOffice/PhpSpreadsheet/pull/4453)
 
diff --git a/docs/index.md b/docs/index.md
index a4b87d32a..d577d8982 100644
--- a/docs/index.md
+++ b/docs/index.md
@@ -51,7 +51,7 @@ Use [composer](https://getcomposer.org) to install PhpSpreadsheet into your proj
 composer require phpoffice/phpspreadsheet
 ```
 
-Or also download the documentation and samples if you plan to use them (note that `git` must be in your path for this to work):
+Or also download the documentation and samples if you plan to use them:
 
 ```sh
 composer require phpoffice/phpspreadsheet --prefer-source
diff --git a/src/PhpSpreadsheet/Worksheet/Worksheet.php b/src/PhpSpreadsheet/Worksheet/Worksheet.php
index e9e0d78c6..1fb73a555 100644
--- a/src/PhpSpreadsheet/Worksheet/Worksheet.php
+++ b/src/PhpSpreadsheet/Worksheet/Worksheet.php
@@ -3,6 +3,7 @@
 namespace PhpOffice\PhpSpreadsheet\Worksheet;
 
 use ArrayObject;
+use Composer\Pcre\Preg;
 use Generator;
 use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
 use PhpOffice\PhpSpreadsheet\Calculation\Functions;
@@ -1205,8 +1206,8 @@ class Worksheet
                 throw new Exception('Sheet not found for name: ' . $worksheetReference[0]);
             }
         } elseif (
-            !preg_match('/^' . Calculation::CALCULATION_REGEXP_CELLREF . '$/i', $coordinate)
-            && preg_match('/^' . Calculation::CALCULATION_REGEXP_DEFINEDNAME . '$/iu', $coordinate)
+            !Preg::isMatch('/^' . Calculation::CALCULATION_REGEXP_CELLREF . '$/i', $coordinate)
+            && Preg::isMatch('/^' . Calculation::CALCULATION_REGEXP_DEFINEDNAME . '$/iu', $coordinate)
         ) {
             // Named range?
             $namedRange = $this->validateNamedRange($coordinate, true);
@@ -1458,7 +1459,7 @@ class Worksheet
     public function getConditionalStyles(string $coordinate, bool $firstOnly = true): array
     {
         $coordinate = strtoupper($coordinate);
-        if (preg_match('/[: ,]/', $coordinate) === 1) {
+        if (Preg::isMatch('/[: ,]/', $coordinate)) {
             return $this->conditionalStylesCollection[$coordinate] ?? [];
         }
 
@@ -1778,7 +1779,7 @@ class Worksheet
             $range .= ":{$range}";
         }
 
-        if (preg_match('/^([A-Z]+)(\d+):([A-Z]+)(\d+)$/', $range, $matches) !== 1) {
+        if (!Preg::isMatch('/^([A-Z]+)(\d+):([A-Z]+)(\d+)$/', $range, $matches)) {
             throw new Exception('Merge must be on a valid range of cells.');
         }
 
@@ -2392,6 +2393,42 @@ class Worksheet
         if ($row < 1) {
             throw new Exception('Rows to be deleted should at least start from row 1.');
         }
+        $startRow = $row;
+        $endRow = $startRow + $numberOfRows - 1;
+        $removeKeys = [];
+        $addKeys = [];
+        foreach ($this->mergeCells as $key => $value) {
+            if (
+                Preg::isMatch(
+                    '/^([a-z]{1,3})(\d+):([a-z]{1,3})(\d+)/i',
+                    $key,
+                    $matches
+                )
+            ) {
+                $startMergeInt = (int) $matches[2];
+                $endMergeInt = (int) $matches[4];
+                if ($startMergeInt >= $startRow) {
+                    if ($startMergeInt <= $endRow) {
+                        $removeKeys[] = $key;
+                    }
+                } elseif ($endMergeInt >= $startRow) {
+                    if ($endMergeInt <= $endRow) {
+                        $temp = $endMergeInt - 1;
+                        $removeKeys[] = $key;
+                        if ($temp !== $startMergeInt) {
+                            $temp3 = $matches[1] . $matches[2] . ':' . $matches[3] . $temp;
+                            $addKeys[] = $temp3;
+                        }
+                    }
+                }
+            }
+        }
+        foreach ($removeKeys as $key) {
+            unset($this->mergeCells[$key]);
+        }
+        foreach ($addKeys as $key) {
+            $this->mergeCells[$key] = $key;
+        }
 
         $holdRowDimensions = $this->removeRowDimensions($row, $numberOfRows);
         $highestRow = $this->getHighestDataRow();
@@ -2448,6 +2485,43 @@ class Worksheet
         if (is_numeric($column)) {
             throw new Exception('Column references should not be numeric.');
         }
+        $startColumnInt = Coordinate::columnIndexFromString($column);
+        $endColumnInt = $startColumnInt + $numberOfColumns - 1;
+        $removeKeys = [];
+        $addKeys = [];
+        foreach ($this->mergeCells as $key => $value) {
+            if (
+                Preg::isMatch(
+                    '/^([a-z]{1,3})(\d+):([a-z]{1,3})(\d+)/i',
+                    $key,
+                    $matches
+                )
+            ) {
+                $startMergeInt = Coordinate::columnIndexFromString($matches[1]);
+                $endMergeInt = Coordinate::columnIndexFromString($matches[3]);
+                if ($startMergeInt >= $startColumnInt) {
+                    if ($startMergeInt <= $endColumnInt) {
+                        $removeKeys[] = $key;
+                    }
+                } elseif ($endMergeInt >= $startColumnInt) {
+                    if ($endMergeInt <= $endColumnInt) {
+                        $temp = Coordinate::columnIndexFromString($matches[3]) - 1;
+                        $temp2 = Coordinate::stringFromColumnIndex($temp);
+                        $removeKeys[] = $key;
+                        if ($temp2 !== $matches[1]) {
+                            $temp3 = $matches[1] . $matches[2] . ':' . $temp2 . $matches[4];
+                            $addKeys[] = $temp3;
+                        }
+                    }
+                }
+            }
+        }
+        foreach ($removeKeys as $key) {
+            unset($this->mergeCells[$key]);
+        }
+        foreach ($addKeys as $key) {
+            $this->mergeCells[$key] = $key;
+        }
 
         $highestColumn = $this->getHighestDataColumn();
         $highestColumnIndex = Coordinate::columnIndexFromString($highestColumn);
@@ -3436,7 +3510,7 @@ class Worksheet
         $collectionCells = [];
         $collectionRanges = [];
         foreach ($this->dataValidationCollection as $key => $dataValidation) {
-            if (preg_match('/[: ]/', $key) === 1) {
+            if (Preg::isMatch('/[: ]/', $key)) {
                 $collectionRanges[$key] = $dataValidation;
             } else {
                 $collectionCells[$key] = $dataValidation;
@@ -3704,7 +3778,7 @@ class Worksheet
 
     public static function nameRequiresQuotes(string $sheetName): bool
     {
-        return preg_match(self::SHEET_NAME_REQUIRES_NO_QUOTES, $sheetName) !== 1;
+        return !Preg::isMatch(self::SHEET_NAME_REQUIRES_NO_QUOTES, $sheetName);
     }
 
     public function isRowVisible(int $row): bool
@@ -3837,7 +3911,7 @@ class Worksheet
             $keys = $this->cellCollection->getCoordinates();
             foreach ($keys as $key) {
                 if ($this->getCell($key)->getDataType() === DataType::TYPE_FORMULA) {
-                    if (preg_match(self::FUNCTION_LIKE_GROUPBY, $this->getCell($key)->getValueString()) !== 1) {
+                    if (!Preg::isMatch(self::FUNCTION_LIKE_GROUPBY, $this->getCell($key)->getValueString())) {
                         $this->getCell($key)->getCalculatedValue();
                     }
                 }


Test patch:
diff --git a/tests/PhpSpreadsheetTests/Worksheet/MergeCellsDeletedTest.php b/tests/PhpSpreadsheetTests/Worksheet/MergeCellsDeletedTest.php
new file mode 100644
index 000000000..5df39bcd8
--- /dev/null
+++ b/tests/PhpSpreadsheetTests/Worksheet/MergeCellsDeletedTest.php
@@ -0,0 +1,169 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpOffice\PhpSpreadsheetTests\Worksheet;
+
+use PhpOffice\PhpSpreadsheet\Reader\Xlsx as XlsxReader;
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
+use PhpOffice\PhpSpreadsheet\Style\Alignment;
+use PhpOffice\PhpSpreadsheet\Style\Fill;
+use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
+use PHPUnit\Framework\TestCase;
+
+class MergeCellsDeletedTest extends TestCase
+{
+    public function testDeletedColumns(): void
+    {
+        $infile = 'tests/data/Reader/XLSX/issue.282.xlsx';
+        $reader = new XlsxReader();
+        $spreadsheet = $reader->load($infile);
+        $sheet = $spreadsheet->getSheetByNameOrThrow('Sheet1');
+
+        $mergeCells = $sheet->getMergeCells();
+        self::assertSame(['B1:F1', 'G1:I1'], array_values($mergeCells));
+
+        // Want to delete column B,C,D,E,F
+        $sheet->removeColumnByIndex(2, 5);
+        $mergeCells2 = $sheet->getMergeCells();
+        self::assertSame(['B1:D1'], array_values($mergeCells2));
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    public function testDeletedRows(): void
+    {
+        $infile = 'tests/data/Reader/XLSX/issue.282.xlsx';
+        $reader = new XlsxReader();
+        $spreadsheet = $reader->load($infile);
+        $sheet = $spreadsheet->getSheetByNameOrThrow('Sheet2');
+
+        $mergeCells = $sheet->getMergeCells();
+        self::assertSame(['A2:A6', 'A7:A9'], array_values($mergeCells));
+
+        // Want to delete rows 2 to 4
+        $sheet->removeRow(2, 3);
+        $mergeCells2 = $sheet->getMergeCells();
+        self::assertSame(['A4:A6'], array_values($mergeCells2));
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    private static function yellowBackground(Worksheet $sheet, string $cells, string $color = 'ffffff00'): void
+    {
+        $sheet->getStyle($cells)
+            ->getFill()
+            ->setFillType(Fill::FILL_SOLID);
+        $sheet->getStyle($cells)
+            ->getFill()
+            ->getStartColor()
+            ->setArgb($color);
+        $sheet->getStyle($cells)
+            ->getAlignment()
+            ->setHorizontal(Alignment::HORIZONTAL_CENTER);
+    }
+
+    public static function testDeletedColumns2(): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->getActiveSheet();
+        $sheet->setTitle('Before');
+        $sheet->getCell('A1')->setValue('a1');
+        $sheet->getCell('J1')->setValue('j1');
+        $sheet->getCell('K1')->setValue('will delete d-f');
+        $sheet->getCell('C1')->setValue('c1-g1');
+        $sheet->mergeCells('C1:G1');
+        self::yellowBackground($sheet, 'C1');
+
+        $sheet->getCell('A2')->setValue('a2');
+        $sheet->getCell('J2')->setValue('j2');
+        $sheet->getCell('B2')->setValue('b2-c2');
+        $sheet->mergeCells('B2:C2');
+        self::yellowBackground($sheet, 'B2');
+        $sheet->getCell('G2')->setValue('g2-h2');
+        $sheet->mergeCells('G2:H2');
+        self::yellowBackground($sheet, 'G2', 'FF00FFFF');
+
+        $sheet->getCell('A3')->setValue('a3');
+        $sheet->getCell('J3')->setValue('j3');
+        $sheet->getCell('D3')->setValue('d3-g3');
+        $sheet->mergeCells('D3:G3');
+        self::yellowBackground($sheet, 'D3');
+
+        $sheet->getCell('A4')->setValue('a4');
+        $sheet->getCell('J4')->setValue('j4');
+        $sheet->getCell('B4')->setValue('b4-d4');
+        $sheet->mergeCells('B4:D4');
+        self::yellowBackground($sheet, 'B4');
+
+        $sheet->getCell('A5')->setValue('a5');
+        $sheet->getCell('J5')->setValue('j5');
+        $sheet->getCell('D5')->setValue('d5-e5');
+        $sheet->mergeCells('D5:E5');
+        self::yellowBackground($sheet, 'D5');
+
+        $sheet->removeColumn('D', 3);
+        $expected = [
+            'C1:D1', // was C1:G1, drop 3 inside cells
+            'B2:C2', // was B2:C2, unaffected
+            'D2:E2', // was G2:H2, move 3 columns left
+            //'D2:E2', // was D3:G3, start in delete range
+            'B4:C4', // was B4:D4, truncated at start of delete range
+            //'D5:E5', // was D5:E5, start in delete range
+        ];
+        self::assertSame($expected, array_keys($sheet->getMergeCells()));
+
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    public static function testDeletedRows2(): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->getActiveSheet();
+        $sheet->setTitle('Before');
+        $sheet->getCell('A1')->setValue('a1');
+        $sheet->getCell('A10')->setValue('a10');
+        $sheet->getCell('A11')->setValue('will delete 4-6');
+        $sheet->getCell('A3')->setValue('a3-a7');
+        $sheet->mergeCells('A3:A7');
+        self::yellowBackground($sheet, 'A3');
+
+        $sheet->getCell('B1')->setValue('b1');
+        $sheet->getCell('B10')->setValue('b10');
+        $sheet->getCell('B2')->setValue('b2-b3');
+        $sheet->mergeCells('B2:B3');
+        self::yellowBackground($sheet, 'B2');
+        $sheet->getCell('B7')->setValue('b7-b8');
+        $sheet->mergeCells('B7:B8');
+        self::yellowBackground($sheet, 'B7', 'FF00FFFF');
+
+        $sheet->getCell('C1')->setValue('c1');
+        $sheet->getCell('C10')->setValue('c10');
+        $sheet->getCell('C4')->setValue('c4-c7');
+        $sheet->mergeCells('C4:C7');
+        self::yellowBackground($sheet, 'C4');
+
+        $sheet->getCell('D1')->setValue('d1');
+        $sheet->getCell('D10')->setValue('d10');
+        $sheet->getCell('D2')->setValue('d2-d4');
+        $sheet->mergeCells('D2:D4');
+        self::yellowBackground($sheet, 'd2');
+
+        $sheet->getCell('E1')->setValue('e1');
+        $sheet->getCell('E10')->setValue('e10');
+        $sheet->getCell('E4')->setValue('e4-e5');
+        $sheet->mergeCells('E4:E5');
+        self::yellowBackground($sheet, 'E4');
+
+        $sheet->removeRow(4, 3);
+        $expected = [
+            'A3:A4', // was A3:A7, drop 3 inside cells
+            'B2:B3', // was B2:B3, unaffected
+            'B4:B5', // was B7:B8, move 3 columns up
+            //'C4:C7', // was C4:C7, start in delete range
+            'D2:D3', // was D2:D4, truncated at start of delete range
+            //'E4:E5', // was E4:E5, start in delete range
+        ];
+        self::assertSame($expected, array_keys($sheet->getMergeCells()));
+
+        $spreadsheet->disconnectWorksheets();
+    }
+}