[BUGFIX] Ensure PHP 8.4 compatibility for nullable parameters #677
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This is the backport of #643. |
|
Another note: This blocks PHP 8.4 support for Emogrifier: MyIntervals/emogrifier#1278 |
oliverklee
force-pushed
the
backport/nullable-paramter
branch
2 times, most recently
from
c6c9f34 to
e421dd4
Compare
JakeQZ
requested changes
Aug 27, 2024
Comment on lines
+60
to
+64
| * @param OutputFormat $oOutputFormat | ||
| * | ||
| * @return string | ||
| */ | ||
| public function render(OutputFormat $oOutputFormat) | ||
| public function render($oOutputFormat) |
There was a problem hiding this comment.
This doesn't look right. It seems to be reverting a previous change.
There was a problem hiding this comment.
Yes, it's removing the type declaration.
The reason for this that there are two possible ways to fix the implicit nullable parameters:
- make the parameters explicitly nullable using a native type declaration (which is not possible with PHP 5.6, which we support in this branch)
- drop the native type declaration for this parameter and use PHPDoc type annotations instead
I'll create a separate pre-PR to do 2. first (without changing the type).
There was a problem hiding this comment.
There is another way, which is to make a 9.0 release with where we're at. The only breaking change I'm aware of is requiring PHP >= 7.2.
oliverklee
force-pushed
the
backport/nullable-paramter
branch
from
e421dd4 to
39a7c72
Compare
Ensure that parameters that have a `null` default value also have `null` as part of their type annotations. Also remove native type declarations for nullable parameters (as nullable parameters are not possible with some of the PHP versions we currently support). https://php.watch/versions/8.4/implicitly-marking-parameter-type-nullable-deprecated Fixes #634
oliverklee
force-pushed
the
backport/nullable-paramter
branch
from
39a7c72 to
f65d376
Compare
|
According to #701, this change does not seem to be necessary after all. Closing. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ensure that parameters that have a
nulldefault value also havenullas part of their type annotations.Also remove native type declarations for nullable parameters (as nullable parameters are not possible with some of the PHP versions we currently support).
https://php.watch/versions/8.4/implicitly-marking-parameter-type-nullable-deprecated
Fixes #634