Clientbase
/
PhpSpreadsheet
mirror of https://github.com/basemaster/PhpSpreadsheet.git
3
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Reader/Html vs. Scrutinizer/Phpstan 

Just reviewing Scrutinizer's list of "bugs". There are 19 ascribed to me. For some, I will definitely take no action (e.g. use of bitwise operators in AND, OR, and XOR functions). However, where I can clean things up so that Scrutinizer is satisfied and the resulting code is not too contorted, I will make an attempt.

This PR corrects 2 problems according to Scrutinizer, and about 30 per Phpstan.
This commit is contained in:
Owen Leibman 2021-06-30 19:19:30 -07:00 committed by Mark Baker
parent 2ae948a319
commit 435ac30b47
2 changed files with 61 additions and 178 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

145
phpstan-baseline.neon
View File

@ -2260,151 +2260,6 @@ parameters:
count: 1 count: 1
path: src/PhpSpreadsheet/Reader/BaseReader.php path: src/PhpSpreadsheet/Reader/BaseReader.php
-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Html\\:\\:\\$rowspan has no typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Html\\:\\:readBeginning\\(\\) has no return typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Html\\:\\:readEnding\\(\\) has no return typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Parameter \\#2 \\$length of function fread expects int, int\\<min, \\-1\\>\\|int\\<1, 2048\\>\\|false given\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Html\\:\\:startsWithTag\\(\\) has no return typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Html\\:\\:startsWithTag\\(\\) has parameter \\$data with no typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Html\\:\\:endsWithTag\\(\\) has no return typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Html\\:\\:endsWithTag\\(\\) has parameter \\$data with no typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Html\\:\\:containsTags\\(\\) has no return typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Html\\:\\:containsTags\\(\\) has parameter \\$data with no typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Html\\:\\:\\$dataArray has no typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Html\\:\\:\\$tableLevel has no typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Html\\:\\:\\$nestedColumn has no typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Html\\:\\:setTableStartColumn\\(\\) has no return typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Html\\:\\:setTableStartColumn\\(\\) has parameter \\$column with no typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Html\\:\\:getTableStartColumn\\(\\) has no return typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Html\\:\\:releaseTableStartColumn\\(\\) has no return typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Html\\:\\:flushCell\\(\\) has parameter \\$cellContent with no typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Html\\:\\:flushCell\\(\\) has parameter \\$column with no typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Html\\:\\:flushCell\\(\\) has parameter \\$row with no typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Argument of an invalid type DOMNamedNodeMap\\|null supplied for foreach, only iterables are supported\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Html\\:\\:\\$spanEtc has no typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Html\\:\\:\\$h1Etc has no typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Parameter \\#2 \\$styleValue of method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Html\\:\\:setBorderStyle\\(\\) expects string, string\\|null given\\.$#"
count: 5
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Parameter \\#3 \\$subject of function str_replace expects array\\|string, string\\|null given\\.$#"
count: 2
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Parameter \\#1 \\$pValue of method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\Alignment\\:\\:setHorizontal\\(\\) expects string, string\\|null given\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Parameter \\#1 \\$pValue of method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\Alignment\\:\\:setVertical\\(\\) expects string, string\\|null given\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Html\\:\\:getStyleColor\\(\\) has parameter \\$value with no typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Html.php
-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Html\\:\\:\\$borderMappings has no typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Html.php
- -
message: "#^Cannot call method getNamespaces\\(\\) on SimpleXMLElement\\|false\\.$#" message: "#^Cannot call method getNamespaces\\(\\) on SimpleXMLElement\\|false\\.$#"
count: 1 count: 1

94
src/PhpSpreadsheet/Reader/Html.php
View File

