Add return types everywhere possible #42496
Closed
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
49343ed to
9194a17
Compare
This was referenced Aug 12, 2021
9194a17 to
36d294a
Compare
5188230 to
49f871f
Compare
49f871f to
04e44c7
Compare
This was referenced Aug 12, 2021
bb11086 to
a95fd2b
Compare
nicolas-grekas
added a commit
that referenced
this pull request
Aug 13, 2021
This PR was merged into the 6.0 branch. Discussion ---------- Add return types - batch 1/n | Q | A | ------------- | --- | Branch? | 6.0 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | #40154 | License | MIT | Doc PR | - Extracted from #42496 for components that shouldn't be extended very often. Commits ------- 1564887 Add return types - batch 1/n
nicolas-grekas
added a commit
that referenced
this pull request
Aug 13, 2021
This PR was merged into the 6.0 branch. Discussion ---------- Add return types - batch 2/n | Q | A | ------------- | --- | Branch? | 6.0 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | #40154 | License | MIT | Doc PR | - Extracted from #42496 for components that shouldn't be extended very often. Commits ------- 606775c Add return types - batch 2/n
nicolas-grekas
added a commit
that referenced
this pull request
Aug 13, 2021
This PR was merged into the 6.0 branch. Discussion ---------- Add return types - batch 3/n | Q | A | ------------- | --- | Branch? | 6.0 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | #40154 | License | MIT | Doc PR | - Extracted from #42496 for components that shouldn't be extended very often. Commits ------- aef9069 Add return types - batch 3/n
nicolas-grekas
added a commit
that referenced
this pull request
Aug 13, 2021
This PR was merged into the 6.0 branch. Discussion ---------- Add return types - batch 4/n | Q | A | ------------- | --- | Branch? | 6.0 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | #40154 | License | MIT | Doc PR | - Extracted from #42496 Not all possible return types are patched for the attached components, to save breaking BC cross-components, for now at least. Commits ------- bad9b7d Add return types - batch 4/n
nicolas-grekas
added a commit
that referenced
this pull request
Aug 13, 2021
This PR was merged into the 6.0 branch. Discussion ---------- Add return types - batch 5/n | Q | A | ------------- | --- | Branch? | 6.0 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | #40154 | License | MIT | Doc PR | - Extracted from #42496 Not all possible return types are patched for the attached components, to save breaking BC cross-components, for now at least. Commits ------- 00f08e4 Add return types - batch 5/n
a95fd2b to
fcc06c8
Compare
nicolas-grekas
added a commit
that referenced
this pull request
Aug 16, 2021
This PR was merged into the 6.0 branch. Discussion ---------- Add return types - batch 6/n | Q | A | ------------- | --- | Branch? | 6.0 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | #40154 | License | MIT | Doc PR | - Extracted from #42496 Not all possible return types are patched for the attached components, to save breaking BC cross-components, for now at least. Commits ------- df6b42e Add return types - batch 6/n
2069380 to
6536675
Compare
nicolas-grekas
added a commit
that referenced
this pull request
Aug 19, 2021
This PR was merged into the 6.0 branch. Discussion ---------- [Serializer] add return types | Q | A | ------------- | --- | Branch? | 6.0 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | #40154 | License | MIT | Doc PR | - Extracted from #42496 Not all possible return types are patched for the attached components, to save breaking BC cross-components, for now at least. Serializer's test fails. Help wanted 🙏 Commits ------- b2090d0 [Serializer] add return types
nicolas-grekas
added a commit
that referenced
this pull request
Aug 20, 2021
This PR was merged into the 6.0 branch. Discussion ---------- [Security] add return types | Q | A | ------------- | --- | Branch? | 6.0 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | #40154 | License | MIT | Doc PR | - Extracted from #42496 Not all possible return types are patched for the attached components, to save breaking BC cross-components, for now at least. Commits ------- 8871327 [Security] add return types
4a1506b to
d7fbc2c
Compare
|
This PR is the last remaining one on the return type topic \o/ Unless I made a mistake, adding any of the return types here will break a cross-versions dependency between Symfony components. I'm going to need help to decide, case by case, one of those:
Help wanted. |
f4ae43d to
ea96144
Compare
78cc0f5 to
19f28cb
Compare
nicolas-grekas
added a commit
that referenced
this pull request
Aug 25, 2021
…o native types (nicolas-grekas) This PR was merged into the 6.0 branch. Discussion ---------- [CI] Ensure that all possible ``@return`` are turned into native types | Q | A | ------------- | --- | Branch? | 6.0 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | #40154 | License | MIT | Doc PR | - This PR is the final step to close the topic of return types for Symfony 6. It replaces #42496 by *not* adding return types to the classes/interfaces that break cross-component deps. It enforces that all ``@return`` from 5.4 are turned into native types in 6.0, with the known list of exceptions from #42496. Commits ------- 2b5cd7a [CI] Ensure that all possible ``@return`` are turned into native types
19f28cb to
21146be
Compare
|
Continued in #42735 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 patch is generated by
DebugClassLoaderwith a few manual tweaks when tests failed. These tweaks were isolated in #42490 for 5.4 when applicable.As of now, tests pass for all components in no-deps mode, but for
Security/Http,Security/CoreandSecurityBundle, mostly because #41613 should be merged first. Tests onSerializerdon't pass either yet (help welcome.)As a first step, I'd like we make these no-deps jobs green.
Then we'll be left with cross versions tests (high-deps/low-deps). Because adding return types is a BC break, I expect these cross versions tests to require us to break compatibility between 5.4 and 6.0 components. I anticipate that in some cases, adding all possible return types will hit the community too hard. That's why I think we should review case-by-case before marking a component as incompatible with another. Instead, we could decide to not add the corresponding return type. The canonical example of this is
Command::execute(): I'm not sure adding the native return type in 6.0 is worth the cost it will create. We might better wait for 7.0 to add it.