Problem/Motivation

Method "Symfony\Component\HttpFoundation\RedirectResponse::setTargetUrl()" will return "static" as of its next major version. Doing the same in child class "Drupal\Component\HttpFoundation\SecuredRedirectResponse" will be required when upgrading.

Steps to reproduce

Proposed resolution

Add the "static" return type hint.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#2 3232113-2.patch676 bytesdaffie
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

daffie created an issue. See original summary.

daffie’s picture

daffie CreditAttribution: daffie commented
Status: Active » Needs review
FileSize
3232113-2.patch676 bytes

I could only find 1 instance where in contrib the method was overridden. See: http://grep.xnddx.ru/node/31687281.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Status: Needs review » Needs work

This is going to be tricky, we need a corresponding issue for commerce donation

http://grep.xnddx.ru/node/31687281

There are several other extensions in contrib, but only commerce donation touches that method.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented

Ah, you already found that one 🤦‍♂️

I think we should file and issue for commerce donation and try to get it fixed there

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Title: [Symfony 6] Add "static" type hint to the method Drupal\Component\HttpFoundation\SecuredRedirectResponse::setTargetUrl() » [Symfony 6] [PP-1] Add "static" type hint to the method Drupal\Component\HttpFoundation\SecuredRedirectResponse::setTargetUrl()
Status: Needs work » Postponed

#3232249: Add static type-hint to DonationSecuredRedirectResponse::setTargetUrl

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented

Pinged the maintainer in slack

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented

static return type is only introduced in PHP 8, so another thing we can't do until 10.x opens I guess.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Spokje’s picture

Spokje
Primary language Dutch
CreditAttribution: Spokje as a volunteer commented
Version: 9.4.x-dev » 10.0.x-dev
longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Postponed » Reviewed & tested by the community

Being bold and marking this RTBC for 10.0.x, as commerce_donation does not yet declare compatibility with Drupal 10.

  • catch committed 1553465 on 10.0.x 
    Issue #3232113 by daffie, larowlan, longwave: [Symfony 6] Add "static"...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Title: [Symfony 6] [PP-1] Add "static" type hint to the method Drupal\Component\HttpFoundation\SecuredRedirectResponse::setTargetUrl() » [Symfony 6] Add "static" type hint to the method Drupal\Component\HttpFoundation\SecuredRedirectResponse::setTargetUrl()
Status: Reviewed & tested by the community » Fixed

Yes I think it's fine to do these in Drupal 10 - contrib modules can safely update their code in Drupal 9 as long as they require PHP 8, or if they conditionally declare the method.

Committed 1553465 and pushed to 10.0.x. Thanks!

Status: Fixed » Closed (fixed)

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