Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add union types #41424

Closed
wants to merge 1 commit into from
Closed

Add union types #41424

wants to merge 1 commit into from

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented May 27, 2021

Q A
Branch? 6.0
Bug fix? no
New feature? no
Deprecations? no
Tickets -
License MIT
Doc PR -

Created using https://github.com/rectorphp/rector/ and manual editing (a lot of them. ;))

For the record, I'm using this command to inspect a specific component:
grep -l 'function [a-zA-Z_].*(\([$]\|.*, [$]\)' -- $(find src/Symfony/Component/Messenger -type f|grep Tests -v)

  • src/Symfony/Bridge/Doctrine/
  • src/Symfony/Bridge/Monolog/
  • src/Symfony/Bridge/ProxyManager/
  • src/Symfony/Bridge/Twig/
  • src/Symfony/Bundle/DebugBundle/
  • src/Symfony/Bundle/FrameworkBundle/
  • src/Symfony/Bundle/SecurityBundle/
  • src/Symfony/Bundle/TwigBundle/
  • src/Symfony/Bundle/WebProfilerBundle/
  • src/Symfony/Component/Asset/
  • src/Symfony/Component/BrowserKit/
  • src/Symfony/Component/Cache/
  • src/Symfony/Component/Config/
  • src/Symfony/Component/CssSelector/
  • src/Symfony/Component/Console/
  • src/Symfony/Component/DependencyInjection/
  • src/Symfony/Component/DomCrawler/
  • src/Symfony/Component/Dotenv/
  • src/Symfony/Component/ErrorHandler/
  • src/Symfony/Component/EventDispatcher/
  • src/Symfony/Component/ExpressionLanguage/
  • src/Symfony/Component/Form/
  • src/Symfony/Component/Filesystem/
  • src/Symfony/Component/Finder/
  • src/Symfony/Component/HttpClient/
  • src/Symfony/Component/HttpFoundation/
  • src/Symfony/Component/HttpKernel/
  • src/Symfony/Component/Intl/
  • src/Symfony/Component/Ldap/
  • src/Symfony/Component/Lock/
  • src/Symfony/Component/Mailer/
  • src/Symfony/Component/Messenger/
  • src/Symfony/Component/Mime/
  • src/Symfony/Component/Notifier/
  • src/Symfony/Component/OptionsResolver/
  • src/Symfony/Component/PasswordHasher/
  • src/Symfony/Component/Process/
  • src/Symfony/Component/PropertyAccess/
  • src/Symfony/Component/PropertyInfo/
  • src/Symfony/Component/RateLimiter/
  • src/Symfony/Component/Routing/
  • src/Symfony/Component/Runtime/
  • src/Symfony/Component/Security/
  • src/Symfony/Component/Semaphore/
  • src/Symfony/Component/Serializer/
  • src/Symfony/Component/Stopwatch/
  • src/Symfony/Component/String/
  • src/Symfony/Component/Templating/
  • src/Symfony/Component/Translation/
  • src/Symfony/Component/Uid/
  • src/Symfony/Component/Validator/
  • src/Symfony/Component/VarDumper/
  • src/Symfony/Component/VarExporter/
  • src/Symfony/Component/WebLink/
  • src/Symfony/Component/Workflow/
  • src/Symfony/Component/Yaml/
  • src/Symfony/Contracts/

@kaznovac
Copy link
Contributor

just minor consistency some union definitions have spaces around the pipe character, but some don't

@ro0NL
Copy link
Contributor

ro0NL commented May 27, 2021

is there a CS rule?

Copy link
Member

@wouterj wouterj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fyi, I've reviewed up to the Console component.

This PR does not only introduce union types, but also some object, string and bool types. Is that intended?

@@ -92,7 +92,7 @@ public static function createChoiceName(object $choice, $key, string $value): st
* @internal This method is public to be usable as callback. It should not
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated, but this one, the one above and the one in EntityType below can also be made private in Symfony 5.4 I think. The required public visibility is from 6 years ago, when $this inside a closure was not yet supported (3172d73).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct, anyone up for a PR doing so?

src/Symfony/Bundle/FrameworkBundle/KernelBrowser.php Outdated Show resolved Hide resolved
src/Symfony/Bridge/Doctrine/Form/Type/EntityType.php Outdated Show resolved Hide resolved
src/Symfony/Bridge/Twig/Mime/NotificationEmail.php Outdated Show resolved Hide resolved
src/Symfony/Component/Cache/Traits/RedisTrait.php Outdated Show resolved Hide resolved
src/Symfony/Component/Console/Helper/ProcessHelper.php Outdated Show resolved Hide resolved
@nicolas-grekas
Copy link
Member Author

Thank you all and @wouterj especially for the review. This is going to take ages to get right. The more eyes the better :)

Copy link
Member

@wouterj wouterj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some more :)

