diff --git a/.github/FUNDING.yml b/.github/FUNDING.yml new file mode 100644 index 0000000..51a7b98 --- /dev/null +++ b/.github/FUNDING.yml @@ -0,0 +1 @@ +github: adrilo diff --git a/.gitignore b/.gitignore index 048320b..6253664 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,4 @@ /vendor /composer.lock /.php_cs.cache +/.phpunit.result.cache diff --git a/.travis.yml b/.travis.yml index 4605919..abc92c4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,16 +5,14 @@ language: php matrix: include: - - php: 7.1 - env: WITH_CS=true - - php: 7.1 - env: WITH_PHPSTAN=true - - php: 7.1 - env: WITH_PHPUNIT=true WITH_COVERAGE=true - php: 7.2 env: WITH_PHPUNIT=true + - php: 7.2 + env: WITH_PHPSTAN=true - php: 7.3 env: WITH_PHPUNIT=true + - php: 7.3 + env: WITH_PHPUNIT=true WITH_COVERAGE=true - php: 7.4 env: WITH_PHPUNIT=true @@ -47,7 +45,7 @@ before_script: script: - | if [[ "$WITH_CS" == "true" ]]; then - vendor/bin/php-cs-fixer fix --config=.php_cs.dist --verbose --diff --dry-run + vendor/bin/php-cs-fixer fix --config=.php_cs.dist --verbose --diff --dry-run --diff-format=udiff fi - | if [[ "$WITH_PHPSTAN" == "true" ]]; then diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d0eb666..5db0f6e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -68,11 +68,21 @@ This will add your changes on top of what's already in upstream, minimizing merg Make sure that all tests are passing before submitting a pull request. -- Phpunit: `composer phpunit` -- Phpstan: `composer phpstan` -- Php-cs-fixer: `composer phpcs` +### Step 8: Fix code style -### Step 8: Send the pull request +Run the following command to check the code style of your changes: + +``` +vendor/bin/php-cs-fixer fix --config=.php_cs.dist --verbose --diff --dry-run --diff-format=udiff +``` + +This will print a diff of proposed code style changes. To apply these suggestions, run the following command: + +``` +vendor/bin/php-cs-fixer fix --config=.php_cs.dist +``` + +### Step 9: Send the pull request Send the pull request from your feature branch to us. Be sure to include a description that lets us know what work you did. diff --git a/README.md b/README.md index 517f5a4..7c77242 100644 --- a/README.md +++ b/README.md @@ -8,9 +8,9 @@ [![Total Downloads](https://poser.pugx.org/box/spout/downloads)](https://packagist.org/packages/box/spout) Spout is a PHP library to read and write spreadsheet files (CSV, XLSX and ODS), in a fast and scalable way. -Contrary to other file readers or writers, it is capable of processing very large files while keeping the memory usage really low (less than 3MB). +Unlike other file readers or writers, it is capable of processing very large files, while keeping the memory usage really low (less than 3MB). -Join the community and come discuss about Spout: [![Gitter](https://badges.gitter.im/Join%20Chat.svg)](https://gitter.im/box/spout?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge) +Join the community and come discuss Spout: [![Gitter](https://badges.gitter.im/Join%20Chat.svg)](https://gitter.im/box/spout?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge) ## Documentation @@ -20,7 +20,7 @@ Full documentation can be found at [https://opensource.box.com/spout/](https://o ## Requirements -* PHP version 7.1 or higher +* PHP version 7.2 or higher * PHP extension `php_zip` enabled * PHP extension `php_xmlreader` enabled @@ -43,7 +43,7 @@ For information, the performance tests take about 10 minutes to run (processing ## Support -You can ask questions, submit new features ideas or discuss about Spout in the chat room:
+You can ask questions, submit new features ideas or discuss Spout in the chat room:
[![Gitter](https://badges.gitter.im/Join%20Chat.svg)](https://gitter.im/box/spout?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge) ## Copyright and License diff --git a/UPGRADE-3.0.md b/UPGRADE-3.0.md index 3952443..9b2e370 100644 --- a/UPGRADE-3.0.md +++ b/UPGRADE-3.0.md @@ -11,7 +11,7 @@ With the 3.0 version, this is now possible: each cell can have its own style. Spout 3.0 tries to enforce better typing. For instance, instead of using/returning generic arrays, Spout now makes use of specific `Row` and `Cell` objects that can encapsulate more data such as type, style, value. -Finally, **_Spout 3.0 only supports PHP 7.1 and above_**, as other PHP versions are no longer supported by the community. +Finally, **_Spout 3.2 only supports PHP 7.2 and above_**, as other PHP versions are no longer supported by the community. Reader changes -------------- diff --git a/composer.json b/composer.json index 552db4c..62e4e0e 100644 --- a/composer.json +++ b/composer.json @@ -12,13 +12,13 @@ } ], "require": { - "php": ">=7.1.0", + "php": ">=7.2.0", "ext-zip": "*", - "ext-xmlreader" : "*" + "ext-xmlreader": "*", + "ext-dom": "*" }, "require-dev": { - "ext-dom": "*", - "phpunit/phpunit": "^7", + "phpunit/phpunit": "^8", "friendsofphp/php-cs-fixer": "^2", "phpstan/phpstan": "^0.12.98" }, @@ -43,7 +43,7 @@ }, "config": { "platform": { - "php": "7.1" + "php": "7.2" } } } diff --git a/docs/_pages/documentation.md b/docs/_pages/documentation.md index d63ca03..ce142cf 100755 --- a/docs/_pages/documentation.md +++ b/docs/_pages/documentation.md @@ -116,18 +116,19 @@ $reader->setShouldPreserveEmptyRows(true); For fonts and alignments, {{ site.spout_html }} does not support all the possible formatting options yet. But you can find the most important ones: -| Category | Property | API -|:----------|:---------------|:-------------------------------------- -| Font | Bold | `StyleBuilder::setFontBold()` -| | Italic | `StyleBuilder::setFontItalic()` -| | Underline | `StyleBuilder::setFontUnderline()` -| | Strikethrough | `StyleBuilder::setFontStrikethrough()` -| | Font name | `StyleBuilder::setFontName('Arial')` -| | Font size | `StyleBuilder::setFontSize(14)` -| | Font color | `StyleBuilder::setFontColor(Color::BLUE)`
`StyleBuilder::setFontColor(Color::rgb(0, 128, 255))` -| Alignment | Cell alignment | `StyleBuilder::setCellAlignment(CellAlignment::CENTER)` -| | Wrap text | `StyleBuilder::setShouldWrapText(true)` - +| Category | Property | API +|:---------------------|:---------------|:-------------------------------------- +| Font | Bold | `StyleBuilder::setFontBold()` +| | Italic | `StyleBuilder::setFontItalic()` +| | Underline | `StyleBuilder::setFontUnderline()` +| | Strikethrough | `StyleBuilder::setFontStrikethrough()` +| | Font name | `StyleBuilder::setFontName('Arial')` +| | Font size | `StyleBuilder::setFontSize(14)` +| | Font color | `StyleBuilder::setFontColor(Color::BLUE)`
`StyleBuilder::setFontColor(Color::rgb(0, 128, 255))` +| Alignment | Cell alignment | `StyleBuilder::setCellAlignment(CellAlignment::CENTER)` +| | Wrap text | `StyleBuilder::setShouldWrapText(true)` +| Format _(XLSX only)_ | Number format | `StyleBuilder::setFormat('0.000')` +| | Date format | `StyleBuilder::setFormat('m/d/yy h:mm')` ### Styling rows diff --git a/docs/_pages/getting-started.md b/docs/_pages/getting-started.md index 0004d4d..0e0b336 100755 --- a/docs/_pages/getting-started.md +++ b/docs/_pages/getting-started.md @@ -10,7 +10,7 @@ This guide will help you install {{ site.spout_html }} and teach you how to use ## Requirements -* PHP version 7.1 or higher +* PHP version 7.2 or higher * PHP extension `ext-zip` enabled * PHP extension `ext-xmlreader` enabled diff --git a/docs/_pages/guides/4-symfony-stream-content-large-spreadsheet.md b/docs/_pages/guides/4-symfony-stream-content-large-spreadsheet.md index fb203ae..165a4b3 100644 --- a/docs/_pages/guides/4-symfony-stream-content-large-spreadsheet.md +++ b/docs/_pages/guides/4-symfony-stream-content-large-spreadsheet.md @@ -94,7 +94,7 @@ class MyStreamController extends Controller $i++; // Flushing the buffer every N rows to stream echo'ed content. - if ($i % FLUSH_THRESHOLD === 0) { + if ($i % self::FLUSH_THRESHOLD === 0) { flush(); } } diff --git a/phpunit.xml b/phpunit.xml index 4c56189..4e410e2 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -1,6 +1,6 @@ shouldApplyBorder = true; $this->border = $border; + $this->isEmpty = false; return $this; } @@ -147,6 +154,7 @@ class Style $this->fontBold = true; $this->hasSetFontBold = true; $this->shouldApplyFont = true; + $this->isEmpty = false; return $this; } @@ -175,6 +183,7 @@ class Style $this->fontItalic = true; $this->hasSetFontItalic = true; $this->shouldApplyFont = true; + $this->isEmpty = false; return $this; } @@ -203,6 +212,7 @@ class Style $this->fontUnderline = true; $this->hasSetFontUnderline = true; $this->shouldApplyFont = true; + $this->isEmpty = false; return $this; } @@ -231,6 +241,7 @@ class Style $this->fontStrikethrough = true; $this->hasSetFontStrikethrough = true; $this->shouldApplyFont = true; + $this->isEmpty = false; return $this; } @@ -260,6 +271,7 @@ class Style $this->fontSize = $fontSize; $this->hasSetFontSize = true; $this->shouldApplyFont = true; + $this->isEmpty = false; return $this; } @@ -291,6 +303,7 @@ class Style $this->fontColor = $fontColor; $this->hasSetFontColor = true; $this->shouldApplyFont = true; + $this->isEmpty = false; return $this; } @@ -320,6 +333,7 @@ class Style $this->fontName = $fontName; $this->hasSetFontName = true; $this->shouldApplyFont = true; + $this->isEmpty = false; return $this; } @@ -350,6 +364,7 @@ class Style $this->cellAlignment = $cellAlignment; $this->hasSetCellAlignment = true; $this->shouldApplyCellAlignment = true; + $this->isEmpty = false; return $this; } @@ -386,6 +401,7 @@ class Style { $this->shouldWrapText = $shouldWrap; $this->hasSetWrapText = true; + $this->isEmpty = false; return $this; } @@ -415,6 +431,7 @@ class Style { $this->hasSetBackgroundColor = true; $this->backgroundColor = $color; + $this->isEmpty = false; return $this; } @@ -444,6 +461,7 @@ class Style { $this->hasSetFormat = true; $this->format = $format; + $this->isEmpty = false; return $this; } @@ -463,4 +481,29 @@ class Style { return $this->hasSetFormat; } + + /** + * @return bool + */ + public function isRegistered() : bool + { + return $this->isRegistered; + } + + public function markAsRegistered(?int $id) : void + { + $this->setId($id); + $this->isRegistered = true; + } + + public function unmarkAsRegistered() : void + { + $this->setId(0); + $this->isRegistered = false; + } + + public function isEmpty() : bool + { + return $this->isEmpty; + } } diff --git a/src/Spout/Common/Helper/StringHelper.php b/src/Spout/Common/Helper/StringHelper.php index 0906132..6256b1e 100644 --- a/src/Spout/Common/Helper/StringHelper.php +++ b/src/Spout/Common/Helper/StringHelper.php @@ -13,12 +13,20 @@ class StringHelper /** @var bool Whether the mbstring extension is loaded */ protected $hasMbstringSupport; + /** @var bool Whether the code is running with PHP7 or older versions */ + private $isRunningPhp7OrOlder; + + /** @var array Locale info, used for number formatting */ + private $localeInfo; + /** * */ public function __construct() { $this->hasMbstringSupport = \extension_loaded('mbstring'); + $this->isRunningPhp7OrOlder = \version_compare(PHP_VERSION, '8.0.0') < 0; + $this->localeInfo = \localeconv(); } /** @@ -68,4 +76,30 @@ class StringHelper return ($position !== false) ? $position : -1; } + + /** + * Formats a numeric value (int or float) in a way that's compatible with the expected spreadsheet format. + * + * Formatting of float values is locale dependent in PHP < 8. + * Thousands separators and decimal points vary from locale to locale (en_US: 12.34 vs pl_PL: 12,34). + * However, float values must be formatted with no thousands separator and a "." as decimal point + * to work properly. This method can be used to convert the value to the correct format before storing it. + * + * @see https://wiki.php.net/rfc/locale_independent_float_to_string for the changed behavior in PHP8. + * + * @param int|float $numericValue + * @return string + */ + public function formatNumericValue($numericValue) + { + if ($this->isRunningPhp7OrOlder && is_float($numericValue)) { + return str_replace( + [$this->localeInfo['thousands_sep'], $this->localeInfo['decimal_point']], + ['', '.'], + $numericValue + ); + } + + return $numericValue; + } } diff --git a/src/Spout/Reader/Common/Manager/RowManager.php b/src/Spout/Reader/Common/Manager/RowManager.php index 623c8d8..67c8fb1 100644 --- a/src/Spout/Reader/Common/Manager/RowManager.php +++ b/src/Spout/Reader/Common/Manager/RowManager.php @@ -56,12 +56,25 @@ class RowManager $rowCells = $row->getCells(); $maxCellIndex = $numCells; + // If the row has empty cells, calling "setCellAtIndex" will add the cell + // but in the wrong place (the new cell is added at the end of the array). + // Therefore, we need to sort the array using keys to have proper order. + // @see https://github.com/box/spout/issues/740 + $needsSorting = false; + for ($cellIndex = 0; $cellIndex < $maxCellIndex; $cellIndex++) { if (!isset($rowCells[$cellIndex])) { $row->setCellAtIndex($this->entityFactory->createCell(''), $cellIndex); + $needsSorting = true; } } + if ($needsSorting) { + $rowCells = $row->getCells(); + ksort($rowCells); + $row->setCells($rowCells); + } + return $row; } } diff --git a/src/Spout/Reader/XLSX/Helper/CellValueFormatter.php b/src/Spout/Reader/XLSX/Helper/CellValueFormatter.php index 11bc909..49bc72c 100644 --- a/src/Spout/Reader/XLSX/Helper/CellValueFormatter.php +++ b/src/Spout/Reader/XLSX/Helper/CellValueFormatter.php @@ -31,14 +31,6 @@ class CellValueFormatter /** Constants used for date formatting */ const NUM_SECONDS_IN_ONE_DAY = 86400; - const NUM_SECONDS_IN_ONE_HOUR = 3600; - const NUM_SECONDS_IN_ONE_MINUTE = 60; - - /** - * February 29th, 1900 is NOT a leap year but Excel thinks it is... - * @see https://en.wikipedia.org/wiki/Year_1900_problem#Microsoft_Excel - */ - const ERRONEOUS_EXCEL_LEAP_YEAR_DAY = 60; /** @var SharedStringsManager Manages shared strings */ protected $sharedStringsManager; @@ -130,10 +122,15 @@ class CellValueFormatter */ protected function formatInlineStringCellValue($node) { - // inline strings are formatted this way: - // [INLINE_STRING] - $tNode = $node->getElementsByTagName(self::XML_NODE_INLINE_STRING_VALUE)->item(0); - $cellValue = $this->escaper->unescape($tNode->nodeValue); + // inline strings are formatted this way (they can contain any number of nodes): + // [INLINE_STRING][INLINE_STRING_2] + $tNodes = $node->getElementsByTagName(self::XML_NODE_INLINE_STRING_VALUE); + + $cellValue = ''; + for ($i = 0; $i < $tNodes->count(); $i++) { + $tNode = $tNodes->item($i); + $cellValue .= $this->escaper->unescape($tNode->nodeValue); + } return $cellValue; } diff --git a/src/Spout/Reader/XLSX/Manager/SharedStringsManager.php b/src/Spout/Reader/XLSX/Manager/SharedStringsManager.php index 23070cc..0da6e54 100644 --- a/src/Spout/Reader/XLSX/Manager/SharedStringsManager.php +++ b/src/Spout/Reader/XLSX/Manager/SharedStringsManager.php @@ -16,9 +16,6 @@ use Box\Spout\Reader\XLSX\Manager\SharedStringsCaching\CachingStrategyInterface; */ class SharedStringsManager { - /** Main namespace for the sharedStrings.xml file */ - const MAIN_NAMESPACE_FOR_SHARED_STRINGS_XML = 'http://schemas.openxmlformats.org/spreadsheetml/2006/main'; - /** Definition of XML nodes names used to parse data */ const XML_NODE_SST = 'sst'; const XML_NODE_SI = 'si'; diff --git a/src/Spout/Reader/XLSX/Manager/WorkbookRelationshipsManager.php b/src/Spout/Reader/XLSX/Manager/WorkbookRelationshipsManager.php index ec77031..f9fb82c 100644 --- a/src/Spout/Reader/XLSX/Manager/WorkbookRelationshipsManager.php +++ b/src/Spout/Reader/XLSX/Manager/WorkbookRelationshipsManager.php @@ -17,10 +17,11 @@ class WorkbookRelationshipsManager /** Path of workbook relationships XML file inside the XLSX file */ const WORKBOOK_RELS_XML_FILE_PATH = 'xl/_rels/workbook.xml.rels'; - /** Relationships types */ + /** Relationships types - For Transitional and Strict OOXML */ const RELATIONSHIP_TYPE_SHARED_STRINGS = 'http://schemas.openxmlformats.org/officeDocument/2006/relationships/sharedStrings'; const RELATIONSHIP_TYPE_STYLES = 'http://schemas.openxmlformats.org/officeDocument/2006/relationships/styles'; - const RELATIONSHIP_TYPE_WORKSHEET = 'http://schemas.openxmlformats.org/officeDocument/2006/relationships/worksheet'; + const RELATIONSHIP_TYPE_SHARED_STRINGS_STRICT = 'http://purl.oclc.org/ooxml/officeDocument/relationships/sharedStrings'; + const RELATIONSHIP_TYPE_STYLES_STRICT = 'http://purl.oclc.org/ooxml/officeDocument/relationships/styles'; /** Nodes and attributes used to find relevant information in the workbook relationships XML file */ const XML_NODE_RELATIONSHIP = 'Relationship'; @@ -52,7 +53,8 @@ class WorkbookRelationshipsManager public function getSharedStringsXMLFilePath() { $workbookRelationships = $this->getWorkbookRelationships(); - $sharedStringsXMLFilePath = $workbookRelationships[self::RELATIONSHIP_TYPE_SHARED_STRINGS]; + $sharedStringsXMLFilePath = $workbookRelationships[self::RELATIONSHIP_TYPE_SHARED_STRINGS] + ?? $workbookRelationships[self::RELATIONSHIP_TYPE_SHARED_STRINGS_STRICT]; // the file path can be relative (e.g. "styles.xml") or absolute (e.g. "/xl/styles.xml") $doesContainBasePath = (\strpos($sharedStringsXMLFilePath, self::BASE_PATH) !== false); @@ -71,7 +73,8 @@ class WorkbookRelationshipsManager { $workbookRelationships = $this->getWorkbookRelationships(); - return isset($workbookRelationships[self::RELATIONSHIP_TYPE_SHARED_STRINGS]); + return isset($workbookRelationships[self::RELATIONSHIP_TYPE_SHARED_STRINGS]) + || isset($workbookRelationships[self::RELATIONSHIP_TYPE_SHARED_STRINGS_STRICT]); } /** @@ -81,7 +84,8 @@ class WorkbookRelationshipsManager { $workbookRelationships = $this->getWorkbookRelationships(); - return isset($workbookRelationships[self::RELATIONSHIP_TYPE_STYLES]); + return isset($workbookRelationships[self::RELATIONSHIP_TYPE_STYLES]) + || isset($workbookRelationships[self::RELATIONSHIP_TYPE_STYLES_STRICT]); } /** @@ -90,7 +94,8 @@ class WorkbookRelationshipsManager public function getStylesXMLFilePath() { $workbookRelationships = $this->getWorkbookRelationships(); - $stylesXMLFilePath = $workbookRelationships[self::RELATIONSHIP_TYPE_STYLES]; + $stylesXMLFilePath = $workbookRelationships[self::RELATIONSHIP_TYPE_STYLES] + ?? $workbookRelationships[self::RELATIONSHIP_TYPE_STYLES_STRICT]; // the file path can be relative (e.g. "styles.xml") or absolute (e.g. "/xl/styles.xml") $doesContainBasePath = (\strpos($stylesXMLFilePath, self::BASE_PATH) !== false); diff --git a/src/Spout/Writer/Common/Manager/RegisteredStyle.php b/src/Spout/Writer/Common/Manager/RegisteredStyle.php new file mode 100644 index 0000000..734c2b6 --- /dev/null +++ b/src/Spout/Writer/Common/Manager/RegisteredStyle.php @@ -0,0 +1,38 @@ +style = $style; + $this->isMatchingRowStyle = $isMatchingRowStyle; + } + + public function getStyle() : Style + { + return $this->style; + } + + public function isMatchingRowStyle() : bool + { + return $this->isMatchingRowStyle; + } +} diff --git a/src/Spout/Writer/Common/Manager/Style/PossiblyUpdatedStyle.php b/src/Spout/Writer/Common/Manager/Style/PossiblyUpdatedStyle.php new file mode 100644 index 0000000..6ccaa29 --- /dev/null +++ b/src/Spout/Writer/Common/Manager/Style/PossiblyUpdatedStyle.php @@ -0,0 +1,32 @@ +style = $style; + $this->isUpdated = $isUpdated; + } + + public function getStyle() : Style + { + return $this->style; + } + + public function isUpdated() : bool + { + return $this->isUpdated; + } +} diff --git a/src/Spout/Writer/Common/Manager/Style/StyleManager.php b/src/Spout/Writer/Common/Manager/Style/StyleManager.php index 77eec73..e2b5ebd 100644 --- a/src/Spout/Writer/Common/Manager/Style/StyleManager.php +++ b/src/Spout/Writer/Common/Manager/Style/StyleManager.php @@ -50,13 +50,11 @@ class StyleManager implements StyleManagerInterface * Typically, set "wrap text" if a cell contains a new line. * * @param Cell $cell - * @return Style + * @return PossiblyUpdatedStyle The eventually updated style */ - public function applyExtraStylesIfNeeded(Cell $cell) + public function applyExtraStylesIfNeeded(Cell $cell) : PossiblyUpdatedStyle { - $updatedStyle = $this->applyWrapTextIfCellContainsNewLine($cell); - - return $updatedStyle; + return $this->applyWrapTextIfCellContainsNewLine($cell); } /** @@ -69,21 +67,19 @@ class StyleManager implements StyleManagerInterface * on the Windows version of Excel... * * @param Cell $cell The cell the style should be applied to - * @return \Box\Spout\Common\Entity\Style\Style The eventually updated style + * @return PossiblyUpdatedStyle The eventually updated style */ - protected function applyWrapTextIfCellContainsNewLine(Cell $cell) + protected function applyWrapTextIfCellContainsNewLine(Cell $cell) : PossiblyUpdatedStyle { $cellStyle = $cell->getStyle(); // if the "wrap text" option is already set, no-op - if ($cellStyle->hasSetWrapText()) { - return $cellStyle; - } - - if ($cell->isString() && \strpos($cell->getValue(), "\n") !== false) { + if (!$cellStyle->hasSetWrapText() && $cell->isString() && \strpos($cell->getValue(), "\n") !== false) { $cellStyle->setShouldWrapText(); + + return new PossiblyUpdatedStyle($cellStyle, true); } - return $cellStyle; + return new PossiblyUpdatedStyle($cellStyle, false); } } diff --git a/src/Spout/Writer/Common/Manager/Style/StyleManagerInterface.php b/src/Spout/Writer/Common/Manager/Style/StyleManagerInterface.php index 312588e..6b320b1 100644 --- a/src/Spout/Writer/Common/Manager/Style/StyleManagerInterface.php +++ b/src/Spout/Writer/Common/Manager/Style/StyleManagerInterface.php @@ -24,7 +24,7 @@ interface StyleManagerInterface * Typically, set "wrap text" if a cell contains a new line. * * @param Cell $cell - * @return Style The updated style + * @return PossiblyUpdatedStyle The eventually updated style */ - public function applyExtraStylesIfNeeded(Cell $cell); + public function applyExtraStylesIfNeeded(Cell $cell) : PossiblyUpdatedStyle; } diff --git a/src/Spout/Writer/Common/Manager/Style/StyleRegistry.php b/src/Spout/Writer/Common/Manager/Style/StyleRegistry.php index b8bb608..5a7e5c1 100644 --- a/src/Spout/Writer/Common/Manager/Style/StyleRegistry.php +++ b/src/Spout/Writer/Common/Manager/Style/StyleRegistry.php @@ -36,9 +36,9 @@ class StyleRegistry { $serializedStyle = $this->serialize($style); - if (!$this->hasStyleAlreadyBeenRegistered($style)) { + if (!$this->hasSerializedStyleAlreadyBeenRegistered($serializedStyle)) { $nextStyleId = \count($this->serializedStyleToStyleIdMappingTable); - $style->setId($nextStyleId); + $style->markAsRegistered($nextStyleId); $this->serializedStyleToStyleIdMappingTable[$serializedStyle] = $nextStyleId; $this->styleIdToStyleMappingTable[$nextStyleId] = $style; @@ -48,15 +48,13 @@ class StyleRegistry } /** - * Returns whether the given style has already been registered. + * Returns whether the serialized style has already been registered. * - * @param Style $style + * @param string $serializedStyle The serialized style * @return bool */ - protected function hasStyleAlreadyBeenRegistered(Style $style) + protected function hasSerializedStyleAlreadyBeenRegistered(string $serializedStyle) { - $serializedStyle = $this->serialize($style); - // Using isset here because it is way faster than array_key_exists... return isset($this->serializedStyleToStyleIdMappingTable[$serializedStyle]); } @@ -101,13 +99,13 @@ class StyleRegistry */ public function serialize(Style $style) { - // In order to be able to properly compare style, set static ID value + // In order to be able to properly compare style, set static ID value and reset registration $currentId = $style->getId(); - $style->setId(0); + $style->unmarkAsRegistered(); $serializedStyle = \serialize($style); - $style->setId($currentId); + $style->markAsRegistered($currentId); return $serializedStyle; } diff --git a/src/Spout/Writer/ODS/Manager/Style/StyleRegistry.php b/src/Spout/Writer/ODS/Manager/Style/StyleRegistry.php index 815caa2..47ca61c 100644 --- a/src/Spout/Writer/ODS/Manager/Style/StyleRegistry.php +++ b/src/Spout/Writer/ODS/Manager/Style/StyleRegistry.php @@ -22,6 +22,10 @@ class StyleRegistry extends \Box\Spout\Writer\Common\Manager\Style\StyleRegistry */ public function registerStyle(Style $style) { + if ($style->isRegistered()) { + return $style; + } + $registeredStyle = parent::registerStyle($style); $this->usedFontsSet[$style->getFontName()] = true; diff --git a/src/Spout/Writer/ODS/Manager/WorksheetManager.php b/src/Spout/Writer/ODS/Manager/WorksheetManager.php index 96ed6ee..11b396e 100644 --- a/src/Spout/Writer/ODS/Manager/WorksheetManager.php +++ b/src/Spout/Writer/ODS/Manager/WorksheetManager.php @@ -10,6 +10,7 @@ use Box\Spout\Common\Exception\IOException; use Box\Spout\Common\Helper\Escaper\ODS as ODSEscaper; use Box\Spout\Common\Helper\StringHelper; use Box\Spout\Writer\Common\Entity\Worksheet; +use Box\Spout\Writer\Common\Manager\RegisteredStyle; use Box\Spout\Writer\Common\Manager\Style\StyleMerger; use Box\Spout\Writer\Common\Manager\WorksheetManagerInterface; use Box\Spout\Writer\ODS\Manager\Style\StyleManager; @@ -94,7 +95,7 @@ class WorksheetManager implements WorksheetManagerInterface $escapedSheetName = $this->stringsEscaper->escape($externalSheet->getName()); $tableStyleName = 'ta' . ($externalSheet->getIndex() + 1); - $tableElement = ''; + $tableElement = ''; $tableElement .= ''; return $tableElement; @@ -105,8 +106,8 @@ class WorksheetManager implements WorksheetManagerInterface * * @param Worksheet $worksheet The worksheet to add the row to * @param Row $row The row to be added - * @throws IOException If the data cannot be written * @throws InvalidArgumentException If a cell value's type is not supported + * @throws IOException If the data cannot be written * @return void */ public function addRow(Worksheet $worksheet, Row $row) @@ -126,7 +127,13 @@ class WorksheetManager implements WorksheetManagerInterface $nextCell = isset($cells[$nextCellIndex]) ? $cells[$nextCellIndex] : null; if ($nextCell === null || $cell->getValue() !== $nextCell->getValue()) { - $data .= $this->applyStyleAndGetCellXML($cell, $rowStyle, $currentCellIndex, $nextCellIndex); + $registeredStyle = $this->applyStyleAndRegister($cell, $rowStyle); + $cellStyle = $registeredStyle->getStyle(); + if ($registeredStyle->isMatchingRowStyle()) { + $rowStyle = $cellStyle; // Replace actual rowStyle (possibly with null id) by registered style (with id) + } + + $data .= $this->getCellXMLWithStyle($cell, $cellStyle, $currentCellIndex, $nextCellIndex); $currentCellIndex = $nextCellIndex; } @@ -147,24 +154,46 @@ class WorksheetManager implements WorksheetManagerInterface /** * Applies styles to the given style, merging the cell's style with its row's style - * Then builds and returns xml for the cell. * * @param Cell $cell * @param Style $rowStyle - * @param int $currentCellIndex - * @param int $nextCellIndex * @throws InvalidArgumentException If a cell value's type is not supported - * @return string + * @return RegisteredStyle */ - private function applyStyleAndGetCellXML(Cell $cell, Style $rowStyle, $currentCellIndex, $nextCellIndex) + private function applyStyleAndRegister(Cell $cell, Style $rowStyle) : RegisteredStyle { - // Apply row and extra styles - $mergedCellAndRowStyle = $this->styleMerger->merge($cell->getStyle(), $rowStyle); - $cell->setStyle($mergedCellAndRowStyle); - $newCellStyle = $this->styleManager->applyExtraStylesIfNeeded($cell); + $isMatchingRowStyle = false; + if ($cell->getStyle()->isEmpty()) { + $cell->setStyle($rowStyle); - $registeredStyle = $this->styleManager->registerStyle($newCellStyle); - $styleIndex = $registeredStyle->getId() + 1; // 1-based + $possiblyUpdatedStyle = $this->styleManager->applyExtraStylesIfNeeded($cell); + + if ($possiblyUpdatedStyle->isUpdated()) { + $registeredStyle = $this->styleManager->registerStyle($possiblyUpdatedStyle->getStyle()); + } else { + $registeredStyle = $this->styleManager->registerStyle($rowStyle); + $isMatchingRowStyle = true; + } + } else { + $mergedCellAndRowStyle = $this->styleMerger->merge($cell->getStyle(), $rowStyle); + $cell->setStyle($mergedCellAndRowStyle); + + $possiblyUpdatedStyle = $this->styleManager->applyExtraStylesIfNeeded($cell); + if ($possiblyUpdatedStyle->isUpdated()) { + $newCellStyle = $possiblyUpdatedStyle->getStyle(); + } else { + $newCellStyle = $mergedCellAndRowStyle; + } + + $registeredStyle = $this->styleManager->registerStyle($newCellStyle); + } + + return new RegisteredStyle($registeredStyle, $isMatchingRowStyle); + } + + private function getCellXMLWithStyle(Cell $cell, Style $style, int $currentCellIndex, int $nextCellIndex) : string + { + $styleIndex = $style->getId() + 1; // 1-based $numTimesValueRepeated = ($nextCellIndex - $currentCellIndex); @@ -198,12 +227,14 @@ class WorksheetManager implements WorksheetManagerInterface $data .= ''; } elseif ($cell->isBoolean()) { - $data .= ' office:value-type="boolean" calcext:value-type="boolean" office:boolean-value="' . $cell->getValue() . '">'; + $value = $cell->getValue() ? 'true' : 'false'; // boolean-value spec: http://docs.oasis-open.org/office/v1.2/os/OpenDocument-v1.2-os-part1.html#datatype-boolean + $data .= ' office:value-type="boolean" calcext:value-type="boolean" office:boolean-value="' . $value . '">'; $data .= '' . $cell->getValue() . ''; $data .= ''; } elseif ($cell->isNumeric()) { - $data .= ' office:value-type="float" calcext:value-type="float" office:value="' . $cell->getValue() . '">'; - $data .= '' . $cell->getValue() . ''; + $cellValue = $this->stringHelper->formatNumericValue($cell->getValue()); + $data .= ' office:value-type="float" calcext:value-type="float" office:value="' . $cellValue . '">'; + $data .= '' . $cellValue . ''; $data .= ''; } elseif ($cell->isError() && is_string($cell->getValueEvenIfError())) { // only writes the error value if it's a string diff --git a/src/Spout/Writer/WriterAbstract.php b/src/Spout/Writer/WriterAbstract.php index 84396f9..104539f 100644 --- a/src/Spout/Writer/WriterAbstract.php +++ b/src/Spout/Writer/WriterAbstract.php @@ -128,9 +128,26 @@ abstract class WriterAbstract implements WriterInterface // @see https://github.com/box/spout/issues/241 $this->globalFunctionsHelper->ob_end_clean(); - // Set headers + /* + * Set headers + * + * For newer browsers such as Firefox, Chrome, Opera, Safari, etc., they all support and use `filename*` + * specified by the new standard, even if they do not automatically decode filename; it does not matter; + * and for older versions of Internet Explorer, they are not recognized `filename*`, will automatically + * ignore it and use the old `filename` (the only minor flaw is that there must be an English suffix name). + * In this way, the multi-browser multi-language compatibility problem is perfectly solved, which does not + * require UA judgment and is more in line with the standard. + * + * @see https://github.com/box/spout/issues/745 + * @see https://tools.ietf.org/html/rfc6266 + * @see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Disposition + */ $this->globalFunctionsHelper->header('Content-Type: ' . static::$headerContentType); - $this->globalFunctionsHelper->header('Content-Disposition: attachment; filename="' . $this->outputFilePath . '"'); + $this->globalFunctionsHelper->header( + 'Content-Disposition: attachment; ' . + 'filename="' . rawurldecode($this->outputFilePath) . '"; ' . + 'filename*=UTF-8\'\'' . rawurldecode($this->outputFilePath) + ); /* * When forcing the download of a file over SSL,IE8 and lower browsers fail diff --git a/src/Spout/Writer/XLSX/Manager/Style/StyleRegistry.php b/src/Spout/Writer/XLSX/Manager/Style/StyleRegistry.php index 21fda08..d18d8fd 100644 --- a/src/Spout/Writer/XLSX/Manager/Style/StyleRegistry.php +++ b/src/Spout/Writer/XLSX/Manager/Style/StyleRegistry.php @@ -119,6 +119,10 @@ class StyleRegistry extends \Box\Spout\Writer\Common\Manager\Style\StyleRegistry */ public function registerStyle(Style $style) { + if ($style->isRegistered()) { + return $style; + } + $registeredStyle = parent::registerStyle($style); $this->registerFill($registeredStyle); $this->registerFormat($registeredStyle); diff --git a/src/Spout/Writer/XLSX/Manager/WorksheetManager.php b/src/Spout/Writer/XLSX/Manager/WorksheetManager.php index 48c748f..14ffecb 100644 --- a/src/Spout/Writer/XLSX/Manager/WorksheetManager.php +++ b/src/Spout/Writer/XLSX/Manager/WorksheetManager.php @@ -14,6 +14,7 @@ use Box\Spout\Writer\Common\Creator\InternalEntityFactory; use Box\Spout\Writer\Common\Entity\Options; use Box\Spout\Writer\Common\Entity\Worksheet; use Box\Spout\Writer\Common\Helper\CellHelper; +use Box\Spout\Writer\Common\Manager\RegisteredStyle; use Box\Spout\Writer\Common\Manager\RowManager; use Box\Spout\Writer\Common\Manager\Style\StyleMerger; use Box\Spout\Writer\Common\Manager\WorksheetManagerInterface; @@ -161,7 +162,12 @@ EOD; $rowXML = ''; foreach ($row->getCells() as $columnIndexZeroBased => $cell) { - $rowXML .= $this->applyStyleAndGetCellXML($cell, $rowStyle, $rowIndexOneBased, $columnIndexZeroBased); + $registeredStyle = $this->applyStyleAndRegister($cell, $rowStyle); + $cellStyle = $registeredStyle->getStyle(); + if ($registeredStyle->isMatchingRowStyle()) { + $rowStyle = $cellStyle; // Replace actual rowStyle (possibly with null id) by registered style (with id) + } + $rowXML .= $this->getCellXML($rowIndexOneBased, $columnIndexZeroBased, $cell, $cellStyle->getId()); } $rowXML .= ''; @@ -174,26 +180,43 @@ EOD; /** * Applies styles to the given style, merging the cell's style with its row's style - * Then builds and returns xml for the cell. * * @param Cell $cell * @param Style $rowStyle - * @param int $rowIndexOneBased - * @param int $columnIndexZeroBased * * @throws InvalidArgumentException If the given value cannot be processed - * @return string + * @return RegisteredStyle */ - private function applyStyleAndGetCellXML(Cell $cell, Style $rowStyle, $rowIndexOneBased, $columnIndexZeroBased) + private function applyStyleAndRegister(Cell $cell, Style $rowStyle) : RegisteredStyle { - // Apply row and extra styles - $mergedCellAndRowStyle = $this->styleMerger->merge($cell->getStyle(), $rowStyle); - $cell->setStyle($mergedCellAndRowStyle); - $newCellStyle = $this->styleManager->applyExtraStylesIfNeeded($cell); + $isMatchingRowStyle = false; + if ($cell->getStyle()->isEmpty()) { + $cell->setStyle($rowStyle); - $registeredStyle = $this->styleManager->registerStyle($newCellStyle); + $possiblyUpdatedStyle = $this->styleManager->applyExtraStylesIfNeeded($cell); - return $this->getCellXML($rowIndexOneBased, $columnIndexZeroBased, $cell, $registeredStyle->getId()); + if ($possiblyUpdatedStyle->isUpdated()) { + $registeredStyle = $this->styleManager->registerStyle($possiblyUpdatedStyle->getStyle()); + } else { + $registeredStyle = $this->styleManager->registerStyle($rowStyle); + $isMatchingRowStyle = true; + } + } else { + $mergedCellAndRowStyle = $this->styleMerger->merge($cell->getStyle(), $rowStyle); + $cell->setStyle($mergedCellAndRowStyle); + + $possiblyUpdatedStyle = $this->styleManager->applyExtraStylesIfNeeded($cell); + + if ($possiblyUpdatedStyle->isUpdated()) { + $newCellStyle = $possiblyUpdatedStyle->getStyle(); + } else { + $newCellStyle = $mergedCellAndRowStyle; + } + + $registeredStyle = $this->styleManager->registerStyle($newCellStyle); + } + + return new RegisteredStyle($registeredStyle, $isMatchingRowStyle); } /** @@ -218,7 +241,7 @@ EOD; } elseif ($cell->isBoolean()) { $cellXML .= ' t="b">' . (int) ($cell->getValue()) . ''; } elseif ($cell->isNumeric()) { - $cellXML .= '>' . $cell->getValue() . ''; + $cellXML .= '>' . $this->stringHelper->formatNumericValue($cell->getValue()) . ''; } elseif ($cell->isError() && is_string($cell->getValueEvenIfError())) { // only writes the error value if it's a string $cellXML .= ' t="e">' . $cell->getValueEvenIfError() . ''; diff --git a/tests/Spout/Common/Entity/RowTest.php b/tests/Spout/Common/Entity/RowTest.php index 4c121cf..d734bca 100644 --- a/tests/Spout/Common/Entity/RowTest.php +++ b/tests/Spout/Common/Entity/RowTest.php @@ -129,6 +129,6 @@ class RowTest extends \PHPUnit\Framework\TestCase ->setStyle($this->getStyleMock()) ->setCells([]); - $this->assertInternalType('object', $row); + $this->assertInstanceOf(Row::class, $row); } } diff --git a/tests/Spout/Common/Helper/FileSystemHelperTest.php b/tests/Spout/Common/Helper/FileSystemHelperTest.php index 0c34782..0eb314e 100644 --- a/tests/Spout/Common/Helper/FileSystemHelperTest.php +++ b/tests/Spout/Common/Helper/FileSystemHelperTest.php @@ -16,7 +16,7 @@ class FileSystemHelperTest extends TestCase /** * @return void */ - public function setUp() + public function setUp() : void { $baseFolder = \sys_get_temp_dir(); $this->fileSystemHelper = new FileSystemHelper($baseFolder); diff --git a/tests/Spout/Reader/ODS/ReaderTest.php b/tests/Spout/Reader/ODS/ReaderTest.php index f007487..3bdf70d 100644 --- a/tests/Spout/Reader/ODS/ReaderTest.php +++ b/tests/Spout/Reader/ODS/ReaderTest.php @@ -295,6 +295,10 @@ class ReaderTest extends TestCase */ public function testReadShouldBeProtectedAgainstBillionLaughsAttack() { + if (function_exists('xdebug_code_coverage_started') && xdebug_code_coverage_started()) { + $this->markTestSkipped('test not compatible with code coverage'); + } + $startTime = microtime(true); $fileName = 'attack_billion_laughs.ods'; @@ -318,6 +322,10 @@ class ReaderTest extends TestCase */ public function testReadShouldBeProtectedAgainstQuadraticBlowupAttack() { + if (function_exists('xdebug_code_coverage_started') && xdebug_code_coverage_started()) { + $this->markTestSkipped('test not compatible with code coverage'); + } + $startTime = microtime(true); $fileName = 'attack_quadratic_blowup.ods'; diff --git a/tests/Spout/Reader/XLSX/Helper/CellValueFormatterTest.php b/tests/Spout/Reader/XLSX/Helper/CellValueFormatterTest.php index d6e9e8b..1e044d4 100644 --- a/tests/Spout/Reader/XLSX/Helper/CellValueFormatterTest.php +++ b/tests/Spout/Reader/XLSX/Helper/CellValueFormatterTest.php @@ -197,18 +197,22 @@ class CellValueFormatterTest extends TestCase public function testFormatInlineStringCellValue($value, $expectedFormattedValue) { $nodeListMock = $this->createMock(\DOMNodeList::class); + $nodeListMock + ->expects($this->atLeastOnce()) + ->method('count') + ->willReturn(1); $nodeListMock ->expects($this->atLeastOnce()) ->method('item') ->with(0) - ->will($this->returnValue((object) ['nodeValue' => $value])); + ->willReturn((object) ['nodeValue' => $value]); $nodeMock = $this->createMock(\DOMElement::class); $nodeMock ->expects($this->atLeastOnce()) ->method('getElementsByTagName') ->with(CellValueFormatter::XML_NODE_INLINE_STRING_VALUE) - ->will($this->returnValue($nodeListMock)); + ->willReturn($nodeListMock); /** @var SharedStringsManager $sharedStringManager */ $sharedStringManager = $this->createMock(SharedStringsManager::class); diff --git a/tests/Spout/Reader/XLSX/Manager/SharedStringsManagerTest.php b/tests/Spout/Reader/XLSX/Manager/SharedStringsManagerTest.php index 40e4855..7274904 100644 --- a/tests/Spout/Reader/XLSX/Manager/SharedStringsManagerTest.php +++ b/tests/Spout/Reader/XLSX/Manager/SharedStringsManagerTest.php @@ -25,7 +25,7 @@ class SharedStringsManagerTest extends TestCase /** * @return void */ - public function setUp() + public function setUp() : void { $this->sharedStringsManager = null; } @@ -33,7 +33,7 @@ class SharedStringsManagerTest extends TestCase /** * @return void */ - public function tearDown() + public function tearDown() : void { if ($this->sharedStringsManager !== null) { $this->sharedStringsManager->cleanup(); diff --git a/tests/Spout/Reader/XLSX/ReaderTest.php b/tests/Spout/Reader/XLSX/ReaderTest.php index ac268fa..8cda9de 100644 --- a/tests/Spout/Reader/XLSX/ReaderTest.php +++ b/tests/Spout/Reader/XLSX/ReaderTest.php @@ -72,6 +72,19 @@ class ReaderTest extends TestCase } } + /** + * @return void + */ + public function testReadShouldSupportInlineStringsWithMultipleValueNodes() + { + $allRows = $this->getAllRowsForFile('sheet_with_multiple_value_nodes_in_inline_strings.xlsx'); + + $expectedRows = [ + ['VALUE 1 VALUE 2 VALUE 3 VALUE 4', 's1 - B1'], + ]; + $this->assertEquals($expectedRows, $allRows); + } + /** * @return void */ @@ -539,6 +552,10 @@ class ReaderTest extends TestCase */ public function testReadShouldBeProtectedAgainstQuadraticBlowupAttack() { + if (function_exists('xdebug_code_coverage_started') && xdebug_code_coverage_started()) { + $this->markTestSkipped('test not compatible with code coverage'); + } + $startTime = microtime(true); $this->getAllRowsForFile('attack_quadratic_blowup.xlsx'); @@ -675,13 +692,29 @@ class ReaderTest extends TestCase $allRows = $this->getAllRowsForFile('sheet_with_empty_cells.xlsx'); $expectedRows = [ - ['A', 'B', 'C'], + ['A', '', 'C'], ['0', '', ''], ['1', '1', ''], ]; $this->assertEquals($expectedRows, $allRows, 'There should be 3 rows, with equal length'); } + /** + * https://github.com/box/spout/issues/184 + * @return void + */ + public function testReadShouldCreateOutputEmptyCellPreservedWhenNoDimensionsSpecified() + { + $allRows = $this->getAllRowsForFile('sheet_with_empty_cells_without_dimensions.xlsx'); + + $expectedRows = [ + ['A', '', 'C'], + ['0'], + ['1', '1'], + ]; + $this->assertEquals($expectedRows, $allRows); + } + /** * https://github.com/box/spout/issues/195 * @return void @@ -699,6 +732,18 @@ class ReaderTest extends TestCase $this->assertEquals($expectedRows, $allRows, 'Cell values should not be trimmed'); } + /** + * https://github.com/box/spout/issues/726 + * @return void + */ + public function testReaderShouldSupportStrictOOXML() + { + $allRows = $this->getAllRowsForFile('sheet_with_strict_ooxml.xlsx'); + + $this->assertEquals('UNIQUE_ACCOUNT_IDENTIFIER', $allRows[0][0]); + $this->assertEquals('A2Z34NJA7N2ESJ', $allRows[1][0]); + } + /** * @param string $fileName * @param bool $shouldFormatDates diff --git a/tests/Spout/Writer/CSV/WriterTest.php b/tests/Spout/Writer/CSV/WriterTest.php index 32b1b71..405208f 100644 --- a/tests/Spout/Writer/CSV/WriterTest.php +++ b/tests/Spout/Writer/CSV/WriterTest.php @@ -102,7 +102,7 @@ class WriterTest extends TestCase ]); $writtenContent = $this->writeToCsvFileAndReturnWrittenContent($allRows, 'csv_with_utf8_bom.csv'); - $this->assertContains(EncodingHelper::BOM_UTF8, $writtenContent, 'The CSV file should contain a UTF-8 BOM'); + $this->assertStringStartsWith(EncodingHelper::BOM_UTF8, $writtenContent, 'The CSV file should contain a UTF-8 BOM'); } /** @@ -115,7 +115,7 @@ class WriterTest extends TestCase ]); $writtenContent = $this->writeToCsvFileAndReturnWrittenContent($allRows, 'csv_no_bom.csv', ',', '"', false); - $this->assertNotContains(EncodingHelper::BOM_UTF8, $writtenContent, 'The CSV file should not contain a UTF-8 BOM'); + $this->assertStringNotContainsString(EncodingHelper::BOM_UTF8, $writtenContent, 'The CSV file should not contain a UTF-8 BOM'); } /** diff --git a/tests/Spout/Writer/Common/Entity/SheetTest.php b/tests/Spout/Writer/Common/Entity/SheetTest.php index ea83957..2326cb7 100644 --- a/tests/Spout/Writer/Common/Entity/SheetTest.php +++ b/tests/Spout/Writer/Common/Entity/SheetTest.php @@ -18,7 +18,7 @@ class SheetTest extends TestCase /** * @return void */ - public function setUp() + public function setUp() : void { $this->sheetManager = new SheetManager(new StringHelper()); } diff --git a/tests/Spout/Writer/Common/Manager/Style/StyleManagerTest.php b/tests/Spout/Writer/Common/Manager/Style/StyleManagerTest.php index 82292bb..5bbbe5c 100644 --- a/tests/Spout/Writer/Common/Manager/Style/StyleManagerTest.php +++ b/tests/Spout/Writer/Common/Manager/Style/StyleManagerTest.php @@ -14,7 +14,7 @@ class StyleManagerTest extends TestCase /** * @return StyleManager */ - private function getStyleManager() + private function getStyleManager() : StyleManager { $style = (new StyleBuilder())->build(); $styleRegistry = new StyleRegistry($style); @@ -22,31 +22,37 @@ class StyleManagerTest extends TestCase return new StyleManager($styleRegistry); } - /** - * @return void - */ - public function testApplyExtraStylesIfNeededShouldApplyWrapTextIfCellContainsNewLine() + public function testApplyExtraStylesIfNeededShouldApplyWrapTextIfCellContainsNewLine() : void { $style = (new StyleBuilder())->build(); $this->assertFalse($style->shouldWrapText()); $styleManager = $this->getStyleManager(); - $updatedStyle = $styleManager->applyExtraStylesIfNeeded(new Cell("multi\nlines", $style)); + $possiblyUpdatedStyle = $styleManager->applyExtraStylesIfNeeded(new Cell("multi\nlines", $style)); - $this->assertTrue($updatedStyle->shouldWrapText()); + $this->assertTrue($possiblyUpdatedStyle->isUpdated()); + $this->assertTrue($possiblyUpdatedStyle->getStyle()->shouldWrapText()); } - /** - * @return void - */ - public function testApplyExtraStylesIfNeededShouldDoNothingIfWrapTextAlreadyApplied() + public function testApplyExtraStylesIfNeededShouldReturnNullIfWrapTextNotNeeded() : void + { + $style = (new StyleBuilder())->build(); + $this->assertFalse($style->shouldWrapText()); + + $styleManager = $this->getStyleManager(); + $possiblyUpdatedStyle = $styleManager->applyExtraStylesIfNeeded(new Cell('oneline', $style)); + + $this->assertFalse($possiblyUpdatedStyle->isUpdated()); + } + + public function testApplyExtraStylesIfNeededShouldReturnNullIfWrapTextAlreadyApplied() : void { $style = (new StyleBuilder())->setShouldWrapText()->build(); $this->assertTrue($style->shouldWrapText()); $styleManager = $this->getStyleManager(); - $updatedStyle = $styleManager->applyExtraStylesIfNeeded(new Cell("multi\nlines", $style)); + $possiblyUpdatedStyle = $styleManager->applyExtraStylesIfNeeded(new Cell("multi\nlines", $style)); - $this->assertTrue($updatedStyle->shouldWrapText()); + $this->assertFalse($possiblyUpdatedStyle->isUpdated()); } } diff --git a/tests/Spout/Writer/Common/Manager/Style/StyleMergerTest.php b/tests/Spout/Writer/Common/Manager/Style/StyleMergerTest.php index 46820c0..b7c1607 100644 --- a/tests/Spout/Writer/Common/Manager/Style/StyleMergerTest.php +++ b/tests/Spout/Writer/Common/Manager/Style/StyleMergerTest.php @@ -18,7 +18,7 @@ class StyleMergerTest extends TestCase /** * @return void */ - public function setUp() + public function setUp() : void { $this->styleMerger = new StyleMerger(); } diff --git a/tests/Spout/Writer/Common/Manager/Style/StyleRegistryTest.php b/tests/Spout/Writer/Common/Manager/Style/StyleRegistryTest.php index 41cc571..ac78c9e 100644 --- a/tests/Spout/Writer/Common/Manager/Style/StyleRegistryTest.php +++ b/tests/Spout/Writer/Common/Manager/Style/StyleRegistryTest.php @@ -20,7 +20,7 @@ class StyleRegistryTest extends TestCase /** * @return void */ - public function setUp() + public function setUp() : void { $this->defaultStyle = (new StyleBuilder())->build(); $this->styleRegistry = new StyleRegistry($this->defaultStyle); diff --git a/tests/Spout/Writer/ODS/SheetTest.php b/tests/Spout/Writer/ODS/SheetTest.php index 6e46392..4797406 100644 --- a/tests/Spout/Writer/ODS/SheetTest.php +++ b/tests/Spout/Writer/ODS/SheetTest.php @@ -89,7 +89,7 @@ class SheetTest extends TestCase $pathToContentFile = $resourcePath . '#content.xml'; $xmlContents = file_get_contents('zip://' . $pathToContentFile); - $this->assertContains(' table:display="false"', $xmlContents, 'The sheet visibility should have been changed to "hidden"'); + $this->assertStringContainsString(' table:display="false"', $xmlContents, 'The sheet visibility should have been changed to "hidden"'); } /** @@ -164,6 +164,6 @@ class SheetTest extends TestCase $pathToWorkbookFile = $resourcePath . '#content.xml'; $xmlContents = file_get_contents('zip://' . $pathToWorkbookFile); - $this->assertContains("table:name=\"$expectedName\"", $xmlContents, $message); + $this->assertStringContainsString("table:name=\"$expectedName\"", $xmlContents, $message); } } diff --git a/tests/Spout/Writer/ODS/WriterTest.php b/tests/Spout/Writer/ODS/WriterTest.php index 215b701..4545960 100644 --- a/tests/Spout/Writer/ODS/WriterTest.php +++ b/tests/Spout/Writer/ODS/WriterTest.php @@ -279,6 +279,41 @@ class WriterTest extends TestCase $this->assertValueWasWrittenToSheet($fileName, 1, 10.2); } + /** + * @return void + */ + public function testAddRowShouldSupportFloatValuesInDifferentLocale() + { + $previousLocale = \setlocale(LC_ALL, 0); + + try { + // Pick a supported locale whose decimal point is a comma. + // Installed locales differ from one system to another, so we can't pick + // a given locale. + $supportedLocales = explode("\n", shell_exec('locale -a')); + foreach ($supportedLocales as $supportedLocale) { + \setlocale(LC_ALL, $supportedLocale); + if (\localeconv()['decimal_point'] === ',') { + break; + } + } + $this->assertEquals(',', \localeconv()['decimal_point']); + + $fileName = 'test_add_row_should_support_float_values_in_different_locale.xlsx'; + $dataRows = $this->createRowsFromValues([ + [1234.5], + ]); + + $this->writeToODSFile($dataRows, $fileName); + + $this->assertValueWasNotWrittenToSheet($fileName, 1, "1234,5"); + $this->assertValueWasWrittenToSheet($fileName, 1, "1234.5"); + } finally { + // reset locale + \setlocale(LC_ALL, $previousLocale); + } + } + /** * @return array */ @@ -548,7 +583,7 @@ class WriterTest extends TestCase $pathToContentFile = $resourcePath . '#content.xml'; $xmlContents = file_get_contents('zip://' . $pathToContentFile); - $this->assertContains($value, $xmlContents, $message); + $this->assertStringContainsString($value, $xmlContents, $message); } /** @@ -563,7 +598,7 @@ class WriterTest extends TestCase $sheetXmlAsString = $this->getSheetXmlNodeAsString($fileName, $sheetIndex); $valueAsXmlString = "$value"; - $this->assertContains($valueAsXmlString, $sheetXmlAsString, $message); + $this->assertStringContainsString($valueAsXmlString, $sheetXmlAsString, $message); } /** @@ -578,7 +613,7 @@ class WriterTest extends TestCase $sheetXmlAsString = $this->getSheetXmlNodeAsString($fileName, $sheetIndex); $valueAsXmlString = "$value"; - $this->assertNotContains($valueAsXmlString, $sheetXmlAsString, $message); + $this->assertStringNotContainsString($valueAsXmlString, $sheetXmlAsString, $message); } /** diff --git a/tests/Spout/Writer/ODS/WriterWithStyleTest.php b/tests/Spout/Writer/ODS/WriterWithStyleTest.php index 82ee44b..07e15e1 100644 --- a/tests/Spout/Writer/ODS/WriterWithStyleTest.php +++ b/tests/Spout/Writer/ODS/WriterWithStyleTest.php @@ -30,7 +30,7 @@ class WriterWithStyleTest extends TestCase /** * @return void */ - public function setUp() + public function setUp() : void { $this->defaultStyle = (new StyleBuilder())->build(); } diff --git a/tests/Spout/Writer/XLSX/SheetTest.php b/tests/Spout/Writer/XLSX/SheetTest.php index cebd72e..a243161 100644 --- a/tests/Spout/Writer/XLSX/SheetTest.php +++ b/tests/Spout/Writer/XLSX/SheetTest.php @@ -89,7 +89,7 @@ class SheetTest extends TestCase $pathToWorkbookFile = $resourcePath . '#xl/workbook.xml'; $xmlContents = file_get_contents('zip://' . $pathToWorkbookFile); - $this->assertContains(' state="hidden"', $xmlContents, 'The sheet visibility should have been changed to "hidden"'); + $this->assertStringContainsString(' state="hidden"', $xmlContents, 'The sheet visibility should have been changed to "hidden"'); } /** @@ -166,6 +166,6 @@ class SheetTest extends TestCase $pathToWorkbookFile = $resourcePath . '#xl/workbook.xml'; $xmlContents = file_get_contents('zip://' . $pathToWorkbookFile); - $this->assertContains("assertStringContainsString("assertInlineDataWasWrittenToSheet($fileName, 1, 't="e">#DIV/0'); } + /** + * @return void + */ + public function testAddRowShouldSupportFloatValuesInDifferentLocale() + { + $previousLocale = \setlocale(LC_ALL, 0); + $valueToWrite = 1234.5; // needs to be defined before changing the locale as PHP8 would expect 1234,5 + + try { + // Pick a supported locale whose decimal point is a comma. + // Installed locales differ from one system to another, so we can't pick + // a given locale. + $supportedLocales = explode("\n", shell_exec('locale -a')); + foreach ($supportedLocales as $supportedLocale) { + \setlocale(LC_ALL, $supportedLocale); + if (\localeconv()['decimal_point'] === ',') { + break; + } + } + $this->assertEquals(',', \localeconv()['decimal_point']); + + $fileName = 'test_add_row_should_support_float_values_in_different_locale.xlsx'; + $dataRows = $this->createRowsFromValues([ + [$valueToWrite], + ]); + + $this->writeToXLSXFile($dataRows, $fileName, $shouldUseInlineStrings = false); + + $this->assertInlineDataWasNotWrittenToSheet($fileName, 1, "1234,5"); + $this->assertInlineDataWasWrittenToSheet($fileName, 1, "1234.5"); + } finally { + // reset locale + \setlocale(LC_ALL, $previousLocale); + } + } + /** * @return void */ @@ -609,7 +645,7 @@ class WriterTest extends TestCase $pathToSheetFile = $resourcePath . '#xl/worksheets/sheet' . $sheetIndex . '.xml'; $xmlContents = file_get_contents('zip://' . $pathToSheetFile); - $this->assertContains((string) $inlineData, $xmlContents, $message); + $this->assertStringContainsString((string) $inlineData, $xmlContents, $message); } /** @@ -625,7 +661,7 @@ class WriterTest extends TestCase $pathToSheetFile = $resourcePath . '#xl/worksheets/sheet' . $sheetIndex . '.xml'; $xmlContents = file_get_contents('zip://' . $pathToSheetFile); - $this->assertNotContains((string) $inlineData, $xmlContents, $message); + $this->assertStringNotContainsString((string) $inlineData, $xmlContents, $message); } /** @@ -640,6 +676,6 @@ class WriterTest extends TestCase $pathToSharedStringsFile = $resourcePath . '#xl/sharedStrings.xml'; $xmlContents = file_get_contents('zip://' . $pathToSharedStringsFile); - $this->assertContains($sharedString, $xmlContents, $message); + $this->assertStringContainsString($sharedString, $xmlContents, $message); } } diff --git a/tests/Spout/Writer/XLSX/WriterWithStyleTest.php b/tests/Spout/Writer/XLSX/WriterWithStyleTest.php index a9b9f76..c174151 100644 --- a/tests/Spout/Writer/XLSX/WriterWithStyleTest.php +++ b/tests/Spout/Writer/XLSX/WriterWithStyleTest.php @@ -32,7 +32,7 @@ class WriterWithStyleTest extends TestCase /** * @return void */ - public function setUp() + public function setUp() : void { $this->defaultStyle = (new StyleBuilder())->build(); } diff --git a/tests/resources/xlsx/sheet_with_empty_cells.xlsx b/tests/resources/xlsx/sheet_with_empty_cells.xlsx index d815a1c..8de9522 100644 Binary files a/tests/resources/xlsx/sheet_with_empty_cells.xlsx and b/tests/resources/xlsx/sheet_with_empty_cells.xlsx differ diff --git a/tests/resources/xlsx/sheet_with_empty_cells_without_dimensions.xlsx b/tests/resources/xlsx/sheet_with_empty_cells_without_dimensions.xlsx new file mode 100644 index 0000000..533a634 Binary files /dev/null and b/tests/resources/xlsx/sheet_with_empty_cells_without_dimensions.xlsx differ diff --git a/tests/resources/xlsx/sheet_with_multiple_value_nodes_in_inline_strings.xlsx b/tests/resources/xlsx/sheet_with_multiple_value_nodes_in_inline_strings.xlsx new file mode 100644 index 0000000..2ee574c Binary files /dev/null and b/tests/resources/xlsx/sheet_with_multiple_value_nodes_in_inline_strings.xlsx differ diff --git a/tests/resources/xlsx/sheet_with_strict_ooxml.xlsx b/tests/resources/xlsx/sheet_with_strict_ooxml.xlsx new file mode 100644 index 0000000..0fba9f9 Binary files /dev/null and b/tests/resources/xlsx/sheet_with_strict_ooxml.xlsx differ