From 545fc89d8fd7484cad18e335a6f537e304738686 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Sat, 11 Feb 2023 17:21:19 -0800 Subject: [PATCH] Resolve Phpstan Messages Reader/Xls Shared/Escher (#3355) * Resolve Phpstan Messages Reader/Xls Shared/Escher Reduce number of Phpstan messages by addressing their issues. * Scrutinizer Hyperactivity. * More Scrutinizer Getting close. * Even More Scrutinizer Closer and closer. --- phpstan-baseline.neon | 230 ------------------ src/PhpSpreadsheet/Reader/Xls.php | 50 ++-- src/PhpSpreadsheet/Shared/Escher.php | 8 +- .../Shared/Escher/DgContainer.php | 29 ++- .../Escher/DgContainer/SpgrContainer.php | 2 +- .../Shared/Escher/DggContainer.php | 4 +- .../DggContainer/BstoreContainer/BSE.php | 7 +- src/PhpSpreadsheet/Writer/Xls/Escher.php | 72 +++--- .../Shared/DgContainerTest.php | 24 ++ 9 files changed, 121 insertions(+), 305 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Shared/DgContainerTest.php diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 954ac8cbf5..eb3eae5acf 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -45,216 +45,6 @@ parameters: count: 1 path: src/PhpSpreadsheet/Calculation/Statistical/Trends.php - - - message: "#^Call to an undefined method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Escher\\|PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Escher\\\\DgContainer\\|PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Escher\\\\DgContainer\\\\SpgrContainer\\|PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Escher\\\\DgContainer\\\\SpgrContainer\\\\SpContainer\\|PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Escher\\\\DggContainer\\|PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Escher\\\\DggContainer\\\\BstoreContainer\\|PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Escher\\\\DggContainer\\\\BstoreContainer\\\\BSE\\:\\:getDgContainer\\(\\)\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Call to an undefined method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Escher\\|PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Escher\\\\DgContainer\\|PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Escher\\\\DgContainer\\\\SpgrContainer\\|PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Escher\\\\DgContainer\\\\SpgrContainer\\\\SpContainer\\|PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Escher\\\\DggContainer\\|PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Escher\\\\DggContainer\\\\BstoreContainer\\|PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Escher\\\\DggContainer\\\\BstoreContainer\\\\BSE\\:\\:getDggContainer\\(\\)\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Call to an undefined method object\\:\\:getEndCoordinates\\(\\)\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Call to an undefined method object\\:\\:getEndOffsetX\\(\\)\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Call to an undefined method object\\:\\:getEndOffsetY\\(\\)\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Call to an undefined method object\\:\\:getNestingLevel\\(\\)\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Call to an undefined method object\\:\\:getOPT\\(\\)\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Call to an undefined method object\\:\\:getStartCoordinates\\(\\)\\.$#" - count: 2 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Call to an undefined method object\\:\\:getStartOffsetX\\(\\)\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Call to an undefined method object\\:\\:getStartOffsetY\\(\\)\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Cannot access offset 1 on array\\|false\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^If condition is always true\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xls\\:\\:includeCellRangeFiltered\\(\\) has no return type specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xls\\:\\:includeCellRangeFiltered\\(\\) has parameter \\$cellRangeAddress with no type specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xls\\:\\:parseRichText\\(\\) has no return type specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xls\\:\\:parseRichText\\(\\) has parameter \\$is with no type specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Negated boolean expression is always false\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Parameter \\#1 \\$block of method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xls\\:\\:makeKey\\(\\) expects int, float given\\.$#" - count: 2 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Parameter \\#1 \\$showSummaryBelow of method PhpOffice\\\\PhpSpreadsheet\\\\Worksheet\\\\Worksheet\\:\\:setShowSummaryBelow\\(\\) expects bool, int given\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Parameter \\#1 \\$showSummaryRight of method PhpOffice\\\\PhpSpreadsheet\\\\Worksheet\\\\Worksheet\\:\\:setShowSummaryRight\\(\\) expects bool, int given\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Parameter \\#2 \\$row of method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\IReadFilter\\:\\:readCell\\(\\) expects int, string given\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Parameter \\#2 \\$row of static method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Xls\\:\\:sizeRow\\(\\) expects int, string given\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Parameter \\#2 \\$startRow of static method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Xls\\:\\:getDistanceY\\(\\) expects int, string given\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Parameter \\#3 \\$subject of function str_replace expects array\\|string, int\\|string\\|null given\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Parameter \\#4 \\$endRow of static method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Xls\\:\\:getDistanceY\\(\\) expects int, string given\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xls\\:\\:\\$data \\(string\\) does not accept string\\|null\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xls\\:\\:\\$documentSummaryInformation \\(string\\) does not accept string\\|null\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xls\\:\\:\\$documentSummaryInformation \\(string\\) in isset\\(\\) is not nullable\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xls\\:\\:\\$md5Ctxt is never written, only read\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xls\\:\\:\\$summaryInformation \\(string\\) does not accept string\\|null\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xls\\:\\:\\$summaryInformation \\(string\\) in isset\\(\\) is not nullable\\.$#" - count: 1 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Unreachable statement \\- code above always terminates\\.$#" - count: 7 - path: src/PhpSpreadsheet/Reader/Xls.php - - - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Escher\\\\DgContainer\\:\\:getDgId\\(\\) has no return type specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Shared/Escher/DgContainer.php - - - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Escher\\\\DgContainer\\:\\:getLastSpId\\(\\) has no return type specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Shared/Escher/DgContainer.php - - - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Escher\\\\DgContainer\\:\\:getSpgrContainer\\(\\) has no return type specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Shared/Escher/DgContainer.php - - - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Escher\\\\DgContainer\\:\\:setDgId\\(\\) has parameter \\$value with no type specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Shared/Escher/DgContainer.php - - - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Escher\\\\DgContainer\\:\\:setLastSpId\\(\\) has parameter \\$value with no type specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Shared/Escher/DgContainer.php - - - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Escher\\\\DgContainer\\:\\:setSpgrContainer\\(\\) has no return type specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Shared/Escher/DgContainer.php - - - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Escher\\\\DgContainer\\:\\:setSpgrContainer\\(\\) has parameter \\$spgrContainer with no type specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Shared/Escher/DgContainer.php - - - - message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Escher\\\\DgContainer\\:\\:\\$spgrContainer has no type specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Shared/Escher/DgContainer.php - - - - message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Escher\\\\DgContainer\\\\SpgrContainer\\:\\:getChildren\\(\\) has no return type specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Shared/Escher/DgContainer/SpgrContainer.php - - - - message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Escher\\\\DggContainer\\\\BstoreContainer\\\\BSE\\:\\:\\$parent is never read, only written\\.$#" - count: 1 - path: src/PhpSpreadsheet/Shared/Escher/DggContainer/BstoreContainer/BSE.php - - message: "#^Cannot access offset 1 on array\\|false\\.$#" count: 1 @@ -595,26 +385,6 @@ parameters: count: 1 path: src/PhpSpreadsheet/Shared/Trend/Trend.php - - - message: "#^Elseif condition is always true\\.$#" - count: 1 - path: src/PhpSpreadsheet/Writer/Xls/Escher.php - - - - message: "#^If condition is always true\\.$#" - count: 3 - path: src/PhpSpreadsheet/Writer/Xls/Escher.php - - - - message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Writer\\\\Xls\\\\Escher\\:\\:\\$data has no type specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Writer/Xls/Escher.php - - - - message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Writer\\\\Xls\\\\Escher\\:\\:\\$object has no type specified\\.$#" - count: 1 - path: src/PhpSpreadsheet/Writer/Xls/Escher.php - - message: "#^Cannot access offset 'comp' on array\\|false\\.$#" count: 1 diff --git a/src/PhpSpreadsheet/Reader/Xls.php b/src/PhpSpreadsheet/Reader/Xls.php index 6d82b9a05f..7c17937407 100644 --- a/src/PhpSpreadsheet/Reader/Xls.php +++ b/src/PhpSpreadsheet/Reader/Xls.php @@ -14,6 +14,7 @@ use PhpOffice\PhpSpreadsheet\Shared\CodePage; use PhpOffice\PhpSpreadsheet\Shared\Date; use PhpOffice\PhpSpreadsheet\Shared\Escher; +use PhpOffice\PhpSpreadsheet\Shared\Escher\DgContainer\SpgrContainer\SpContainer; use PhpOffice\PhpSpreadsheet\Shared\Escher\DggContainer\BstoreContainer\BSE; use PhpOffice\PhpSpreadsheet\Shared\File; use PhpOffice\PhpSpreadsheet\Shared\OLE; @@ -161,14 +162,14 @@ class Xls extends BaseReader /** * Summary Information stream data. * - * @var string + * @var ?string */ private $summaryInformation; /** * Extended Summary Information stream data. * - * @var string + * @var ?string */ private $documentSummaryInformation; @@ -387,7 +388,7 @@ class Xls extends BaseReader /** * The current RC4 decryption object. * - * @var Xls\RC4 + * @var ?Xls\RC4 */ private $rc4Key; @@ -400,10 +401,11 @@ class Xls extends BaseReader /** * The current MD5 context state. + * It is never set in the program, so code which uses it is suspect. * * @var string */ - private $md5Ctxt; + private $md5Ctxt; // @phpstan-ignore-line /** * @var int @@ -1088,7 +1090,8 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet $escherWorksheet = $reader->load($this->drawingData); // get all spContainers in one long array, so they can be mapped to OBJ records - $allSpContainers = $escherWorksheet->getDgContainer()->getSpgrContainer()->getAllSpContainers(); + /** @var SpContainer[] */ + $allSpContainers = method_exists($escherWorksheet, 'getDgContainer') ? $escherWorksheet->getDgContainer()->getSpgrContainer()->getAllSpContainers() : []; } // treat OBJ records @@ -1103,7 +1106,9 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet } // calculate the width and height of the shape + /** @var int $startRow */ [$startColumn, $startRow] = Coordinate::coordinateFromString($spContainer->getStartCoordinates()); + /** @var int $endRow */ [$endColumn, $endRow] = Coordinate::coordinateFromString($spContainer->getEndCoordinates()); $startOffsetX = $spContainer->getStartOffsetX(); @@ -1145,7 +1150,7 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet } if ($escherWorkbook) { - $BSECollection = $escherWorkbook->getDggContainer()->getBstoreContainer()->getBSECollection(); + $BSECollection = method_exists($escherWorkbook, 'getDggContainer') ? $escherWorkbook->getDggContainer()->getBstoreContainer()->getBSECollection() : []; $BSE = $BSECollection[$BSEindex - 1]; $blipType = $BSE->getBlipType(); @@ -1195,6 +1200,7 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet // treat SHAREDFMLA records if ($this->version == self::XLS_BIFF8) { foreach ($this->sharedFormulaParts as $cell => $baseCell) { + /** @var int $row */ [$column, $row] = Coordinate::coordinateFromString($cell); if (($this->getReadFilter() !== null) && $this->getReadFilter()->readCell($column, $row, $this->phpSheet->getTitle())) { $formula = $this->getFormulaFromStructure($this->sharedFormulas[$baseCell], $cell); @@ -1335,8 +1341,8 @@ private function readRecordData($data, $pos, $len) $recordData = ''; if ($this->encryption == self::MS_BIFF_CRYPTO_RC4) { $oldBlock = floor($this->rc4Pos / self::REKEY_BLOCK); - $block = floor($pos / self::REKEY_BLOCK); - $endBlock = floor(($pos + $len) / self::REKEY_BLOCK); + $block = (int) floor($pos / self::REKEY_BLOCK); + $endBlock = (int) floor(($pos + $len) / self::REKEY_BLOCK); // Spin an RC4 decryptor to the right spot. If we have a decryptor sitting // at a point earlier in the current block, re-use it as we can save some time. @@ -1382,7 +1388,7 @@ private function loadOLE($filename): void // get excel data, $ole->read($filename); // Get workbook data: workbook stream + sheet streams - $this->data = $ole->getStream($ole->wrkbook); + $this->data = $ole->getStream($ole->wrkbook); // @phpstan-ignore-line // Get summary information data $this->summaryInformation = $ole->getStream($ole->summaryInformation); // Get additional document summary information data @@ -1723,7 +1729,7 @@ private function readNote(): void $cellAddress = array_pop($arrayKeys); } - $cellAddress = str_replace('$', '', $cellAddress); + $cellAddress = str_replace('$', '', (string) $cellAddress); $noteLength = self::getUInt2d($recordData, 4); $noteText = trim(substr($recordData, 6)); @@ -3154,11 +3160,11 @@ private function readSheetPr(): void // bit: 6; mask: 0x0040; 0 = outline buttons above outline group $isSummaryBelow = (0x0040 & self::getUInt2d($recordData, 0)) >> 6; - $this->phpSheet->setShowSummaryBelow($isSummaryBelow); + $this->phpSheet->setShowSummaryBelow((bool) $isSummaryBelow); // bit: 7; mask: 0x0080; 0 = outline buttons left of outline group $isSummaryRight = (0x0080 & self::getUInt2d($recordData, 0)) >> 7; - $this->phpSheet->setShowSummaryRight($isSummaryRight); + $this->phpSheet->setShowSummaryRight((bool) $isSummaryRight); // bit: 8; mask: 0x100; 0 = scale printout in percent, 1 = fit printout to number of pages // this corresponds to radio button setting in page setup dialog in Excel @@ -4540,7 +4546,7 @@ private function readSelection(): void } } - private function includeCellRangeFiltered($cellRangeAddress) + private function includeCellRangeFiltered(string $cellRangeAddress): bool { $includeCellRange = true; if ($this->getReadFilter() !== null) { @@ -5637,8 +5643,6 @@ private function getNextToken($formulaData, $baseCell = 'A1') break; default: throw new Exception('Unrecognized space type in tAttrSpace token'); - - break; } // offset: 3; size: 1; number of inserted spaces/carriage returns $spacecount = ord($formulaData[3]); @@ -5648,8 +5652,6 @@ private function getNextToken($formulaData, $baseCell = 'A1') break; default: throw new Exception('Unrecognized attribute flag in tAttr token'); - - break; } break; @@ -6500,8 +6502,6 @@ private function getNextToken($formulaData, $baseCell = 'A1') break; default: throw new Exception('Unrecognized function in formula'); - - break; } $data = ['function' => $function, 'args' => $args]; @@ -6870,8 +6870,6 @@ private function getNextToken($formulaData, $baseCell = 'A1') break; default: throw new Exception('Unrecognized function in formula'); - - break; } $data = ['function' => $function, 'args' => $args]; @@ -7004,8 +7002,6 @@ private function getNextToken($formulaData, $baseCell = 'A1') // Unknown cases // don't know how to deal with default: throw new Exception('Unrecognized token ' . sprintf('%02X', $id) . ' in formula'); - - break; } return [ @@ -7413,13 +7409,9 @@ private function readSheetRangeByRefIndex($index) } return $sheetRange; - - break; default: // TODO: external sheet support throw new Exception('Xls reader only supports internal sheets in formulas'); - - break; } } @@ -7821,7 +7813,7 @@ public static function getUInt2d($data, $pos) */ public static function getInt2d($data, $pos) { - return unpack('s', $data[$pos] . $data[$pos + 1])[1]; + return unpack('s', $data[$pos] . $data[$pos + 1])[1]; // @phpstan-ignore-line } /** @@ -7848,7 +7840,7 @@ public static function getInt4d($data, $pos) return ord($data[$pos]) | (ord($data[$pos + 1]) << 8) | (ord($data[$pos + 2]) << 16) | $_ord_24; } - private function parseRichText($is) + private function parseRichText(string $is): RichText { $value = new RichText(); $value->createText($is); diff --git a/src/PhpSpreadsheet/Shared/Escher.php b/src/PhpSpreadsheet/Shared/Escher.php index c6d0a6f811..466e7e8254 100644 --- a/src/PhpSpreadsheet/Shared/Escher.php +++ b/src/PhpSpreadsheet/Shared/Escher.php @@ -7,21 +7,21 @@ class Escher /** * Drawing Group Container. * - * @var Escher\DggContainer + * @var ?Escher\DggContainer */ private $dggContainer; /** * Drawing Container. * - * @var Escher\DgContainer + * @var ?Escher\DgContainer */ private $dgContainer; /** * Get Drawing Group Container. * - * @return Escher\DggContainer + * @return ?Escher\DggContainer */ public function getDggContainer() { @@ -43,7 +43,7 @@ public function setDggContainer($dggContainer) /** * Get Drawing Container. * - * @return Escher\DgContainer + * @return ?Escher\DgContainer */ public function getDgContainer() { diff --git a/src/PhpSpreadsheet/Shared/Escher/DgContainer.php b/src/PhpSpreadsheet/Shared/Escher/DgContainer.php index b0d75d7833..51c6860cbe 100644 --- a/src/PhpSpreadsheet/Shared/Escher/DgContainer.php +++ b/src/PhpSpreadsheet/Shared/Escher/DgContainer.php @@ -2,50 +2,63 @@ namespace PhpOffice\PhpSpreadsheet\Shared\Escher; +use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException; + class DgContainer { /** * Drawing index, 1-based. * - * @var int + * @var ?int */ private $dgId; /** * Last shape index in this drawing. * - * @var int + * @var ?int */ private $lastSpId; + /** @var ?DgContainer\SpgrContainer */ private $spgrContainer; - public function getDgId() + public function getDgId(): ?int { return $this->dgId; } - public function setDgId($value): void + public function setDgId(int $value): void { $this->dgId = $value; } - public function getLastSpId() + public function getLastSpId(): ?int { return $this->lastSpId; } - public function setLastSpId($value): void + public function setLastSpId(int $value): void { $this->lastSpId = $value; } - public function getSpgrContainer() + public function getSpgrContainer(): ?DgContainer\SpgrContainer { return $this->spgrContainer; } - public function setSpgrContainer($spgrContainer) + public function getSpgrContainerOrThrow(): DgContainer\SpgrContainer + { + if ($this->spgrContainer !== null) { + return $this->spgrContainer; + } + + throw new SpreadsheetException('spgrContainer is unexpectedly null'); + } + + /** @param DgContainer\SpgrContainer $spgrContainer */ + public function setSpgrContainer($spgrContainer): DgContainer\SpgrContainer { return $this->spgrContainer = $spgrContainer; } diff --git a/src/PhpSpreadsheet/Shared/Escher/DgContainer/SpgrContainer.php b/src/PhpSpreadsheet/Shared/Escher/DgContainer/SpgrContainer.php index 6bdc8f7dca..260df9cd4c 100644 --- a/src/PhpSpreadsheet/Shared/Escher/DgContainer/SpgrContainer.php +++ b/src/PhpSpreadsheet/Shared/Escher/DgContainer/SpgrContainer.php @@ -48,7 +48,7 @@ public function addChild($child): void /** * Get collection of Shape Containers. */ - public function getChildren() + public function getChildren(): array { return $this->children; } diff --git a/src/PhpSpreadsheet/Shared/Escher/DggContainer.php b/src/PhpSpreadsheet/Shared/Escher/DggContainer.php index 36806aa683..ba5e7980b2 100644 --- a/src/PhpSpreadsheet/Shared/Escher/DggContainer.php +++ b/src/PhpSpreadsheet/Shared/Escher/DggContainer.php @@ -28,7 +28,7 @@ class DggContainer /** * BLIP Store Container. * - * @var DggContainer\BstoreContainer + * @var ?DggContainer\BstoreContainer */ private $bstoreContainer; @@ -109,7 +109,7 @@ public function setCSpSaved($value): void /** * Get BLIP Store Container. * - * @return DggContainer\BstoreContainer + * @return ?DggContainer\BstoreContainer */ public function getBstoreContainer() { diff --git a/src/PhpSpreadsheet/Shared/Escher/DggContainer/BstoreContainer/BSE.php b/src/PhpSpreadsheet/Shared/Escher/DggContainer/BstoreContainer/BSE.php index d24af3f7c7..328ac6b6c2 100644 --- a/src/PhpSpreadsheet/Shared/Escher/DggContainer/BstoreContainer/BSE.php +++ b/src/PhpSpreadsheet/Shared/Escher/DggContainer/BstoreContainer/BSE.php @@ -19,15 +19,16 @@ class BSE /** * The parent BLIP Store Entry Container. + * Property is never currently read. * * @var BstoreContainer */ - private $parent; + private $parent; // @phpstan-ignore-line /** * The BLIP (Big Large Image or Picture). * - * @var BSE\Blip + * @var ?BSE\Blip */ private $blip; @@ -49,7 +50,7 @@ public function setParent(BstoreContainer $parent): void /** * Get the BLIP. * - * @return BSE\Blip + * @return ?BSE\Blip */ public function getBlip() { diff --git a/src/PhpSpreadsheet/Writer/Xls/Escher.php b/src/PhpSpreadsheet/Writer/Xls/Escher.php index d5d60c6469..3786ee8c9f 100644 --- a/src/PhpSpreadsheet/Writer/Xls/Escher.php +++ b/src/PhpSpreadsheet/Writer/Xls/Escher.php @@ -3,6 +3,7 @@ namespace PhpOffice\PhpSpreadsheet\Writer\Xls; use PhpOffice\PhpSpreadsheet\Cell\Coordinate; +use PhpOffice\PhpSpreadsheet\Shared\Escher as SharedEscher; use PhpOffice\PhpSpreadsheet\Shared\Escher\DgContainer; use PhpOffice\PhpSpreadsheet\Shared\Escher\DgContainer\SpgrContainer; use PhpOffice\PhpSpreadsheet\Shared\Escher\DgContainer\SpgrContainer\SpContainer; @@ -15,11 +16,15 @@ class Escher { /** * The object we are writing. + * + * @var Blip|BSE|BstoreContainer|DgContainer|DggContainer|Escher|SpContainer|SpgrContainer */ private $object; /** * The written binary data. + * + * @var string */ private $data; @@ -58,11 +63,11 @@ public function close() $this->data = ''; switch (get_class($this->object)) { - case \PhpOffice\PhpSpreadsheet\Shared\Escher::class: - if ($dggContainer = $this->object->getDggContainer()) { + case SharedEscher::class: + if ($dggContainer = $this->object->/** @scrutinizer ignore-call */ getDggContainer()) { $writer = new self($dggContainer); $this->data = $writer->close(); - } elseif ($dgContainer = $this->object->getDgContainer()) { + } elseif ($dgContainer = $this->object->/** @scrutinizer ignore-call */ getDgContainer()) { $writer = new self($dgContainer); $this->data = $writer->close(); $this->spOffsets = $writer->getSpOffsets(); @@ -88,13 +93,14 @@ public function close() $dggData = pack( 'VVVV', - $this->object->getSpIdMax(), // maximum shape identifier increased by one - $this->object->getCDgSaved() + 1, // number of file identifier clusters increased by one - $this->object->getCSpSaved(), - $this->object->getCDgSaved() // count total number of drawings saved + $this->object->/** @scrutinizer ignore-call */ getSpIdMax(), // maximum shape identifier increased by one + $this->object->/** @scrutinizer ignore-call */ getCDgSaved() + 1, // number of file identifier clusters increased by one + $this->object->/** @scrutinizer ignore-call */ getCSpSaved(), + $this->object->/** @scrutinizer ignore-call */ getCDgSaved() // count total number of drawings saved ); // add file identifier clusters (one per drawing) + /** @scrutinizer ignore-call */ $IDCLs = $this->object->getIDCLs(); foreach ($IDCLs as $dgId => $maxReducedSpId) { @@ -105,7 +111,7 @@ public function close() $innerData .= $header . $dggData; // write the bstoreContainer - if ($bstoreContainer = $this->object->getBstoreContainer()) { + if ($bstoreContainer = $this->object->/** @scrutinizer ignore-call */ getBstoreContainer()) { $writer = new self($bstoreContainer); $innerData .= $writer->close(); } @@ -131,7 +137,7 @@ public function close() $innerData = ''; // treat the inner data - if ($BSECollection = $this->object->getBSECollection()) { + if ($BSECollection = $this->object->/** @scrutinizer ignore-call */ getBSECollection()) { foreach ($BSECollection as $BSE) { $writer = new self($BSE); $innerData .= $writer->close(); @@ -140,7 +146,7 @@ public function close() // write the record $recVer = 0xF; - $recInstance = count($this->object->getBSECollection()); + $recInstance = count($this->object->/** @scrutinizer ignore-call */ getBSECollection()); $recType = 0xF001; $length = strlen($innerData); @@ -159,7 +165,7 @@ public function close() $innerData = ''; // here we treat the inner data - if ($blip = $this->object->getBlip()) { + if ($blip = $this->object->/** @scrutinizer ignore-call */ getBlip()) { $writer = new self($blip); $innerData .= $writer->close(); } @@ -167,7 +173,9 @@ public function close() // initialize $data = ''; + /** @scrutinizer ignore-call */ $btWin32 = $this->object->getBlipType(); + /** @scrutinizer ignore-call */ $btMacOS = $this->object->getBlipType(); $data .= pack('CC', $btWin32, $btMacOS); @@ -188,6 +196,7 @@ public function close() // write the record $recVer = 0x2; + /** @scrutinizer ignore-call */ $recInstance = $this->object->getBlipType(); $recType = 0xF007; $length = strlen($data); @@ -206,7 +215,7 @@ public function close() // this is an atom record // write the record - switch ($this->object->getParent()->getBlipType()) { + switch ($this->object->/** @scrutinizer ignore-call */ getParent()->/** @scrutinizer ignore-call */ getBlipType()) { case BSE::BLIPTYPE_JPEG: // initialize $innerData = ''; @@ -217,7 +226,7 @@ public function close() $tag = 0xFF; // todo $innerData .= pack('C', $tag); - $innerData .= $this->object->getData(); + $innerData .= $this->object->/** @scrutinizer ignore-call */ getData(); $recVer = 0x0; $recInstance = 0x46A; @@ -244,7 +253,7 @@ public function close() $tag = 0xFF; // todo $innerData .= pack('C', $tag); - $innerData .= $this->object->getData(); + $innerData .= $this->object->/** @scrutinizer ignore-call */ getData(); $recVer = 0x0; $recInstance = 0x6E0; @@ -272,6 +281,7 @@ public function close() // write the dg $recVer = 0x0; + /** @scrutinizer ignore-call */ $recInstance = $this->object->getDgId(); $recType = 0xF008; $length = 8; @@ -282,11 +292,11 @@ public function close() $header = pack('vvV', $recVerInstance, $recType, $length); // number of shapes in this drawing (including group shape) - $countShapes = count($this->object->getSpgrContainer()->getChildren()); - $innerData .= $header . pack('VV', $countShapes, $this->object->getLastSpId()); + $countShapes = count($this->object->/** @scrutinizer ignore-call */ getSpgrContainerOrThrow()->getChildren()); + $innerData .= $header . pack('VV', $countShapes, $this->object->/** @scrutinizer ignore-call */ getLastSpId()); // write the spgrContainer - if ($spgrContainer = $this->object->getSpgrContainer()) { + if ($spgrContainer = $this->object->/** @scrutinizer ignore-call */ getSpgrContainer()) { $writer = new self($spgrContainer); $innerData .= $writer->close(); @@ -329,7 +339,7 @@ public function close() $spTypes = []; // treat the inner data - foreach ($this->object->getChildren() as $spContainer) { + foreach ($this->object->/** @scrutinizer ignore-call */ getChildren() as $spContainer) { $writer = new self($spContainer); $spData = $writer->close(); $innerData .= $spData; @@ -364,7 +374,7 @@ public function close() // build the data // write group shape record, if necessary? - if ($this->object->getSpgr()) { + if ($this->object->/** @scrutinizer ignore-call */ getSpgr()) { $recVer = 0x1; $recInstance = 0x0000; $recType = 0xF009; @@ -377,10 +387,12 @@ public function close() $data .= $header . pack('VVVV', 0, 0, 0, 0); } + /** @scrutinizer ignore-call */ $this->spTypes[] = ($this->object->getSpType()); // write the shape record $recVer = 0x2; + /** @scrutinizer ignore-call */ $recInstance = $this->object->getSpType(); // shape type $recType = 0xF00A; $length = 0x00000008; @@ -390,16 +402,16 @@ public function close() $header = pack('vvV', $recVerInstance, $recType, $length); - $data .= $header . pack('VV', $this->object->getSpId(), $this->object->getSpgr() ? 0x0005 : 0x0A00); + $data .= $header . pack('VV', $this->object->/** @scrutinizer ignore-call */ getSpId(), $this->object->/** @scrutinizer ignore-call */ getSpgr() ? 0x0005 : 0x0A00); // the options - if ($this->object->getOPTCollection()) { + if ($this->object->/** @scrutinizer ignore-call */ getOPTCollection()) { $optData = ''; $recVer = 0x3; - $recInstance = count($this->object->getOPTCollection()); + $recInstance = count($this->object->/** @scrutinizer ignore-call */ getOPTCollection()); $recType = 0xF00B; - foreach ($this->object->getOPTCollection() as $property => $value) { + foreach ($this->object->/** @scrutinizer ignore-call */ getOPTCollection() as $property => $value) { $optData .= pack('vV', $property, $value); } $length = strlen($optData); @@ -412,34 +424,38 @@ public function close() } // the client anchor - if ($this->object->getStartCoordinates()) { + if ($this->object->/** @scrutinizer ignore-call */ getStartCoordinates()) { $recVer = 0x0; $recInstance = 0x0; $recType = 0xF010; // start coordinates - [$column, $row] = Coordinate::indexesFromString($this->object->getStartCoordinates()); + [$column, $row] = Coordinate::indexesFromString($this->object->/** @scrutinizer ignore-call */ getStartCoordinates()); $c1 = $column - 1; $r1 = $row - 1; // start offsetX + /** @scrutinizer ignore-call */ $startOffsetX = $this->object->getStartOffsetX(); // start offsetY + /** @scrutinizer ignore-call */ $startOffsetY = $this->object->getStartOffsetY(); // end coordinates - [$column, $row] = Coordinate::indexesFromString($this->object->getEndCoordinates()); + [$column, $row] = Coordinate::indexesFromString($this->object->/** @scrutinizer ignore-call */ getEndCoordinates()); $c2 = $column - 1; $r2 = $row - 1; // end offsetX + /** @scrutinizer ignore-call */ $endOffsetX = $this->object->getEndOffsetX(); // end offsetY + /** @scrutinizer ignore-call */ $endOffsetY = $this->object->getEndOffsetY(); - $clientAnchorData = pack('vvvvvvvvv', $this->object->getSpFlag(), $c1, $startOffsetX, $r1, $startOffsetY, $c2, $endOffsetX, $r2, $endOffsetY); + $clientAnchorData = pack('vvvvvvvvv', $this->object->/** @scrutinizer ignore-call */ getSpFlag(), $c1, $startOffsetX, $r1, $startOffsetY, $c2, $endOffsetX, $r2, $endOffsetY); $length = strlen($clientAnchorData); @@ -451,7 +467,7 @@ public function close() } // the client data, just empty for now - if (!$this->object->getSpgr()) { + if (!$this->object->/** @scrutinizer ignore-call */ getSpgr()) { $clientDataData = ''; $recVer = 0x0; diff --git a/tests/PhpSpreadsheetTests/Shared/DgContainerTest.php b/tests/PhpSpreadsheetTests/Shared/DgContainerTest.php new file mode 100644 index 0000000000..0a539f2845 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Shared/DgContainerTest.php @@ -0,0 +1,24 @@ +expectException(SpreadsheetException::class); + $this->expectExceptionMessage('spgrContainer is unexpectedly null'); + $container = new DgContainer(); + $container->getSpgrContainerOrThrow(); + } + + public function testNullContainerWithoutThrow(): void + { + $container = new DgContainer(); + self::assertNull($container->getSpgrContainer()); + } +}