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

Add failing test fixture for DowngradeParameterTypeWideningRector #5995

Merged
merged 4 commits into from
Apr 2, 2021

Conversation

leoloso
Copy link
Contributor

@leoloso leoloso commented Mar 25, 2021

Failing Test for DowngradeParameterTypeWideningRector

Based on https://getrector.org/demo/1b8ee6a0-7d50-4fae-8e15-f19d19591fc5

Fixes #5994

@leoloso
Copy link
Contributor Author

leoloso commented Mar 25, 2021

Issue: #5994

@leoloso
Copy link
Contributor Author

leoloso commented Apr 2, 2021

Alright, bug fixed!

Traits must be skipped for this rule, so I added:

if ($ancestorClassReflection->isTrait()) {
    continue;
}

Please check and merge 🙏

@TomasVotruba
Copy link
Member

Thank you Leo

@TomasVotruba TomasVotruba merged commit d2e4c0a into rectorphp:main Apr 2, 2021
@samsonasik
Copy link
Member

@leoloso it possibly cause issue on rector prefixed build which string parameter not removed:

https://github.com/rectorphp/rector-prefixed/blob/056cdf6b69602977e4227fe74e2c8b55ecb26c7f/vendor/symfony/service-contracts/ServiceLocatorTrait.php#L41

so make error:

PHP Fatal error:  Declaration of RectorPrefix20210420\Symfony\Component\DependencyInjection\ServiceLocator::has(string $id) must be compatible with RectorPrefix20210420\Psr\Container\ContainerInterface::has($id) in /home/runner/work/rector-prefixed/rector-prefixed/vendor/symfony/dependency-injection/ServiceLocator.php on line 25

Fatal error: Declaration of RectorPrefix20210420\Symfony\Component\DependencyInjection\ServiceLocator::has(string $id) must be compatible with RectorPrefix20210420\Psr\Container\ContainerInterface::has($id) in /home/runner/work/rector-prefixed/rector-prefixed/vendor/symfony/dependency-injection/ServiceLocator.php on line 25
Error: Process completed with exit code 255.

@leoloso
Copy link
Contributor Author

leoloso commented Apr 20, 2021

I don't think this code I added is the one causing the issue.

I explained in #6176 but also, this rule was originally modifying code in the class, if it implemented a trait:

foreach ($classReflection->getAncestors() as $ancestorClassReflection) {

But that's an error, because the same method defined in the class will override the one from the trait (check the fixture I added).

And in any case, the offending code is in the trait ServiceLocatorTrait, not in the class.

So these are related, but different issues.

@samsonasik
Copy link
Member

@leoloso thank you for verify 👍

TomasVotruba added a commit that referenced this pull request Jun 21, 2024
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.

Incorrect behavior of DowngradeParameterTypeWideningRector
3 participants