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 a rule to force-add or force-remove ? on nullable types #3883

Closed
nicolas-grekas opened this issue Jul 6, 2018 · 4 comments
Closed

Add a rule to force-add or force-remove ? on nullable types #3883

nicolas-grekas opened this issue Jul 6, 2018 · 4 comments

Comments

@nicolas-grekas
Copy link

Sorry if this already exists, I didn't find it with a quick lookup,

Since PHP 7.1, nullable arguments can be written with or without the ? prefix:
Form 1: function (string $s = null) {}
vs
Form 2: function (?string $s = null) {}

It would be great to have a rule to normalize to either form 1 or 2.
(On Symfony, we would stick to Form 1 for consistency with branches that still support PHP 5.5)

@SpacePossum
Copy link
Contributor

Hi,
Sounds like a good idea, we currently don't have a rule for this nor an open request for it, so all good 👍

@keradus
Copy link
Member

keradus commented Jul 6, 2018

note that
function (?string $s = null, $b) {}
shall be later autofixed into
function (?string $s, $b) {}

so, I would even say
Form 3: function (?string $s) {}

(as often, = null was provided not with intention to make argument optional (in terms of number of args), but to make it nullable)

@gharlan
Copy link
Contributor

gharlan commented Jul 12, 2018

@keradus I think for this case the NoUnreachableDefaultArgumentValueFixer should be updated.

https://github.com/FriendsOfPHP/PHP-CS-Fixer/blob/2.12/src/Fixer/FunctionNotation/NoUnreachableDefaultArgumentValueFixer.php#L102

isTypehintedNullableVariable should be replaced by
isNonNullableTypehintedNullableVariable

keradus added a commit that referenced this issue Jul 20, 2018
…for nullable typehints (gharlan, keradus)

This PR was squashed before being merged into the 2.13-dev branch (closes #3914).

Discussion
----------

NoUnreachableDefaultArgumentValueFixer - remove `null` for nullable typehints

#3883 (comment)

Commits
-------

dad732f NoUnreachableDefaultArgumentValueFixer - remove `null` for nullable typehints
SpacePossum added a commit that referenced this issue Oct 8, 2019
…peMC)

This PR was squashed before being merged into the 2.16-dev branch (closes #4401).

Discussion
----------

Add NullableTypeDeclarationForDefaultNullValueFixer

Resolves #3883

Commits
-------

0dffadb Add NullableTypeDeclarationForDefaultNullValueFixer
@keradus
Copy link
Member

keradus commented Oct 21, 2019

Closing due to merge of #4401

@keradus keradus closed this as completed Oct 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants