Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Breaking Change to toFormattedString and 3 toArray Methods #3304

Merged
merged 4 commits into from
Jan 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ and this project adheres to [Semantic Versioning](https://semver.org).

### Changed

- Nothing
- `toFormattedString` will now always return a string. This can affect the results of `toArray`, `namedRangeToArray`, and `rangeToArray`. [PR #3304](https://github.com/PHPOffice/PhpSpreadsheet/pull/3304)

### Deprecated

Expand Down
50 changes: 0 additions & 50 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -790,56 +790,6 @@ parameters:
count: 1
path: src/PhpSpreadsheet/Shared/Trend/Trend.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\Formatter\\:\\:splitFormat\\(\\) has no return type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Style/NumberFormat/Formatter.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\Formatter\\:\\:splitFormat\\(\\) has parameter \\$sections with no type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Style/NumberFormat/Formatter.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\Formatter\\:\\:splitFormat\\(\\) has parameter \\$value with no type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Style/NumberFormat/Formatter.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\Formatter\\:\\:splitFormatCompare\\(\\) has no return type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Style/NumberFormat/Formatter.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\Formatter\\:\\:splitFormatCompare\\(\\) has parameter \\$cond with no type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Style/NumberFormat/Formatter.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\Formatter\\:\\:splitFormatCompare\\(\\) has parameter \\$dfcond with no type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Style/NumberFormat/Formatter.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\Formatter\\:\\:splitFormatCompare\\(\\) has parameter \\$dfval with no type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Style/NumberFormat/Formatter.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\Formatter\\:\\:splitFormatCompare\\(\\) has parameter \\$val with no type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Style/NumberFormat/Formatter.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\Formatter\\:\\:splitFormatCompare\\(\\) has parameter \\$value with no type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Style/NumberFormat/Formatter.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\Formatter\\:\\:toFormattedString\\(\\) should return string but returns float\\|int\\|string\\.$#"
count: 1
path: src/PhpSpreadsheet/Style/NumberFormat/Formatter.php

-
message: "#^Negated boolean expression is always false\\.$#"
count: 1
Expand Down
27 changes: 19 additions & 8 deletions src/PhpSpreadsheet/Style/NumberFormat/Formatter.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,19 @@

namespace PhpOffice\PhpSpreadsheet\Style\NumberFormat;

use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
use PhpOffice\PhpSpreadsheet\RichText\RichText;
use PhpOffice\PhpSpreadsheet\Style\Color;
use PhpOffice\PhpSpreadsheet\Style\NumberFormat;

class Formatter
{
private static function splitFormatCompare($value, $cond, $val, $dfcond, $dfval)
/**
* @param mixed $value
* @param mixed $val
* @param mixed $dfval
*/
private static function splitFormatCompare($value, ?string $cond, $val, string $dfcond, $dfval): bool
{
if (!$cond) {
$cond = $dfcond;
Expand All @@ -33,7 +40,8 @@ private static function splitFormatCompare($value, $cond, $val, $dfcond, $dfval)
return $value >= $val;
}

private static function splitFormat($sections, $value)
/** @param mixed $value */
private static function splitFormat(array $sections, $value): array
{
// Extract the relevant section depending on whether number is positive, negative, or zero?
// Text not supported yet.
Expand Down Expand Up @@ -93,23 +101,26 @@ private static function splitFormat($sections, $value)
/**
* Convert a value in a pre-defined format to a PHP string.
*
* @param mixed $value Value to format
* @param null|bool|float|int|RichText|string $value Value to format
* @param string $format Format code, see = NumberFormat::FORMAT_*
* @param array $callBack Callback function for additional formatting of string
*
* @return string Formatted string
*/
public static function toFormattedString($value, $format, $callBack = null)
{
if (is_bool($value)) {
return $value ? Calculation::getTRUE() : Calculation::getFALSE();
}
// For now we do not treat strings although section 4 of a format code affects strings
if (!is_numeric($value)) {
return $value;
return (string) $value;
}

// For 'General' format code, we just pass the value although this is not entirely the way Excel does it,
// it seems to round numbers to a total of 10 digits.
if (($format === NumberFormat::FORMAT_GENERAL) || ($format === NumberFormat::FORMAT_TEXT)) {
return $value;
return (string) $value;
}

// Ignore square-$-brackets prefix in format string, like "[$-411]ge.m.d", "[$-010419]0%", etc
Expand All @@ -127,7 +138,7 @@ function ($matches) {
$format = (string) preg_replace('/(\\\(((.)(?!((AM\/PM)|(A\/P))))|([^ ])))(?=(?:[^"]|"[^"]*")*$)/ui', '"${2}"', $format);

// Get the sections, there can be up to four sections, separated with a semi-colon (but only if not a quoted literal)
$sections = preg_split('/(;)(?=(?:[^"]|"[^"]*")*$)/u', $format);
$sections = preg_split('/(;)(?=(?:[^"]|"[^"]*")*$)/u', $format) ?: [];

[$colors, $format, $value] = self::splitFormat($sections, $value);

Expand All @@ -145,8 +156,8 @@ function ($matches) {
if (substr($format, 0, 1) === '"' && substr($format, -1, 1) === '"' && substr_count($format, '"') === 2) {
$value = substr($format, 1, -1);
} elseif (preg_match('/[0#, ]%/', $format)) {
// % number format
$value = PercentageFormatter::format($value, $format);
// % number format - avoid weird '-0' problem
$value = PercentageFormatter::format(0 + (float) $value, $format);
} else {
$value = NumberFormatter::format($value, $format);
}
Expand Down
37 changes: 17 additions & 20 deletions tests/PhpSpreadsheetTests/Reader/Ods/HiddenMergeCellsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,46 +3,43 @@
namespace PhpOffice\PhpSpreadsheetTests\Reader\Ods;

use PhpOffice\PhpSpreadsheet\Reader\Ods;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PHPUnit\Framework\TestCase;

class HiddenMergeCellsTest extends TestCase
{
/**
* @var Spreadsheet
*/
private $spreadsheet;

protected function setup(): void
{
$filename = 'tests/data/Reader/Ods/HiddenMergeCellsTest.ods';
$reader = new Ods();
$this->spreadsheet = $reader->load($filename);
}
private const FILENAME = 'tests/data/Reader/Ods/HiddenMergeCellsTest.ods';

public function testHiddenMergeCells(): void
{
$c2InMergeRange = $this->spreadsheet->getActiveSheet()->getCell('C2')->isInMergeRange();
$reader = new Ods();
$spreadsheet = $reader->load(self::FILENAME);
$c2InMergeRange = $spreadsheet->getActiveSheet()->getCell('C2')->isInMergeRange();
self::assertTrue($c2InMergeRange);
$a2InMergeRange = $this->spreadsheet->getActiveSheet()->getCell('A2')->isInMergeRange();
$a2InMergeRange = $spreadsheet->getActiveSheet()->getCell('A2')->isInMergeRange();
self::assertTrue($a2InMergeRange);
$a2MergeRangeValue = $this->spreadsheet->getActiveSheet()->getCell('A2')->isMergeRangeValueCell();
$a2MergeRangeValue = $spreadsheet->getActiveSheet()->getCell('A2')->isMergeRangeValueCell();
self::assertTrue($a2MergeRangeValue);

$cellArray = $this->spreadsheet->getActiveSheet()->rangeToArray('A2:C2');
$cellArray = $spreadsheet->getActiveSheet()->rangeToArray('A2:C2');
self::assertSame([['12', '4', '3']], $cellArray);
$cellArray = $spreadsheet->getActiveSheet()->rangeToArray('A2:C2', null, true, false);
self::assertSame([[12, 4, 3]], $cellArray);
$spreadsheet->disconnectWorksheets();
}

public function testUnmergeHiddenMergeCells(): void
{
$this->spreadsheet->getActiveSheet()->unmergeCells('A2:C2');
$reader = new Ods();
$spreadsheet = $reader->load(self::FILENAME);
$spreadsheet->getActiveSheet()->unmergeCells('A2:C2');

$c2InMergeRange = $this->spreadsheet->getActiveSheet()->getCell('C2')->isInMergeRange();
$c2InMergeRange = $spreadsheet->getActiveSheet()->getCell('C2')->isInMergeRange();
self::assertFalse($c2InMergeRange);
$a2InMergeRange = $this->spreadsheet->getActiveSheet()->getCell('A2')->isInMergeRange();
$a2InMergeRange = $spreadsheet->getActiveSheet()->getCell('A2')->isInMergeRange();
self::assertFalse($a2InMergeRange);

$cellArray = $this->spreadsheet->getActiveSheet()->rangeToArray('A2:C2', null, false, false, false);
$cellArray = $spreadsheet->getActiveSheet()->rangeToArray('A2:C2', null, false, false, false);
self::assertSame([[12, '=6-B1', '=A2/B2']], $cellArray);
$spreadsheet->disconnectWorksheets();
}
}
17 changes: 4 additions & 13 deletions tests/PhpSpreadsheetTests/Reader/Ods/HiddenWorksheetTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,29 +3,19 @@
namespace PhpOffice\PhpSpreadsheetTests\Reader\Ods;

use PhpOffice\PhpSpreadsheet\Reader\Ods;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
use PHPUnit\Framework\TestCase;

class HiddenWorksheetTest extends TestCase
{
/**
* @var Spreadsheet
*/
private $spreadsheet;

protected function setup(): void
public function testPageSetup(): void
{
$filename = 'tests/data/Reader/Ods/HiddenSheet.ods';
$reader = new Ods();
$this->spreadsheet = $reader->load($filename);
}

public function testPageSetup(): void
{
$spreadsheet = $reader->load($filename);
$assertions = $this->worksheetAssertions();

foreach ($this->spreadsheet->getAllSheets() as $worksheet) {
foreach ($spreadsheet->getAllSheets() as $worksheet) {
if (!array_key_exists($worksheet->getTitle(), $assertions)) {
continue;
}
Expand All @@ -40,6 +30,7 @@ public function testPageSetup(): void
);
}
}
$spreadsheet->disconnectWorksheets();
}

private function worksheetAssertions(): array
Expand Down
37 changes: 17 additions & 20 deletions tests/PhpSpreadsheetTests/Reader/Xls/HiddenMergeCellsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,46 +3,43 @@
namespace PhpOffice\PhpSpreadsheetTests\Reader\Xls;

use PhpOffice\PhpSpreadsheet\Reader\Xls;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PHPUnit\Framework\TestCase;

class HiddenMergeCellsTest extends TestCase
{
/**
* @var Spreadsheet
*/
private $spreadsheet;

protected function setup(): void
{
$filename = 'tests/data/Reader/XLS/HiddenMergeCellsTest.xls';
$reader = new Xls();
$this->spreadsheet = $reader->load($filename);
}
private const FILENAME = 'tests/data/Reader/XLS/HiddenMergeCellsTest.xls';

public function testHiddenMergeCells(): void
{
$c2InMergeRange = $this->spreadsheet->getActiveSheet()->getCell('C2')->isInMergeRange();
$reader = new Xls();
$spreadsheet = $reader->load(self::FILENAME);
$c2InMergeRange = $spreadsheet->getActiveSheet()->getCell('C2')->isInMergeRange();
self::assertTrue($c2InMergeRange);
$a2InMergeRange = $this->spreadsheet->getActiveSheet()->getCell('A2')->isInMergeRange();
$a2InMergeRange = $spreadsheet->getActiveSheet()->getCell('A2')->isInMergeRange();
self::assertTrue($a2InMergeRange);
$a2MergeRangeValue = $this->spreadsheet->getActiveSheet()->getCell('A2')->isMergeRangeValueCell();
$a2MergeRangeValue = $spreadsheet->getActiveSheet()->getCell('A2')->isMergeRangeValueCell();
self::assertTrue($a2MergeRangeValue);

$cellArray = $this->spreadsheet->getActiveSheet()->rangeToArray('A2:C2');
$cellArray = $spreadsheet->getActiveSheet()->rangeToArray('A2:C2');
self::assertSame([['12', '4', '3']], $cellArray);
$cellArray = $spreadsheet->getActiveSheet()->rangeToArray('A2:C2', null, true, false);
self::assertSame([[12, 4, 3]], $cellArray);
$spreadsheet->disconnectWorksheets();
}

public function testUnmergeHiddenMergeCells(): void
{
$this->spreadsheet->getActiveSheet()->unmergeCells('A2:C2');
$reader = new Xls();
$spreadsheet = $reader->load(self::FILENAME);
$spreadsheet->getActiveSheet()->unmergeCells('A2:C2');

$c2InMergeRange = $this->spreadsheet->getActiveSheet()->getCell('C2')->isInMergeRange();
$c2InMergeRange = $spreadsheet->getActiveSheet()->getCell('C2')->isInMergeRange();
self::assertFalse($c2InMergeRange);
$a2InMergeRange = $this->spreadsheet->getActiveSheet()->getCell('A2')->isInMergeRange();
$a2InMergeRange = $spreadsheet->getActiveSheet()->getCell('A2')->isInMergeRange();
self::assertFalse($a2InMergeRange);

$cellArray = $this->spreadsheet->getActiveSheet()->rangeToArray('A2:C2', null, false, false, false);
$cellArray = $spreadsheet->getActiveSheet()->rangeToArray('A2:C2', null, false, false, false);
self::assertSame([[12, '=6-B1', '=A2/B2']], $cellArray);
$spreadsheet->disconnectWorksheets();
}
}
17 changes: 4 additions & 13 deletions tests/PhpSpreadsheetTests/Reader/Xls/HiddenWorksheetTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,29 +3,19 @@
namespace PhpOffice\PhpSpreadsheetTests\Reader\Xls;

use PhpOffice\PhpSpreadsheet\Reader\Xls;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
use PHPUnit\Framework\TestCase;

class HiddenWorksheetTest extends TestCase
{
/**
* @var Spreadsheet
*/
private $spreadsheet;

protected function setup(): void
public function testPageSetup(): void
{
$filename = 'tests/data/Reader/XLS/HiddenSheet.xls';
$reader = new Xls();
$this->spreadsheet = $reader->load($filename);
}

public function testPageSetup(): void
{
$spreadsheet = $reader->load($filename);
$assertions = $this->worksheetAssertions();

foreach ($this->spreadsheet->getAllSheets() as $worksheet) {
foreach ($spreadsheet->getAllSheets() as $worksheet) {
if (!array_key_exists($worksheet->getTitle(), $assertions)) {
continue;
}
Expand All @@ -40,6 +30,7 @@ public function testPageSetup(): void
);
}
}
$spreadsheet->disconnectWorksheets();
}

private function worksheetAssertions(): array
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public function testGeneral(): void
self::assertSame(2.95, $sheet->getCell('D2')->getValue());
self::assertSame(2.95, $sheet->getCell('D2')->getCalculatedValue());
self::assertSame('€ 2.95', $sheet->getCell('D2')->getFormattedValue());
self::assertSame(21, $array[1][4]);
self::assertSame('21', $array[1][4]);
self::assertSame(21, $sheet->getCell('E2')->getValue());
self::assertSame(21, $sheet->getCell('E2')->getCalculatedValue());
self::assertSame('21', $sheet->getCell('E2')->getFormattedValue());
Expand Down
Loading