Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP 8.4 support (Implicitly marking parameter $request as nullable is deprecated, the explicit nullable type must be used instead) #3006

Closed
wants to merge 2 commits into from

Conversation

puzzledmonkey
Copy link

This PR adds support for PHP8.4, which has changed how we need to define nullable function parameters - they must be marked explicitly now (i.e. Service|null $service = null instead of just Service $service = null)

I've tried to minimise the code changes and stick with the code style (i.e. Service|null instead of ?Service) of the original files but please let me know if there is anything you would like to to fix/revisit/explain more.

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

@Jubeki
Copy link
Contributor

Jubeki commented Sep 30, 2024

@puzzledmonkey Support for Union Types was only added in PHP 8.0:
See https://www.php.net/manual/en/language.types.declarations.php

This library supports PHP >= 7.2.5.

To support PHP 8.4 and keep backwards compatibility you would need the ?, which was added in PHP 7.1. I also attempted a PR #2995 with that in mind, so maybe that PR can be reviewed, or a new one be attempted.

For the maintainers:
The explicit nullable type (?) is 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.

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') {}
}

@puzzledmonkey
Copy link
Author

Sorry @Jubeki I didn't spot your PR for some reason (I did search!) so I'll close this one now.

@Jubeki
Copy link
Contributor

Jubeki commented Sep 30, 2024

@puzzledmonkey you can keep your PR open. My PR was closed, so it would be great if another PR is considered.

@puzzledmonkey puzzledmonkey reopened this Sep 30, 2024
@puzzledmonkey
Copy link
Author

This PR also now uses the backwards-compatible ? syntax rather than |null, and this should therefore mean this PR has no breaking changes at all, and just adds forwards-compatibility for PHP 8.4

@yenfryherrerafeliz
Copy link
Contributor

Hi @puzzledmonkey, thanks for your contribution. I will be closing this PR in favor of this one which was opened a bit earlier for addressing this. Please track any further updates there.

Thanks!

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.

3 participants