Cell indexes not being respected when rendering row

Fixes #682
When calling `Row::setCellIndex`, it's possible to create a Row with holes.
Instead of iterating over existing cells of a Row, we should instead use the cell indexes (from 0 to max cell index).

src/Spout/Common/Entity/Row.php

@@ -70,7 +70,7 @@ class Row
      */
     public function getCellAtIndex($cellIndex)
     {
-        return isset($this->cells[$cellIndex]) ? $this->cells[$cellIndex] : null;
+        return $this->cells[$cellIndex] ?? null;
     }
 
     /**
@@ -89,7 +89,13 @@ class Row
      */
     public function getNumCells()
     {
-        return count($this->cells);
+        // When using "setCellAtIndex", it's possible to
+        // have "$this->cells" contain holes.
+        if (empty($this->cells)) {
+            return 0;
+        }
+
+        return max(array_keys($this->cells)) + 1;
     }
 
     /**

src/Spout/Reader/Common/Manager/RowManager.php

@@ -47,12 +47,14 @@ class RowManager
      */
     public function fillMissingIndexesWithEmptyCells(Row $row)
     {
-        if ($row->getNumCells() === 0) {
+        $numCells = $row->getNumCells();
+
+        if ($numCells === 0) {
             return $row;
         }
 
         $rowCells = $row->getCells();
-        $maxCellIndex = max(array_keys($rowCells));
+        $maxCellIndex = $numCells;
 
         for ($cellIndex = 0; $cellIndex < $maxCellIndex; $cellIndex++) {
             if (!isset($rowCells[$cellIndex])) {

src/Spout/Writer/Common/Helper/CellHelper.php

@@ -8,38 +8,39 @@ namespace Box\Spout\Writer\Common\Helper;
  */
 class CellHelper
 {
-    /** @var array Cache containing the mapping column index => cell index */
+    /** @var array Cache containing the mapping column index => column letters */
-    private static $columnIndexToCellIndexCache = [];
+    private static $columnIndexToColumnLettersCache = [];
 
     /**
-     * Returns the cell index (base 26) associated to the base 10 column index.
+     * Returns the column letters (base 26) associated to the base 10 column index.
      * Excel uses A to Z letters for column indexing, where A is the 1st column,
      * Z is the 26th and AA is the 27th.
      * The mapping is zero based, so that 0 maps to A, B maps to 1, Z to 25 and AA to 26.
      *
-     * @param int $columnIndex The Excel column index (0, 42, ...)
+     * @param int $columnIndexZeroBased The Excel column index (0, 42, ...)
+     *
      * @return string The associated cell index ('A', 'BC', ...)
      */
-    public static function getCellIndexFromColumnIndex($columnIndex)
+    public static function getColumnLettersFromColumnIndex($columnIndexZeroBased)
     {
-        $originalColumnIndex = $columnIndex;
+        $originalColumnIndex = $columnIndexZeroBased;
 
         // Using isset here because it is way faster than array_key_exists...
-        if (!isset(self::$columnIndexToCellIndexCache[$originalColumnIndex])) {
+        if (!isset(self::$columnIndexToColumnLettersCache[$originalColumnIndex])) {
-            $cellIndex = '';
+            $columnLetters = '';
             $capitalAAsciiValue = ord('A');
 
             do {
-                $modulus = $columnIndex % 26;
+                $modulus = $columnIndexZeroBased % 26;
-                $cellIndex = chr($capitalAAsciiValue + $modulus) . $cellIndex;
+                $columnLetters = chr($capitalAAsciiValue + $modulus) . $columnLetters;
 
                 // substracting 1 because it's zero-based
-                $columnIndex = (int) ($columnIndex / 26) - 1;
+                $columnIndexZeroBased = (int) ($columnIndexZeroBased / 26) - 1;
-            } while ($columnIndex >= 0);
+            } while ($columnIndexZeroBased >= 0);
 
-            self::$columnIndexToCellIndexCache[$originalColumnIndex] = $cellIndex;
+            self::$columnIndexToColumnLettersCache[$originalColumnIndex] = $columnLetters;
         }
 
-        return self::$columnIndexToCellIndexCache[$originalColumnIndex];
+        return self::$columnIndexToColumnLettersCache[$originalColumnIndex];
     }
 }

src/Spout/Writer/XLSX/Manager/WorksheetManager.php

@@ -153,16 +153,14 @@ EOD;
      */
     private function addNonEmptyRow(Worksheet $worksheet, Row $row)
     {
-        $cellIndex = 0;
         $rowStyle = $row->getStyle();
-        $rowIndex = $worksheet->getLastWrittenRowIndex() + 1;
+        $rowIndexOneBased = $worksheet->getLastWrittenRowIndex() + 1;
         $numCells = $row->getNumCells();
 
-        $rowXML = '<row r="' . $rowIndex . '" spans="1:' . $numCells . '">';
+        $rowXML = '<row r="' . $rowIndexOneBased . '" spans="1:' . $numCells . '">';
 
-        foreach ($row->getCells() as $cell) {
+        foreach ($row->getCells() as $columnIndexZeroBased => $cell) {
-            $rowXML .= $this->applyStyleAndGetCellXML($cell, $rowStyle, $rowIndex, $cellIndex);
+            $rowXML .= $this->applyStyleAndGetCellXML($cell, $rowStyle, $rowIndexOneBased, $columnIndexZeroBased);
-            $cellIndex++;
         }
 
         $rowXML .= '</row>';
@@ -179,12 +177,13 @@ EOD;
      *
      * @param Cell  $cell
      * @param Style $rowStyle
-     * @param int $rowIndex
+     * @param int   $rowIndexOneBased
-     * @param int $cellIndex
+     * @param int   $columnIndexZeroBased
+     *
      * @throws InvalidArgumentException If the given value cannot be processed
      * @return string
      */
-    private function applyStyleAndGetCellXML(Cell $cell, Style $rowStyle, $rowIndex, $cellIndex)
+    private function applyStyleAndGetCellXML(Cell $cell, Style $rowStyle, $rowIndexOneBased, $columnIndexZeroBased)
     {
         // Apply row and extra styles
         $mergedCellAndRowStyle = $this->styleMerger->merge($cell->getStyle(), $rowStyle);
@@ -193,23 +192,24 @@ EOD;
 
         $registeredStyle = $this->styleManager->registerStyle($newCellStyle);
 
-        return $this->getCellXML($rowIndex, $cellIndex, $cell, $registeredStyle->getId());
+        return $this->getCellXML($rowIndexOneBased, $columnIndexZeroBased, $cell, $registeredStyle->getId());
     }
 
     /**
      * Builds and returns xml for a single cell.
      *
-     * @param int $rowIndex
+     * @param int  $rowIndexOneBased
-     * @param int $cellNumber
+     * @param int  $columnIndexZeroBased
      * @param Cell $cell
      * @param int  $styleId
+     *
      * @throws InvalidArgumentException If the given value cannot be processed
      * @return string
      */
-    private function getCellXML($rowIndex, $cellNumber, Cell $cell, $styleId)
+    private function getCellXML($rowIndexOneBased, $columnIndexZeroBased, Cell $cell, $styleId)
     {
-        $columnIndex = CellHelper::getCellIndexFromColumnIndex($cellNumber);
+        $columnLetters = CellHelper::getColumnLettersFromColumnIndex($columnIndexZeroBased);
-        $cellXML = '<c r="' . $columnIndex . $rowIndex . '"';
+        $cellXML = '<c r="' . $columnLetters . $rowIndexOneBased . '"';
         $cellXML .= ' s="' . $styleId . '"';
 
         if ($cell->isString()) {

tests/Spout/Common/Entity/RowTest.php

@@ -78,10 +78,24 @@ class RowTest extends \PHPUnit\Framework\TestCase
         $row = new Row([], null);
         $cellMock = $this->getCellMock();
         $row->setCellAtIndex($cellMock, 3);
+
         $this->assertEquals($cellMock, $row->getCellAtIndex(3));
         $this->assertNull($row->getCellAtIndex(10));
     }
 
+    /**
+     * @return void
+     */
+    public function testSetCellAtIndex()
+    {
+        $row = new Row([], null);
+        $cellMock = $this->getCellMock();
+        $row->setCellAtIndex($cellMock, 1);
+
+        $this->assertEquals(2, $row->getNumCells());
+        $this->assertNull($row->getCellAtIndex(0));
+    }
+
     /**
      * @return void
      */

tests/Spout/Writer/Common/Helper/CellHelperTest.php

@@ -12,7 +12,7 @@ class CellHelperTest extends TestCase
     /**
      * @return array
      */
-    public function dataProviderForTestGetCellIndexFromColumnIndex()
+    public function dataProviderForTestGetColumnLettersFromColumnIndex()
     {
         return [
             [0, 'A'],
@@ -24,14 +24,15 @@ class CellHelperTest extends TestCase
     }
 
     /**
-     * @dataProvider dataProviderForTestGetCellIndexFromColumnIndex
+     * @dataProvider dataProviderForTestGetColumnLettersFromColumnIndex
      *
      * @param int    $columnIndex
-     * @param string $expectedCellIndex
+     * @param string $expectedColumnLetters
+     *
      * @return void
      */
-    public function testGetCellIndexFromColumnIndex($columnIndex, $expectedCellIndex)
+    public function testGetColumnLettersFromColumnIndex($columnIndex, $expectedColumnLetters)
     {
-        $this->assertEquals($expectedCellIndex, CellHelper::getCellIndexFromColumnIndex($columnIndex));
+        $this->assertEquals($expectedColumnLetters, CellHelper::getColumnLettersFromColumnIndex($columnIndex));
     }
 }