Problem/Motivation

In \Drupal\Component\Datetime\DateTimePlus::checkArray() we do

      if (@checkdate($array['month'], $array['day'], $array['year'])) {
        $valid_date = TRUE;
      }

The $array['month'], $array['day'], $array['year'] values are not guaranteed to be integers and they should be. In PHP8 this causes test failures.

This error is produced when running Drupal\Tests\datetime\Functional\DateTimeFieldTest::testDatelistWidget see https://dispatcher.drupalci.org/job/drupal_patches/51890/testReport/juni...

Proposed resolution

Casting each value to an integer works but is it the best fix?

This issue should also be backported to 8.9.x so that Drupal 8 can be PHP 8.0 compatible.

Remaining tasks

User interface changes

N/a

API changes

N/a

Data model changes

N/a

Release notes snippet

N/a

Comments

alexpott created an issue. See original summary.

alexpott’s picture

Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new762 bytes

This is a bug on PHP 8 :)

andypost’s picture

Status: Needs review » Needs work
Issue tags: +Needs steps to reproduce

This change sounds strange, tested on alpha 3 locally

/srv # php -v
PHP 8.0.0alpha3 (cli) (built: Jul 23 2020 19:54:36) ( NTS )
Copyright (c) The PHP Group
Zend Engine v4.0.0-dev, Copyright (c) Zend Technologies
    with Zend OPcache v8.0.0alpha3, Copyright (c), by Zend Technologies
/srv # php -r "var_dump(checkdate('07', '26', '2020'));"
bool(true)

It means that wrong data passed to function in tests, so setting NW to get which tests are broken without this change

alexpott’s picture

Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs steps to reproduce
StatusFileSize
new3.57 KB
new2.49 KB
new3.56 KB

The important thing about the code in HEAD is the suppression of warnings - i.e the @ sign. The problem we have is https://3v4l.org/Slqii

Sometimes we're passing empty strings into checkdate.

So we can definitely improve this - compare https://3v4l.org/HBXvM vs https://3v4l.org/poPFV vs https://3v4l.org/p6SKR

andypost’s picture

Status: Needs review » Reviewed & tested by the community

As empty value allows to skip function already ++ to patch approach!
I'd say it a bit improvement as now invalid dates could be caught

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 4: 3156878-4.patch, failed testing. View results

andypost’s picture

Status: Needs work » Reviewed & tested by the community

Test known as randomly failed

  • Gábor Hojtsy committed dc21aa7 on 9.1.x
    Issue #3156878 by alexpott, andypost: \Drupal\Component\Datetime\...

  • Gábor Hojtsy committed 54dbe46 on 9.0.x
    Issue #3156878 by alexpott, andypost: \Drupal\Component\Datetime\...

  • Gábor Hojtsy committed 5becd6a on 8.9.x
    Issue #3156878 by alexpott, andypost: \Drupal\Component\Datetime\...
gábor hojtsy’s picture

Version: 9.1.x-dev » 8.9.x-dev

Thanks! Committed and cherry-picked.

andypost’s picture

Status: Reviewed & tested by the community » Fixed

Thank you!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.