diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 13ce6f9e..0e2ede6f 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -4180,16 +4180,6 @@ parameters: count: 1 path: src/PhpSpreadsheet/Worksheet/Worksheet.php - - - message: "#^Parameter \\#1 \\$row of method PhpOffice\\\\PhpSpreadsheet\\\\Collection\\\\Cells\\:\\:removeRow\\(\\) expects string, int given\\.$#" - count: 1 - path: src/PhpSpreadsheet/Worksheet/Worksheet.php - - - - message: "#^Parameter \\#1 \\$row of method PhpOffice\\\\PhpSpreadsheet\\\\Collection\\\\Cells\\:\\:removeRow\\(\\) expects string, int\\<1, max\\> given\\.$#" - count: 1 - path: src/PhpSpreadsheet/Worksheet/Worksheet.php - - message: "#^Parameter \\#2 \\$format of static method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\:\\:toFormattedString\\(\\) expects string, string\\|null given\\.$#" count: 2 diff --git a/src/PhpSpreadsheet/Cell/Coordinate.php b/src/PhpSpreadsheet/Cell/Coordinate.php index b2dd65c4..fac3c629 100644 --- a/src/PhpSpreadsheet/Cell/Coordinate.php +++ b/src/PhpSpreadsheet/Cell/Coordinate.php @@ -47,15 +47,17 @@ abstract class Coordinate * * @param string $coordinates eg: 'A1', '$B$12' * - * @return array{0: int, 1: int} Array containing column index and row index (indexes 0 and 1) + * @return array{0: int, 1: int, 2: string} Array containing column and row index, and column string */ public static function indexesFromString(string $coordinates): array { - [$col, $row] = self::coordinateFromString($coordinates); + [$column, $row] = self::coordinateFromString($coordinates); + $column = ltrim($column, '$'); return [ - self::columnIndexFromString(ltrim($col, '$')), + self::columnIndexFromString($column), (int) ltrim($row, '$'), + $column, ]; } diff --git a/src/PhpSpreadsheet/Collection/Cells.php b/src/PhpSpreadsheet/Collection/Cells.php index 72790e9a..20fccf48 100644 --- a/src/PhpSpreadsheet/Collection/Cells.php +++ b/src/PhpSpreadsheet/Collection/Cells.php @@ -12,6 +12,8 @@ use Psr\SimpleCache\CacheInterface; class Cells { + protected const MAX_COLUMN_ID = 16384; + /** * @var CacheInterface */ @@ -46,9 +48,10 @@ class Cells private $currentCellIsDirty = false; /** - * An index of existing cells. Booleans indexed by their coordinate. + * An index of existing cells. int pointer to the coordinate (0-base-indexed row * 16,384 + 1-base indexed column) + * indexed by their coordinate. * - * @var bool[] + * @var int[] */ private $index = []; @@ -93,12 +96,7 @@ class Cells */ public function has($cellCoordinate) { - if ($cellCoordinate === $this->currentCoordinate) { - return true; - } - - // Check if the requested entry exists in the index - return isset($this->index[$cellCoordinate]); + return ($cellCoordinate === $this->currentCoordinate) || isset($this->index[$cellCoordinate]); } /** @@ -150,15 +148,9 @@ class Cells */ public function getSortedCoordinates() { - $sortKeys = []; - foreach ($this->getCoordinates() as $coordinate) { - sscanf($coordinate, '%[A-Z]%d', $column, $row); - $key = (--$row * 16384) + Coordinate::columnIndexFromString($column); - $sortKeys[$key] = $coordinate; - } - ksort($sortKeys); + asort($this->index); - return array_values($sortKeys); + return array_keys($this->index); } /** @@ -203,21 +195,17 @@ class Cells public function getHighestRowAndColumn() { // Lookup highest column and highest row - $columns = ['1A']; - $rows = [1]; - foreach ($this->getCoordinates() as $coordinate) { - sscanf($coordinate, '%[A-Z]%d', $column, $row); - $rows[$row] = $rows[$row] ?? $row; - $columns[$column] = $columns[$column] ?? strlen($column) . $column; + $maxRow = $maxColumn = 1; + foreach ($this->index as $coordinate) { + $row = (int) floor($coordinate / self::MAX_COLUMN_ID) + 1; + $maxRow = ($maxRow > $row) ? $maxRow : $row; + $column = $coordinate % self::MAX_COLUMN_ID; + $maxColumn = ($maxColumn > $column) ? $maxColumn : $column; } - // Determine highest column and row - $highestRow = max($rows); - $highestColumn = substr((string) max($columns), 1); - return [ - 'row' => $highestRow, - 'column' => $highestColumn, + 'row' => $maxRow, + 'column' => Coordinate::stringFromColumnIndex($maxColumn), ]; } @@ -235,17 +223,23 @@ class Cells return $this->getHighestRowAndColumn()['column']; } - $maxColumn = '1A'; - foreach ($this->getCoordinates() as $coord) { - sscanf($coord, '%[A-Z]%d', $c, $r); - if ($r != $row) { - continue; - } - $sortableColum = strlen($c) . $c; - $maxColumn = $maxColumn > $sortableColum ? $maxColumn : $sortableColum; + $row = (int) $row; + if ($row <= 0) { + throw new PhpSpreadsheetException('Row number must be a positive integer'); } - return substr($maxColumn, 1); + $maxColumn = 1; + $toRow = $row * self::MAX_COLUMN_ID; + $fromRow = --$row * self::MAX_COLUMN_ID; + foreach ($this->index as $coordinate) { + if ($coordinate < $fromRow || $coordinate >= $toRow) { + continue; + } + $column = $coordinate % self::MAX_COLUMN_ID; + $maxColumn = $maxColumn > $column ? $maxColumn : $column; + } + + return Coordinate::stringFromColumnIndex($maxColumn); } /** @@ -263,12 +257,13 @@ class Cells } $maxRow = 1; - foreach ($this->getCoordinates() as $coord) { - sscanf($coord, '%[A-Z]%d', $c, $r); - if ($c != $column) { + $columnIndex = Coordinate::columnIndexFromString($column); + foreach ($this->index as $coordinate) { + if ($coordinate % self::MAX_COLUMN_ID !== $columnIndex) { continue; } - $maxRow = ($maxRow > $r) ? $maxRow : $r; + $row = (int) floor($coordinate / self::MAX_COLUMN_ID) + 1; + $maxRow = ($maxRow > $row) ? $maxRow : $row; } return $maxRow; @@ -297,28 +292,17 @@ class Cells $newCollection = clone $this; $newCollection->parent = $worksheet; - if (is_object($newCollection->currentCell)) { - $newCollection->currentCell->attach($this); - } - - // Get old values - $oldKeys = $newCollection->getAllCacheKeys(); - $oldValues = $newCollection->cache->getMultiple($oldKeys); - $newValues = []; - $oldCachePrefix = $newCollection->cachePrefix; - - // Change prefix $newCollection->cachePrefix = $newCollection->getUniqueID(); - foreach ($oldValues as $oldKey => $value) { - /** @var string $newKey */ - $newKey = str_replace($oldCachePrefix, $newCollection->cachePrefix, $oldKey); - $newValues[$newKey] = clone $value; - } - // Store new values - $stored = $newCollection->cache->setMultiple($newValues); - if ($stored === false) { - $this->destructIfNeeded($newCollection, 'Failed to copy cells in cache'); + foreach ($this->index as $key => $value) { + $newCollection->index[$key] = $value; + $stored = $newCollection->cache->set( + $newCollection->cachePrefix . $key, + clone $this->cache->get($this->cachePrefix . $key) + ); + if ($stored === false) { + $this->destructIfNeeded($newCollection, 'Failed to copy cells in cache'); + } } return $newCollection; @@ -327,14 +311,23 @@ class Cells /** * Remove a row, deleting all cells in that row. * - * @param string $row Row number to remove + * @param int|string $row Row number to remove */ public function removeRow($row): void { - foreach ($this->getCoordinates() as $coord) { - sscanf($coord, '%[A-Z]%d', $c, $r); - if ($r == $row) { - $this->delete($coord); + $this->storeCurrentCell(); + $row = (int) $row; + if ($row <= 0) { + throw new PhpSpreadsheetException('Row number must be a positive integer'); + } + + $toRow = $row * self::MAX_COLUMN_ID; + $fromRow = --$row * self::MAX_COLUMN_ID; + foreach ($this->index as $coordinate) { + if ($coordinate >= $fromRow && $coordinate < $toRow) { + $row = (int) floor($coordinate / self::MAX_COLUMN_ID) + 1; + $column = Coordinate::stringFromColumnIndex($coordinate % self::MAX_COLUMN_ID); + $this->delete("{$column}{$row}"); } } } @@ -346,10 +339,14 @@ class Cells */ public function removeColumn($column): void { - foreach ($this->getCoordinates() as $coord) { - sscanf($coord, '%[A-Z]%d', $c, $r); - if ($c == $column) { - $this->delete($coord); + $this->storeCurrentCell(); + + $columnIndex = Coordinate::columnIndexFromString($column); + foreach ($this->index as $coordinate) { + if ($coordinate % self::MAX_COLUMN_ID === $columnIndex) { + $row = (int) floor($coordinate / self::MAX_COLUMN_ID) + 1; + $column = Coordinate::stringFromColumnIndex($coordinate % self::MAX_COLUMN_ID); + $this->delete("{$column}{$row}"); } } } @@ -394,7 +391,8 @@ class Cells if ($cellCoordinate !== $this->currentCoordinate) { $this->storeCurrentCell(); } - $this->index[$cellCoordinate] = true; + sscanf($cellCoordinate, '%[A-Z]%d', $column, $row); + $this->index[$cellCoordinate] = (--$row * self::MAX_COLUMN_ID) + Coordinate::columnIndexFromString($column); $this->currentCoordinate = $cellCoordinate; $this->currentCell = $cell; @@ -422,7 +420,7 @@ class Cells return null; } - // Check if the entry that has been requested actually exists + // Check if the entry that has been requested actually exists in the cache $cell = $this->cache->get($this->cachePrefix . $cellCoordinate); if ($cell === null) { throw new PhpSpreadsheetException("Cell entry {$cellCoordinate} no longer exists in cache. This probably means that the cache was cleared by someone else."); diff --git a/src/PhpSpreadsheet/Worksheet/Worksheet.php b/src/PhpSpreadsheet/Worksheet/Worksheet.php index 0baf6ecd..4c0237f5 100644 --- a/src/PhpSpreadsheet/Worksheet/Worksheet.php +++ b/src/PhpSpreadsheet/Worksheet/Worksheet.php @@ -181,13 +181,6 @@ class Worksheet implements IComparable */ private $conditionalStylesCollection = []; - /** - * Is the current cell collection sorted already? - * - * @var bool - */ - private $cellCollectionIsSorted = false; - /** * Collection of breaks. * @@ -1336,24 +1329,22 @@ class Worksheet implements IComparable */ public function createNewCell($coordinate) { + [$column, $row, $columnString] = Coordinate::indexesFromString($coordinate); $cell = new Cell(null, DataType::TYPE_NULL, $this); $this->cellCollection->add($coordinate, $cell); - $this->cellCollectionIsSorted = false; // Coordinates - [$column, $row] = Coordinate::coordinateFromString($coordinate); - $aIndexes = Coordinate::indexesFromString($coordinate); - if ($this->cachedHighestColumn < $aIndexes[0]) { - $this->cachedHighestColumn = $aIndexes[0]; + if ($column > $this->cachedHighestColumn) { + $this->cachedHighestColumn = $column; } - if ($aIndexes[1] > $this->cachedHighestRow) { - $this->cachedHighestRow = $aIndexes[1]; + if ($row > $this->cachedHighestRow) { + $this->cachedHighestRow = $row; } // Cell needs appropriate xfIndex from dimensions records // but don't create dimension records if they don't already exist $rowDimension = $this->rowDimensions[$row] ?? null; - $columnDimension = $this->columnDimensions[$column] ?? null; + $columnDimension = $this->columnDimensions[$columnString] ?? null; if ($rowDimension !== null && $rowDimension->getXfIndex() > 0) { // then there is a row dimension with explicit style, assign it to the cell diff --git a/tests/PhpSpreadsheetTests/Collection/CellsTest.php b/tests/PhpSpreadsheetTests/Collection/CellsTest.php index 866f4117..5731d581 100644 --- a/tests/PhpSpreadsheetTests/Collection/CellsTest.php +++ b/tests/PhpSpreadsheetTests/Collection/CellsTest.php @@ -38,18 +38,21 @@ class CellsTest extends TestCase $cell2 = $sheet->getCell('A1'); self::assertSame($cell2, $collection->add('A1', $cell2), 'adding a second cell should return the cell'); self::assertEquals(['B2', 'A1'], $collection->getCoordinates(), 'cell list should contains the second cell'); + // Note that sorts orders the collection itself, so these cells will aread be ordered for the subsequent asserions self::assertEquals(['A1', 'B2'], $collection->getSortedCoordinates(), 'sorted cell list contains the second cell'); // Add a third cell $cell3 = $sheet->getCell('AA1'); self::assertSame($cell3, $collection->add('AA1', $cell3), 'adding a third cell should return the cell'); - self::assertEquals(['B2', 'A1', 'AA1'], $collection->getCoordinates(), 'cell list should contains the third cell'); + self::assertEquals(['A1', 'B2', 'AA1'], $collection->getCoordinates(), 'cell list should contains the third cell'); + // Note that sorts orders the collection itself, so these cells will aread be ordered for the subsequent asserions self::assertEquals(['A1', 'AA1', 'B2'], $collection->getSortedCoordinates(), 'sorted cell list contains the third cell'); // Add a fourth cell $cell4 = $sheet->getCell('Z1'); self::assertSame($cell4, $collection->add('Z1', $cell4), 'adding a fourth cell should return the cell'); - self::assertEquals(['B2', 'A1', 'AA1', 'Z1'], $collection->getCoordinates(), 'cell list should contains the fourth cell'); + self::assertEquals(['A1', 'AA1', 'B2', 'Z1'], $collection->getCoordinates(), 'cell list should contains the fourth cell'); + // Note that sorts orders the collection itself, so these cells will aread be ordered for the subsequent asserions self::assertEquals(['A1', 'Z1', 'AA1', 'B2'], $collection->getSortedCoordinates(), 'sorted cell list contains the fourth cell'); // Assert collection copy @@ -65,7 +68,7 @@ class CellsTest extends TestCase // Assert deletion $collection->delete('B2'); self::assertFalse($collection->has('B2'), 'cell should have been deleted'); - self::assertEquals(['A1', 'AA1', 'Z1'], $collection->getCoordinates(), 'cell list should still contains the A1,Z1 and A11 cells'); + self::assertEquals(['A1', 'Z1', 'AA1'], $collection->getCoordinates(), 'cell list should still contains the A1,Z1 and A11 cells'); // Assert update $cell2 = $sheet->getCell('A1'); @@ -74,7 +77,7 @@ class CellsTest extends TestCase $cell3 = $sheet->getCell('C3'); self::assertSame($cell3, $collection->update($cell3), 'should silently add non-existing C3 cell'); - self::assertEquals(['A1', 'AA1', 'Z1', 'C3'], $collection->getCoordinates(), 'cell list should contains the C3 cell'); + self::assertEquals(['A1', 'Z1', 'AA1', 'C3'], $collection->getCoordinates(), 'cell list should contains the C3 cell'); } public function testCacheLastCell(): void diff --git a/tests/data/Cell/IndexesFromString.php b/tests/data/Cell/IndexesFromString.php index c2fe1c09..0a79e47f 100644 --- a/tests/data/Cell/IndexesFromString.php +++ b/tests/data/Cell/IndexesFromString.php @@ -5,6 +5,7 @@ return [ [ 1, 1, + 'A', ], 'A1', ], @@ -12,6 +13,7 @@ return [ [ 1, 12, + 'A', ], 'A12', ], @@ -19,6 +21,7 @@ return [ [ 10, 1, + 'J', ], 'J1', ], @@ -26,6 +29,7 @@ return [ [ 10, 20, + 'J', ], 'J20', ], @@ -33,6 +37,7 @@ return [ [ 35, 1, + 'AI', ], 'AI1', ], @@ -40,6 +45,7 @@ return [ [ 35, 2012, + 'AI', ], 'AI2012', ], @@ -47,6 +53,7 @@ return [ [ 2, 3, + 'B', ], 'B3', ], @@ -54,6 +61,7 @@ return [ [ 2, 3, + 'B', ], '$B3', ], @@ -61,6 +69,7 @@ return [ [ 2, 3, + 'B', ], 'B$3', ], @@ -68,6 +77,7 @@ return [ [ 2, 3, + 'B', ], '$B$3', ],