Repository navigation
null default-value promoted class parameter before required parameter is silently ignored #11488
Description
Description
The following code:
<?php
class Foobar {
public function __construct(
public string|null $a = null,
public $b,
) {}
}
new Foobar(b: 'x');Resulted in this output:
PHP Fatal error: Uncaught ArgumentCountError: Foobar::__construct(): Argument #1 ($a) not passed
But I expected this output instead:
PHP Deprecated: Optional parameter $a declared before required parameter $b is implicitly treated as a required parameter
PHP Fatal error: Uncaught ArgumentCountError: Foobar::__construct(): Argument #1 ($a) not passed
To explain; $a does not end up being optional, and if you perform reflection on the parameter and/or property, neither parameter/property has a default-value. So PHP is silently ignoring the user-specified behavior, which only surfaces once you do the call to the function (or as it is sometimes in our case, it doesn't show up at all because some objects are created via reflection).
Further information
Normal functions and methods support "poor mans nullable types" for now, which makes the above case a weird quirk of the backwards compatibility, as noted by @KapitanOczywisty in this post #11485 (comment).
However, constructors with parameter/property promotion behave in a stricter manner.
class C1 {
public function __construct(
public string|null $a = null,
public $b,
) {}
}
class C2 {
public function __construct(
public string|null $a = "a",
public $b,
) {}
}
class C3 {
public function __construct(
public string $a = null,
public $b,
) {}
}
class C4 {
public function __construct(
public string $a = "a",
public $b,
) {}
}
class C5 {
public function __construct(
public $a = null,
public $b,
) {}
}- C1 emits no error, but silently makes the parameter/property mandatory.
- C2 emits "Deprecated: Optional parameter $a declared before required parameter $b is implicitly treated as a required parameter".
- C3 emits "Fatal error: Cannot use null as default value for parameter $a of type string".
- C4 emits "Deprecated: Optional parameter $a declared before required parameter $b is implicitly treated as a required parameter".
- C5 emits "Deprecated: Optional parameter $a declared before required parameter $b is implicitly treated as a required parameter".
C3 proves that poor mans nullable types aren't intended to apply to constructors with typed promoted parameters/properties like they do for normal parameters, and C5 proves that it's deprecated for them regardless. And C2 and C4 proves that optional parameters aren't allowed before required parameters and emits a deprecation.
However, C1 emits no warning at all, even though parameter/property doesn't end up having a null default-value, nor is it optional. So C1 behaves identically to not specifying a default-value at all, whereas the only purpose of specifying the null default-value is to make it optional, which it isn't.
So it definitely seems like C1 should emit a deprecation as well.
PHP Version
PHP 8.2.7
Operating System
No response
Activity
KapitanOczywisty commentedon Jun 20, 2023
Deprecation notice would be added before Fatal error, so expected result should be:
syranide commentedon Jun 20, 2023
Fair point, in-terms of clarity, I'll update it. I was debating not including the function call at all, but it helps illustrate the context better.
iluuu1994 commentedon Jun 21, 2023
I was about to classify this as a feature, but noticed that this already works correctly for
?string, but not forstring|null(https://3v4l.org/uvbF7). The reason is that thenullcheck on the type declaration is AST based, and naively assumes only?stringexists. We should instead delay this check to type compilation.php-src/Zend/zend_compile.c
Lines 6958 to 6960 in 6c015ae
Constructor property promotion fails for a different reason, as stated in the RFC:
https://wiki.php.net/rfc/constructor_promotion
The RFC requires an explicit
?to avoid confusion on whether property type includesnullor not.iluuu1994 commentedon Jun 21, 2023
#11497 I still targeted master for this. The fix was not completely trivial, the bug is non-crucial and 8.3 will still allow fixing the error before removal (if that's even planned).
Fix missing "Optional parameter before required" deprecation on union…