Fix root namespace classes DebugClassLoader forward compatibility warnings

Typehints make sense. Removing from deprecation-ignore didn't cause failures, which I find is a good sign for these kind of tickets.

Nice to see these finally all have their correct types.

Committed ff7c5de and pushed to 11.x. Thanks!

longwave → closed merge request !6887

This is causing unit tests to fail with a deprecation warning https://git.drupalcode.org/project/drupal/-/jobs/978330 Not sure how the pipeline here passed, maybe a cross-commit?

Reverted - let's try again.

mondrake → reopened merge request !6887

Deprecation of the DebugClassLoader are very fragile... also noticed that in 📌 Upgrade PHPUnit to 10, drop Symfony PHPUnit-bridge dependency Needs work , where by filling the gap of symfony-bridge I found more errors reported. The strange thing here is that this appears to have dfferent behaviour in MR testing vs branch testing...

I rebased and checked, the error reported in #11 was fixed in the MR already, so 🤷

Maybe I missed it somehow when I committed?! Trying again...

Should we have another issue to remove all remaining #[ReturnTypeWillChange] in 11.x?

Indeed, some seem not to have been reported... I will check with 📌 Upgrade PHPUnit to 10, drop Symfony PHPUnit-bridge dependency Needs work . Sure we need a followup.

If I run this test locally:

../vendor/bin/phpunit modules/file/tests/src/Functional/FileAddPermissionsUpdateTest.php 

I get

1) Drupal\Tests\file\Functional\FileAddPermissionsUpdateTest::testUpdate
Exception: Deprecated function: Return type of Drupal\Component\Annotation\Doctrine\StaticReflectionClass::getStaticProperties(): ?array should either be compatible with ReflectionClass::getStaticProperties(): array, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice
include()() (Line: 32)

@catch just found this too
I found this issue because for some reason gitlabci started to test with 8.3 when we were testing the Automatic Updates module with core 11.x

In the commit

    -  #[\ReturnTypeWillChange]
    - public function getStaticProperties()
    + public function getStaticProperties(): ?array

I think this will cause an error on PHP 8.3 because
ReflectionClass::getStaticProperties(): ?array was changed to ReflectionClass::getStaticProperties(): array so removing the `ReturnTypeWillChange` actually makes this not compatible with PHP 8.3's return type. I couldn't tell if core itself or this merge request was tested on PHP 8.3(still getting use to the UI) but looks like 11.x should support PHP 8.3, correct?

Notice is right, the signature is public ReflectionClass::getStaticProperties(): array, https://www.php.net/manual/it/reflectionclass.getstaticproperties.php

Will fix it in 📌 Fix remaining methods with #[\ReturnTypeWillChange] attribute RTBC that is still to be committed.

Maybe it's time to enable 8.3 as default for 11.x?

Yep we already agreed 🌱 [11.x] [policy] Require PHP 8.3 for Drupal 11 RTBC we should change this in 11.x now.

I think this needs to be temporarily reverted, and 📌 Make PHP 8.3 the default environment for gitlab CI runs Fixed done first. BTW there’s a sorting test that fails on PHP 8.3 and I think it’s unrelated.

@mondrake was watching your test runs on the other issue and have come to the same conclusion.

Seems OK now after the PHP 8.3 bump in CI

Third time lucky. Now we are testing on PHP 8.3 I think this is finally OK!

Committed f330fa3 and pushed to 11.x. Thanks!

longwave → closed merge request !6887

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