Escapers should not be singletons

Instead, they should be proper object that can be injected where needed.

src/Spout/Common/Helper/Escaper/CSV.php

@@ -1,12 +1,12 @@
 <?php
 
-namespace Box\Spout\Common\Escaper;
+namespace Box\Spout\Common\Helper\Escaper;
 
 /**
  * Class CSV
  * Provides functions to escape and unescape data for CSV files
  *
- * @package Box\Spout\Common\Escaper
+ * @package Box\Spout\Common\Helper\Escaper
  */
 class CSV implements EscaperInterface
 {

src/Spout/Common/Helper/Escaper/EscaperInterface.php

@@ -1,11 +1,11 @@
 <?php
 
-namespace Box\Spout\Common\Escaper;
+namespace Box\Spout\Common\Helper\Escaper;
 
 /**
  * Interface EscaperInterface
  *
- * @package Box\Spout\Common\Escaper
+ * @package Box\Spout\Common\Helper\Escaper
  */
 interface EscaperInterface
 {

src/Spout/Common/Helper/Escaper/ODS.php

@@ -1,19 +1,15 @@
 <?php
 
-namespace Box\Spout\Common\Escaper;
+namespace Box\Spout\Common\Helper\Escaper;
-
-use Box\Spout\Common\Singleton;
 
 /**
  * Class ODS
  * Provides functions to escape and unescape data for ODS files
  *
- * @package Box\Spout\Common\Escaper
+ * @package Box\Spout\Common\Helper\Escaper
  */
 class ODS implements EscaperInterface
 {
-    use Singleton;
-
     /**
      * Escapes the given string to make it compatible with XLSX
      *

src/Spout/Common/Helper/Escaper/XLSX.php

@@ -1,36 +1,39 @@
 <?php
 
-namespace Box\Spout\Common\Escaper;
+namespace Box\Spout\Common\Helper\Escaper;
-
-use Box\Spout\Common\Singleton;
 
 /**
  * Class XLSX
  * Provides functions to escape and unescape data for XLSX files
  *
- * @package Box\Spout\Common\Escaper
+ * @package Box\Spout\Common\Helper\Escaper
  */
 class XLSX implements EscaperInterface
 {
-    use Singleton;
+    /** @var bool Whether the escaper has already been initialized */
+    private $isAlreadyInitialized = false;
 
     /** @var string Regex pattern to detect control characters that need to be escaped */
-    protected $escapableControlCharactersPattern;
+    private $escapableControlCharactersPattern;
 
     /** @var string[] Map containing control characters to be escaped (key) and their escaped value (value) */
-    protected $controlCharactersEscapingMap;
+    private $controlCharactersEscapingMap;
 
     /** @var string[] Map containing control characters to be escaped (value) and their escaped value (key) */
-    protected $controlCharactersEscapingReverseMap;
+    private $controlCharactersEscapingReverseMap;
 
     /**
-     * Initializes the singleton instance
+     * Initializes the control characters if not already done
      */
-    protected function init()
+    protected function initIfNeeded()
     {
+        if (!$this->isAlreadyInitialized) {
             $this->escapableControlCharactersPattern = $this->getEscapableControlCharactersPattern();
             $this->controlCharactersEscapingMap = $this->getControlCharactersEscapingMap();
             $this->controlCharactersEscapingReverseMap = array_flip($this->controlCharactersEscapingMap);
+
+            $this->isAlreadyInitialized = true;
+        }
     }
 
     /**
@@ -41,6 +44,8 @@ class XLSX implements EscaperInterface
      */
     public function escape($string)
     {
+        $this->initIfNeeded();
+
         $escapedString = $this->escapeControlCharacters($string);
         // @NOTE: Using ENT_NOQUOTES as only XML entities ('<', '>', '&') need to be encoded.
         //        Single and double quotes can be left as is.
@@ -57,6 +62,8 @@ class XLSX implements EscaperInterface
      */
     public function unescape($string)
     {
+        $this->initIfNeeded();
+
         // ==============
         // =   WARNING  =
         // ==============

src/Spout/Common/Singleton.php

@@ -1,41 +0,0 @@
-<?php
-
-namespace Box\Spout\Common;
-
-/**
- * Class Singleton
- * Defines a class as a singleton.
- *
- * @package Box\Spout\Common
- */
-trait Singleton
-{
-    protected static $instance;
-
-    /**
-     * @return static
-     */
-    final public static function getInstance()
-    {
-        return isset(static::$instance)
-            ? static::$instance
-            : static::$instance = new static;
-    }
-
-    /**
-     * Singleton constructor.
-     */
-    final private function __construct()
-    {
-        $this->init();
-    }
-
-    /**
-     * Initializes the singleton
-     * @return void
-     */
-    protected function init() {}
-
-    final private function __wakeup() {}
-    final private function __clone() {}
-}

src/Spout/Reader/ODS/Creator/HelperFactory.php

@@ -20,7 +20,8 @@ class HelperFactory extends \Box\Spout\Common\Creator\HelperFactory
      */
     public function createCellValueFormatter($shouldFormatDates)
     {
-        return new CellValueFormatter($shouldFormatDates);
+        $escaper = $this->createStringsEscaper();
+        return new CellValueFormatter($shouldFormatDates, $escaper);
     }
 
     /**
@@ -30,4 +31,13 @@ class HelperFactory extends \Box\Spout\Common\Creator\HelperFactory
     {
         return new SettingsHelper();
     }
+
+    /**
+     * @return \Box\Spout\Common\Helper\Escaper\ODS
+     */
+    public function createStringsEscaper()
+    {
+        /** @noinspection PhpUnnecessaryFullyQualifiedNameInspection */
+        return new \Box\Spout\Common\Helper\Escaper\ODS();
+    }
 }

src/Spout/Reader/ODS/Helper/CellValueFormatter.php

@@ -38,18 +38,17 @@ class CellValueFormatter
     /** @var bool Whether date/time values should be returned as PHP objects or be formatted as strings */
     protected $shouldFormatDates;
 
-    /** @var \Box\Spout\Common\Escaper\ODS Used to unescape XML data */
+    /** @var \Box\Spout\Common\Helper\Escaper\ODS Used to unescape XML data */
     protected $escaper;
 
     /**
      * @param bool $shouldFormatDates Whether date/time values should be returned as PHP objects or be formatted as strings
+     * @param \Box\Spout\Common\Helper\Escaper\ODS $escaper Used to unescape XML data
      */
-    public function __construct($shouldFormatDates)
+    public function __construct($shouldFormatDates, $escaper)
     {
         $this->shouldFormatDates = $shouldFormatDates;
-
+        $this->escaper = $escaper;
-        /** @noinspection PhpUnnecessaryFullyQualifiedNameInspection */
-        $this->escaper = \Box\Spout\Common\Escaper\ODS::getInstance();
     }
 
     /**

src/Spout/Reader/ODS/SheetIterator.php

@@ -36,7 +36,7 @@ class SheetIterator implements IteratorInterface
     /** @var XMLReader The XMLReader object that will help read sheet's XML data */
     protected $xmlReader;
 
-    /** @var \Box\Spout\Common\Escaper\ODS Used to unescape XML data */
+    /** @var \Box\Spout\Common\Helper\Escaper\ODS Used to unescape XML data */
     protected $escaper;
 
     /** @var bool Whether there are still at least a sheet to be read */
@@ -61,8 +61,7 @@ class SheetIterator implements IteratorInterface
         $this->entityFactory = $entityFactory;
         $this->xmlReader = $entityFactory->createXMLReader();
 
-        /** @noinspection PhpUnnecessaryFullyQualifiedNameInspection */
+        $this->escaper = $helperFactory->createStringsEscaper();
-        $this->escaper = \Box\Spout\Common\Escaper\ODS::getInstance();
 
         $settingsHelper = $helperFactory->createSettingsHelper();
         $this->activeSheetName = $settingsHelper->getActiveSheetName($filePath);

src/Spout/Reader/XLSX/Creator/HelperFactory.php

@@ -2,6 +2,7 @@
 
 namespace Box\Spout\Reader\XLSX\Creator;
 
+use Box\Spout\Common\Helper\Escaper;
 use Box\Spout\Reader\XLSX\Helper\CellValueFormatter;
 use Box\Spout\Reader\XLSX\Helper\SharedStringsCaching\CachingStrategyFactory;
 use Box\Spout\Reader\XLSX\Helper\SharedStringsHelper;
@@ -49,7 +50,8 @@ class HelperFactory extends \Box\Spout\Common\Creator\HelperFactory
      */
     public function createSheetHelper($filePath, $optionsManager, $sharedStringsHelper, $globalFunctionsHelper, $entityFactory)
     {
-        return new SheetHelper($filePath, $optionsManager, $sharedStringsHelper, $globalFunctionsHelper, $entityFactory);
+        $escaper = $this->createStringsEscaper();
+        return new SheetHelper($filePath, $optionsManager, $sharedStringsHelper, $globalFunctionsHelper, $escaper, $entityFactory);
     }
 
     /**
@@ -70,6 +72,16 @@ class HelperFactory extends \Box\Spout\Common\Creator\HelperFactory
      */
     public function createCellValueFormatter($sharedStringsHelper, $styleHelper, $shouldFormatDates)
     {
-        return new CellValueFormatter($sharedStringsHelper, $styleHelper, $shouldFormatDates);
+        $escaper = $this->createStringsEscaper();
+        return new CellValueFormatter($sharedStringsHelper, $styleHelper, $shouldFormatDates, $escaper);
+    }
+
+    /**
+     * @return Escaper\XLSX
+     */
+    public function createStringsEscaper()
+    {
+        /** @noinspection PhpUnnecessaryFullyQualifiedNameInspection */
+        return new Escaper\XLSX();
     }
 }

src/Spout/Reader/XLSX/Helper/CellValueFormatter.php

@@ -47,22 +47,21 @@ class CellValueFormatter
     /** @var bool Whether date/time values should be returned as PHP objects or be formatted as strings */
     protected $shouldFormatDates;
 
-    /** @var \Box\Spout\Common\Escaper\XLSX Used to unescape XML data */
+    /** @var \Box\Spout\Common\Helper\Escaper\XLSX Used to unescape XML data */
     protected $escaper;
 
     /**
      * @param SharedStringsHelper $sharedStringsHelper Helper to work with shared strings
      * @param StyleHelper $styleHelper Helper to work with styles
      * @param bool $shouldFormatDates Whether date/time values should be returned as PHP objects or be formatted as strings
+     * @param \Box\Spout\Common\Helper\Escaper\XLSX $escaper Used to unescape XML data
      */
-    public function __construct($sharedStringsHelper, $styleHelper, $shouldFormatDates)
+    public function __construct($sharedStringsHelper, $styleHelper, $shouldFormatDates, $escaper)
     {
         $this->sharedStringsHelper = $sharedStringsHelper;
         $this->styleHelper = $styleHelper;
         $this->shouldFormatDates = $shouldFormatDates;
-
+        $this->escaper = $escaper;
-        /** @noinspection PhpUnnecessaryFullyQualifiedNameInspection */
-        $this->escaper = \Box\Spout\Common\Escaper\XLSX::getInstance();
     }
 
     /**

src/Spout/Reader/XLSX/Helper/SheetHelper.php

@@ -46,19 +46,24 @@ class SheetHelper
     /** @var EntityFactory Factory to create entities */
     protected $entityFactory;
 
+    /** @var \Box\Spout\Common\Helper\Escaper\XLSX Used to unescape XML data */
+    protected $escaper;
+
     /**
      * @param string $filePath Path of the XLSX file being read
      * @param \Box\Spout\Common\Manager\OptionsManagerInterface $optionsManager Reader's options manager
      * @param \Box\Spout\Reader\XLSX\Helper\SharedStringsHelper Helper to work with shared strings
      * @param \Box\Spout\Common\Helper\GlobalFunctionsHelper $globalFunctionsHelper
+     * @param \Box\Spout\Common\Helper\Escaper\XLSX $escaper Used to unescape XML data
      * @param EntityFactory $entityFactory Factory to create entities
      */
-    public function __construct($filePath, $optionsManager, $sharedStringsHelper, $globalFunctionsHelper, $entityFactory)
+    public function __construct($filePath, $optionsManager, $sharedStringsHelper, $globalFunctionsHelper, $escaper, $entityFactory)
     {
         $this->filePath = $filePath;
         $this->optionsManager = $optionsManager;
         $this->sharedStringsHelper = $sharedStringsHelper;
         $this->globalFunctionsHelper = $globalFunctionsHelper;
+        $this->escaper = $escaper;
         $this->entityFactory = $entityFactory;
     }
 
@@ -112,10 +117,7 @@ class SheetHelper
     {
         $sheetId = $xmlReaderOnSheetNode->getAttribute(self::XML_ATTRIBUTE_R_ID);
         $escapedSheetName = $xmlReaderOnSheetNode->getAttribute(self::XML_ATTRIBUTE_NAME);
-
+        $sheetName = $this->escaper->unescape($escapedSheetName);
-        /** @noinspection PhpUnnecessaryFullyQualifiedNameInspection */
-        $escaper = \Box\Spout\Common\Escaper\XLSX::getInstance();
-        $sheetName = $escaper->unescape($escapedSheetName);
 
         $sheetDataXMLFilePath = $this->getSheetDataXMLFilePathForSheetId($sheetId);
 

src/Spout/Writer/ODS/Creator/InternalFactory.php

@@ -13,7 +13,7 @@ use Box\Spout\Writer\ODS\Manager\Style\StyleManager;
 use Box\Spout\Writer\ODS\Manager\Style\StyleRegistry;
 use Box\Spout\Writer\ODS\Manager\WorkbookManager;
 use Box\Spout\Writer\ODS\Manager\WorksheetManager;
-use \Box\Spout\Common\Escaper;
+use Box\Spout\Common\Helper\Escaper;
 
 /**
  * Class InternalFactory
@@ -109,7 +109,7 @@ class InternalFactory implements InternalFactoryInterface
      */
     private function createStringsEscaper()
     {
-        return Escaper\ODS::getInstance();
+        return new Escaper\ODS();
     }
 
     /**

src/Spout/Writer/ODS/Manager/WorksheetManager.php

@@ -19,7 +19,7 @@ use Box\Spout\Writer\ODS\Manager\Style\StyleManager;
  */
 class WorksheetManager implements WorksheetManagerInterface
 {
-    /** @var \Box\Spout\Common\Escaper\ODS Strings escaper */
+    /** @var \Box\Spout\Common\Helper\Escaper\ODS Strings escaper */
     private $stringsEscaper;
 
     /** @var StringHelper String helper */
@@ -28,11 +28,11 @@ class WorksheetManager implements WorksheetManagerInterface
     /**
      * WorksheetManager constructor.
      *
-     * @param \Box\Spout\Common\Escaper\ODS $stringsEscaper
+     * @param \Box\Spout\Common\Helper\Escaper\ODS $stringsEscaper
      * @param StringHelper $stringHelper
      */
     public function __construct(
-        \Box\Spout\Common\Escaper\ODS $stringsEscaper,
+        \Box\Spout\Common\Helper\Escaper\ODS $stringsEscaper,
         StringHelper $stringHelper)
     {
         $this->stringsEscaper = $stringsEscaper;

src/Spout/Writer/XLSX/Creator/InternalFactory.php

@@ -2,7 +2,7 @@
 
 namespace Box\Spout\Writer\XLSX\Creator;
 
-use Box\Spout\Common\Escaper;
+use Box\Spout\Common\Helper\Escaper;
 use Box\Spout\Common\Helper\StringHelper;
 use Box\Spout\Writer\Common\Creator\EntityFactory;
 use Box\Spout\Writer\Common\Creator\InternalFactoryInterface;
@@ -113,8 +113,9 @@ class InternalFactory implements InternalFactoryInterface
     {
         $tempFolder = $optionsManager->getOption(Options::TEMP_FOLDER);
         $zipHelper = $this->createZipHelper();
+        $escaper = $this->createStringsEscaper();
 
-        return new FileSystemHelper($tempFolder, $zipHelper);
+        return new FileSystemHelper($tempFolder, $zipHelper, $escaper);
     }
 
     /**
@@ -130,7 +131,7 @@ class InternalFactory implements InternalFactoryInterface
      */
     private function createStringsEscaper()
     {
-        return Escaper\XLSX::getInstance();
+        return new Escaper\XLSX();
     }
 
     /**

src/Spout/Writer/XLSX/Helper/FileSystemHelper.php

@@ -34,6 +34,9 @@ class FileSystemHelper extends \Box\Spout\Common\Helper\FileSystemHelper impleme
     /** @var ZipHelper Helper to perform tasks with Zip archive */
     private $zipHelper;
 
+    /** @var \Box\Spout\Common\Helper\Escaper\XLSX Used to escape XML data */
+    private $escaper;
+
     /** @var string Path to the root folder inside the temp folder where the files to create the XLSX will be stored */
     private $rootFolder;
 
@@ -55,11 +58,13 @@ class FileSystemHelper extends \Box\Spout\Common\Helper\FileSystemHelper impleme
     /**
      * @param string $baseFolderPath The path of the base folder where all the I/O can occur
      * @param ZipHelper $zipHelper Helper to perform tasks with Zip archive
+     * @param \Box\Spout\Common\Helper\Escaper\XLSX $escaper Used to escape XML data
      */
-    public function __construct($baseFolderPath, $zipHelper)
+    public function __construct($baseFolderPath, $zipHelper, $escaper)
     {
         parent::__construct($baseFolderPath);
         $this->zipHelper = $zipHelper;
+        $this->escaper = $escaper;
     }
 
     /**
@@ -298,14 +303,11 @@ EOD;
     <sheets>
 EOD;
 
-        /** @noinspection PhpUnnecessaryFullyQualifiedNameInspection */
-        $escaper = \Box\Spout\Common\Escaper\XLSX::getInstance();
-
         /** @var Worksheet $worksheet */
         foreach ($worksheets as $worksheet) {
             $worksheetName = $worksheet->getExternalSheet()->getName();
             $worksheetId = $worksheet->getId();
-            $workbookXmlFileContents .= '<sheet name="' . $escaper->escape($worksheetName) . '" sheetId="' . $worksheetId . '" r:id="rIdSheet' . $worksheetId . '"/>';
+            $workbookXmlFileContents .= '<sheet name="' . $this->escaper->escape($worksheetName) . '" sheetId="' . $worksheetId . '" r:id="rIdSheet' . $worksheetId . '"/>';
         }
 
         $workbookXmlFileContents .= <<<EOD

src/Spout/Writer/XLSX/Manager/SharedStringsManager.php

@@ -3,7 +3,7 @@
 namespace Box\Spout\Writer\XLSX\Manager;
 
 use Box\Spout\Common\Exception\IOException;
-use Box\Spout\Common\Escaper;
+use Box\Spout\Common\Helper\Escaper;
 
 /**
  * Class SharedStringsManager

src/Spout/Writer/XLSX/Manager/WorksheetManager.php

@@ -44,7 +44,7 @@ EOD;
     /** @var SharedStringsManager Helper to write shared strings */
     private $sharedStringsManager;
 
-    /** @var \Box\Spout\Common\Escaper\XLSX Strings escaper */
+    /** @var \Box\Spout\Common\Helper\Escaper\XLSX Strings escaper */
     private $stringsEscaper;
 
     /** @var StringHelper String helper */
@@ -56,14 +56,14 @@ EOD;
      * @param OptionsManagerInterface $optionsManager
      * @param StyleManager $styleManager
      * @param SharedStringsManager $sharedStringsManager
-     * @param \Box\Spout\Common\Escaper\XLSX $stringsEscaper
+     * @param \Box\Spout\Common\Helper\Escaper\XLSX $stringsEscaper
      * @param StringHelper $stringHelper
      */
     public function __construct(
         OptionsManagerInterface $optionsManager,
         StyleManager $styleManager,
         SharedStringsManager $sharedStringsManager,
-        \Box\Spout\Common\Escaper\XLSX $stringsEscaper,
+        \Box\Spout\Common\Helper\Escaper\XLSX $stringsEscaper,
         StringHelper $stringHelper)
     {
         $this->shouldUseInlineStrings = $optionsManager->getOption(Options::SHOULD_USE_INLINE_STRINGS);

tests/Spout/Common/Helper/Escaper/ODSTest.php

@@ -1,11 +1,13 @@
 <?php
 
-namespace Box\Spout\Common\Escaper;
+namespace Box\Spout\Common\Helper\Escaper;
+
+use Box\Spout\Common\Helper\Escaper;
 
 /**
  * Class ODSTest
  *
- * @package Box\Spout\Common\Escaper
+ * @package Box\Spout\Common\Helper\Escaper
  */
 class ODSTest extends \PHPUnit_Framework_TestCase
 {
@@ -34,7 +36,7 @@ class ODSTest extends \PHPUnit_Framework_TestCase
      */
     public function testEscape($stringToEscape, $expectedEscapedString)
     {
-        $escaper = \Box\Spout\Common\Escaper\ODS::getInstance();
+        $escaper = new Escaper\ODS();
         $escapedString = $escaper->escape($stringToEscape);
 
         $this->assertEquals($expectedEscapedString, $escapedString, 'Incorrect escaped string');

tests/Spout/Common/Helper/Escaper/XLSXTest.php

@@ -1,11 +1,13 @@
 <?php
 
-namespace Box\Spout\Common\Escaper;
+namespace Box\Spout\Common\Helper\Escaper;
+
+use Box\Spout\Common\Helper\Escaper;
 
 /**
  * Class XLSXTest
  *
- * @package Box\Spout\Common\Escaper
+ * @package Box\Spout\Common\Helper\Escaper
  */
 class XLSXTest extends \PHPUnit_Framework_TestCase
 {
@@ -38,8 +40,7 @@ class XLSXTest extends \PHPUnit_Framework_TestCase
      */
     public function testEscape($stringToEscape, $expectedEscapedString)
     {
-        /** @noinspection PhpUnnecessaryFullyQualifiedNameInspection */
+        $escaper = new Escaper\XLSX();
-        $escaper = \Box\Spout\Common\Escaper\XLSX::getInstance();
         $escapedString = $escaper->escape($stringToEscape);
 
         $this->assertEquals($expectedEscapedString, $escapedString, 'Incorrect escaped string');
@@ -74,8 +75,7 @@ class XLSXTest extends \PHPUnit_Framework_TestCase
      */
     public function testUnescape($stringToUnescape, $expectedUnescapedString)
     {
-        /** @noinspection PhpUnnecessaryFullyQualifiedNameInspection */
+        $escaper = new Escaper\XLSX();
-        $escaper = \Box\Spout\Common\Escaper\XLSX::getInstance();
         $unescapedString = $escaper->unescape($stringToUnescape);
 
         $this->assertEquals($expectedUnescapedString, $unescapedString, 'Incorrect escaped string');

tests/Spout/Reader/XLSX/Helper/CellValueFormatterTest.php

@@ -2,6 +2,8 @@
 
 namespace Box\Spout\Reader\XLSX\Helper;
 
+use Box\Spout\Common\Helper\Escaper;
+
 /**
  * Class CellValueFormatterTest
  *
@@ -71,7 +73,7 @@ class CellValueFormatterTest extends \PHPUnit_Framework_TestCase
             ->with(123)
             ->will($this->returnValue(true));
 
-        $formatter = new CellValueFormatter(null, $styleHelperMock, false);
+        $formatter = new CellValueFormatter(null, $styleHelperMock, false, new Escaper\XLSX());
         $result = $formatter->extractAndFormatNodeValue($nodeMock);
 
         if ($expectedDateAsString === null) {
@@ -124,7 +126,7 @@ class CellValueFormatterTest extends \PHPUnit_Framework_TestCase
             ->method('shouldFormatNumericValueAsDate')
             ->will($this->returnValue(false));
 
-        $formatter = new CellValueFormatter(null, $styleHelperMock, false);
+        $formatter = new CellValueFormatter(null, $styleHelperMock, false, new Escaper\XLSX());
         $formattedValue = \ReflectionHelper::callMethodOnObject($formatter, 'formatNumericCellValue', $value, 0);
 
         $this->assertEquals($expectedFormattedValue, $formattedValue);
@@ -167,7 +169,7 @@ class CellValueFormatterTest extends \PHPUnit_Framework_TestCase
             ->with(CellValueFormatter::XML_NODE_INLINE_STRING_VALUE)
             ->will($this->returnValue($nodeListMock));
 
-        $formatter = new CellValueFormatter(null, null, false);
+        $formatter = new CellValueFormatter(null, null, false, new Escaper\XLSX());
         $formattedValue = \ReflectionHelper::callMethodOnObject($formatter, 'formatInlineStringCellValue', $nodeMock);
 
         $this->assertEquals($expectedFormattedValue, $formattedValue);