Extend Symfony DebugClassLoader to report missing cross-module @return types

this makes the bot crash, i don't have 10G or ram on it

Wonder if this counts as an API change?

Or least warrants a CR?

Only moving to NR for the pipeline revert as not sure it's in scope.

If can rebase too since it's kinda older (sorry took so long to look at)

If you are another contributor eager to jump in, please allow the original poster @mondrake at least 48 hours to respond to feedback first, so they have the opportunity to finish what they started!

Probably phpstan-drupal right?

Rebased.

Let's create a first version CR. It will be in draft state and by using the revision log any future edits can be tracked and logged.

I have put some comments on Slack however making them more formal:

As a Contrib maintainer I do not see the value.

For me PHPStan exists to identify bugs in code, not future compatibility problems. Not having a return type when the parent method itself does not have a return type is not a bug.

PHPStan scans your whole codebase and looks for both obvious & tricky bugs. Even in those rarely executed if statements that certainly aren't covered by tests.

-- https://phpstan.org/

Looking at a few of the projects I maintain there is a good chance there will be a different branch for D12 and my current branches will never need to implement a hard return type (although in many cases I already added strong code typing as part of implementing Level 9 PHPStan).

A 'feature' like this will just be noise to me, and distract from working on issues more pressing (security hardening, bug fixes, and features).

My opinion is that #3513474: Consider always having the next major branch open is the better method to implement, it would allow modules to opt-in to scanning against the next major core branch early to identify where they need to make changes, and reduce creating future technical debt (no need to come back in D12 to remove the type-hint warning).

As an aside. I have suggested in Slack that if this is felt to be useful it might be worth considering putting this in its own third party repository to allow it to be available to others as a way for the Drupal Community to "give back" for all it receives. If this does not make sense to be an independent project I would suggest that should be treated as a warning that indicates other methods should be preferred.

Looking at the MR itself: I note the presence of // @phpstan-ignore phpstanApi.method, should code that uses PHPStan off-api be allowed considering the maintenance burden that could create?

Should this be postponed on #3513474: Consider always having the next major branch open

Going to go on a limb and say this may be worth waiting for #14 to be decided.

Could we extend DebugClassLoader to work around the same-namespace limitation? Might have a play with this.

Maybe upstream would accept a configuration option to handle namespace prefixes, but we can also just extend it ourselves - see MR!14882.

This MR was partially written by Claude Code.

@mondrake unit tests are now passing, wondering what you think of this approach before I continue either fixing or ignoring more deprecations?

It's getting hard to draw the line on what to fix here and what to fix in followups for test modules. Maybe we just add all test modules to the skip set and fix them in batches?

Given this is going to be quite disruptive for contrib and custom code, maybe we want to commit to 12.0 or even wait for 12.1, and announce it in some way? (maybe via a core blog post)

I would probably go for 12.0 here, not many modules have updated for 12.x compatibility yet so it's something that can be tackled alongside other changes. Seems a bit too disruptive for 11.4, we'd break a lot of CI pipelines even though sites would be OK.

This unfortunately means we won't be able to actually bring in the return type hint changes everywhere until 13.0 but at least we can prepare everything for then.

Hopefully this is green now. We can pick off the test fixes in followups, then wait for contrib to fix their deprecations, and finally add return types everywhere in the next major.

Good call, looks like we might need a bit more work for the other database drivers.

@mondrake actually the fails in other database drivers all look like false positives.

@quietone thanks for the review, I reworded "module" to "extension" in all cases.

Added an initial change record: https://www.drupal.org/node/3579056

One question about whether any of the test subclass methods should have the {@inheritdoc} annotation to be more representative of what code actually looks like. The detection in the loader ignores that annotation, but maybe a test case can make that clear.

Addressed #38, leaving as RTBC.

Committed 90c0add and pushed to main. Thanks!

Reviewed and published the CR.