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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove |null from union types when null is the default #5785

Closed
nicolas-grekas opened this issue Jun 30, 2021 · 8 comments 路 Fixed by #5819
Closed

Remove |null from union types when null is the default #5785

nicolas-grekas opened this issue Jun 30, 2021 · 8 comments 路 Fixed by #5819
Labels
contribution welcome Core team is looking for a contribution kind/feature request topic/types FQCNs and types used in signatures (class-like, function, property)

Comments

@nicolas-grekas
Copy link

nicolas-grekas commented Jun 30, 2021

I don't remember: do we have a rule to turn an argument like ?foo $bar = null into foo $bat = null, don't we?
We might need one if not :)

And with union types, this rule should also turn foo|bar|null $bar = null into foo|bar $bar = null.

馃檹

@keradus
Copy link
Member

keradus commented Jun 30, 2021

can you share the reasoning behind such rule, @nicolas-grekas ?
frankly speaking, I would rather to the opposite, if argument is optional, I would add ?/null to the typehint, if missing

@nicolas-grekas
Copy link
Author

With union types, signatures are already very long. That CS style makes them a bit shorter without any downside.
Also, that's two possible styles. A CS fixer tool is supposed to normalize, and there are two ways here.

@julienfalque
Copy link
Member

do we have a rule to turn an argument like ?foo $bar = null into foo = null

Did you mean

-?foo $bar = null
+foo $bar = null

?

@nicolas-grekas
Copy link
Author

nicolas-grekas commented Jun 30, 2021

@julienfalque yes, updated

@kubawerlos
Copy link
Contributor

kubawerlos commented Jun 30, 2021

@julienfalque
Copy link
Member

Ah yes, I forgot this rule has an option to make the reverse change. Closing then, thanks @kubawerlos :)

@julienfalque
Copy link
Member

Mmmh actually the rule does not cover the |null part in union types. Maybe we could simply update it.

@julienfalque julienfalque reopened this Jun 30, 2021
nicolas-grekas added a commit to symfony/symfony that referenced this issue Jun 30, 2021
This PR was merged into the 4.4 branch.

Discussion
----------

CS fixes

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

With a rule hinted by `@julienfalque` in PHP-CS-Fixer/PHP-CS-Fixer#5785

Commits
-------

31e1a97 CS fixes
@Wirone Wirone added contribution welcome Core team is looking for a contribution topic/types FQCNs and types used in signatures (class-like, function, property) labels May 15, 2023
@Wirone
Copy link
Member

Wirone commented Jun 25, 2023

@nicolas-grekas it took 2 years, but finally it's done 馃榿. Thank you @HypeMC for this great improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution welcome Core team is looking for a contribution kind/feature request topic/types FQCNs and types used in signatures (class-like, function, property)
Projects
None yet
5 participants