@ -123,6 +123,7 @@ class Html extends BaseReader
], // Italic ], // Italic
]; ];
/** @var array */
protected $rowspan = []; protected $rowspan = [];
/** /**
@ -160,19 +161,19 @@ class Html extends BaseReader
return $startWithTag && $containsTags && $endsWithTag; return $startWithTag && $containsTags && $endsWithTag;
} }
private function readBeginning() private function readBeginning(): string
{ {
fseek($this->fileHandle, 0); fseek($this->fileHandle, 0);
return fread($this->fileHandle, self::TEST_SAMPLE_SIZE); return (string) fread($this->fileHandle, self::TEST_SAMPLE_SIZE);
} }
private function readEnding() private function readEnding(): string
{ {
$meta = stream_get_meta_data($this->fileHandle); $meta = stream_get_meta_data($this->fileHandle);
$filename = $meta['uri']; $filename = $meta['uri'];
$size = filesize($filename); $size = (int) filesize($filename);
if ($size === 0) { if ($size === 0) {
return ''; return '';
} }
@ -184,20 +185,20 @@ class Html extends BaseReader
fseek($this->fileHandle, $size - $blockSize); fseek($this->fileHandle, $size - $blockSize);
return fread($this->fileHandle, $blockSize); return (string) fread($this->fileHandle, $blockSize);
} }
private static function startsWithTag($data) private static function startsWithTag(string $data): bool
{ {
return '<' === substr(trim($data), 0, 1); return '<' === substr(trim($data), 0, 1);
} }
private static function endsWithTag($data) private static function endsWithTag(string $data): bool
{ {
return '>' === substr(trim($data), -1, 1); return '>' === substr(trim($data), -1, 1);
} }
private static function containsTags($data) private static function containsTags(string $data): bool
{ {
return strlen($data) !== strlen(strip_tags($data)); return strlen($data) !== strlen(strip_tags($data));
} }
@ -251,13 +252,17 @@ class Html extends BaseReader
} }
// Data Array used for testing only, should write to Spreadsheet object on completion of tests // Data Array used for testing only, should write to Spreadsheet object on completion of tests
/** @var array */
protected $dataArray = []; protected $dataArray = [];
/** @var int */
protected $tableLevel = 0; protected $tableLevel = 0;
/** @var array */
protected $nestedColumn = ['A']; protected $nestedColumn = ['A'];
protected function setTableStartColumn($column) protected function setTableStartColumn(string $column): string
{ {
if ($this->tableLevel == 0) { if ($this->tableLevel == 0) {
$column = 'A'; $column = 'A';
@ -268,18 +273,25 @@ class Html extends BaseReader
return $this->nestedColumn[$this->tableLevel]; return $this->nestedColumn[$this->tableLevel];
} }
protected function getTableStartColumn() protected function getTableStartColumn(): string
{ {
return $this->nestedColumn[$this->tableLevel]; return $this->nestedColumn[$this->tableLevel];
} }
protected function releaseTableStartColumn() protected function releaseTableStartColumn(): string
{ {
--$this->tableLevel; --$this->tableLevel;
return array_pop($this->nestedColumn); return array_pop($this->nestedColumn);
} }
/**
* Flush cell.
*
* @param string $column
* @param int|string $row
* @param mixed $cellContent
*/
protected function flushCell(Worksheet $sheet, $column, $row, &$cellContent): void protected function flushCell(Worksheet $sheet, $column, $row, &$cellContent): void
{ {
if (is_string($cellContent)) { if (is_string($cellContent)) {
@ -302,7 +314,7 @@ class Html extends BaseReader
private function processDomElementBody(Worksheet $sheet, int &$row, string &$column, string &$cellContent, DOMElement $child): void private function processDomElementBody(Worksheet $sheet, int &$row, string &$column, string &$cellContent, DOMElement $child): void
{ {
$attributeArray = []; $attributeArray = [];
foreach ($child->attributes as $attribute) { foreach (($child->attributes ?? []) as $attribute) {
$attributeArray[$attribute->name] = $attribute->value; $attributeArray[$attribute->name] = $attribute->value;
} }
@ -328,11 +340,11 @@ class Html extends BaseReader
} }
} }
private static $spanEtc = ['span', 'div', 'font', 'i', 'em', 'strong', 'b']; private const SPAN_ETC = ['span', 'div', 'font', 'i', 'em', 'strong', 'b'];
private function processDomElementSpanEtc(Worksheet $sheet, int &$row, string &$column, string &$cellContent, DOMElement $child, array &$attributeArray): void private function processDomElementSpanEtc(Worksheet $sheet, int &$row, string &$column, string &$cellContent, DOMElement $child, array &$attributeArray): void
{ {
if (in_array($child->nodeName, self::$spanEtc)) { if (in_array((string) $child->nodeName, self::SPAN_ETC, true)) {
if (isset($attributeArray['class']) && $attributeArray['class'] === 'comment') { if (isset($attributeArray['class']) && $attributeArray['class'] === 'comment') {
$sheet->getComment($column . $row) $sheet->getComment($column . $row)
->getText() ->getText()
@ -405,11 +417,11 @@ class Html extends BaseReader
} }
} }
private static $h1Etc = ['h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'ol', 'ul', 'p']; private const H1_ETC = ['h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'ol', 'ul', 'p'];
private function processDomElementH1Etc(Worksheet $sheet, int &$row, string &$column, string &$cellContent, DOMElement $child, array &$attributeArray): void private function processDomElementH1Etc(Worksheet $sheet, int &$row, string &$column, string &$cellContent, DOMElement $child, array &$attributeArray): void
{ {
if (in_array($child->nodeName, self::$h1Etc)) { if (in_array((string) $child->nodeName, self::H1_ETC, true)) {
if ($this->tableLevel > 0) { if ($this->tableLevel > 0) {
// If we're inside a table, replace with a \n // If we're inside a table, replace with a \n
$cellContent .= $cellContent ? "\n" : ''; $cellContent .= $cellContent ? "\n" : '';
@ -628,6 +640,16 @@ class Html extends BaseReader
} }
} }
/**
* Make sure mb_convert_encoding returns string.
*
* @param mixed $result
*/
private static function ensureString($result): string
{
return is_string($result) ? $result : '';
}
/** /**
* Loads PhpSpreadsheet from file into PhpSpreadsheet instance. * Loads PhpSpreadsheet from file into PhpSpreadsheet instance.
* *
@ -646,7 +668,8 @@ class Html extends BaseReader
$dom = new DOMDocument(); $dom = new DOMDocument();
// Reload the HTML file into the DOM object // Reload the HTML file into the DOM object
try { try {
$loaded = $dom->loadHTML(mb_convert_encoding($this->securityScanner->scanFile($pFilename), 'HTML-ENTITIES', 'UTF-8')); $convert = mb_convert_encoding($this->securityScanner->scanFile($pFilename), 'HTML-ENTITIES', 'UTF-8');
$loaded = $dom->loadHTML(self::ensureString($convert));
} catch (Throwable $e) { } catch (Throwable $e) {
$loaded = false; $loaded = false;
} }
@ -668,7 +691,8 @@ class Html extends BaseReader
$dom = new DOMDocument(); $dom = new DOMDocument();
// Reload the HTML file into the DOM object // Reload the HTML file into the DOM object
try { try {
$loaded = $dom->loadHTML(mb_convert_encoding($this->securityScanner->scan($content), 'HTML-ENTITIES', 'UTF-8')); $convert = mb_convert_encoding($this->securityScanner->scan($content), 'HTML-ENTITIES', 'UTF-8');
$loaded = $dom->loadHTML(self::ensureString($convert));
} catch (Throwable $e) { } catch (Throwable $e) {
$loaded = false; $loaded = false;
} }
@ -774,6 +798,7 @@ class Html extends BaseReader
$value = explode(':', $st); $value = explode(':', $st);
$styleName = isset($value[0]) ? trim($value[0]) : null; $styleName = isset($value[0]) ? trim($value[0]) : null;
$styleValue = isset($value[1]) ? trim($value[1]) : null; $styleValue = isset($value[1]) ? trim($value[1]) : null;
$styleValueString = (string) $styleValue;
if (!$styleName) { if (!$styleName) {
continue; continue;
@ -782,7 +807,7 @@ class Html extends BaseReader
switch ($styleName) { switch ($styleName) {
case 'background': case 'background':
case 'background-color': case 'background-color':
$styleColor = $this->getStyleColor($styleValue); $styleColor = $this->getStyleColor($styleValueString);
if (!$styleColor) { if (!$styleColor) {
continue 2; continue 2;
@ -792,7 +817,7 @@ class Html extends BaseReader
break; break;
case 'color': case 'color':
$styleColor = $this->getStyleColor($styleValue); $styleColor = $this->getStyleColor($styleValueString);
if (!$styleColor) { if (!$styleColor) {
continue 2; continue 2;
@ -803,27 +828,27 @@ class Html extends BaseReader
break; break;
case 'border': case 'border':
$this->setBorderStyle($cellStyle, $styleValue, 'allBorders'); $this->setBorderStyle($cellStyle, $styleValueString, 'allBorders');
break; break;
case 'border-top': case 'border-top':
$this->setBorderStyle($cellStyle, $styleValue, 'top'); $this->setBorderStyle($cellStyle, $styleValueString, 'top');
break; break;
case 'border-bottom': case 'border-bottom':
$this->setBorderStyle($cellStyle, $styleValue, 'bottom'); $this->setBorderStyle($cellStyle, $styleValueString, 'bottom');
break; break;
case 'border-left': case 'border-left':
$this->setBorderStyle($cellStyle, $styleValue, 'left'); $this->setBorderStyle($cellStyle, $styleValueString, 'left');
break; break;
case 'border-right': case 'border-right':
$this->setBorderStyle($cellStyle, $styleValue, 'right'); $this->setBorderStyle($cellStyle, $styleValueString, 'right');
break; break;
@ -849,7 +874,7 @@ class Html extends BaseReader
break; break;
case 'font-family': case 'font-family':
$cellStyle->getFont()->setName(str_replace('\'', '', $styleValue)); $cellStyle->getFont()->setName(str_replace('\'', '', $styleValueString));
break; break;
@ -868,12 +893,12 @@ class Html extends BaseReader
break; break;
case 'text-align': case 'text-align':
$cellStyle->getAlignment()->setHorizontal($styleValue); $cellStyle->getAlignment()->setHorizontal($styleValueString);
break; break;
case 'vertical-align': case 'vertical-align':
$cellStyle->getAlignment()->setVertical($styleValue); $cellStyle->getAlignment()->setVertical($styleValueString);
break; break;
@ -900,7 +925,7 @@ class Html extends BaseReader
case 'text-indent': case 'text-indent':
$cellStyle->getAlignment()->setIndent( $cellStyle->getAlignment()->setIndent(
(int) str_replace(['px'], '', $styleValue) (int) str_replace(['px'], '', $styleValueString)
); );
break; break;
@ -911,15 +936,18 @@ class Html extends BaseReader
/** /**
* Check if has #, so we can get clean hex. * Check if has #, so we can get clean hex.
* *
* @param mixed $value
*
* @return null|string * @return null|string
*/ */
public function getStyleColor($value) public function getStyleColor($value)
{ {
$value = (string) $value;
if (strpos($value ?? '', '#') === 0) { if (strpos($value ?? '', '#') === 0) {
return substr($value, 1); return substr($value, 1);
} }
return \PhpOffice\PhpSpreadsheet\Helper\Html::colourNameLookup((string) $value); return \PhpOffice\PhpSpreadsheet\Helper\Html::colourNameLookup($value);
} }
/** /**
@ -966,7 +994,7 @@ class Html extends BaseReader
); );
} }
private static $borderMappings = [ private const BORDER_MAPPINGS = [
'dash-dot' => Border::BORDER_DASHDOT, 'dash-dot' => Border::BORDER_DASHDOT,
'dash-dot-dot' => Border::BORDER_DASHDOTDOT, 'dash-dot-dot' => Border::BORDER_DASHDOTDOT,
'dashed' => Border::BORDER_DASHED, 'dashed' => Border::BORDER_DASHED,
@ -985,7 +1013,7 @@ class Html extends BaseReader
public static function getBorderMappings(): array public static function getBorderMappings(): array
{ {
return self::$borderMappings; return self::BORDER_MAPPINGS;
} }
/** /**
@ -997,7 +1025,7 @@ class Html extends BaseReader
*/ */
public function getBorderStyle($style) public function getBorderStyle($style)
{ {
return (array_key_exists($style, self::$borderMappings)) ? self::$borderMappings[$style] : null; return self::BORDER_MAPPINGS[$style] ?? null;
} }
/** /**