Switched to non-null defaults in exception constructors #40271
Conversation
5b1becb to
b4b3ebe
Compare
| @@ -18,7 +18,7 @@ | |||
| */ | |||
| class FileLoaderImportCircularReferenceException extends LoaderLoadException | |||
| { | |||
| public function __construct(array $resources, int $code = null, \Throwable $previous = null) | |||
| public function __construct(array $resources, ?int $code = 0, \Throwable $previous = null) | |||
There was a problem hiding this comment.
shouldn't we pass $code ?? 0 to the parent?
There was a problem hiding this comment.
This is a decision we have to make. PHP 8.1 deprecates passing null as $code. Do we want to hide that deprecation from the caller? Then we should map null to zero. Or do we want to pass that deprecation down to the caller instead?
Note that all of our exception classes that don't override the constructor will trigger the new deprecation on null arguments, so not mapping null arguments would be more or less consistent behavior.
There was a problem hiding this comment.
ok, I see, let's keep as is then, we can figure out later anyway
There was a problem hiding this comment.
@derrabus we hit this in 4.4 now. I fix it on our side which has to be done anyway, but yet seems unhandled and undecided in this codebase.
There was a problem hiding this comment.
Yes, you have to do it anyway. See my explanation above.
b4b3ebe to
f8e1009
Compare
|
Thank you @derrabus. |
|
Merged in 5.2, in case there are remaining ones there. |
|
I'll check. |
…tructors (derrabus) This PR was merged into the 5.2 branch. Discussion ---------- [Config] Switched to non-null defaults in exception constructors | Q | A | ------------- | --- | Branch? | 5.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | N/A | License | MIT | Doc PR | N/A Follow-up to #40271 on the 5.2 branch. Commits ------- 2e865ac Switched to non-null defaults in exception constructors
…ons (derrabus) This PR was merged into the 5.3-dev branch. Discussion ---------- Deprecate passing null as $message or $code to exceptions | Q | A | ------------- | --- | Branch? | 5.x | Bug fix? | no | New feature? | no | Deprecations? | yes | Tickets | N/A | License | MIT | Doc PR | N/A Follow-up to #40271. Following the example of the PHP core, this PR introduces deprecation warnings that are triggered if a developer attempts to pass null as `$code` or `$message` to an exception constructor. Commits ------- 8e3058d Deprecate passing null as $message or $code to exceptions
PHP 8.1 will trigger a deprecation warning if we pass
nullas$messageor$codeto the constructor of\Exception. However, many of our own exception acceptnullfor those parameters and even use them as default.This is unfortunate because code like the following snippet would trigger that deprecation although the code itself is perfectly fine:
With this PR, I'd like to change our defaults to
''and0while still allowing to passnullfor BC. In a follow-up PR for the 5.x branch, I'd like to deprecate passingnull, matching the future behavior of PHP.This PR also adjust various PHPDoc blocks with inaccurate types.