Wrap actual file loading in protected methods, while just providing a stub load() method in the BaseReader abstract. This is to allow a wrapper in the file loader logic across all Readers, with a try/catch block that can be used to handle special loader settings. (#2620)

Author: Mark Baker

src/PhpSpreadsheet/Reader/BaseReader.php

@@ -2,9 +2,11 @@
 
 namespace PhpOffice\PhpSpreadsheet\Reader;
 
+use PhpOffice\PhpSpreadsheet\Exception as PhpSpreadsheetException;
 use PhpOffice\PhpSpreadsheet\Reader\Exception as ReaderException;
 use PhpOffice\PhpSpreadsheet\Reader\Security\XmlScanner;
 use PhpOffice\PhpSpreadsheet\Shared\File;
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
 
 abstract class BaseReader implements IReader
 {
@@ -144,25 +146,41 @@ protected function processFlags(int $flags): void
         }
     }
 
+    protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
+    {
+        throw new PhpSpreadsheetException('Reader classes must implement their own loadSpreadsheetFromFile() method');
+    }
+
+    /**
+     * Loads Spreadsheet from file.
+     */
+    public function load(string $filename, int $flags = 0): Spreadsheet
+    {
+        $this->processFlags($flags);
+
+        try {
+            return $this->loadSpreadsheetFromFile($filename);
+        } catch (ReaderException $e) {
+            throw $e;
+        }
+    }
+
     /**
      * Open file for reading.
-     *
-     * @param string $filename
      */
-    protected function openFile($filename): void
+    protected function openFile(string $filename): void
     {
+        $fileHandle = false;
         if ($filename) {
             File::assertFile($filename);
 
             // Open file
             $fileHandle = fopen($filename, 'rb');
-        } else {
-            $fileHandle = false;
         }
-        if ($fileHandle !== false) {
-            $this->fileHandle = $fileHandle;
-        } else {
+        if ($fileHandle === false) {
             throw new ReaderException('Could not open file ' . $filename . ' for reading.');
         }
+
+        $this->fileHandle = $fileHandle;
     }
 }

src/PhpSpreadsheet/Reader/Csv.php

@@ -236,13 +236,9 @@ public function listWorksheetInfo(string $filename): array
 
     /**
      * Loads Spreadsheet from file.
-     *
-     * @return Spreadsheet
      */
-    public function load(string $filename, int $flags = 0)
+    protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
     {
-        $this->processFlags($flags);
-
         // Create new Spreadsheet
         $spreadsheet = new Spreadsheet();
 

src/PhpSpreadsheet/Reader/Gnumeric.php

@@ -227,13 +227,9 @@ private static function testSimpleXml($value): SimpleXMLElement
 
     /**
      * Loads Spreadsheet from file.
-     *
-     * @return Spreadsheet
      */
-    public function load(string $filename, int $flags = 0)
+    protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
     {
-        $this->processFlags($flags);
-
         // Create new Spreadsheet
         $spreadsheet = new Spreadsheet();
         $spreadsheet->removeSheetByIndex(0);

src/PhpSpreadsheet/Reader/Html.php

@@ -19,7 +19,6 @@
 use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
 use Throwable;
 
-/** PhpSpreadsheet root directory */
 class Html extends BaseReader
 {
     /**
@@ -201,13 +200,9 @@ private static function containsTags(string $data): bool
 
     /**
      * Loads Spreadsheet from file.
-     *
-     * @return Spreadsheet
      */
-    public function load(string $filename, int $flags = 0)
+    protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
     {
-        $this->processFlags($flags);
-
         // Create new Spreadsheet
         $spreadsheet = new Spreadsheet();
 

src/PhpSpreadsheet/Reader/Ods.php

@@ -217,13 +217,9 @@ public function listWorksheetInfo($filename)
 
     /**
      * Loads PhpSpreadsheet from file.
-     *
-     * @return Spreadsheet
      */
-    public function load(string $filename, int $flags = 0)
+    protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
     {
-        $this->processFlags($flags);
-
         // Create new Spreadsheet
         $spreadsheet = new Spreadsheet();
 

src/PhpSpreadsheet/Reader/Slk.php

@@ -191,13 +191,9 @@ public function listWorksheetInfo($filename)
 
     /**
      * Loads PhpSpreadsheet from file.
-     *
-     * @return Spreadsheet
      */
-    public function load(string $filename, int $flags = 0)
+    protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
     {
-        $this->processFlags($flags);
-
         // Create new Spreadsheet
         $spreadsheet = new Spreadsheet();
 

src/PhpSpreadsheet/Reader/Xls.php

@@ -620,13 +620,9 @@ public function listWorksheetInfo($filename)
 
     /**
      * Loads PhpSpreadsheet from file.
-     *
-     * @return Spreadsheet
      */
-    public function load(string $filename, int $flags = 0)
+    protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
     {
-        $this->processFlags($flags);
-
         // Read the OLE file
         $this->loadOLE($filename);
 

src/PhpSpreadsheet/Reader/Xlsx.php

@@ -384,10 +384,9 @@ private function getFromZipArchive(ZipArchive $archive, $fileName = '')
     /**
      * Loads Spreadsheet from file.
      */
-    public function load(string $filename, int $flags = 0): Spreadsheet
+    protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
     {
         File::assertFile($filename, self::INITIAL_FILE);
-        $this->processFlags($flags);
 
         // Initialisations
         $excel = new Spreadsheet();

src/PhpSpreadsheet/Reader/Xml.php

@@ -231,13 +231,9 @@ public function listWorksheetInfo($filename)
 
     /**
      * Loads Spreadsheet from file.
-     *
-     * @return Spreadsheet
      */
-    public function load(string $filename, int $flags = 0)
+    protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
     {
-        $this->processFlags($flags);
-
         // Create new Spreadsheet
         $spreadsheet = new Spreadsheet();
         $spreadsheet->removeSheetByIndex(0);

tests/PhpSpreadsheetTests/Calculation/CalclationFunctionListTest.php renamed to tests/PhpSpreadsheetTests/Calculation/CalculationFunctionListTest.php

@@ -7,7 +7,7 @@
 use PhpOffice\PhpSpreadsheet\Spreadsheet;
 use PHPUnit\Framework\TestCase;
 
-class CalclationFunctionListTest extends TestCase
+class CalculationFunctionListTest extends TestCase
 {
     /**
      * @var string