IOFactory::identify and Custom Reader/Writer

Fix PHPOffice#4357.`identify` returns a type, not a class name, and this result is not always usable for createReader. The docs say that it is usable in that manner. Changing the behavior of `identify` would be a breaking change, but adding an optional parameter (defaulting to current behavior) allowing it to return a class name rather than a type would not. This PR adds that parameter and updates the documentation to suggest its use for the code in question.

To complete this change, `createReader` now accepts either a type (current behavior) or class name (new). Although it is not particularly identified as a possible problem by the issue, `createWriter` is similarly changed for consistency's sake.

docs/topics/reading-files.md

@@ -123,7 +123,10 @@ method to identify the reader that you need, before using the
 ```php
 $inputFileName = './sampleData/example1.xls';
 
-/**  Identify the type of $inputFileName  **/
+/**
+ * Identify the type of $inputFileName.
+ * See below for a possible improvement for release 4.1.0+.
+ */
 $inputFileType = \PhpOffice\PhpSpreadsheet\IOFactory::identify($inputFileName);
 /**  Create a new Reader of the type that has been identified  **/
 $reader = \PhpOffice\PhpSpreadsheet\IOFactory::createReader($inputFileType);
@@ -134,6 +137,13 @@ $spreadsheet = $reader->load($inputFileName);
 See `samples/Reader/04_Simple_file_reader_using_the_IOFactory_to_identify_a_reader_to_use.php`
 for a working example of this code.
 
+Prior to release 4.1.0, `identify` returns a file type.
+It may be more useful to return a fully-qualified class name,
+which can be accomplished using a parameter introduced in 4.1.0:
+```php
+$inputFileType = \PhpOffice\PhpSpreadsheet\IOFactory::identify($inputFileName, null, true);
+```
+
 As with the IOFactory `load()` method, you can also pass an array of formats
 for  the `identify()` method to check against if you know that it will only
 be in a subset of the possible formats that PhpSpreadsheet supports.

src/PhpSpreadsheet/IOFactory.php

@@ -59,12 +59,16 @@ abstract class IOFactory
      */
     public static function createWriter(Spreadsheet $spreadsheet, string $writerType): IWriter
     {
-        if (!isset(self::$writers[$writerType])) {
-            throw new Writer\Exception("No writer found for type $writerType");
-        }
+        /** @var class-string<IWriter> */
+        $className = $writerType;
+        if (!in_array($writerType, self::$writers, true)) {
+            if (!isset(self::$writers[$writerType])) {
+                throw new Writer\Exception("No writer found for type $writerType");
+            }
 
-        // Instantiate writer
-        $className = self::$writers[$writerType];
+            // Instantiate writer
+            $className = self::$writers[$writerType];
+        }
 
         return new $className($spreadsheet);
     }
@@ -74,12 +78,16 @@ public static function createWriter(Spreadsheet $spreadsheet, string $writerType
      */
     public static function createReader(string $readerType): IReader
     {
-        if (!isset(self::$readers[$readerType])) {
-            throw new Reader\Exception("No reader found for type $readerType");
-        }
+        /** @var class-string<IReader> */
+        $className = $readerType;
+        if (!in_array($readerType, self::$readers, true)) {
+            if (!isset(self::$readers[$readerType])) {
+                throw new Reader\Exception("No reader found for type $readerType");
+            }
 
-        // Instantiate reader
-        $className = self::$readers[$readerType];
+            // Instantiate reader
+            $className = self::$readers[$readerType];
+        }
 
         return new $className();
     }
@@ -109,12 +117,14 @@ public static function load(string $filename, int $flags = 0, ?array $readers =
     /**
      * Identify file type using automatic IReader resolution.
      */
-    public static function identify(string $filename, ?array $readers = null): string
+    public static function identify(string $filename, ?array $readers = null, bool $fullClassName = false): string
     {
         $reader = self::createReaderForFile($filename, $readers);
         $className = $reader::class;
+        if ($fullClassName) {
+            return $className;
+        }
         $classType = explode('\\', $className);
-        unset($reader);
 
         return array_pop($classType);
     }
@@ -224,6 +234,8 @@ public static function registerWriter(string $writerType, string $writerClass):
 
     /**
      * Register a reader with its type and class name.
+     *
+     * @param class-string<IReader> $readerClass
      */
     public static function registerReader(string $readerType, string $readerClass): void
     {

tests/PhpSpreadsheetTests/CustomReader.php

@@ -0,0 +1,12 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpOffice\PhpSpreadsheetTests;
+
+use PhpOffice\PhpSpreadsheet\Reader\Xlsx as XlsxReader;
+
+// Used in IOFactoryRegister tests
+class CustomReader extends XlsxReader
+{
+}

tests/PhpSpreadsheetTests/CustomWriter.php

@@ -0,0 +1,12 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpOffice\PhpSpreadsheetTests;
+
+use PhpOffice\PhpSpreadsheet\Writer\Html as HtmlWriter;
+
+// Used in IOFactoryRegister tests
+class CustomWriter extends HtmlWriter
+{
+}

tests/PhpSpreadsheetTests/IOFactoryRegisterTest.php

@@ -0,0 +1,88 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpOffice\PhpSpreadsheetTests;
+
+use PhpOffice\PhpSpreadsheet\IOFactory;
+use PhpOffice\PhpSpreadsheet\Reader;
+use PhpOffice\PhpSpreadsheet\Reader\Exception as ReaderException;
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
+use PhpOffice\PhpSpreadsheet\Writer;
+use PHPUnit\Framework\Attributes;
+use PHPUnit\Framework\TestCase;
+
+// Separate processes because register arrays are static
+#[Attributes\RunTestsInSeparateProcesses]
+class IOFactoryRegisterTest extends TestCase
+{
+    public function testRegisterWriter(): void
+    {
+        IOFactory::registerWriter('Pdf', Writer\Pdf\Mpdf::class);
+        $spreadsheet = new Spreadsheet();
+        $actual = IOFactory::createWriter($spreadsheet, 'Pdf');
+        self::assertInstanceOf(Writer\Pdf\Mpdf::class, $actual);
+    }
+
+    public function testRegisterReader(): void
+    {
+        IOFactory::registerReader('Custom', Reader\Html::class);
+        $actual = IOFactory::createReader('Custom');
+        self::assertInstanceOf(Reader\Html::class, $actual);
+    }
+
+    public function testRegisterInvalidWriter(): void
+    {
+        $this->expectException(Writer\Exception::class);
+        $this->expectExceptionMessage('writers must implement');
+        IOFactory::registerWriter('foo', 'bar'); // @phpstan-ignore-line
+    }
+
+    public function testRegisterInvalidReader(): void
+    {
+        $this->expectException(ReaderException::class);
+        $this->expectExceptionMessage('readers must implement');
+        IOFactory::registerReader('foo', 'bar'); // @phpstan-ignore-line
+    }
+
+    public static function testRegisterCustomReader(): void
+    {
+        IOFactory::registerReader(IOFactory::READER_XLSX, CustomReader::class);
+        $inputFileName = 'tests/data/Reader/Xlsx/1900_Calendar.xlsx';
+        $loadSpreadsheet = IOFactory::load($inputFileName);
+        $sheet = $loadSpreadsheet->getActiveSheet();
+        self::assertSame('2022-01-01', $sheet->getCell('A1')->getFormattedValue());
+        $loadSpreadsheet->disconnectWorksheets();
+
+        $reader = new CustomReader();
+        $newSpreadsheet = $reader->load($inputFileName);
+        $newSheet = $newSpreadsheet->getActiveSheet();
+        self::assertSame('2022-01-01', $newSheet->getCell('A1')->getFormattedValue());
+        $newSpreadsheet->disconnectWorksheets();
+
+        $inputFileType = IOFactory::identify($inputFileName, null, true);
+        $objReader = IOFactory::createReader($inputFileType);
+        self::assertInstanceOf(CustomReader::class, $objReader);
+        $objSpreadsheet = $objReader->load($inputFileName);
+        $objSheet = $objSpreadsheet->getActiveSheet();
+        self::assertSame('2022-01-01', $objSheet->getCell('A1')->getFormattedValue());
+        $objSpreadsheet->disconnectWorksheets();
+    }
+
+    public static function testRegisterCustomWriter(): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->getActiveSheet();
+        $sheet->setCellValue('A1', 1);
+        $writer = new CustomWriter($spreadsheet);
+        $html = $writer->generateHtmlAll();
+        self::assertStringContainsString('<td class="column0 style0 n">1</td>', $html);
+        IOFactory::registerWriter(IOFactory::WRITER_HTML, CustomWriter::class);
+        $objWriter = IOFactory::createWriter($spreadsheet, CustomWriter::class);
+        self::assertInstanceOf(CustomWriter::class, $objWriter);
+        self::assertTrue(method_exists($objWriter, 'generateHtmlAll'));
+        $html2 = $objWriter->generateHtmlAll();
+        self::assertStringContainsString('<td class="column0 style0 n">1</td>', $html2);
+        $spreadsheet->disconnectWorksheets();
+    }
+}

tests/PhpSpreadsheetTests/IOFactoryTest.php

@@ -9,11 +9,12 @@
 use PhpOffice\PhpSpreadsheet\Reader\Exception as ReaderException;
 use PhpOffice\PhpSpreadsheet\Spreadsheet;
 use PhpOffice\PhpSpreadsheet\Writer;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
 
 class IOFactoryTest extends TestCase
 {
-    #[\PHPUnit\Framework\Attributes\DataProvider('providerCreateWriter')]
+    #[DataProvider('providerCreateWriter')]
     public function testCreateWriter(string $name, string $expected): void
     {
         $spreadsheet = new Spreadsheet();
@@ -35,15 +36,7 @@ public static function providerCreateWriter(): array
         ];
     }
 
-    public function testRegisterWriter(): void
-    {
-        IOFactory::registerWriter('Pdf', Writer\Pdf\Mpdf::class);
-        $spreadsheet = new Spreadsheet();
-        $actual = IOFactory::createWriter($spreadsheet, 'Pdf');
-        self::assertInstanceOf(Writer\Pdf\Mpdf::class, $actual);
-    }
-
-    #[\PHPUnit\Framework\Attributes\DataProvider('providerCreateReader')]
+    #[DataProvider('providerCreateReader')]
     public function testCreateReader(string $name, string $expected): void
     {
         $actual = IOFactory::createReader($name);
@@ -64,22 +57,14 @@ public static function providerCreateReader(): array
         ];
     }
 
-    public function testRegisterReader(): void
-    {
-        IOFactory::registerReader('Custom', Reader\Html::class);
-        $actual = IOFactory::createReader('Custom');
-        self::assertInstanceOf(Reader\Html::class, $actual);
-    }
-
-    #[\PHPUnit\Framework\Attributes\DataProvider('providerIdentify')]
+    #[DataProvider('providerIdentify')]
     public function testIdentifyCreateLoad(string $file, string $expectedName, string $expectedClass): void
     {
         $actual = IOFactory::identify($file);
         self::assertSame($expectedName, $actual);
         $actual = IOFactory::createReaderForFile($file);
         self::assertSame($expectedClass, $actual::class);
-        $actual = IOFactory::load($file);
-        self::assertInstanceOf(Spreadsheet::class, $actual);
+        IOFactory::load($file);
     }
 
     public static function providerIdentify(): array
@@ -159,21 +144,6 @@ public function testIdentifyExistingDirectoryThrowExceptions(): void
         IOFactory::identify('.');
     }
 
-    public function testRegisterInvalidWriter(): void
-    {
-        $this->expectException(Writer\Exception::class);
-
-        // @phpstan-ignore-next-line
-        IOFactory::registerWriter('foo', 'bar');
-    }
-
-    public function testRegisterInvalidReader(): void
-    {
-        $this->expectException(ReaderException::class);
-
-        IOFactory::registerReader('foo', 'bar');
-    }
-
     public function testCreateInvalidWriter(): void
     {
         $this->expectException(Writer\Exception::class);