fix: PHP 8.4 deprecations with implicit nullable parameters

[Jubeki]

Issue #, if available:
Not available.

Description of changes:
As of PHP 8.4 parameters marked as implicit nullable will throw a deprecation warning. Explicitly Declaring is Supported as of PHP 7.1 (See https://www.php.net/manual/en/migration71.new-features.php). This SDK requires PHP >=7.2.5 so no lower bounds need to be updated.

This PR adds the question mark or null to the parameter types to to avoid the deprecation warning in PHP 8.4.

This was done with the help of laravel/pint and the following configuration in the pint.json:

{
    "preset": "empty",
    "rules": {
        "nullable_type_declaration_for_default_null_value": true
    }
}

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[yenfryherrerafeliz]

Hi @Jubeki, thanks for taking the time for opening this PR. While your suggestion is valid, we can not accept this change right now since it will break customers extending our SDK, and it will force them to also need to apply this change. In other words it is not a backward compatibility change.

Thanks!

[Jubeki]

Hey @yenfryherrerafeliz

the changes in this PR are fully backwards compatible and contain no breaking changes.
See the following article: https://php.watch/versions/8.4/implicitly-marking-parameter-type-nullable-deprecated#change

The explicit declaration is compatible with PHP 7.1 and later, even if you extend the class.

Would you please consider opening this PR again for further review?

Thanks!

[Jubeki]

You can also take a look at the following snippet at PHP Playground at php-play.dev

<?php

class ParentClass {
   public function tester(?string $value = null) {}
}

// This class is compatible after PHP 7.1, because it keeps the null type implicitly.
class SubClass extends ParentClass {
     public function tester(string $value = null) {}
}

// This class is compatible after PHP 7.1, because it keeps the null type explicitly.
class SubClass2 {
   public function tester(?string $value) {}
}

// Fatal Error: This class is not compatible with the parent class even before PHP 8.4,
// because the implicit null type is removed from the declaration.
class SubClass3 extends ParentClass {
     public function tester(string $value) {}
}

// Fatal Error: This class is not compatible with the parent class even before PHP 8.4,
// because the implicit null type is removed from the declaration.
class SubClass4 extends ParentClass {
     public function tester(string $value = 'x') {}
}

[Jubeki]

Hey @yenfryherrerafeliz and @stobrien89,

would you be willing to take another look at this PR, given the above arguments, that this is not a breaking change, but an improvement for all upcoming PHP 8.4 installations?

Thanks!

[puzzledmonkey]

@yenfryherrerafeliz Is there any chance this can get revisited? This needs fixing to support PHP 8.4 and as @Jubeki says it should be entirely backwards-compatible.

[yenfryherrerafeliz]

Hey @Jubeki, sorry, I think I jumped too fast in answering to this. I got biased with the fact that changing method signatures for classes will break any super class relaying on those parent classes. But in this case you are right. It is a backward compatible change. I will do another review to this as soon as possible.

Thanks!

[Jubeki]

Hey @yenfryherrerafeliz, that is fully understandable on my side. I also could have done a better job explaining something about the rfc and the backwards compatibility. I simply assumed everybody would know because I already submitted similar PRs where people already knew that this change would be okay.

Thanks!

[puzzledmonkey]

thanks @Jubeki and @yenfryherrerafeliz !

[stobrien89]

Hi @Jubeki,

Sorry for the confusion— looks good to me. I added a changelog entry and we'll merge after the current CI run. Thanks for the contribution!