Skip to content

Commit bc315a3

Browse files
committed
A Win For Scrutinizer!
It flagged a statement as dead code. It was correct - there was a typo in the variable name. But no tests had failed. The explanation was, of course, that this particular path was not adequately tested. PhpSpreadsheet extends Excel (dating back to PHPExcel) by allowing the ordinal form of days in the DATE function, implemented as "take the numeric portion if the field is a string consisting of a numeric portion followed by some alphabetics". Whether or not this is a good idea, it would be a breaking change to eliminate it, so that's not going to happen. However, the same logic has been applied to month, and I don't see a use case for that, so I'm eliminating it - any non-numeric string used as the month parameter will now result in a VALUE error. It also turns out that Excel accepts null, false, and true for the month, and PhpSpreadsheet will now do likewise.
1 parent e3ef02a commit bc315a3

File tree

4 files changed

+22
-14
lines changed

4 files changed

+22
-14
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org).
2323
- Start migration to Phpstan level 9. [PR #4396](https://github.com/PHPOffice/PhpSpreadsheet/pull/4396)
2424
- Calculation locale logic moved to separate class. [PR #4398](https://github.com/PHPOffice/PhpSpreadsheet/pull/4398)
2525
- TREND_POLYNOMIAL_* and TREND_BEST_FIT do not work, and are changed to throw Exceptions if attempted. (TREND_BEST_FIT_NO_POLY works.) An attempt to use an unknown trend type will now also throw an exception. [Issue #4400](https://github.com/PHPOffice/PhpSpreadsheet/issues/4400) [PR #4339](https://github.com/PHPOffice/PhpSpreadsheet/pull/4339)
26+
- Month parameter of DATE function will now return VALUE if an ordinal string (e.g. '3rd') is used, but will accept bool or null. [PR #4420](https://github.com/PHPOffice/PhpSpreadsheet/pull/4420)
2627

2728
### Moved
2829

src/PhpSpreadsheet/Calculation/DateTimeExcel/Date.php

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ class Date
6464
* If an array of numbers is passed as the argument, then the returned result will also be an array
6565
* with the same dimensions
6666
*/
67-
public static function fromYMD(array|float|int|string $year, array|float|int|string $month, array|float|int|string $day): float|int|DateTime|string|array
67+
public static function fromYMD(array|float|int|string $year, null|array|bool|float|int|string $month, array|float|int|string $day): float|int|DateTime|string|array
6868
{
6969
if (is_array($year) || is_array($month) || is_array($day)) {
7070
return self::evaluateArrayArguments([self::class, __FUNCTION__], $year, $month, $day);
@@ -121,13 +121,14 @@ private static function getYear(mixed $year, int $baseYear): int
121121
*/
122122
private static function getMonth(mixed $month): int
123123
{
124-
if (is_string($month) && !is_numeric($month)) {
125-
$month = SharedDateHelper::monthStringToNumber($month);
126-
}
127-
if ($month === null) {
124+
if (is_string($month)) {
125+
if (!is_numeric($month)) {
126+
$month = SharedDateHelper::monthStringToNumber($month);
127+
}
128+
} elseif ($month === null) {
128129
$month = 0;
129-
} elseif (is_scalar($month)) {
130-
$year = StringHelper::testStringAsNumeric((string) $month);
130+
} elseif (is_bool($month)) {
131+
$month = (int) $month;
131132
}
132133
if (!is_numeric($month)) {
133134
throw new Exception(ExcelError::VALUE());

tests/PhpSpreadsheetTests/Calculation/Functions/DateTime/DateTest.php

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use PhpOffice\PhpSpreadsheet\Shared\Date as SharedDate;
1313
use PhpOffice\PhpSpreadsheet\Spreadsheet;
1414
use PhpOffice\PhpSpreadsheetTests\Calculation\Functions\FormulaArguments;
15+
use PHPUnit\Framework\Attributes\DataProvider;
1516
use PHPUnit\Framework\TestCase;
1617

1718
class DateTest extends TestCase
@@ -36,14 +37,14 @@ protected function tearDown(): void
3637
Functions::setReturnDateType($this->returnDateType);
3738
}
3839

39-
#[\PHPUnit\Framework\Attributes\DataProvider('providerDATE')]
40-
public function testDirectCallToDATE(float|string $expectedResult, int|string $year, float|int|string $month, float|int|string $day): void
40+
#[DataProvider('providerDATE')]
41+
public function testDirectCallToDATE(float|string $expectedResult, int|string $year, null|bool|float|int|string $month, float|int|string $day): void
4142
{
4243
$result = Date::fromYMD($year, $month, $day);
4344
self::assertSame($expectedResult, $result);
4445
}
4546

46-
#[\PHPUnit\Framework\Attributes\DataProvider('providerDATE')]
47+
#[DataProvider('providerDATE')]
4748
public function testDATEAsFormula(mixed $expectedResult, mixed ...$args): void
4849
{
4950
$arguments = new FormulaArguments(...$args);
@@ -55,7 +56,7 @@ public function testDATEAsFormula(mixed $expectedResult, mixed ...$args): void
5556
self::assertSame($expectedResult, $result);
5657
}
5758

58-
#[\PHPUnit\Framework\Attributes\DataProvider('providerDATE')]
59+
#[DataProvider('providerDATE')]
5960
public function testDATEInWorksheet(mixed $expectedResult, mixed ...$args): void
6061
{
6162
$arguments = new FormulaArguments(...$args);
@@ -78,7 +79,7 @@ public static function providerDATE(): array
7879
return require 'tests/data/Calculation/DateTime/DATE.php';
7980
}
8081

81-
#[\PHPUnit\Framework\Attributes\DataProvider('providerUnhappyDATE')]
82+
#[DataProvider('providerUnhappyDATE')]
8283
public function testDATEUnhappyPath(string $expectedException, mixed ...$args): void
8384
{
8485
$arguments = new FormulaArguments(...$args);
@@ -136,7 +137,7 @@ public function testDATEWith1904Calendar(): void
136137
self::assertEquals($result, ExcelError::NAN());
137138
}
138139

139-
#[\PHPUnit\Framework\Attributes\DataProvider('providerDateArray')]
140+
#[DataProvider('providerDateArray')]
140141
public function testDateArray(array $expectedResult, string $year, string $month, string $day): void
141142
{
142143
$calculation = Calculation::getInstance();
@@ -200,7 +201,7 @@ public static function providerDateArray(): array
200201
];
201202
}
202203

203-
#[\PHPUnit\Framework\Attributes\DataProvider('providerDateArrayException')]
204+
#[DataProvider('providerDateArrayException')]
204205
public function testDateArrayException(string $year, string $month, string $day): void
205206
{
206207
$calculation = Calculation::getInstance();

tests/data/Calculation/DateTime/DATE.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,14 @@
6262
[39844.0, 2008, 13, 31],
6363
[39813.0, 2009, 1, 0],
6464
[39812.0, 2009, 1, -1],
65+
'month expressed as true' => [39812.0, 2009, true, -1],
6566
[39782.0, 2009, 0, 0],
6667
[39781.0, 2009, 0, -1],
6768
[39752.0, 2009, -1, 0],
6869
[39751.0, 2009, -1, -1],
6970
[40146.0, 2010, 0, -1],
71+
'month expressed as false' => [40146.0, 2010, false, -1],
72+
'month expressed as null' => [40146.0, 2010, null, -1],
7073
[40329.0, 2010, 5, 31],
7174
[40199.0, 2010, 1, '21st'], // Excel can't parse ordinal, PhpSpreadsheet can
7275
[40200.0, 2010, 1, '22nd'], // Excel can't parse ordinal, PhpSpreadsheet can
@@ -75,6 +78,8 @@
7578
[40258.0, 2010, 'March', '21st'], // ordinal and month name
7679
// MS Excel will fail with a #VALUE return, but PhpSpreadsheet can parse this date
7780
[40258.0, 2010, 'March', 21], // Excel can't parse month name, PhpSpreadsheet can
81+
'month expressed as string' => [40258.0, 2010, '03', 21],
82+
'month expressed as invalid string' => [ExcelError::VALUE(), 2010, '03x', 21],
7883
[ExcelError::VALUE(), 'ABC', 1, 21],
7984
[ExcelError::VALUE(), 2010, 'DEF', 21],
8085
[ExcelError::VALUE(), 2010, 3, 'GHI'],

0 commit comments

Comments
 (0)