Add @property $date to the DateTimeItem

Created on 4 March 2024, 10 months ago

Problem/Motivation

If I have a datetime field, and i can access the date property:

    return $this->get('field_foo_date')->date;

PHPStan doesn't like this:

  27     Access to an undefined property Drupal\Core\Field\FieldItemListInterface::$date.
         💡 Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property

Steps to reproduce

Proposed resolution

Similar to LayoutSectionItem.php use the @property tag for date in DateTimeItem.

@property \Drupal\Core\Datetime\DrupalDateTime $date

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Feature request
Status

Active

Version

10.3

Component
Datetime 

Last updated 6 days ago

Created by

🇦🇺Australia sime Melbourne

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @sime
  • First commit to issue fork.
  • Merge request !6880Add @property to the DateTimeItem → (Open) created by dineshkumarbollu
  • Pipeline finished with Failed
    10 months ago
    Total: 514s
    #110092
  • Pipeline finished with Failed
    10 months ago
    Total: 1043s
    #110098
  • Status changed to Needs work 10 months ago
  • 🇦🇺Australia mstrelan

    Nice, the phpstan fail is because we are ignoring this error in the phpstan baseline and that is no longer necessary. That validates that adding this @property is useful not only downstream but in core also. Needs work to remove the ignore rule from the phpstan baseline.

  • Pipeline finished with Success
    10 months ago
    Total: 495s
    #111261
  • Status changed to Needs review 10 months ago
  • 🇮🇳India dineshkumarbollu

    Now MR looks good phpstan issue fix.

  • Status changed to Needs work 10 months ago
  • 🇦🇺Australia mstrelan

    This fixes phpstan errors in \Drupal\datetime\Plugin\Field\FieldType\DateTimeItem::onChange that were in the baseline.

    I think we should update \Drupal\datetime\Plugin\Field\FieldFormatter\DateTimePlainFormatter::viewElements and other related formatters. It seems we're using the local variable $date simply so we can typehint it to DrupalDateTime. I think instead we should typehint $item to DateTimeItemInterface and drop the local variable.

    Wondering though if DateTimeItem::$date could be null though, in which case we need a union type on the @property.

  • 🇦🇺Australia sime Melbourne

    I'm wondering why we wouldn't protect the date property and add something like a getDate() method in that case (from a position of ignorance).

  • 🇳🇿New Zealand quietone

    Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.

Production build 0.71.5 2024