Problem/Motivation

When a class uses ComputedItemListTrait, a PHP 8.1 deprecation is trigger on offsetExists because return is not typed as bool.

Steps to reproduce

For example the profile module uses this trait in the class Drupal\profile\Plugin\Field\ProfileEntityFieldItemList. Displaying the login pane in a commerce checkout displays this error.

Proposed resolution

As for the others deprecated functions of this trait we could simply add #[\ReturnTypeWillChange] to suppress the deprecation.

CommentFileSizeAuthor
#2 drupal-offsetexists_deprecation-3257713-2.patch522 bytesaerzas

Comments

Aerzas created an issue. See original summary.

aerzas’s picture

aerzas commented
Status: Active » Needs review
Issue tags: +Needs review
StatusFileSize
new drupal-offsetexists_deprecation-3257713-2.patch522 bytes

Here is the patch.

mondrake’s picture

mondrake
Location 🇮🇹
commented

Will be fixed in #3257654: Fix PHPStan L0 failures that cannot be included in baseline

aerzas’s picture

aerzas commented
Status: Needs review » Closed (duplicate)
Related issues: +#3257654: Fix PHPStan L0 failures that cannot be included in baseline

Thank you for the heads up, I didn't find the issue.
Closing this one in favor of https://www.drupal.org/project/drupal/issues/3257654

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

The duplicate has been fixed. What's interesting here is that we didn't catch with with core testing. I was going to ask for further testing to be added to code but there's no way that \Drupal\content_moderation\Plugin\Field\ModerationStateFieldItemList is not tested by core - that is tested by \Drupal\content_moderation\Plugin\Field\ModerationStateFieldItemList - or maybe we need to ensure the offsetExists method is called... weird.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Yeah if I remove the annotation from \Drupal\Core\TypedData\Plugin\DataType\ItemList::offsetExists() and run \Drupal\Tests\content_moderation\Kernel\ModerationStateFieldItemListTest::testArrayIndex() then the test fails with 

3x: Return type of Drupal\Core\TypedData\Plugin\DataType\ItemList::offsetExists($offset) should either be compatible with ArrayAccess::offsetExists(mixed $offset): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice
    3x in ModerationStateFieldItemListTest::testArrayIndex from Drupal\Tests\content_moderation\Kernel

I'm not sure why this deprecation is being reported for ProfileEntityFieldItemList as that extends from ItemList too....

aerzas’s picture

aerzas commented

If I understand it correctly, ProfileEntityFieldItemList extends FieldItemList which extends ItemList, and then it uses the trait ComputedItemListTrait. So the method offsetExists on ProfileEntityFieldItemList is not inherited from ItemList but injected by the trait ComputedItemListTrait which misses the return type comment. To test this behaviour we would need a dummy class that reproduces this inheritance/trait architecture.

aerzas’s picture

aerzas commented

Regarding ModerationStateFieldItemList, the class explicitly only inherit from the get method of the trait so there is no overridance of offsetExists like ProfileEntityFieldItemList.

Of course ProfileEntityFieldItemList could be more selective to avoid this override and deprecation but fixing the trait in Drupal core would also be needed for consistency.

quietone’s picture

quietone commented
Issue tags: -Needs review

ignore, cleaning up tag.