Skip to content

[Meta] PHP 8.1 Support #41552

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

Closed
derrabus opened this issue Jun 5, 2021 · 6 comments
Closed

[Meta] PHP 8.1 Support #41552

derrabus opened this issue Jun 5, 2021 · 6 comments
Labels

Comments

@derrabus
Copy link
Member

derrabus commented Jun 5, 2021

Symfony version(s) affected: 4.4, 5.3, 5.4, 6.0

Description

In November, PHP 8.1 will be released and we should be prepared. In this PR, I'd like to track the efforts necessary to make all actively maintained branches ready for that release.

Ready means: if an application uses features of PHP 8.1, Symfony must not blow up.

We do already have a GitHub action that tests all branches with PHP 8.1. This action will always pass, but the output will show us if we're compatible yet: https://github.com/symfony/symfony/runs/2752054643

I will update this issue as we progress.

How to Contribute

If you want to join the effort, please start with the components and bridges. Fixing those will also fix many broken bundle tests. Reminder: We treat compatibility issues as bugs. As always, please fix bugs on the lowest maintained branch that suffers from the bug.

Progress

Components

Bridges

Bundles

@derrabus derrabus added the Bug label Jun 5, 2021
nicolas-grekas added a commit that referenced this issue Jun 6, 2021
This PR was merged into the 5.2 branch.

Discussion
----------

[String] Fix implicit float-to-int casts

| Q             | A
| ------------- | ---
| Branch?       | 5.2
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #41552
| License       | MIT
| Doc PR        | N/A

`str_repeat()` expects an integer, but the result of the division might be a float. PHP 8.1 will complain here, so I'm using an integer division instead.

Commits
-------

448a017 [String] Fix implicit float-to-int casts
nicolas-grekas added a commit that referenced this issue Jun 6, 2021
This PR was merged into the 5.3 branch.

Discussion
----------

[Messenger] Fix broken mock

| Q             | A
| ------------- | ---
| Branch?       | 5.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #41552
| License       | MIT
| Doc PR        | N/A

In reality, `$response->getInfo('http_code')` will always yield an integer, but the mock created in `AmazonSqsTransportTest` will return `null`. This causes trouble on PHP 8.1 because the status code is eventually passed as `$code` to an exception constructor that does not allow `null` values anymore.

Commits
-------

7d1435b [Messenger] Fix broken mock
nicolas-grekas added a commit that referenced this issue Jun 6, 2021
This PR was merged into the 4.4 branch.

Discussion
----------

[DependencyInjection] Don't pass null to trim()

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #41552
| License       | MIT
| Doc PR        | N/A

Commits
-------

e14dd67 [DependencyInjection] Don't pass null to trim()
nicolas-grekas added a commit that referenced this issue Jun 6, 2021
…rrabus)

This PR was merged into the 4.4 branch.

Discussion
----------

Add return types to JsonSerializable implementations

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #41552
| License       | MIT
| Doc PR        | N/A

PHP 8.1 will trigger a deprecation warning if we implement `JsonSerializable` without a return type. I've added return types to the two mock implementations that were affected.

Commits
-------

9246b53 Add return types to JsonSerializable implementations
nicolas-grekas added a commit that referenced this issue Jun 6, 2021
…derrabus)

This PR was merged into the 4.4 branch.

Discussion
----------

Fix Serializable deprecations triggered by token mocks

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #41552
| License       | MIT
| Doc PR        | N/A

On 4.4, `TokenInterface` extends `Serializable` but does not enforce implementations to implement `__serialize()` as well. When creating a mock from this interface, PHPUnit will emit code that triggers a deprecation warning on PHP 8.1.

This PR introduces stricter versions of `TokenInterface` to work around that problem.

My goal was to keep the diff small and mergable. We can probably drop most of the changes when merging to 5.2 because the two methods have been added there.

Commits
-------

48e76fa Fix Serializable deprecations triggered by token mocks
nicolas-grekas added a commit that referenced this issue Jun 6, 2021
…abus)

This PR was merged into the 4.4 branch.

Discussion
----------

Fix incompatible implicit float-to-int conversions

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #41552
| License       | MIT
| Doc PR        | N/A

