From 0d1c9e4e0e93fa374cc3973d21258460dd528f6f Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Thu, 7 Sep 2023 18:29:45 -0700 Subject: [PATCH 1/2] ListWorksheetInfo/Names for Html/Csv/Slk (#3709) * ListWorksheetInfo/Names for Html/Csv/Slk Fix #3706. ListWorksheetInfo is implemented for all Readers except Html. For most (not all), ListWorksheetInfo is more efficient than reading the spreadsheet. I can't think of a way to make that so for Html, but that shouldn't be a reason to leave it unimplemented. ListWorksheetNames is not implemented for Html, Csv, or Slk. It isn't terribly useful for those formats, but that isn't a reason to omit it. The requester's use case consists of using IOFactory to create a reader for a file of unknown format and determining the first sheet name. That seems legitimate, but it is currently not possible without extra user code if the file is Html, Csv, or Slk; this PR will make it possible. When Excel opens a Slk or Csv file, the sheet name is based on the file name. PhpSpreadsheet does this for Slk, but it uses a default name for Csv. I am not interested in creating a break for that behavior, but I have added a new boolean property `sheetNameIsFileName` with a setter to Csv Reader. The requester actually mentioned that possibility in our discussion, although it is not essential to the request. As an adjunct to the issue, the requester wishes to use the worksheet name in `setLoadSheetsOnly`. That is already possible for Html, Csv, and Slk, but that particular property is ignored for those formats. I do not see a reason to change that behavior. This treatment is now explicitly noted in the documentation for property `loadSheetsOnly`. There had been no tests for what happens when `loadSheetsOnly` is specified but no sheets match the criteria for the formats for which this makes sense (Xlsx, Xls, Ods, Gnumeric, Xml). The behavior was not consistent - some formats threw an Exception while others continued with a single empty worksheet. All cases attempt to set the active sheet, and they will now all throw identical Exceptions when they attempt to do so in this situation. Tests are added for each. There also had been no tests for `loadSheetsOnly` returning more than one sheet. One is added. * Update LoadSheetsOnlyTest.php Add strict types to this new test, consistent with work being done in PR #3718. * Update LoadSheetsOnlyTest.php Add strict types to this new test, consistent with work being done in PR #3718. --- src/PhpSpreadsheet/Reader/BaseReader.php | 36 +++++++++ src/PhpSpreadsheet/Reader/Csv.php | 20 ++++- src/PhpSpreadsheet/Reader/Html.php | 25 ++++++ src/PhpSpreadsheet/Reader/Ods.php | 5 +- src/PhpSpreadsheet/Reader/Slk.php | 2 +- src/PhpSpreadsheet/Reader/Xls.php | 8 ++ .../Reader/Xlsx/WorkbookView.php | 3 - .../PhpSpreadsheetTests/Reader/BaseNoLoad.php | 5 -- .../Reader/BaseNoLoadTest.php | 10 ++- .../Reader/Csv/CsvCallbackTest.php | 8 ++ .../Reader/Csv/CsvEncodingTest.php | 15 ++-- .../Reader/Gnumeric/GnumericLoadTest.php | 11 +++ .../Reader/Html/Issue2942Test.php | 26 ++++++ .../Reader/Ods/OdsTest.php | 14 +++- .../Reader/Slk/SlkTest.php | 15 ++-- .../Reader/Xls/LoadSheetsOnlyTest.php | 61 +++++++++++++++ .../Reader/Xlsx/LoadSheetsOnlyTest.php | 74 ++++++++++++++++++ .../Reader/Xml/XmlLoadTest.php | 13 +++ tests/data/Reader/HTML/utf8chars.charset.html | 43 ++++++++++ tests/data/Reader/XLSX/threesheets.xlsx | Bin 0 -> 12116 bytes 20 files changed, 369 insertions(+), 25 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Reader/Xls/LoadSheetsOnlyTest.php create mode 100644 tests/PhpSpreadsheetTests/Reader/Xlsx/LoadSheetsOnlyTest.php create mode 100644 tests/data/Reader/HTML/utf8chars.charset.html create mode 100644 tests/data/Reader/XLSX/threesheets.xlsx diff --git a/src/PhpSpreadsheet/Reader/BaseReader.php b/src/PhpSpreadsheet/Reader/BaseReader.php index aa380aa95..d395f8f3a 100644 --- a/src/PhpSpreadsheet/Reader/BaseReader.php +++ b/src/PhpSpreadsheet/Reader/BaseReader.php @@ -39,6 +39,7 @@ abstract class BaseReader implements IReader /** * Restrict which sheets should be loaded? * This property holds an array of worksheet names to be loaded. If null, then all worksheets will be loaded. + * This property is ignored for Csv, Html, and Slk. * * @var null|string[] */ @@ -203,4 +204,39 @@ abstract class BaseReader implements IReader $this->fileHandle = $fileHandle; } + + /** + * Return worksheet info (Name, Last Column Letter, Last Column Index, Total Rows, Total Columns). + * + * @param string $filename + * + * @return array + */ + public function listWorksheetInfo($filename) + { + throw new PhpSpreadsheetException('Reader classes must implement their own listWorksheetInfo() method'); + } + + /** + * Returns names of the worksheets from a file, + * possibly without parsing the whole file to a Spreadsheet object. + * Readers will often have a more efficient method with which + * they can override this method. + * + * @param string $filename + * + * @return array + */ + public function listWorksheetNames($filename) + { + $returnArray = []; + $info = $this->listWorksheetInfo($filename); + foreach ($info as $infoArray) { + if (isset($infoArray['worksheetName'])) { + $returnArray[] = $infoArray['worksheetName']; + } + } + + return $returnArray; + } } diff --git a/src/PhpSpreadsheet/Reader/Csv.php b/src/PhpSpreadsheet/Reader/Csv.php index be9a2a32c..10107e237 100644 --- a/src/PhpSpreadsheet/Reader/Csv.php +++ b/src/PhpSpreadsheet/Reader/Csv.php @@ -10,6 +10,7 @@ use PhpOffice\PhpSpreadsheet\Reader\Exception as ReaderException; use PhpOffice\PhpSpreadsheet\Shared\StringHelper; use PhpOffice\PhpSpreadsheet\Spreadsheet; use PhpOffice\PhpSpreadsheet\Style\NumberFormat; +use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet; class Csv extends BaseReader { @@ -106,6 +107,9 @@ class Csv extends BaseReader /** @var bool */ private $preserveNullString = false; + /** @var bool */ + private $sheetNameIsFileName = false; + /** * Create a new CSV Reader instance. */ @@ -220,8 +224,12 @@ class Csv extends BaseReader /** * Return worksheet info (Name, Last Column Letter, Last Column Index, Total Rows, Total Columns). + * + * @param string $filename + * + * @return array */ - public function listWorksheetInfo(string $filename): array + public function listWorksheetInfo($filename) { // Open file $this->openFileOrMemory($filename); @@ -381,6 +389,9 @@ class Csv extends BaseReader $spreadsheet->createSheet(); } $sheet = $spreadsheet->setActiveSheetIndex($this->sheetIndex); + if ($this->sheetNameIsFileName) { + $sheet->setTitle(substr(basename($filename, '.csv'), 0, Worksheet::SHEET_TITLE_MAXIMUM_LENGTH)); + } // Set our starting row based on whether we're in contiguous mode or not $currentRow = 1; @@ -643,4 +654,11 @@ class Csv extends BaseReader { return $this->preserveNullString; } + + public function setSheetNameIsFileName(bool $sheetNameIsFileName): self + { + $this->sheetNameIsFileName = $sheetNameIsFileName; + + return $this; + } } diff --git a/src/PhpSpreadsheet/Reader/Html.php b/src/PhpSpreadsheet/Reader/Html.php index 860e9bcbe..fa49a79f0 100644 --- a/src/PhpSpreadsheet/Reader/Html.php +++ b/src/PhpSpreadsheet/Reader/Html.php @@ -1183,4 +1183,29 @@ class Html extends BaseReader ], ]); } + + /** + * Return worksheet info (Name, Last Column Letter, Last Column Index, Total Rows, Total Columns). + * + * @param string $filename + * + * @return array + */ + public function listWorksheetInfo($filename) + { + $info = []; + $spreadsheet = new Spreadsheet(); + $this->loadIntoExisting($filename, $spreadsheet); + foreach ($spreadsheet->getAllSheets() as $sheet) { + $newEntry = ['worksheetName' => $sheet->getTitle()]; + $newEntry['lastColumnLetter'] = $sheet->getHighestDataColumn(); + $newEntry['lastColumnIndex'] = Coordinate::columnIndexFromString($sheet->getHighestDataColumn()) - 1; + $newEntry['totalRows'] = $sheet->getHighestDataRow(); + $newEntry['totalColumns'] = $newEntry['lastColumnIndex'] + 1; + $info[] = $newEntry; + } + $spreadsheet->disconnectWorksheets(); + + return $info; + } } diff --git a/src/PhpSpreadsheet/Reader/Ods.php b/src/PhpSpreadsheet/Reader/Ods.php index 2ffc2b52c..30fc36d91 100644 --- a/src/PhpSpreadsheet/Reader/Ods.php +++ b/src/PhpSpreadsheet/Reader/Ods.php @@ -240,6 +240,7 @@ class Ods extends BaseReader { // Create new Spreadsheet $spreadsheet = new Spreadsheet(); + $spreadsheet->removeSheetByIndex(0); // Load into this instance return $this->loadIntoExisting($filename, $spreadsheet); @@ -345,9 +346,7 @@ class Ods extends BaseReader $worksheetStyleName = $worksheetDataSet->getAttributeNS($tableNs, 'style-name'); // Create sheet - if ($worksheetID > 0) { - $spreadsheet->createSheet(); // First sheet is added by default - } + $spreadsheet->createSheet(); $spreadsheet->setActiveSheetIndex($worksheetID); if ($worksheetName || is_numeric($worksheetName)) { diff --git a/src/PhpSpreadsheet/Reader/Slk.php b/src/PhpSpreadsheet/Reader/Slk.php index 525e9c7dc..64c3bb8ee 100644 --- a/src/PhpSpreadsheet/Reader/Slk.php +++ b/src/PhpSpreadsheet/Reader/Slk.php @@ -168,7 +168,7 @@ class Slk extends BaseReader break; case 'Y': - $rowIndex = substr($rowDatum, 1); + $rowIndex = (int) substr($rowDatum, 1); break; } diff --git a/src/PhpSpreadsheet/Reader/Xls.php b/src/PhpSpreadsheet/Reader/Xls.php index a006d308b..76eccfbff 100644 --- a/src/PhpSpreadsheet/Reader/Xls.php +++ b/src/PhpSpreadsheet/Reader/Xls.php @@ -417,6 +417,9 @@ class Xls extends BaseReader */ private $baseCell; + /** @var bool */ + private $activeSheetSet = false; + /** * Create a new Xls Reader instance. */ @@ -829,6 +832,7 @@ class Xls extends BaseReader } // Parse the individual sheets + $this->activeSheetSet = false; foreach ($this->sheets as $sheet) { if ($sheet['sheetType'] != 0x00) { // 0x00: Worksheet, 0x02: Chart, 0x06: Visual Basic module @@ -1240,6 +1244,9 @@ class Xls extends BaseReader } } } + if ($this->activeSheetSet === false) { + $this->spreadsheet->setActiveSheetIndex(0); + } // add the named ranges (defined names) foreach ($this->definedname as $definedName) { @@ -4401,6 +4408,7 @@ class Xls extends BaseReader $isActive = (bool) ((0x0400 & $options) >> 10); if ($isActive) { $this->spreadsheet->setActiveSheetIndex($this->spreadsheet->getIndex($this->phpSheet)); + $this->activeSheetSet = true; } // bit: 11; mask: 0x0800; 0 = normal view, 1 = page break view diff --git a/src/PhpSpreadsheet/Reader/Xlsx/WorkbookView.php b/src/PhpSpreadsheet/Reader/Xlsx/WorkbookView.php index 4743afbf9..d7db6240e 100644 --- a/src/PhpSpreadsheet/Reader/Xlsx/WorkbookView.php +++ b/src/PhpSpreadsheet/Reader/Xlsx/WorkbookView.php @@ -22,9 +22,6 @@ class WorkbookView */ public function viewSettings(SimpleXMLElement $xmlWorkbook, $mainNS, array $mapSheetId, bool $readDataOnly): void { - if ($this->spreadsheet->getSheetCount() == 0) { - $this->spreadsheet->createSheet(); - } // Default active sheet index to the first loaded worksheet from the file $this->spreadsheet->setActiveSheetIndex(0); diff --git a/tests/PhpSpreadsheetTests/Reader/BaseNoLoad.php b/tests/PhpSpreadsheetTests/Reader/BaseNoLoad.php index 02298e8d9..177d8a939 100644 --- a/tests/PhpSpreadsheetTests/Reader/BaseNoLoad.php +++ b/tests/PhpSpreadsheetTests/Reader/BaseNoLoad.php @@ -10,9 +10,4 @@ class BaseNoLoad extends BaseReader { return $filename !== ''; } - - public function loadxxx(string $filename): void - { - $this->loadSpreadsheetFromFile($filename); - } } diff --git a/tests/PhpSpreadsheetTests/Reader/BaseNoLoadTest.php b/tests/PhpSpreadsheetTests/Reader/BaseNoLoadTest.php index c79bb9e91..558189673 100644 --- a/tests/PhpSpreadsheetTests/Reader/BaseNoLoadTest.php +++ b/tests/PhpSpreadsheetTests/Reader/BaseNoLoadTest.php @@ -12,6 +12,14 @@ class BaseNoLoadTest extends TestCase $this->expectException(SpreadsheetException::class); $this->expectExceptionMessage('Reader classes must implement their own loadSpreadsheetFromFile() method'); $reader = new BaseNoLoad(); - $reader->loadxxx('unknown.file'); + $reader->load('unknown.file'); + } + + public function testBaseNoLoadInfo(): void + { + $this->expectException(SpreadsheetException::class); + $this->expectExceptionMessage('Reader classes must implement their own listWorksheetInfo() method'); + $reader = new BaseNoLoad(); + $reader->listWorksheetInfo('unknown.file'); } } diff --git a/tests/PhpSpreadsheetTests/Reader/Csv/CsvCallbackTest.php b/tests/PhpSpreadsheetTests/Reader/Csv/CsvCallbackTest.php index 46f4a5277..51f4c760b 100644 --- a/tests/PhpSpreadsheetTests/Reader/Csv/CsvCallbackTest.php +++ b/tests/PhpSpreadsheetTests/Reader/Csv/CsvCallbackTest.php @@ -30,12 +30,14 @@ class CsvCallbackTest extends TestCase $spreadsheet = $reader->load($filename); $sheet = $spreadsheet->getActiveSheet(); self::assertEquals('Å', $sheet->getCell('A1')->getValue()); + $spreadsheet->disconnectWorksheets(); } public function callbackSetFallbackEncoding(Csv $reader): void { $reader->setFallbackEncoding('ISO-8859-2'); $reader->setInputEncoding(Csv::GUESS_ENCODING); + $reader->setSheetNameIsFileName(true); $reader->setEscapeCharacter(''); } @@ -48,6 +50,7 @@ class CsvCallbackTest extends TestCase $sheet = $spreadsheet->getActiveSheet(); self::assertEquals('premičre', $sheet->getCell('A1')->getValue()); self::assertEquals('sixičme', $sheet->getCell('C2')->getValue()); + $spreadsheet->disconnectWorksheets(); } public function testIOFactory(): void @@ -58,6 +61,7 @@ class CsvCallbackTest extends TestCase $sheet = $spreadsheet->getActiveSheet(); self::assertEquals('premičre', $sheet->getCell('A1')->getValue()); self::assertEquals('sixičme', $sheet->getCell('C2')->getValue()); + $spreadsheet->disconnectWorksheets(); } public function testNonFallbackEncoding(): void @@ -69,6 +73,7 @@ class CsvCallbackTest extends TestCase $sheet = $spreadsheet->getActiveSheet(); self::assertEquals('première', $sheet->getCell('A1')->getValue()); self::assertEquals('sixième', $sheet->getCell('C2')->getValue()); + $spreadsheet->disconnectWorksheets(); } public function testDefaultEscape(): void @@ -79,6 +84,7 @@ class CsvCallbackTest extends TestCase $sheet = $spreadsheet->getActiveSheet(); // this is not how Excel views the file self::assertEquals('a\"hello', $sheet->getCell('A1')->getValue()); + $spreadsheet->disconnectWorksheets(); } public function testBetterEscape(): void @@ -89,5 +95,7 @@ class CsvCallbackTest extends TestCase $sheet = $spreadsheet->getActiveSheet(); // this is how Excel views the file self::assertEquals('a\"hello;hello;hello;\"', $sheet->getCell('A1')->getValue()); + self::assertSame('escape', $sheet->getTitle(), 'callback set sheet title to use file name rather than default'); + $spreadsheet->disconnectWorksheets(); } } diff --git a/tests/PhpSpreadsheetTests/Reader/Csv/CsvEncodingTest.php b/tests/PhpSpreadsheetTests/Reader/Csv/CsvEncodingTest.php index 7a9438864..a216eee5f 100644 --- a/tests/PhpSpreadsheetTests/Reader/Csv/CsvEncodingTest.php +++ b/tests/PhpSpreadsheetTests/Reader/Csv/CsvEncodingTest.php @@ -33,11 +33,13 @@ class CsvEncodingTest extends TestCase $reader = new Csv(); $reader->setInputEncoding($encoding); $info = $reader->listWorksheetInfo($filename); - self::assertEquals('Worksheet', $info[0]['worksheetName']); - self::assertEquals('B', $info[0]['lastColumnLetter']); - self::assertEquals(1, $info[0]['lastColumnIndex']); - self::assertEquals(2, $info[0]['totalRows']); - self::assertEquals(2, $info[0]['totalColumns']); + self::assertCount(1, $info); + self::assertSame('Worksheet', $info[0]['worksheetName']); + self::assertSame('B', $info[0]['lastColumnLetter']); + self::assertSame(1, $info[0]['lastColumnIndex']); + self::assertSame(2, $info[0]['totalRows']); + self::assertSame(2, $info[0]['totalColumns']); + self::assertSame(['Worksheet'], $reader->listWorksheetNames($filename)); } public static function providerEncodings(): array @@ -78,6 +80,9 @@ class CsvEncodingTest extends TestCase $filename = 'tests/data/Reader/CSV/premiere.utf16le.csv'; $reader = new Csv(); $reader->setInputEncoding(Csv::guessEncoding($filename)); + $names = $reader->listWorksheetNames($filename); + // Following ignored, just make sure it's executable. + $reader->setLoadSheetsOnly([$names[0]]); $spreadsheet = $reader->load($filename); $sheet = $spreadsheet->getActiveSheet(); self::assertEquals('𐐀', $sheet->getCell('A3')->getValue()); diff --git a/tests/PhpSpreadsheetTests/Reader/Gnumeric/GnumericLoadTest.php b/tests/PhpSpreadsheetTests/Reader/Gnumeric/GnumericLoadTest.php index c413b3a74..7e89e3acf 100644 --- a/tests/PhpSpreadsheetTests/Reader/Gnumeric/GnumericLoadTest.php +++ b/tests/PhpSpreadsheetTests/Reader/Gnumeric/GnumericLoadTest.php @@ -3,6 +3,7 @@ namespace PhpOffice\PhpSpreadsheetTests\Reader\Gnumeric; use DateTimeZone; +use PhpOffice\PhpSpreadsheet\Exception as PhpSpreadsheetException; use PhpOffice\PhpSpreadsheet\Reader\Exception as ReaderException; use PhpOffice\PhpSpreadsheet\Reader\Gnumeric; use PhpOffice\PhpSpreadsheet\Shared\Date; @@ -170,6 +171,16 @@ class GnumericLoadTest extends TestCase $spreadsheet->disconnectWorksheets(); } + public function testLoadNoSelectedSheets(): void + { + $this->expectException(PhpSpreadsheetException::class); + $this->expectExceptionMessage('You tried to set a sheet active by the out of bounds index'); + $filename = 'samples/templates/GnumericTest.gnumeric'; + $reader = new Gnumeric(); + $reader->setLoadSheetsOnly(['Unknown Sheet', 'xReport Data']); + $reader->load($filename); + } + public function testLoadNotGnumeric(): void { $this->expectException(ReaderException::class); diff --git a/tests/PhpSpreadsheetTests/Reader/Html/Issue2942Test.php b/tests/PhpSpreadsheetTests/Reader/Html/Issue2942Test.php index 3a41805c9..8e25bd6f7 100644 --- a/tests/PhpSpreadsheetTests/Reader/Html/Issue2942Test.php +++ b/tests/PhpSpreadsheetTests/Reader/Html/Issue2942Test.php @@ -14,6 +14,7 @@ class Issue2942Test extends TestCase $spreadsheet = $reader->loadFromString($content); $sheet = $spreadsheet->getActiveSheet(); self::assertSame('éàâèî', $sheet->getCell('A1')->getValue()); + $spreadsheet->disconnectWorksheets(); } public function testLoadFromFile(): void @@ -32,5 +33,30 @@ class Issue2942Test extends TestCase self::assertSame('അആ', $sheet->getCell('B3')->getValue()); self::assertSame('กขฃ', $sheet->getCell('C3')->getValue()); self::assertSame('✀✐✠', $sheet->getCell('D3')->getValue()); + $spreadsheet->disconnectWorksheets(); + } + + public function testInfo(): void + { + $file = 'tests/data/Reader/HTML/utf8chars.charset.html'; + $reader = new Html(); + $info = $reader->listWorksheetInfo($file); + self::assertCount(1, $info); + $info0 = $info[0]; + self::assertSame('Test Utf-8 characters voilà', $info0['worksheetName']); + self::assertSame('D', $info0['lastColumnLetter']); + self::assertSame(3, $info0['lastColumnIndex']); + self::assertSame(7, $info0['totalRows']); + self::assertSame(4, $info0['totalColumns']); + $names = $reader->listWorksheetNames($file); + self::assertCount(1, $names); + self::assertSame('Test Utf-8 characters voilà', $names[0]); + + // Following ignored, just make sure it's executable. + $reader->setLoadSheetsOnly([$names[0]]); + $spreadsheet = $reader->load($file); + $sheet = $spreadsheet->getActiveSheet(); + self::assertSame('✀✐✠', $sheet->getCell('D3')->getValue()); + $spreadsheet->disconnectWorksheets(); } } diff --git a/tests/PhpSpreadsheetTests/Reader/Ods/OdsTest.php b/tests/PhpSpreadsheetTests/Reader/Ods/OdsTest.php index 6e05709e9..115b0c019 100644 --- a/tests/PhpSpreadsheetTests/Reader/Ods/OdsTest.php +++ b/tests/PhpSpreadsheetTests/Reader/Ods/OdsTest.php @@ -3,6 +3,7 @@ namespace PhpOffice\PhpSpreadsheetTests\Reader\Ods; use PhpOffice\PhpSpreadsheet\Cell\DataType; +use PhpOffice\PhpSpreadsheet\Exception as PhpSpreadsheetException; use PhpOffice\PhpSpreadsheet\Reader\Exception as ReaderException; use PhpOffice\PhpSpreadsheet\Reader\Ods; use PhpOffice\PhpSpreadsheet\Spreadsheet; @@ -78,7 +79,9 @@ class OdsTest extends TestCase public function testLoadOneWorksheet(): void { $reader = new Ods(); - $reader->setLoadSheetsOnly(['Sheet1']); + //$reader->setLoadSheetsOnly(['Sheet1']); + $names = $reader->listWorksheetNames(self::ODS_DATA_FILE); + $reader->setLoadSheetsOnly([$names[0]]); $spreadsheet = $reader->load(self::ODS_DATA_FILE); self::assertEquals(1, $spreadsheet->getSheetCount()); @@ -99,6 +102,15 @@ class OdsTest extends TestCase $spreadsheet->disconnectWorksheets(); } + public function testLoadNoSelectedWorksheet(): void + { + $this->expectException(PhpSpreadsheetException::class); + $this->expectExceptionMessage('You tried to set a sheet active by the out of bounds index'); + $reader = new Ods(); + $reader->setLoadSheetsOnly(['xSecond Sheet']); + $reader->load(self::ODS_DATA_FILE); + } + public function testLoadBadFile(): void { $this->expectException(ReaderException::class); diff --git a/tests/PhpSpreadsheetTests/Reader/Slk/SlkTest.php b/tests/PhpSpreadsheetTests/Reader/Slk/SlkTest.php index 3a7dff6c4..2d51955c9 100644 --- a/tests/PhpSpreadsheetTests/Reader/Slk/SlkTest.php +++ b/tests/PhpSpreadsheetTests/Reader/Slk/SlkTest.php @@ -33,12 +33,14 @@ class SlkTest extends \PHPUnit\Framework\TestCase { $reader = new Slk(); $workSheetInfo = $reader->listWorkSheetInfo(self::$testbook); + self::assertCount(1, $workSheetInfo); $info0 = $workSheetInfo[0]; - self::assertEquals('SylkTest', $info0['worksheetName']); - self::assertEquals('J', $info0['lastColumnLetter']); - self::assertEquals(9, $info0['lastColumnIndex']); - self::assertEquals(18, $info0['totalRows']); - self::assertEquals(10, $info0['totalColumns']); + self::assertSame('SylkTest', $info0['worksheetName']); + self::assertSame('J', $info0['lastColumnLetter']); + self::assertSame(9, $info0['lastColumnIndex']); + self::assertSame(18, $info0['totalRows']); + self::assertSame(10, $info0['totalColumns']); + self::assertSame(['SylkTest'], $reader->listWorksheetNames(self::$testbook)); } public function testBadFileName(): void @@ -158,6 +160,9 @@ class SlkTest extends \PHPUnit\Framework\TestCase . '/123456789a123456789b123456789c12345.slk'; file_put_contents($this->filename, $contents); $reader = new Slk(); + $names = $reader->listWorksheetNames($this->filename); + // Following ignored, just make sure it's executable. + $reader->setLoadSheetsOnly([$names[0]]); $spreadsheet = $reader->load($this->filename); $sheet = $spreadsheet->getActiveSheet(); self::assertEquals('123456789a123456789b123456789c1', $sheet->getTitle()); diff --git a/tests/PhpSpreadsheetTests/Reader/Xls/LoadSheetsOnlyTest.php b/tests/PhpSpreadsheetTests/Reader/Xls/LoadSheetsOnlyTest.php new file mode 100644 index 000000000..e723dbda2 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Reader/Xls/LoadSheetsOnlyTest.php @@ -0,0 +1,61 @@ +spreadsheet !== null) { + $this->spreadsheet->disconnectWorksheets(); + $this->spreadsheet = null; + } + } + + public function testLoadSheet1Only(): void + { + $filename = self::$testbook; + $reader = new Xls(); + //$reader->setLoadSheetsOnly(['Sheet1']); + $names = $reader->listWorksheetNames($filename); + $reader->setLoadSheetsOnly([$names[0]]); + $this->spreadsheet = $reader->load($filename); + self::assertSame(1, $this->spreadsheet->getSheetCount()); + self::assertSame('Sheet1', $this->spreadsheet->getActiveSheet()->getTitle()); + } + + public function testLoadSheet2Only(): void + { + $filename = self::$testbook; + $reader = new Xls(); + $reader->setLoadSheetsOnly(['Sheet2']); + $this->spreadsheet = $reader->load($filename); + self::assertSame(1, $this->spreadsheet->getSheetCount()); + self::assertSame('Sheet2', $this->spreadsheet->getActiveSheet()->getTitle()); + } + + public function testLoadNoSheet(): void + { + $this->expectException(PhpSpreadsheetException::class); + $this->expectExceptionMessage('You tried to set a sheet active by the out of bounds index'); + $filename = self::$testbook; + $reader = new Xls(); + $reader->setLoadSheetsOnly(['Sheet3']); + $reader->load($filename); + } +} diff --git a/tests/PhpSpreadsheetTests/Reader/Xlsx/LoadSheetsOnlyTest.php b/tests/PhpSpreadsheetTests/Reader/Xlsx/LoadSheetsOnlyTest.php new file mode 100644 index 000000000..3513fd981 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Reader/Xlsx/LoadSheetsOnlyTest.php @@ -0,0 +1,74 @@ +spreadsheet !== null) { + $this->spreadsheet->disconnectWorksheets(); + $this->spreadsheet = null; + } + } + + public function testLoadSheet1Only(): void + { + $filename = self::$testbook; + $reader = new Xlsx(); + //$reader->setLoadSheetsOnly(['Sheet1']); + $names = $reader->listWorksheetNames($filename); + $reader->setLoadSheetsOnly([$names[0]]); + $this->spreadsheet = $reader->load($filename); + self::assertSame(1, $this->spreadsheet->getSheetCount()); + self::assertSame('Sheet1', $this->spreadsheet->getActiveSheet()->getTitle()); + } + + public function testLoadSheet2Only(): void + { + $filename = self::$testbook; + $reader = new Xlsx(); + $reader->setLoadSheetsOnly(['Sheet2']); + $this->spreadsheet = $reader->load($filename); + self::assertSame(1, $this->spreadsheet->getSheetCount()); + self::assertSame('Sheet2', $this->spreadsheet->getActiveSheet()->getTitle()); + } + + public function testLoadNoSheet(): void + { + $this->expectException(PhpSpreadsheetException::class); + $this->expectExceptionMessage('You tried to set a sheet active by the out of bounds index'); + $filename = self::$testbook; + $reader = new Xlsx(); + $reader->setLoadSheetsOnly(['Sheet3']); + $reader->load($filename); + } + + public function testLoadMultipleSheets(): void + { + $filename = 'tests/data/Reader/XLSX/threesheets.xlsx'; + $reader = new Xlsx(); + $reader->setLoadSheetsOnly(['Sheet3', 'Sheet1']); + $spreadsheet = $this->spreadsheet = $reader->load($filename); + self::assertSame(2, $spreadsheet->getSheetCount()); + $sheet = $spreadsheet->getSheetByNameOrThrow('Sheet1'); + self::assertSame('First', $sheet->getCell('A1')->getValue()); + $sheet = $spreadsheet->getSheetByNameOrThrow('Sheet3'); + self::assertSame('Third', $sheet->getCell('A1')->getValue()); + } +} diff --git a/tests/PhpSpreadsheetTests/Reader/Xml/XmlLoadTest.php b/tests/PhpSpreadsheetTests/Reader/Xml/XmlLoadTest.php index 9846b8617..34716da13 100644 --- a/tests/PhpSpreadsheetTests/Reader/Xml/XmlLoadTest.php +++ b/tests/PhpSpreadsheetTests/Reader/Xml/XmlLoadTest.php @@ -3,6 +3,7 @@ namespace PhpOffice\PhpSpreadsheetTests\Reader\Xml; use DateTimeZone; +use PhpOffice\PhpSpreadsheet\Exception as PhpSpreadsheetException; use PhpOffice\PhpSpreadsheet\Reader\Xml; use PhpOffice\PhpSpreadsheet\Settings; use PhpOffice\PhpSpreadsheet\Shared\Date; @@ -127,6 +128,18 @@ class XmlLoadTest extends TestCase self::assertEquals('Third Heading', $sheet->getCell('C2')->getValue()); } + public function testLoadNoSelectedSheets(): void + { + $this->expectException(PhpSpreadsheetException::class); + $this->expectExceptionMessage('You tried to set a sheet active by the out of bounds index'); + $filename = __DIR__ + . '/../../../..' + . '/samples/templates/excel2003.xml'; + $reader = new Xml(); + $reader->setLoadSheetsOnly(['Unknown Sheet', 'xReport Data']); + $this->spreadsheet = $reader->load($filename); + } + public function testLoadUnusableSample(): void { // Sample spreadsheet is not readable by Excel. diff --git a/tests/data/Reader/HTML/utf8chars.charset.html b/tests/data/Reader/HTML/utf8chars.charset.html new file mode 100644 index 000000000..e180dfd7c --- /dev/null +++ b/tests/data/Reader/HTML/utf8chars.charset.html @@ -0,0 +1,43 @@ + + + + +Test Utf-8 characters voilà + + + + + + + + + + + + + + + + + + + + +
éàâèîαβγδε
𐐁𐐂𐐃 & だけちאבגדה𪔀𪔁𪔂
᠐᠑᠒അആกขฃ✀✐✠
+ + + + + + +
second table
+ + + + + + + +
third tablesecond cell
+ + diff --git a/tests/data/Reader/XLSX/threesheets.xlsx b/tests/data/Reader/XLSX/threesheets.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..de30d849eb101c815aa1a97078eeca34723aa64a GIT binary patch literal 12116 zcmeHNWl$W8nni*;1Pc&!aEAnU0>RzgeQ*mF9Kzu48a%iLcY?bIcMX={>?HT?%e~yY z|F&v>?CYxOfvP^Ix_i#&pS%?Gv*!@75bzKX5Tp=N6rrQOkPr~run-Vf5b#i1BDO#$ z6QGlxvb&v$qYk5+jWtQmGbrjT2q^IN|2h62zk#9n70XWM=e;Lz9>Fn-<6=Z0mQvlr z(Jjg$KEZ^v1;dnpwB3s%*OcOZyASGfz!wXfo-0gz!?SB{DhI|0FX^ltW=7x9Ly~_g z9-=oRo=m*R0N@2E2x~!eX+c+=?%r5(>hQD&6r*gHB6slFMN|c7>YEbbrO=d?zsb@P z*pTgSOoAl7c;~9EiPav!&i=xh-P2E)-sx?I+C>B4G%FeqdDPiUXV%H3BY_bT#}t0E z76T+?Ee2UtloBqZ!*0M4r6jMQymq#|u=M;$k_=C*`$h)O^W$~m<++^I&b)mzbX*-Z zEyBRS8yQ;?ksTpwdPw0<)tsiag+mFqAwJq5Uex;tp_KbKYiwMwj{zIsBRQeG9@{rA z&k{tFu~4#NjRjweL-8!1Wt<_vB3q&8EY{}~q)Dv_wZO?)OmnrjerG}D9>lUGQY?1o zY(rR_vn*B;7hV;CXSo23AYYdS!r+dFK?4?FMX{kB`t0 z@_(^hEZtTUD6r+OfDID~Y`J<4Cf1Hjj6cu+vDp8|X8NZ`FO89rf@VPvI`MlX9ctlO zq$QiMW_q(LV|)+El6{#5d6|i zC!16|rS2OV;4oqx#KI5XcKVW87ax>dvW3%bWzSvme<)qg319>~zQ=PGSMBlRYt**j zRF8MS)4Y&GD+XxCZxIEpypSJe?-_Xu&*KQ4!++I$-TazErrcuio$Z&86k{Ln2q|&_4+&!@xS5y=O1g1czZ(kpB!qhb9y>Z0y zSxOf3++l8JMJrGTGJVPU!BWX*G@>@jfjP2Vbpw72e>WNQCFhJLu(^1E4GIl>g_|{# ztF42Tp{=de&p?-_q9C`(jP9LT^N4=UMV$_%#^MW-e_^fB%HFxF>k1cDL26Y+MxnCwH@r1+T#dK_G9ypyTmcT;?<`eul)U7D ztL$Q<%Gr~rKWwS2NQrTUc{L&0RX`4(O8V`UQzl~y$rTh(6Fx=(#w+6J#8qNqq_^Vs ztL%7ZQPN|WBQyivdOs7PPo)32L-t8(z!{0mLWq4#jjKdCSTmoC(^l#U161O^>R zmHB?@Jz0r#?)3*G2^^S!mJVnu@-^H};wEOduM+zEL@HIvh=7C}_^k8|1|vY1Tot0L`WgYAOB*OiNHbiO?CmnuI4lqF<%>*u4Ix{tJy`@IC z`3nGDK32Z6%zZWi<%=9;2{CqwvaHEM>0=+=6)F#5zjT2ahY_lY2sX3Wv;tucfe zzOzH~)P&!U>uWjcFR$GFpnQ?5Y#v9lq2_#6i$LGH!USFnM^MF%y?o}+#$y-E=CPSM zd>Cec7w~Z5h9hfQjt)Z3RgTi!SzI94#e;;I3v%Q8)dkVWN#>^}haIvBlBgL1&|8qvR3NeuY(Esiq(PRVt{U%0+8o4m?v9 zZhfa^*3a+zUUxx~n?))*W*>!pTkC`*SGB) zB_m%ncNpyu9lGC!3vx@+&?xRwvEgF^jSgE0Hhlh3=e&%A@uc9;u>1@H;spdeBv|Kv zM2A1C{l6mxBsjMNU;F?2XpS4S>|#cL^CR%Vf4nu}sK$aix|$}C{%h2gw>*)0=bCgdgM;jHxV1IwxR%qnD^# z8h)kTItGde)IU~_dMv8nylbnY85HZNR>&kW_v!G%8Y-!I8#AG@3Ue#fUBl*$uU}2s zUT<$!3V9d6oF9com5M&Jk;1Gl1=rK$s(d(~Nl})*Of<_bX06|<|U=W z@t{Gt9(CW9@_oP@#nSvT)F1)bcc~m<-J-BPfH}ZZy-)L7kMe*ISQ>wKq=zLg-fCb+ z$_5q~9#}d5a-@#tCMHgfOuxQZetOij7!BKX=I4Pc;`=@!cNv@n@?^r(m}3_2isQrS8B~dT|po$#OGbM(os7%!}MqybB{Gl;*})z6@8VT)zH7R zlaNU1Jx!veWaq*{^g!!k4_}qXZ&k{=$2SyWjl679bvhPSx_g;s=+bl`3{(ydsg3H) zgr!D!&ZNt+pk9}-dz-Wgf)fv8=*iGA4!R@&02*G6u=5ME^38f$`A8PTozm^iJnAG- z6ySW%d2B~hzNw9g8IO{U~L)UKNg{Y>E_Zo#jy9XZHclrhO%US)r!D3h?} z)T~agyFP7oyoO+ z89ru09Gk)HzS?XE}PD`^XqbctD=3Mn^Y6vbqB0ry^XlpuW z^x^KuIcitWR`K0n?u36n8!vwV<ooTGF^nx9vs1<8%J~{lm}6a>dr_K)LD9c4dyfg( zVVzf1hIx>jbmEoVuGZI{pm>|8fIqbBfJqS>GD6L;vz2$(JK2oc+U2FvFQ~Js0s6vD z9JHg))B<;3|ng zs#uas4DK}E{6zd`CHyZ4&HVpRXy#IGin3s#k^j$LkL{PxCZ)o_c_hTDNA{!8E^siC zqMLLihN`JvabX$ToiJ>7Ej|wS<710auraO!0@CXQhKpg)tCZHXS~77vT}`D4c3Emj za%;eFSzeouO%4ly-%iQi1SReQDx-C%)h9KhbnzCJ1pPUP$pTB5$6zQ7mi&!^g3 z;5v#CMZ=Om)VMRu+7)LfW}Gn0;Ms{9^MRlXu0W*iP5#L;afGt`xj&-7(H{0@8G@TK z>?wN5{(_;Rj_NARck-Z!bJEX`zhc-gA^xv8JN^Gd4R+w7x5~mnK=hLQ?AQH@yiVpO zHYQBJ&cA~0p~i3+0mt(;+#3O8N7uVo846W5ZfG~<^j)C@^}X#R&&25?B=TQQ!lC=9_z90Z=1SMkP~O9fm6ZGt zC0^3Uqo`SKjU`$b1w_4~R$K|luH5LV5)KL6RXiplT{ttyVBvYW?lJL$8WYpo{2Fk- zgImvpGp&V> z!0cc)>$#p**R0f$u6+H4mAt5gLqiZ_V6CNV;{vvE=WYbnGP1u=OAO7ZLKI5Lp`wzs zIvK~7Cg_-zAL2cfB89~ImmXxfG5D&6Flw&V?nS^PJf>|BCZ&FZG7XBP%2heP??XQM zhdO4~ym%e`P;~;u?{0?OU!?2q=(46cNSHGCJRZL7od>t^`CRN?cj;9%*3wD=;Q&fX z$J3E|kN5Yu%X)1O-}X;A_NJ&#JG;P%WCU`-bLCzwP8t06%`QXwYwUSrXjAA|wAC@AWGTdt9kwjB zb;bGc3X^S=5166(oE;NNZIvP*qLL<&kgZuh)^+KM(2HaAa{rJ?fQW-WK#{RV#4u#A z(}W~o;cR%>ro5}rYMRf}nA`w-AI#n%MO(dt=8Ou5id>>BuZUX1j5k>wd^*G_%a|xc z-+iDtzcHc@klN-BzI-9nhKRg@dizqR`*n?RpUYghJg?(|bWDG@Hzg|R zn`l>m30%*}c^*+d5kmPYmZf@xy3Tu3vIdw<%ak~(kQ*Ki8xzyKa(9gwGTd-m`!+SN$4YMtK3ai>z7?1=hSBm5wy zw|ZxuvD9uCQ|dV5uHvXWef4--Y&xmSUv4sDm3*$*d-?rR$TGRv(|3UKU|*wyP<^Ee2Oxo}b8GgYG{98}$KHbWmyG4di%9=HneoiSuf{D3yqtOl*NtDdr)#N)_{xo&q zy{qi1kqApYKZ$1`ZAzfvjHjT7D*Rm6EI@ZYS$*6|eO!J&FRL>-*}P zmaSl+1)(5Q!nhqH?1Gx)W>6X8NV3?wShk{MNO?mhHj+purTNeH?v~CL#Vv307|20R z)#1z&%zn6Q^gP1}w;H{llY^&N_)=0Tjdk+?uWK#g>uJf^ zt*)_l@w#T+U>nJ&v~Y8j8eH%tGJsdE1_}7MRl%Zh+}g3S1F$U7Ngi?C40;Sc@hI%$ zggG{JYVXncH8)ZO>4;P9(Vcku-B>%wErPz{DP+hJ`>rXK**z5ZtPqt_%oBFD5Qj&$ zKy&YKVC1Q*Gtt3CguV~+ci10h@lCD3j^4UbPri*2*~9@_mWr`WwRhWjcx#}N0zpbcHwUojh_t|ZU}Pj&FVLVMqt7Hw5OT0t9i^56zi+hFXCi$!_g zEaUOU7Q=8LMp&4%-Vko>+Rb{a_PnS42?j~}ZS6pohK-A5plK~1^|Kb)+_A$M?m*p7 zS|=#)zE+GA@_uU}M?qbf2Ff3BAl5q6Yr$IrOuTb73gO}2V6Gj z5$XmfrsQ{fvxp8$3nzweZbW1K)afxJ3))G zsAU30>07~!m?B)+Jo+M@7~KJygwQ&38m2^JCv>N9!Xq?UkvlLoYpNGvkWd(wPbrqpMY)&DboN&Fg^lx38ARgl=WT$8gsbd!K!O>Cpf{0X zToiL_VGCQiXbO6l%hF)*dCkm#hVs5D7dlqR4Gw)q{6iVy;~-qHj=QB#77;djDDxMk zHvob=E!wLEJL1x!O+$eIBjVj!zQb5T?j!$hGd_A8RC`icLc>f{D0f6dE>mSlRPH&s zTUcX7yK>kh?-cLP-{je zKTL+JDqktWM#VyI#Yu;V`^>gYDp*V{+j z_j}SUZ$=PtEH(0n17<`@C2l0s$pZ@N7;0S^s==nbw_;1NYW#?#p*d^rr<<{X8!-rF z5t>BUFT{l?>>d@LanTBm$oV`F{8PQWJiItl0@m&-`1AZ<8a6j@FfmqgaW!Tom@$l;jV~%O66J5>M_Q*HZT};GFx0NTUOt z75s!Pm2h`0*OsPA_^5H@(upy%1F2Y*fsM>VIok}Cfx5=L=EF9TscHojg#YWP}BLL-JLj7qHC|$P6tQq?8LMugO~6bMt1A3`myRlAFPR3 z2g{3hf{$VT>EX;2@;q4ISWnMx!IFpps+Ar>=<^0Pp*oji<@LYfvbH_hAOI0-j%Ew4J2T~g1U zCJZD8@s<^u3Qs#;*!Z06^jAkvi@o@CMVQ|B#x-m-(^Q$Rr^w%I1{&F9XLnbd2L(zh zQX7+Jf)#vkP+#_we&I z{sz;Y;o|iT`cap^*8qy3_kpj}plQmROb!F#4mJ4xyMZfb0Cr}@vp)JE^)zwzQR-2N zm*S*@IT9blW$XGKcW{=w``MaV3E7QDaTnWbV#X2|c4blGaH>z5my&g7e6y$1vpq!! z@jFKZ-|;cHh|LD|^Xu_y*ED1_595a*TDJ6UIYI)l<(Aq@wQaQ-d1EHE_6D)ao#|8$ z;E72zoNzdCKx%nb+8yqirlmu76rV5!BB;*??MLqDdyQmIDLPbCE>fO9QVwI}Tda7X zis0qwvmljCx*MrqbX3?8QI(j)+zzrY-kiR$y;Dg5c{NL@mYlk!%JyDG8mwMEx zethuXLYt#+QlTUD>(+_Z4|{DsU2pjoHYCMOagpZR52C)cDmqd~G%uk_vyrsM>#<}Z z{37g7ooG6&f$&-r9(d`h4yYmM_`%qQ}9LeBnxC{883<3U|@_TKj1k^ zTZ`?(AhH7}`cIAPv1Thp*^xg!TAyx>T4g)!R~FL{!`#{|%t*rMqtlf;!}No#25 zCdlw3sc++oNAMBN3wLZ$AB#M4`4k)T1GyMG%a=@>>=yVHSQf>q%3qh3?#=eYwh~+pY5g+0~w+MFWOUCS`!Y#fw<7d&98j z<)SW|*ay<30pgj1xgi`i;fQK?<~!aiB%_LU3)~YaOSyVNK=}{4CKhS)la0n5rldAQ zj^VJEdLS+_Ti!rE6{;0~d+o4W$jKv4{3A2kt!bPM`o3+RI8`-iE|QPUBTaFcd7S5& zn0ago;WXx%rqiN z`{6YsX9$WC=o>BB65)Ln|8DmKbaAk^=Jp+}a$^EX*^f&dG;R$)PZjCJt8!h$mspC< zX%d}^??SEsd@?{_DoinwqO_tze2hlysJYOt(~aW#%6smz>uZ@yD)3ACyS^4=xIiDk z+F}QXH}pU2%fQa=Kk@>D0`ytxo%P_ib8h92T1(DuJG73K=nw$xr8s}S3wlXMvjqc`)J=`iJ+ z>dMmfP`4?BN6uxNSB0uGZVl`DI{1-Un`RPxa9XnHXdJ-F9_-ZVm>Sgr1PRjL5w` z*wO;iON5sg>jyIQ$@}X4t9+wYXbYxc?*!P1(@!@-EGq({u8hsAM%59sL90gVeKNJh zGJY|MPILX)*+BQXFRr9&wFm^%0=#;G$6mZBnSyeoRM)&a(~<8az4=}5!6EhU`t3iz z9~=hj_XwQ4f!7WHO z*8^m)IsSD*fS3G8p}#A$h}rJ={oPUO*~fDRr|cm9P|UK-Y%h45==_#tk=&amx5M?& zOPyTdC71G|`ns8lvzxj1NHpZeDf>pmp8ksh4!Z@q_J|^Y@LAl?4;$Z?#FpHzrS8(^ zD5p$=TFbf}YB5Fi>k_1-5p?vqqVXxYNUb{(BF_x&Q2<9oLzOo6r>EthPXVG-&<+(%bZ)twyF zMBS$E@Yjipd$k2SD`2hvVFi#-4B%?(pVw}G|LA}G^$$z9@=|{#`0MuHp9wyLL&KkT z0iOb&?m+#99)a`VCmT^u!GB#W`3;4Dm`3;o{vXy%p7K21ANbAm8u{Nh@waV)rz}qg zzrR_!z|Q9%b`pM#fuFKGo$CE&p~v_~7Pg;W>M6_9xy)~t3h}4HfX(FR R`XCy_7qF!{)BXJKzW@Y*Y6<`V literal 0 HcmV?d00001 From 52f5b24b6de1bbaffb794341a4c587e2a6c3c294 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Thu, 7 Sep 2023 18:38:08 -0700 Subject: [PATCH 2/2] Inconsistency Between Actual and Declared Type - Minor Break (#3715) * Inconsistency Between Actual and Declared Type - Minor Break Fix #3711. User set a cell value to float (implicitly by default value binder), then used `setDataType` to change its type to string. This caused a problem for Xlsx Writer, which uses the string cell values as an index into a Shared String array. However, as the cell actually contained a floating point value, Php treated it as an integer index; such a treatment is both deprecated, and leads to invalid values in the spreadsheet. The use case for `setDataType` is not strong. The user always has the option to use `setValueExplicit` if the type is important. Setting a type afterwards, i.e. irrespective of the value, seems like a peculiar action. Indeed, there are no tests whatever for such use in the unit test suite. There are two possible approaches to fixing this problem. The first is to add casts to the 3 or 4 places in Writer Xlsx which might be affected by this problem (hoping that you've found them all and realizing that similar changes might be needed for other Writers). The second is to change `setDataType` to call `setValueExplicit` using the current value of the cell, thereby possibly changing the cell value. I have gone with the second option - it seems like a much more logical approach, and guarantees that the content of the cell will always be consistent with its declared type. It is, however, a breaking change; if, for example, you have a cell with a string or numeric value and specify `boolean` to `setDataType`, the cell's value will change to `true` or `false` with no way to get back to the original. * Strict Types for New Tests Consistent with work being done in PR #3718. * Improve Test Better match to original issue. * Typo --- src/PhpSpreadsheet/Cell/Cell.php | 7 +-- .../Cell/DataType2Test.php | 44 +++++++++++++++++++ .../Writer/Xlsx/Issue3711Test.php | 32 ++++++++++++++ 3 files changed, 78 insertions(+), 5 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Cell/DataType2Test.php create mode 100644 tests/PhpSpreadsheetTests/Writer/Xlsx/Issue3711Test.php diff --git a/src/PhpSpreadsheet/Cell/Cell.php b/src/PhpSpreadsheet/Cell/Cell.php index b9d7e8f87..36f27a27a 100644 --- a/src/PhpSpreadsheet/Cell/Cell.php +++ b/src/PhpSpreadsheet/Cell/Cell.php @@ -457,12 +457,9 @@ class Cell */ public function setDataType($dataType): self { - if ($dataType == DataType::TYPE_STRING2) { - $dataType = DataType::TYPE_STRING; - } - $this->dataType = $dataType; + $this->setValueExplicit($this->value, $dataType); - return $this->updateInCollection(); + return $this; } /** diff --git a/tests/PhpSpreadsheetTests/Cell/DataType2Test.php b/tests/PhpSpreadsheetTests/Cell/DataType2Test.php new file mode 100644 index 000000000..29c614735 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Cell/DataType2Test.php @@ -0,0 +1,44 @@ +getActiveSheet(); + $sheet->getCell('A1')->setValue(28.1); + self::assertSame(28.1, $sheet->getCell('A1')->getValue()); + self::assertSame('28.1', (string) $sheet->getCell('A1')); + $sheet->getCell('A1')->setDataType(DataType::TYPE_STRING); + self::assertSame('28.1', $sheet->getCell('A1')->getValue()); + $sheet->getCell('A1')->setDataType(DataType::TYPE_NUMERIC); + self::assertSame(28.1, $sheet->getCell('A1')->getValue()); + $sheet->getCell('A1')->setDataType(DataType::TYPE_STRING2); + self::assertSame('28.1', $sheet->getCell('A1')->getValue()); + $sheet->getCell('A1')->setDataType(DataType::TYPE_INLINE); + self::assertSame('28.1', $sheet->getCell('A1')->getValue()); + $sheet->getCell('A1')->setDataType(DataType::TYPE_BOOL); + self::assertTrue($sheet->getCell('A1')->getValue()); + $sheet->getCell('A1')->setDataType(DataType::TYPE_NUMERIC); + self::assertSame(1, $sheet->getCell('A1')->getValue()); + + $sheet->getCell('A2')->setValue('X'); + + try { + $sheet->getCell('A2')->setDataType(DataType::TYPE_NUMERIC); + } catch (PhpSpreadsheetException $e) { + self::assertSame('Invalid numeric value for datatype Numeric', $e->getMessage()); + } + + $spreadsheet->disconnectWorksheets(); + } +} diff --git a/tests/PhpSpreadsheetTests/Writer/Xlsx/Issue3711Test.php b/tests/PhpSpreadsheetTests/Writer/Xlsx/Issue3711Test.php new file mode 100644 index 000000000..b454a4724 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Writer/Xlsx/Issue3711Test.php @@ -0,0 +1,32 @@ +getActiveSheet(); + $sheet->getCell('A1')->setValue('21.5'); + self::assertSame(21.5, $sheet->getCell('A1')->getValue()); + $sheet->getCell('A1')->setDataType(DataType::TYPE_STRING); + $sheet->getCell('A2')->setValue('21'); + self::assertSame(21, $sheet->getCell('A2')->getValue()); + $sheet->getCell('A2')->setDataType(DataType::TYPE_STRING); + + $reloadedSpreadsheet = $this->writeAndReload($spreadsheet, 'Xlsx'); + $spreadsheet->disconnectWorksheets(); + $rsheet = $reloadedSpreadsheet->getActiveSheet(); + self::assertSame('21.5', $rsheet->getCell('A1')->getValue()); + self::assertSame('21', $rsheet->getCell('A2')->getValue()); + $reloadedSpreadsheet->disconnectWorksheets(); + } +}