[PHP 8.4] Fix implicitly nullable type declarations

Ideally the phpdoc could stay in sync with the code?

e.g.

   * @param array $ids
   *   An array of entity IDs, or NULL to load all entities.
   *
   * @return static[]
   *   An array of entity objects indexed by their IDs.
   */
  public static function loadMultiple(array $ids = NULL);

becomes

   * @param ?array $ids
   *   An array of entity IDs, or NULL to load all entities.
   *
   * @return static[]
   *   An array of entity objects indexed by their IDs.
   */
  public static function loadMultiple(?array $ids = NULL);

Ideally should be solved before D11, since it has BC implications

First core run on 8.4 showed that we need to wait contrib fixes #3427903-3: [META] Make Drupal 10.3/11 compatible with PHP 8.4 →

D7 also needs fix

It affects in D10 hook_entity_field_access()

- function rest_test_entity_field_access($operation, FieldDefinitionInterface $field_definition, AccountInterface $account, FieldItemListInterface $items = NULL) {
+ function rest_test_entity_field_access($operation, FieldDefinitionInterface $field_definition, AccountInterface $account, ?FieldItemListInterface $items = NULL) {

Got a first with all deprecations filed (the failures are unrelated to the issue) https://git.drupalcode.org/issue/drupal-3427903/-/jobs/1459523

Filed first child issue 📌 [8.4] Fix implicitly nullable type declarations in composer Needs review

Filed remaining child issues, fill free to re-split/group

There's SlevomatCodingStandard.TypeHints.NullableTypeForNullDefaultValue sniffer to catch such issues

Discussed in slack with @catch and @smushgrave and decided to not split automated fixes

The issue is postponed follow-up 📌 [8.4] Fix implicitly nullable type declarations in docblocks Postponed
That's because docblocks are not yet covered with sniffer #2572645: [Meta] Fix 'Drupal.Commenting.FunctionComment' coding standard →

D7 also needs fix

Actually, as D7 still has to support PHP 5.6 and PHP 7.0, we would probably need to drop type declarations where needed and add type checks to the functions itself, see: https://php.watch/versions/8.4/implicitly-marking-parameter-type-nullabl...

Because nullable types were added only in PHP 7.1.

But thanks for the initial research. I will create a separate issues for D7 soon.

There's also rector rule https://github.com/rectorphp/rector-src/commit/ff32c0c08a89f27ea34187d00... since 1.0.4

andypost → changed the visibility of the branch 3427999-php-8.4-fix to hidden.

Re-rolled and re-based https://git.drupalcode.org/project/drupal/-/merge_requests/8004 as reproducible set of commits

PS: Draft MR !7976 is just a attempt to run current state on PHP 8.4

Seems you already mentioned the rector rule, awesome.

Think i might need to add that to drupal-rector, since it is a deprecation for 11, and its shouldn't break earlier code afaik.

Reviewing MR 8004 and see there is a 1 to 1 deletion to addition. One extra addition being phpcs.dist

Spot checking the files and nothing seemed super off based on our slack conversation.

Needs a rebase.

rebased.

Thank you! back to RTBC

Committed/pushed to 11.x and cherry-picked to 11.0.x, thanks!

We also need an MR for 10.4.x/10.3.x. I don't see any bc implication here because PHP treats these the same in terms of inheritance.

andypost → closed merge request !8004

Created MR for 10.4/10.3 there's extra files but sniffer and `phpcbf` produce reproducible result

andypost → closed merge request !7976

10.4 version seems fine.

Committed/pushed to 10.4.x and cherry-picked to 10.3.x, thanks!

andypost → closed merge request !8266

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