Problem/Motivation

Method "Symfony\Component\HttpKernel\HttpKernelInterface::handle()" will return "Response" as of its next major version.

Steps to reproduce

Proposed resolution

Add the "Response" return type hint.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

daffie created an issue. See original summary.

daffie’s picture

Title: [Symfony 6] Add "Response" type hint to methods that implement the method ::handle() » [Symfony 6] Add "Response" type hint to methods that implement the method Symfony\Component\HttpKernel\HttpKernelInterface::handle()
daffie’s picture

Status: Active » Needs review
FileSize
11.13 KB

The problem for adding this return type hint is that a lot of contrib modules override the method. See: http://grep.xnddx.ru/search?text=public+function+handle%28&filename=. Maybe we need to wait for D10.0 to commit this patch.

Status: Needs review » Needs work

The last submitted patch, 3: 3232082-3.patch, failed testing. View results

catch’s picture

Looks like we're returning NULL in some places from the test results, should we open another issue to fix those in 9.x?

daffie’s picture

Version: 9.3.x-dev » 10.0.x-dev
Priority: Normal » Critical
Issue tags: +Drupal 10
FileSize
3.72 KB
14.85 KB

Looks like we're returning NULL in some places from the test results, should we open another issue to fix those in 9.x?

The problem is that to fix the tests you also have to add the return type hinting. The required test changes are in the interdiff patch file and they will fail. Therefor for this and the overrides in contrib, I am moving this to 10.0.

This issue is part of the Symfony 6 stuff and that is all critical.

longwave’s picture

Version: 10.0.x-dev » 9.3.x-dev
Status: Needs work » Needs review

For this to be problematic to contrib, they have to both implement this method *and* extend one of our classes. From a random sample of the results in #3 they either implement HttpKernelInterface directly (in which case they also need to add return types, but it doesn't stop us from doing so beforehand) or are false positives (the method signature is different). I didn't see anything that extends Drupal core code directly.

Therefore moving this back to 9.x for review.

The last submitted patch, 6: interdiff-3232082-3-6.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 6: 3232082-6.patch, failed testing. View results

daffie’s picture

Status: Needs work » Needs review
FileSize
2.73 KB
15.86 KB

Fixed the testbot failures.

Status: Needs review » Needs work

The last submitted patch, 10: 3232082-10.patch, failed testing. View results

daffie’s picture

Status: Needs work » Needs review
longwave’s picture

Status: Needs review » Reviewed & tested by the community
alexpott’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: +Needs change record updates

Committed e147702 and pushed to 9.3.x. Thanks!

Thanks for checking contrib and documenting your checks @longwave.

I think we might need a single CR to collect all the SF6 preparations together?

  • alexpott committed e147702 on 9.3.x
    Issue #3232082 by daffie, longwave, catch: [Symfony 6] Add "Response"...
catch’s picture

There's a CR here: https://www.drupal.org/node/3236232

I've added this issue.

Status: Fixed » Closed (fixed)

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