Problem/Motivation
Symfony 7 has added void returns to a number of methods. These were added as deprecations in Symfony 6 but we worked around them by adding @return tags, which skipped the deprecation for us but still should have notified any downstream subclasses via the Symfony debug classloader. This means that contrib/custom code has been given the chance to catch and now we need to actually add the void returns to core in order to run on Symfony 7.
Steps to reproduce
Proposed resolution
Convert e.g.
/**
* {@inheritdoc}
*
* phpcs:ignore Drupal.Commenting.FunctionComment.VoidReturn
* @return void
*/
public function set($id, $service) {
to
/**
* {@inheritdoc}
*/
public function set($id, $service): void {
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Issue fork drupal-3407159
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:
- 3407159-add-void-returns changes, plain diff MR !5748
Comments
Comment #2
longwaveComment #3
longwaveNeedless to say this is for 11.x only.
Comment #5
SpokjeUnhappy PHPStan
Comment #6
longwaveHm, maybe the debug class loader isn't working like I thought, because we should have been notified already about these subclasses?
Comment #7
SpokjeNot sure I follow.
In the MR we added return types where none where there before, now PHPStan complains some sub-classes have defined types that aren't covariant with the newly defined ones.
How could we be warned before we added the return types on the "super" classes?
Comment #8
longwaveRe #7 e.g.
AllowedValuesConstraintValidator::validate()
extends a Symfony validator that was tagged@return void
in Symfony 6. But I guess if we have no test coverage of this, then we won't trigger the deprecation.But then what about
SupernovaGenerator::setContext()
, which is part of a test - wonder if this is just because the inheritance chain is more complicated, but ultimatelysetContext()
belongs to a Symfony interface method that was also tagged@return void
.Comment #9
smustgrave CreditAttribution: smustgrave at Mobomo commentedWasn't sure how best to find others.
Did a search for phpcs:ignore Drupal.Commenting.FunctionComment.VoidReturn and all instances have been removed in core. Tests are green so leaning on that.
This seems like a small enough change to mark without having to wait a week.
Comment #11
quietone CreditAttribution: quietone at PreviousNext commentedRebase. Only conflict was in DrupalKernel.php and it was straightforward to fix.
Comment #12
Gábor HojtsyComment #13
alexpottI'm not too concerned about missing some - things will crash if we do and we can fix.
Committed f4167b8 and pushed to 11.x. Thanks!