Skip to content

Conversation

@jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Sep 16, 2024

PHP 8.4 deprecates implicitly nullable parameters, i.e. typed parameters with a null default value, which are not explicitly declared as nullable.

As this code base still has a minimum supported PHP version of PHP 5.6, changing these parameters to explicitly nullable is not an option as that syntax was only introduced in PHP 7.1.

With that in mind, I'm proposing to change the default value of the parameters to comply with the type declaration.

Even though this is not a final class, this is not a breaking change for two reasons:

  1. The signature check does not get applied to constructors.
  2. Even if it did, default values can be different between parent vs child, as long as they comply with the expected type.

Ref: https://wiki.php.net/rfc/deprecate-implicitly-nullable-types

PHP 8.4 deprecates implicitly nullable parameters, i.e. typed parameters with a `null` default value, which are not explicitly declared as nullable.

As this code base still has a minimum supported PHP version of PHP 5.6, changing these parameters to explicitly nullable is not an option as that syntax was only introduced in PHP 7.1.

With that in mind, I'm proposing to change the default value of the parameters to comply with the type declaration.

Even though this is not a `final` class, this is not a breaking change for two reasons:
1. The signature check does not get applied to constructors.
2. Even if it did, default values can be different between parent vs child, as long as they comply with the expected type.

Ref: https://wiki.php.net/rfc/deprecate-implicitly-nullable-types
@jrfnl jrfnl requested a review from a team as a code owner September 16, 2024 17:42
@swissspidy swissspidy added this to the 0.12.1 milestone Sep 17, 2024
@swissspidy swissspidy merged commit 46ddc1d into wp-cli:master Sep 17, 2024
@jrfnl jrfnl deleted the feature/php-8.4-fix-implicitly-nullable-params branch September 17, 2024 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants