Reader/Slk vs. Scrutinizer/Phpstan (#2192)
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 3 problems (2 mine) according to Scrutinizer, and 7 per Phpstan. It also moves the Reader Slk tests under their own directory, as is the case for all the other Reader types.
This commit is contained in:
oleibman
GitHub
parent
49e97f0914
commit
2ae948a319
|
|
@ -2565,41 +2565,6 @@ parameters:
|
|||
count: 1
|
||||
path: src/PhpSpreadsheet/Reader/Security/XmlScanner.php
|
||||
|
||||
-
|
||||
message: "#^Parameter \\#1 \\$haystack of function substr_count expects string, string\\|false given\\.$#"
|
||||
count: 1
|
||||
path: src/PhpSpreadsheet/Reader/Slk.php
|
||||
|
||||
-
|
||||
message: "#^Parameter \\#2 \\$str of function explode expects string, string\\|false given\\.$#"
|
||||
count: 1
|
||||
path: src/PhpSpreadsheet/Reader/Slk.php
|
||||
|
||||
-
|
||||
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Slk\\:\\:\\$colorArray has no typehint specified\\.$#"
|
||||
count: 1
|
||||
path: src/PhpSpreadsheet/Reader/Slk.php
|
||||
|
||||
-
|
||||
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Slk\\:\\:\\$fontStyleMappings has no typehint specified\\.$#"
|
||||
count: 1
|
||||
path: src/PhpSpreadsheet/Reader/Slk.php
|
||||
|
||||
-
|
||||
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Slk\\:\\:\\$styleSettingsFont has no typehint specified\\.$#"
|
||||
count: 1
|
||||
path: src/PhpSpreadsheet/Reader/Slk.php
|
||||
|
||||
-
|
||||
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Slk\\:\\:\\$styleSettingsBorder has no typehint specified\\.$#"
|
||||
count: 1
|
||||
path: src/PhpSpreadsheet/Reader/Slk.php
|
||||
|
||||
-
|
||||
message: "#^Parameter \\#1 \\$columnIndex of static method PhpOffice\\\\PhpSpreadsheet\\\\Cell\\\\Coordinate\\:\\:stringFromColumnIndex\\(\\) expects int, string given\\.$#"
|
||||
count: 3
|
||||
path: src/PhpSpreadsheet/Reader/Slk.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
|
||||
|
|
|
|||
|
|
@ -79,7 +79,7 @@ class Slk extends BaseReader
|
|||
}
|
||||
|
||||
// Read sample data (first 2 KB will do)
|
||||
$data = fread($this->fileHandle, 2048);
|
||||
$data = (string) fread($this->fileHandle, 2048);
|
||||
|
||||
// Count delimiters in file
|
||||
$delimiterCount = substr_count($data, ';');
|
||||
|
|
@ -210,7 +210,7 @@ class Slk extends BaseReader
|
|||
return $this->loadIntoExisting($pFilename, $spreadsheet);
|
||||
}
|
||||
|
||||
private $colorArray = [
|
||||
private const COLOR_ARRAY = [
|
||||
'FF00FFFF', // 0 - cyan
|
||||
'FF000000', // 1 - black
|
||||
'FFFFFFFF', // 2 - white
|
||||
|
|
@ -221,7 +221,7 @@ class Slk extends BaseReader
|
|||
'FFFF00FF', // 7 - magenta
|
||||
];
|
||||
|
||||
private $fontStyleMappings = [
|
||||
private const FONT_STYLE_MAPPINGS = [
|
||||
'B' => 'bold',
|
||||
'I' => 'italic',
|
||||
'U' => 'underline',
|
||||
|
|
@ -235,7 +235,8 @@ class Slk extends BaseReader
|
|||
$key = false;
|
||||
foreach ($temp as &$value) {
|
||||
// Only count/replace in alternate array entries
|
||||
if ($key = !$key) {
|
||||
$key = !$key;
|
||||
if ($key) {
|
||||
preg_match_all('/(R(\[?-?\d*\]?))(C(\[?-?\d*\]?))/', $value, $cellReferences, PREG_SET_ORDER + PREG_OFFSET_CAPTURE);
|
||||
// Reverse the matches array, otherwise all our offsets will become incorrect if we modify our way
|
||||
// through the formula from left to right. Reversing means that we work right to left.through
|
||||
|
|
@ -357,9 +358,9 @@ class Slk extends BaseReader
|
|||
$this->addWidth($spreadsheet, $columnWidth, $startCol, $endCol);
|
||||
}
|
||||
|
||||
private $styleSettingsFont = ['D' => 'bold', 'I' => 'italic'];
|
||||
private const STYLE_SETTINGS_FONT = ['D' => 'bold', 'I' => 'italic'];
|
||||
|
||||
private $styleSettingsBorder = [
|
||||
private const STYLE_SETTINGS_BORDER = [
|
||||
'B' => 'bottom',
|
||||
'L' => 'left',
|
||||
'R' => 'right',
|
||||
|
|
@ -372,10 +373,10 @@ class Slk extends BaseReader
|
|||
$iMax = strlen($styleSettings);
|
||||
for ($i = 0; $i < $iMax; ++$i) {
|
||||
$char = $styleSettings[$i];
|
||||
if (array_key_exists($char, $this->styleSettingsFont)) {
|
||||
$styleData['font'][$this->styleSettingsFont[$char]] = true;
|
||||
} elseif (array_key_exists($char, $this->styleSettingsBorder)) {
|
||||
$styleData['borders'][$this->styleSettingsBorder[$char]]['borderStyle'] = Border::BORDER_THIN;
|
||||
if (array_key_exists($char, self::STYLE_SETTINGS_FONT)) {
|
||||
$styleData['font'][self::STYLE_SETTINGS_FONT[$char]] = true;
|
||||
} elseif (array_key_exists($char, self::STYLE_SETTINGS_BORDER)) {
|
||||
$styleData['borders'][self::STYLE_SETTINGS_BORDER[$char]]['borderStyle'] = Border::BORDER_THIN;
|
||||
} elseif ($char == 'S') {
|
||||
$styleData['fill']['fillType'] = \PhpOffice\PhpSpreadsheet\Style\Fill::FILL_PATTERN_GRAY125;
|
||||
} elseif ($char == 'M') {
|
||||
|
|
@ -409,7 +410,7 @@ class Slk extends BaseReader
|
|||
private function addStyle(Spreadsheet &$spreadsheet, array $styleData, string $row, string $column): void
|
||||
{
|
||||
if ((!empty($styleData)) && $column > '' && $row > '') {
|
||||
$columnLetter = Coordinate::stringFromColumnIndex($column);
|
||||
$columnLetter = Coordinate::stringFromColumnIndex((int) $column);
|
||||
$spreadsheet->getActiveSheet()->getStyle($columnLetter . $row)->applyFromArray($styleData);
|
||||
}
|
||||
}
|
||||
|
|
@ -421,8 +422,8 @@ class Slk extends BaseReader
|
|||
$startCol = Coordinate::stringFromColumnIndex((int) $startCol);
|
||||
$spreadsheet->getActiveSheet()->getColumnDimension($startCol)->setWidth((float) $columnWidth);
|
||||
} else {
|
||||
$startCol = Coordinate::stringFromColumnIndex($startCol);
|
||||
$endCol = Coordinate::stringFromColumnIndex($endCol);
|
||||
$startCol = Coordinate::stringFromColumnIndex((int) $startCol);
|
||||
$endCol = Coordinate::stringFromColumnIndex((int) $endCol);
|
||||
$spreadsheet->getActiveSheet()->getColumnDimension($startCol)->setWidth((float) $columnWidth);
|
||||
do {
|
||||
$spreadsheet->getActiveSheet()->getColumnDimension(++$startCol)->setWidth((float) $columnWidth);
|
||||
|
|
@ -469,7 +470,7 @@ class Slk extends BaseReader
|
|||
{
|
||||
if (preg_match('/L([1-9]\\d*)/', $rowDatum, $matches)) {
|
||||
$fontColor = $matches[1] % 8;
|
||||
$formatArray['font']['color']['argb'] = $this->colorArray[$fontColor];
|
||||
$formatArray['font']['color']['argb'] = self::COLOR_ARRAY[$fontColor];
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -478,8 +479,8 @@ class Slk extends BaseReader
|
|||
$styleSettings = substr($rowDatum, 1);
|
||||
$iMax = strlen($styleSettings);
|
||||
for ($i = 0; $i < $iMax; ++$i) {
|
||||
if (array_key_exists($styleSettings[$i], $this->fontStyleMappings)) {
|
||||
$formatArray['font'][$this->fontStyleMappings[$styleSettings[$i]]] = true;
|
||||
if (array_key_exists($styleSettings[$i], self::FONT_STYLE_MAPPINGS)) {
|
||||
$formatArray['font'][self::FONT_STYLE_MAPPINGS[$styleSettings[$i]]] = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
<?php
|
||||
|
||||
namespace PhpOffice\PhpSpreadsheetTests\Reader;
|
||||
namespace PhpOffice\PhpSpreadsheetTests\Reader\Slk;
|
||||
|
||||
use PhpOffice\PhpSpreadsheet\Reader\Exception as ReaderException;
|
||||
use PhpOffice\PhpSpreadsheet\Reader\Slk;
|
||||
|
|
@ -14,7 +14,7 @@ class SlkTest extends \PHPUnit\Framework\TestCase
|
|||
/**
|
||||
* @var string
|
||||
*/
|
||||
private static $testbook = __DIR__ . '/../../../samples/templates/SylkTest.slk';
|
||||
private static $testbook = __DIR__ . '/../../../../samples/templates/SylkTest.slk';
|
||||
|
||||
/**
|
||||
* @var string
|
||||
|
|
@ -23,7 +23,7 @@ class SlkTest extends \PHPUnit\Framework\TestCase
|
|||
|
||||
protected function teardown(): void
|
||||
{
|
||||
if ($this->filename) {
|
||||
if ($this->filename !== '') {
|
||||
unlink($this->filename);
|
||||
$this->filename = '';
|
||||
}
|
||||
|
|
@ -134,6 +134,7 @@ class SlkTest extends \PHPUnit\Framework\TestCase
|
|||
self::assertEquals(Border::BORDER_THIN, $sheet->getCell('C18')->getStyle()->getBorders()->getBottom()->getBorderStyle());
|
||||
self::assertEquals(Border::BORDER_THIN, $sheet->getCell('C18')->getStyle()->getBorders()->getLeft()->getBorderStyle());
|
||||
// Have not yet figured out how C6/C7 are centred
|
||||
$spreadsheet->disconnectWorksheets();
|
||||
}
|
||||
|
||||
public function testSheetIndex(): void
|
||||
|
|
@ -147,6 +148,7 @@ class SlkTest extends \PHPUnit\Framework\TestCase
|
|||
self::assertEquals('SylkTest', $sheet->getTitle());
|
||||
|
||||
self::assertEquals('FFFF0000', $sheet->getCell('A1')->getStyle()->getFont()->getColor()->getARGB());
|
||||
$spreadsheet->disconnectWorksheets();
|
||||
}
|
||||
|
||||
public function testLongName(): void
|
||||
|
|
@ -160,5 +162,6 @@ class SlkTest extends \PHPUnit\Framework\TestCase
|
|||
$sheet = $spreadsheet->getActiveSheet();
|
||||
self::assertEquals('123456789a123456789b123456789c1', $sheet->getTitle());
|
||||
self::assertEquals('FFFF0000', $sheet->getCell('A1')->getStyle()->getFont()->getColor()->getARGB());
|
||||
$spreadsheet->disconnectWorksheets();
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue