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

Indirect deprecation notice #61

Open
claudiu-cristea opened this issue Dec 21, 2023 · 3 comments
Open

Indirect deprecation notice #61

claudiu-cristea opened this issue Dec 21, 2023 · 3 comments

Comments

@claudiu-cristea
Copy link

I see these notice after running PHPUnit tests in a Drupal project

Method "Consolidation\Config\ConfigInterface::import()" might add "Config" as a native return type declaration in the future. Do the same in implementation "Robo\Config\Config" now to avoid errors or add an explicit @return annotation to suppress this message.

However, I don't think we can do the suggested change, even we cover also Robo, because it might break other extending classes

@claudiu-cristea
Copy link
Author

Tried to add #[\ReturnTypeWillChange] but the deprecation isn't going away

@greg-1-anderson
Copy link
Member

I think we only need #[\ReturnTypeWillChange] if we add a return type to the function definition. I think the code breaks for PHP 8.0 and earlier if we do that, though, because #[\ReturnTypeWillChange] was introduced in PHP 8.1.

I think that the error is indicating that an explicit @return is desired in Robo\Config\Config; in theory, though, this shouldn't be necessary, as Robo Config should inherit the @return annotation from ConfigInterface.

Unsure what the right way forward is. Maybe what is needed is a new major version for PHP 8.3+ or wherever this is first introduced. That often implied cascading major version upgrades along the dependency tree, though.

@greg-1-anderson
Copy link
Member

Can you reproduce this problem with the tests in this repository? I tried running the existing tests with PHP 8.3; I was not too surprised that the problem was not shown by the config tests, but I also did not see the reported error when running the Robo tests.

Could you submit a failing test here that exhibits the same problem that you're seeing in your Drupal module test?

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

No branches or pull requests

2 participants