[3.x] refactor: Add initial backwards-compatible return types #1763
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
@carlos-granados This is quite a big PR, but hopefully keeping it as separate commits will make it easier to review. |
acoulton
commented
Dec 8, 2025
stof
approved these changes
Dec 8, 2025
7f4cfc1 to
d1cb94c
Compare
Contributor
|
@acoulton I'd like to take a good look at this PR, will try to do it this evening, please don't merge before I check it |
The rector run in CI runs on PHP 8.1, which means it also installs symfony 5.4. In symfony/console 5.4, `getOption` has no strict return type so Rector ignores use of it. Newer symfony explicitly types this as `:mixed`, so running Rector on a later php version (e.g. locally) will by default add a (string) cast to the `explode` statement. However, in practice this is unnecessary because symfony/console guarantees that an option with `InputOption::VALUE_REQUIRED` will give a value of `?string`.
Rules applied: - AddArrowFunctionReturnTypeRector - NullToStrictStringFuncCallArgRector
Rules applied: - BoolReturnTypeFromBooleanConstReturnsRector
These can have a more specific type in the interface phpdoc than we had before, and they can also have strict returns on their final implementations. Also improves the phpdoc type of internal class maps.
Where these are guaranteed to return explicit arrays. Based on the Rector rule but manually applied to improve the phpdoc typing of the results.
Rules applied: - ReturnTypeFromStrictConstantReturnRector
Rules applied: - StringReturnTypeFromStrictScalarReturnsRector
Rules applied: - NumericReturnTypeFromStrictScalarReturnsRector
Rules applied: - BoolReturnTypeFromBooleanStrictReturnsRector
Rules applied: - StringReturnTypeFromStrictStringReturnsRector
Rules applied: - NumericReturnTypeFromStrictReturnsRector - StringReturnTypeFromStrictStringReturnsRector
Rules applied: - ReturnTypeFromStrictTernaryRector
With manual fixes to some phpdoc array/list types Rules applied: - ReturnTypeFromReturnDirectArrayRector
Started off with Rector to apply level 17, then adjusted manually to match interfaces (which Rector does not fully respect as they are only phpdoc). Rules applied: - ReturnTypeFromReturnNewRector
Applied rules: - AddVoidReturnTypeWhereNoReturnRector
Rules applied: - ReturnTypeFromStrictTypedPropertyRector - AddArrowFunctionReturnTypeRector - ExplicitBoolCompareRector
Although these classes are not final, the individual methods are, and therefore it is BC to add a strict return type to them. Note that roave/backward-compatibility-check does not recognise that this is safe. Rules applied: - ReturnTypeFromStrictTypedPropertyRector
Line numbering in RuntimeCallHandler has changed due to removal of now-redundant `@return` tags.
d1cb94c to
4b95d58
Compare
Contributor
Author
I'd be delighted if you did, very happy to wait for your review whenever you get a chance :) |
7 tasks
carlos-granados
approved these changes
Dec 8, 2025
Contributor
carlos-granados
left a comment
There was a problem hiding this comment.
Finished reviewing these changes, outstanding work, many thanks @acoulton!
Contributor
Author
|
This is now updated to revert the temporary Rector config changes. I will squash and release it after we release 3.28.0. |
And make users aware of the upcoming 4.0 release.
Contributor
Author
|
@carlos-granados I have added the changelog for 3.29.0 directly to this branch for simplicity. There are no changes to code (other than reverting the rector config) since you reviewed. |
carlos-granados
approved these changes
Dec 11, 2025
Contributor
carlos-granados
left a comment
There was a problem hiding this comment.
Let's go ahead with this
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a first pass at adding backwards-compatible return types that we can safely merge into the 3.x series, as we work towards #1744.
The return types applied are based on:
@returntype of any interface the method implements, if this was less specific than the type that Rector detected. This is the type Rector would have used if the interface had a strict return type, but we cannot add those until 4.x.I've committed this a level at a time, running phpstan at each commit and resolving any errors it produced (there were a few due to invalid / old-style phpdoc
@returntags for lists / arrays etc that were previously ignored but now don't match the runtime return type.Return types are added to
finalclassesnon-finalclassesfinal publicmethods innon-finalclassesRector mostly limited itself to these rules, but there were a few cases where by default it added types to non-final classes (I think because they are not extended anywhere by us, and the parent method has no strict type). I've excluded those changes.
Note that this depends on #1762 and will be rebased after that is merged.now rebased