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

Command icon 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:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

longwave created an issue. See original summary.

longwave’s picture

Issue summary: View changes
longwave’s picture

Title: Add void returns for Symfony 7 » [11.x] Add void returns for Symfony 7
Status: Active » Needs review

Needless to say this is for 11.x only.

Spokje’s picture

Status: Needs review » Needs work

Unhappy PHPStan

longwave’s picture

Hm, maybe the debug class loader isn't working like I thought, because we should have been notified already about these subclasses?

Spokje’s picture

, because we should have been notified already about these subclasses?

Not 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?

longwave’s picture

Status: Needs work » Needs review

Re #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 ultimately setContext() belongs to a Symfony interface method that was also tagged @return void.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Wasn'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.

quietone made their first commit to this issue’s fork.

quietone’s picture

Rebase. Only conflict was in DrupalKernel.php and it was straightforward to fix.

Gábor Hojtsy’s picture

Issue tags: +Drupal 11
alexpott’s picture

Status: Reviewed & tested by the community » Fixed

I'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!

  • alexpott committed f4167b86 on 11.x
    Issue #3407159 by longwave, quietone: [11.x] Add void returns for...

Status: Fixed » Closed (fixed)

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