PHP 8.1 is picky about implicit float-to-int casts if we lose precision because of that cast. This PR should fix all cases that bubbled up in our test suite.

Commits
-------

fc74476 Fix incompatible implicit float-to-int conversions
nicolas-grekas added a commit that referenced this issue Jun 6, 2021
This PR was merged into the 5.3 branch.

Discussion
----------

[Asset] Don't pass null to strpos()

| Q             | A
| ------------- | ---
| Branch?       | 5.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #41552
| License       | MIT
| Doc PR        | N/A

Commits
-------

83b966c [Asset] Don't pass null to strpos()
nicolas-grekas added a commit that referenced this issue Jun 6, 2021
This PR was merged into the 4.4 branch.

Discussion
----------

[Translation] Don't pass null to strtoupper()

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #41552
| License       | MIT
| Doc PR        | N/A

This error popped up on 5.3, but not on 4.4. That's why I missed it when fixing the other occurrence with #41240.

Commits
-------

3c8cf9a [Translation] Don't pass null to strtoupper()
@derrabus derrabus added Help wanted Issues and PRs which are looking for volunteers to complete them. Keep open and removed Status: Needs Review labels Jun 6, 2021
nicolas-grekas added a commit that referenced this issue Jun 7, 2021
…string parameters (derrabus)

This PR was merged into the 4.4 branch.

Discussion
----------

[FrameworkBundle][WebProfilerBundle] Don't pass null to string parameters

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #41552
| License       | MIT
| Doc PR        | N/A

This should fix the remaining issues from our bundle tests

Commits
-------

5af6eda [FrameworkBundle][WebProfilerBundle] Don't pass null to string parameters
nicolas-grekas added a commit that referenced this issue Jun 8, 2021
…8.1 (alexandre-daubois)

This PR was merged into the 4.4 branch.

Discussion
----------

[DependencyInjection] Fix testServiceSubscriber for PHP 8.1

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix #41592 and part of #41552 <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT
| Doc PR        | N/A

Commits
-------

a5be196 [DependencyInjection] Fix testServiceSubscriber for PHP 8.1
derrabus added a commit that referenced this issue Aug 31, 2021
…(derrabus)

This PR was merged into the 4.4 branch.

Discussion
----------

[VarExporter] Suppress deprecations for legacy fixtures

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #41552
| License       | MIT
| Doc PR        | N/A

Two test fixtures currently trigger a deprecation on PHP 8.1 because they implement `Serializable` without implementing `__serialize()` and `__unserialize()`. However, I believe that we want to test the behavior of the component with this kind of implementation. This why I decided to suppress the deprecation instead of upgrading the fixtures.

Commits
-------

2db2c00 [VarExporter] Suppress deprecations for legacy fixtures
fabpot added a commit that referenced this issue Sep 3, 2021
This PR was merged into the 4.4 branch.

Discussion
----------

[Cache] Fix implicit float to int cast

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #41552
| License       | MIT
| Doc PR        | N/A

Passing a float to a method that expects an integer triggers a deprecation on PHP 8.1 if that implicit float to int cast would mean a loss of precision. This PR fixes such an error in the cache component by making sure an integer is passed. This fixes an error in our 5.3 CI.

Commits
-------

93f27d9 Fix implicit float to int cast
fabpot added a commit that referenced this issue Sep 3, 2021
This PR was merged into the 4.4 branch.

Discussion
----------

Run PHP 8.1 CI with all extensions

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #41552
| License       | MIT
| Doc PR        | N/A

Let's activate the same extensions for PHP 8.1 as we do for other PHP versions. The MongoDB extension is still not available, but that does not break the CI. All other extensions can be enabled by Setup PHP now.

Commits
-------

9523fcd Run PHP 8.1 CI with all extensions
nicolas-grekas added a commit that referenced this issue Sep 7, 2021
This PR was merged into the 4.4 branch.

Discussion
----------

[PropertyInfo] Support for intersection types

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | #41552
| License       | MIT
| Doc PR        | N/A

I was a bit unsure how to fix this one. PropertyInfo does not know about the concept of intersection types yet. With this PR, the component will behave exactly the same for an intersection type as for a union: You will get an array of all atomic types.

