Report error for non-nullable typed parameters with null as default value

[Majkl578]

This is a feature request to abolish legacy syntax:

public function bar(int $baz = null): void

this code should contain proper and explicit nullable type:

public function bar(?int $baz = null): void

https://phpstan.org/r/ebb01a22d284b66db7a131b376fe821c - no errors

This code is currently functionally valid, both in PHP and PHPStan. But it's broken semantically. Making the type nullable by assigning null as default value should be considered a bug in 7.1+, it has ever appeared only because 7.0 had no nullable types.

I have already had multiple discussions about this with multiple people. Basically the concensus has always been in favor of explicit nullable since omitting it may introduce unintended LSP violations, especially in inheritance (i.e. you change default value from null to a number in a subclass and end up with violated LSP).

I know this can be done i.e. by CodeSniffer (Slevomat CS has a sniff aleady), but I think this should be checked in PHPStan (as well).
It should probably go somewhere around level 1.

[ondrejmirtes]

Hi, I don’t think it’s a bug, it’s not worth detecting and reporting. ? => nullable = null => nullable and optional Although I prefer adding ? for consistency, there’s no difference for PHP when the code is executed. There’s no reason this should be reported by PHPStan core.

On Thu, 14 Dec 2017 at 04:18, Michael Moravec ***@***.***> wrote: This is a feature request to abolish legacy syntax: public function bar(int $baz = null): void this code should contain proper and explicit nullable type: public function bar(?int $baz = null): void https://phpstan.org/r/ebb01a22d284b66db7a131b376fe821c - no errors This code is currently functionally valid, both in PHP and PHPStan. But it's broken semantically. Making the type nullable by assigning null as default value should be considered a bug in 7.1+, it has ever appeared only because 7.0 had no nullable types. I have already had multiple discussions about this with multiple people. Basically the concensus has always been in favor of explicit nullable since omitting it may introduce unintended LSP violations, especially in inheritance (i.e. you change default value from null to a number and end up with violated LSP). I know this can be done i.e. by CodeSniffer (Slevomat CS has a sniff aleady), but I think this should be checked in PHPStan (as well). It should probably go somewhere around level 1. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#702>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAGZuEif14CvIld034R2x5_zJ8eehjH0ks5tAJOCgaJpZM4RBfV_> .

-- Ondřej Mirtes

[Majkl578]

Please read again what I wrote.

[ondrejmirtes]

I don't agree with this statement:

Making the type nullable by assigning null as default value should be considered a bug in 7.1+

AFAIK there's nothing like this in the related RFC.

[Majkl578]

Well I thought PHPStan aims to make PHP world better. I was wrong apparently. Thanks.