Problem/Motivation
ppl
In PHP 8.4, declaring functions/methods with parameters containing null as a default value, but without null as one of the types (either as a nullable syntax or as a Union type with null) is deprecated.
https://php.watch/versions/8.4/implicitly-marking-parameter-type-nullabl...
Steps to reproduce
Run Drupal in PHP 8.4.
Proposed resolution
Update all instances of such declarations to use Union types or nullable types.
This is a rather big set of changes. To replicate the results:
cd core
../vendor/bin/phpcbf --standard=SlevomatCodingStandard --sniffs=SlevomatCodingStandard.TypeHints.NullableTypeForNullDefaultValue lib modules themes profiles tests
or
php-cs-fixer fix . --rules nullable_type_declaration_for_default_null_value
Add SlevomatCodingStandard.TypeHints.NullableTypeForNullDefaultValue sniffer to prevent new issues
Remaining tasks
- decide on merge strategy (all at once after fixing manual set or split per module/system)
- patch/commit
User interface changes
None.
API changes
None.
Data model changes
None.
Release notes snippet
Several fixes in Drupal core to fix the deprecation notices due on implicitly nullable function/method parameter declarations.
See:
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | 3427999-d7-8.patch | 1.13 KB | andypost |
Issue fork drupal-3427999
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
mfbIdeally the phpdoc could stay in sync with the code?
e.g.
becomes
Comment #4
mondrakeIdeally should be solved before D11, since it has BC implications
Comment #5
poker10 commentedComment #6
andypostFirst core run on 8.4 showed that we need to wait contrib fixes #3427903-3: [META] Make Drupal 10/11 compatible with PHP 8.4
Comment #7
andypostComment #8
andypostD7 also needs fix
Comment #9
andypostIt affects in D10
hook_entity_field_access()Comment #10
andypostGot a first with all deprecations filed (the failures are unrelated to the issue) https://git.drupalcode.org/issue/drupal-3427903/-/jobs/1459523
Comment #11
andypostFiled first child issue #3444020: [8.4] Fix implicitly nullable type declarations in composer plugin
Comment #12
andypostFiled remaining child issues, fill free to re-split/group
Comment #15
andypostThere's
SlevomatCodingStandard.TypeHints.NullableTypeForNullDefaultValuesniffer to catch such issuesEDIT
Comment #16
andypostDiscussed in slack with @catch and @smushgrave and decided to not split automated fixes
The issue is postponed follow-up #3444025: [8.4] Fix implicitly nullable type declarations in docblocks
That's because docblocks are not yet covered with sniffer #2572645: [Meta] Fix 'Drupal.Commenting.FunctionComment' coding standard
Comment #17
andypostComment #18
poker10 commentedActually, 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.
Comment #19
andypostThere's also rector rule https://github.com/rectorphp/rector-src/commit/ff32c0c08a89f27ea34187d00... since 1.0.4
Comment #21
andypostRe-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
Comment #22
bbralaSeems 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.
Comment #23
smustgrave commentedReviewing 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.
Comment #24
catchNeeds a rebase.
Comment #25
bbralarebased.
Comment #26
andypostThank you! back to RTBC
Comment #27
catchCommitted/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.
Comment #32
andypostCreated MR for 10.4/10.3 there's extra files but sniffer and `phpcbf` produce reproducible result
Comment #34
smustgrave commented10.4 version seems fine.
Comment #37
catchCommitted/pushed to 10.4.x and cherry-picked to 10.3.x, thanks!
Comment #40
solideogloria commentedSome were missed. I tried using Drupal 10.4.1 and PHP 8, and I get these errors:
Comment #41
solideogloria commentedCould a maintainer please reopen the issue? Or should I open a new one?
Comment #42
andypostThis files are not from core, please file new issue
EDIT it's from patch #2916876: Add visibility control conditions to blocks within Layout Builder
Comment #43
solideogloria commentedAh, right you are. Thanks