Phpspreadsheet: removeColumn() does not remove last column / removes the wrong column

Created on 3 Apr 2019 · 9Comments · Source: PHPOffice/PhpSpreadsheet

This is:

- [x] a bug report
- [ ] a feature request
- [x] **not** a usage question (ask them on https://stackoverflow.com/questions/tagged/phpspreadsheet or https://gitter.im/PHPOffice/PhpSpreadsheet)

What is the expected behavior?

When using removeColumn() to remove the last column in a worksheet I would expect the last column to be removed.

What is the current behavior?

It is not possible to remove the last column in a worksheet. The last column is emptied, but not removed.

Additionally, if you index beyond the last column using removeColumn() then the last column will be emptied.

What are the steps to reproduce?

Consider the following example sheet:

.|A|B|C
-|-|-|-
1|A1|B1|C1
2|A2|B2|C2

Performing removeColumn('C') on this sheet will result in:

.|A|B|C
-|-|-|-
1|A1|B1|
2|A2|B2|

Also removeColumn('D') yields the same result, which is even worse as column C should not have been touched at all in this case.

However, removeColumn('A') (or 'B') works as expected:

.|A|B
-|-|-
1|B1|C1|
2|B2|C2|

I consider this to be two separate bugs, even though they may be related:

It should be possible to remove the last column in a worksheet. As of now it is not.
Indexing out of bounds should have a well defined and unsurprising behavior. Throwing an exception, returning an error code, issuing a warning, or doing nothing are all acceptable (to me) as long as the behavior is defined and documented. Removing (the content of) a different column than the one requested is not.

Please provide a Minimal, Complete, and Verifiable example of code that exhibits the issue without relying on an external Excel file or a web server:

<?php
require_once 'vendor/autoload.php';
$spreadsheet = new \PhpOffice\PhpSpreadsheet\Spreadsheet();
$html_writer = new \PhpOffice\PhpSpreadsheet\Writer\Html( $spreadsheet );
$sheet = $spreadsheet->getActiveSheet();
$remove_column = 'C';
$data = [
    ['A1', 'B1', 'C1'],
    ['A2', 'B2', 'C2'],
];
$sheet->fromArray( $data );
$html_table_before = $html_writer->generateSheetData();
$sheet->removeColumn( $remove_column );
$html_table_after = $html_writer->generateSheetData();
?>
<html>
<head>
    <title>Bug report</title>
    <style>
        table { border-collapse: collapse; }
        td { border: 1px solid silver; padding: 2px 10px; }
    </style>
</head>
<body>
    <h3>PHP version <?php echo phpversion(); ?></h3>
    <p>Before</p>
    <?php echo $html_table_before; ?>
    <p>After removing column <?php echo $remove_column; ?></p>
    <?php echo $html_table_after; ?>
</body>
</html>

Which versions of PhpSpreadsheet and PHP are affected?

Tested with PhpSpreadsheet 1.6.0, PHP 5.6.25, 7.0.10, 7.3.2

P.S.
By the way, how do I get the PhpSpreadsheet version from code?

Hacktoberfest

Source

tomas-eklund

All 9 comments

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If this is still an issue for you, please try to help by debugging it further and sharing your results.
Thank you for your contributions.

stale[bot] on 2 Jun 2019

👎1

It's not the issue that's stale, it's this project. Which is a shame, because it's a really useful product. Bumping to keep the issue active until resolved.

tomas-eklund on 2 Jun 2019

stale[bot] on 1 Aug 2019

👎1

This issue is too serious to let slide.

tomas-eklund on 1 Aug 2019

stale[bot] on 30 Sep 2019

👎1

I'm afraid I don't have the skills to resolve this issue myself so I'm keeping this issue alive until some who does fixes it. If it never gets fixed, at least this issue will serve to warn other users about the bug.

tomas-eklund on 6 Oct 2019

Hi @tomas-eklund,

I've investigated the issue and found that the column is completely removed as expected BUT some cached indexes were not updated and contained stale values.

That meant that when the HTML printer calculated the number of columns, it hit a stale value of 'highest column' with value 'C' in your example, where it should have been 'B' after the column was removed.

Calling the garbage collector after the columns is removed solves the issue.

I've added unit tests to show the issue, you can run vendor/bin/phpunit tests/PhpSpreadsheetTests/Worksheet/WorksheetTest.php on my branch, if you comment out the line I've added to call the garbage collector, the test fails.