Rule nullable_type_declaration_for_default_null_value should remove default value on non-optional arguments

[nicolas-grekas]

Bug report

When I applied this rule to the Symfony codebase, everything worked really well except for non-optional arguments with a null default.

For those, I had to patch them manually, see
symfony/symfony@fb9fa26

This means that in the context of https://wiki.php.net/rfc/deprecate-implicitly-nullable-types, php-cs-fixer is currently not totally safe to prepare a codebase to PHP 9. (The reason being that non-optional arguments with default values are also deprecated).

nullable_type_declaration_for_default_null_value should remove those non-optional arguments with a null default, so that upgrading is made super easy.

Runtime version

latest

Used command

php-cs-fixer fix --rules nullable_type_declaration_for_default_null_value

[kubawerlos]

@nicolas-grekas do you mean that nullable_type_declaration_for_default_null_value should do, what currently no_unreachable_default_argument_value does?

[nicolas-grekas]

Something like that yes, but specific to null defaults. Because otherwise nullable_type_declaration_for_default_null_value is not safe since it will generate code that will trigger new deprecations, see https://3v4l.org/nSpos

[nicolas-grekas]

I also posted php/php-src#13353

[nicolas-grekas]

From the end user pov, the current behavior is risky because it turns
a non-deprecated syntax: (int $a = null, $b)
into a deprecated one: (?int $a = null, $b).

[SVGAnimate]

@nicolas-grekas I believe in the example below there is an update to present.

PHP 5	PHP 7	PHP 8
<?php function ($a = null, $b) { }		function (?$a, $b) { }
	<?php function (int $a = null, $b) { }	function (?int $a, $b) { }

In PHP5 nullable parameter is supported thusfunction foo($a = null, $b){}
In PHP7 the nullable parameter is a mixed type ( union of int|float|string|array|object|null )
In PHP8 nullable parameter is supported thus function foo(?$a, $b){}

[github-actions]

Since this issue has not had any activity within the last 90 days, I have marked it as stale.

The purpose of this action is to enforce backlog review once in a while. This is mostly for maintainers and helps with keeping repository in good condition, because stale issues and PRs can accumulate over time and make it harder for others to find relevant information. It is also possible that some changes has been made to the repo already, and issue or PR became outdated, but wasn't closed for some reason. This action helps with periodic review and closing of such stale items in automated way.

You may let maintainers handle this or verify current relevancy by yourself, to help with re-triage. Any activity will remove stale label so it won't be automatically closed at this point.

I will close it if no further activity occurs within the next 30 days.