src/Symfony/Component/HttpClient/HttpClientTrait.php Outdated Show resolved Hide resolved
src/Symfony/Component/Form/ChoiceList/ChoiceList.php Outdated Show resolved Hide resolved
src/Symfony/Component/Filesystem/Filesystem.php Outdated Show resolved Hide resolved
src/Symfony/Component/Filesystem/Filesystem.php Outdated Show resolved Hide resolved
src/Symfony/Component/ExpressionLanguage/TokenStream.php Outdated Show resolved Hide resolved
src/Symfony/Component/ErrorHandler/ErrorHandler.php Outdated Show resolved Hide resolved
src/Symfony/Component/DomCrawler/Field/ChoiceFormField.php Outdated Show resolved Hide resolved
nicolas-grekas added a commit that referenced this pull request May 28, 2021
This PR was merged into the 6.0 branch.

Discussion
----------

[Yaml] add types to all arguments

| Q             | A
| ------------- | ---
| Branch?       | 6.0
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Extracted from #41424

Commits
-------

129411e [Yaml] add types to all arguments
nicolas-grekas added a commit that referenced this pull request May 28, 2021
This PR was merged into the 6.0 branch.

Discussion
----------

[Workflow] add types to all arguments

| Q             | A
| ------------- | ---
| Branch?       | 6.0
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Extracted from #41424

Commits
-------

6a94b77 [Workflow] add types to all arguments
nicolas-grekas added a commit that referenced this pull request May 28, 2021
This PR was merged into the 6.0 branch.

Discussion
----------

[VarDumper] add types to all arguments

| Q             | A
| ------------- | ---
| Branch?       | 6.0
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Extracted from #41424

Commits
-------

e68b368 [VarDumper] add types to all arguments
nicolas-grekas added a commit that referenced this pull request Jun 29, 2021
This PR was merged into the 6.0 branch.

Discussion
----------

Add more types

| Q             | A
| ------------- | ---
| Branch?       | 6.0
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Related to #41424

Commits
-------

c21a7be Add more types
nicolas-grekas added a commit that referenced this pull request Jun 29, 2021
This PR was merged into the 6.0 branch.

Discussion
----------

[Config] Add parameter types

| Q             | A
| ------------- | ---
| Branch?       | 6.0
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #41424
| License       | MIT
| Doc PR        | N/A

This PR adds parameter types to all methods of the Config component.

Commits
-------

e7e5256 [Config] Add parameter types
nicolas-grekas added a commit that referenced this pull request Jun 29, 2021
This PR was merged into the 6.0 branch.

Discussion
----------

[DependencyInjection] add union types

| Q             | A
| ------------- | ---
| Branch?       | 6.0
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #41424
| License       | MIT
| Doc PR        | -

Commits
-------

e017977 [DependencyInjection] add union types
nicolas-grekas added a commit that referenced this pull request Jun 30, 2021
This PR was merged into the 6.0 branch.

Discussion
----------

[HttpFoundation] add union types

| Q             | A
| ------------- | ---
| Branch?       | 6.0
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #41424
| License       | MIT
| Doc PR        | -

Commits
-------

9f4a5fb [HttpFoundation] add union types
nicolas-grekas added a commit that referenced this pull request Jul 1, 2021
This PR was merged into the 6.0 branch.

Discussion
----------

[FrameworkBundle] add union types

| Q             | A
| ------------- | ---
| Branch?       | 6.0
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #41424
| License       | MIT
| Doc PR        | -

Commits
-------

e85e459 [FrameworkBundle] add union types
nicolas-grekas added a commit that referenced this pull request Jul 1, 2021
This PR was merged into the 6.0 branch.

Discussion
----------

[Security] add union types

| Q             | A
| ------------- | ---
| Branch?       | 6.0
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #41424
| License       | MIT
| Doc PR        | -

Commits
-------

24e042c [Security] add union types
nicolas-grekas added a commit that referenced this pull request Jul 1, 2021
This PR was merged into the 6.0 branch.

Discussion
----------

[Serializer] add union types

| Q             | A
| ------------- | ---
| Branch?       | 6.0
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #41424
| License       | MIT
| Doc PR        | -

Commits
-------

4db6d56 [Serializer] add union types
nicolas-grekas added a commit that referenced this pull request Jul 2, 2021
This PR was merged into the 6.0 branch.

Discussion
----------

[Console] add union types

| Q             | A
| ------------- | ---
| Branch?       | 6.0
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #41424
| License       | MIT
| Doc PR        | -

Commits
-------

66b6865 [Console] add union types
@nicolas-grekas
Copy link
Member Author

Closing as the only remaining changes are for Cache, and there is already #41290.
Tasks completed 🎉

@nicolas-grekas nicolas-grekas deleted the unions branch July 6, 2021 09:02
nicolas-grekas added a commit that referenced this pull request Jul 7, 2021
This PR was merged into the 6.0 branch.

Discussion
----------

[Cache] Leverage union types

| Q             | A
| ------------- | ---
| Branch?       | 6.0
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #41424
| License       | MIT
| Doc PR        | -

Commits
-------

3275859 [Cache] Leverage union types
nicolas-grekas added a commit that referenced this pull request Jul 7, 2021
This PR was squashed before being merged into the 6.0 branch.

Discussion
----------

[Form] add union types

| Q             | A
| ------------- | ---
| Branch?       | 6.0
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #41424
| License       | MIT
| Doc PR        | -

Commits
-------

0480be6 [Form] add union types
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Help wanted Issues and PRs which are looking for volunteers to complete them. Status: Needs Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants