-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
add the DAYS() function #594
Conversation
The bug was reported by the scrutinizer here: https://scrutinizer-ci.com/g/PHPOffice/PhpSpreadsheet/inspections/225db0af-829f-4823-9591-efaade761da4/issues/files/src/PhpSpreadsheet/Calculation/DateTime.php?status=failed&conditionIds%5B0%5D=289959 The expression return PhpOffice\PhpSpre...tion\Functions::VALUE() returns the type string which is incompatible with the documented return type integer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks promising ! only a few changes required to be merged and also please add it to the changelog
* | ||
* @category Date/Time Functions | ||
* | ||
* @param mixed $endDate Excel date serial value (float), PHP date timestamp (integer), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type should enumerate all possibitilies here: \DateTime|int|float|string
$startDate = Functions::flattenSingleValue($startDate); | ||
$endDate = Functions::flattenSingleValue($endDate); | ||
|
||
if (is_string($startDate = self::getDateValue($startDate))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's avoid assignment within condition and do it beforehand instead
if (is_string($startDate = self::getDateValue($startDate))) { | ||
return Functions::VALUE(); | ||
} | ||
if (is_string($endDate = self::getDateValue($endDate))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
idem
@@ -0,0 +1,89 @@ | |||
<?php | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests cases miss a few cases for different input type such as DateTime, int and float
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
Merged in 2c981e4 |
Adds the DAYS() function
This is:
Checklist:
Why this change is needed?
The DAYS() function was not implemented so this PR adds it
Fixes #593