Commits
-------

9070047 [PropertyInfo] Support for intersection types
@nicolas-grekas
Copy link
Member

Task completed, kudos @derrabus! 💪

@derrabus
Copy link
Member Author

Thanks everyone who helped with making Symfony ready for PHP 8.1! 🎉

nicolas-grekas added a commit that referenced this issue Sep 17, 2021
This PR was merged into the 4.4 branch.

Discussion
----------

[Serializer] Don't pass `null` to `preg_match()`

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #41552
| License       | MIT
| Doc PR        | N/A

Calling `preg_match()` on `null` will trigger a deprecation error on PHP 8.1. This issue has been caught by our CI on the 5.4 branch.

Commits
-------

4e6ea37 [Serializer] Don't pass null to preg_match()
nicolas-grekas added a commit that referenced this issue Sep 29, 2021
This PR was merged into the 4.4 branch.

Discussion
----------

[PhpUnitBridge] Use PHPUnit 9.5 on PHP 8.1

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #41552
| License       | MIT
| Doc PR        | N/A

On PHP 8.1, PhpUnitBridge currently defaults to an incompatible PHPUnit version. This PR proposes to use PHPUnit 9.5 instead.

Commits
-------

b177c08 Use PHPUnit 9.5 on PHP 8.1
nicolas-grekas added a commit that referenced this issue Oct 4, 2021
…s on PHP 8.1 (nicolas-grekas)

This PR was merged into the 4.4 branch.

Discussion
----------

[DependencyInjection] fix support for "new" in initializers on PHP 8.1

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #41552
| License       | MIT
| Doc PR        | -

Commits
-------

fae3659 [DependencyInjection] fix support for "new" in initializers on PHP 8.1
nicolas-grekas added a commit that referenced this issue Oct 5, 2021
….1 (nicolas-grekas)

This PR was merged into the 4.4 branch.

Discussion
----------

[VarDumper] Fix handling of "new" in initializers on PHP 8.1

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #41552
| License       | MIT
| Doc PR        | -

Commits
-------

f617c0b [VarDumper] Fix handling of "new" in initializers on PHP 8.1
@derrabus derrabus removed Help wanted Issues and PRs which are looking for volunteers to complete them. Keep open labels Oct 7, 2021
nicolas-grekas added a commit that referenced this issue Oct 29, 2021
…n PHP 8.1 (agustingomes)

This PR was squashed before being merged into the 4.4 branch.

Discussion
----------

[Config] Fix signature generation with nested attributes on PHP 8.1

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #43260, part of #41552
| License       | MIT
| Doc PR        | -

This fix allows the new PHP 8.1 syntax of [new in initializers](https://wiki.php.net/rfc/new_in_initializers)

It allows the method signature to be inferred from the object that is the default parameter value.

Commits
-------

6300a17 [Config] Fix signature generation with nested attributes on PHP 8.1
nicolas-grekas added a commit that referenced this issue Nov 6, 2021
…haljusiega)

This PR was merged into the 5.3 branch.

Discussion
----------

[Asset][Security] Fixed leftover deprecations PHP 8.1

| Q             | A
| ------------- | ---
| Branch?       | 5.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Part of #41552, leftover #41559
| License       | MIT
| Doc PR        | ~

Found during testing **PHP 8.1** and Symfony **5.4.0-BETA1**

1. src/Symfony/Component/Asset/VersionStrategy/JsonManifestVersionStrategy.php
 ![obraz](https://user-images.githubusercontent.com/16488888/140545163-837ea4c4-907b-44e7-8f8e-7454f7d66be3.png)

Looks like #41559 didn't solve the deprecation completely.

2. src/Symfony/Component/Security/Http/Authenticator/JsonLoginAuthenticator.php
![obraz](https://user-images.githubusercontent.com/16488888/140545313-794c0136-cef8-4434-a8c2-21c3d77ef196.png)

`Request::getRequestFormat()` and `Request::getContentType()` may return nulls.

Commits
-------

86934e6 fixed leftover deprecations PHP 8.1
@derrabus derrabus mentioned this issue Nov 26, 2021
29 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants