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

Properly support PHP 8.4 #49

Merged
merged 4 commits into from
Jan 19, 2025
Merged

Properly support PHP 8.4 #49

merged 4 commits into from
Jan 19, 2025

Conversation

Spea
Copy link
Contributor

@Spea Spea commented Jan 17, 2025

PHP 8.4 deprecated types that are implicitly marked as nullable so the following should be changed 👇

function foo(string $string = null) {}
// becomes
function foo(?string $string = null) {}

To fix this in the current code, I updated php-cs-fixer to the latest version v3.68.1 which also includes the nullable_type_declaration_for_default_null_value rule that takes care of this change.

The second commit contains all the changes that were done by executing composer cs-fixer

Spea added 2 commits January 17, 2025 15:29
With the latest version we can ensure, that typed properties which also have a default `null` value will also be marked explicitly as nullable.
E.g.

```php
function foo(string $string = null) {}
```

becomes

```php
function foo(?string $string = null) {}
```

This is necessary, as PHP 8.4 will otherwise trigger a deprecation message if the type is not marked properly as nullable.
Some unnecessary rules that come with this new version were also deactivated, as they do not make a lot of sense in this project.
@dbu
Copy link
Member

dbu commented Jan 17, 2025

thanks!
this is no longer compatible with php 7.4 - but that one is so old that it is fine to be removed. can you please adjust composer.json to only allow php 8 and adjust the github actions to not build for 7.4 anymore (and add 8.3 and 8.4 instead while you are at it 😁 ?)

@Spea
Copy link
Contributor Author

Spea commented Jan 17, 2025

thanks! this is no longer compatible with php 7.4 - but that one is so old that it is fine to be removed. can you please adjust composer.json to only allow php 8 and adjust the github actions to not build for 7.4 anymore (and add 8.3 and 8.4 instead while you are at it 😁 ?)

Of course 😁 Done via 4471178 and d016fe7

Copy link
Member

@dbu dbu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks a lot!

@dbu dbu merged commit a69d991 into liip:2.x Jan 19, 2025
7 checks passed
@Spea
Copy link
Contributor Author

Spea commented Feb 14, 2025

@dbu Are you planning a 2.0 release for this? Also do you have any suggestions/plans on using this new version in the liip/serializer package? I can also take care of that once a new version for the metadata parser is released.

@Spea Spea deleted the support-php-8-4 branch February 14, 2025 10:20
@dbu
Copy link
Member

dbu commented Feb 14, 2025

we unfortunately don't use this library actively anymore, so it is a bit on the backburner.

for releasing 2.0, there are 2 things in the milestone that would be quite good to do (and should probably not be very difficult): https://github.com/liip/metadata-parser/milestone/1

if you would have time to do those, i can release 2.0.0. or at least the cleanup of deprecations, providing the attribute can be done as a 2.1 version as its not a BC break as long as we keep supporting the annotation.

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