Problem/Motivation

In #3425337: Fix root namespace classes DebugClassLoader forward compatibility warnings we fixed methods to be return typehinted that were reported by DebugClassLoader deprecations.

Some were left over, fix them here.

Proposed resolution

Typehint the methods.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3425537

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:

About issue forks

Comments

mondrake created an issue. See original summary.

mondrake opened merge request !6904

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Active » Needs review

Had to add : void to one __wakeup() call that was being reported, from there I had to add it to all such methods otherwise they would not be covariant; by extension, I added : array to all __sleep() methods.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Applied the MR and searched the repo for "#[\ReturnTypeWillChange]" using phpstorm. All instances appear to be replaced and caused no test failures.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Reviewed & tested by the community » Needs review

Added a fix for #3425337-21: Fix root namespace classes DebugClassLoader forward compatibility warnings

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs review » Postponed

Parent was reverted and is blocked on making PHP 8.3 the default CI environment

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Postponed » Needs review
mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs review » Reviewed & tested by the community

The implementation of the deprecation reporting in #3417066: Upgrade PHPUnit to 10, drop Symfony PHPUnit-bridge dependency is now reporting about these missing fixes, that the current phpunit-bridge is not: https://git.drupalcode.org/issue/drupal-3417066/-/jobs/1012561

I think I can set this back to RTBC since the MR now is exactly as RTBCed in #4.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented 
-  #[\ReturnTypeWillChange]
-  public function asort($flags = SORT_REGULAR): bool {
+  public function asort($flags = SORT_REGULAR): TRUE {

IIRC TRUE return type is PHP 8.2, so we should drop PHP 8.1 support first?

mondrake’s picture

mondrake
Location 🇮🇹
commented

Yes it's PHP 8.2. But aren't we shipping D11 on PHP 8.3? Meaning, haven't we dropped PHP < 8.3 in the branch already? For sure we are testing on PHP 8.3 only.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

We haven't technically done that until #3413268: Add PHP 8.3 requirement to Drupal 11.0.x lands although CI is PHP 8.3 only.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Reviewed & tested by the community » Postponed

If we need to wait for #11 then ok; I wouldn't revert the MR change from bool to TRUE, since it would need to readd #[\ReturnTypeWillChange] to prevent PHP 8.3 to throw its own deprecation.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Related issues: +#3413268: Add PHP 8.3 requirement to Drupal 11.0.x
mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Postponed » Needs work
Issue tags: +Needs reroll

The blocker is in, this needs a rebase

mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue tags: -Needs reroll

Merged with 11.x, no conflicts. Back to RTBC.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs work » Reviewed & tested by the community

  • catch committed aca58777 on 10.3.x 
    Issue #3425537 by mondrake: Fix remaining methods with #[\...

  • catch committed f2f91eb1 on 11.x 
    Issue #3425537 by mondrake: Fix remaining methods with #[\...

  • catch committed 3e8edacc on 10.3.x 
    Revert "Issue #3425537 by mondrake: Fix remaining methods with #[\...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x, thanks!

Ignore the cherry-pick and revert, copy/pasted the wrong commit hash when cherry-picking another commit.

mondrake closed merge request !6904

Status: Fixed » Closed (fixed)

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