search

rectorphp / rector

Instant Upgrades and Automated Refactoring of any PHP 5.3+ code
https://getrector.com
MIT License
8.72k stars 686 forks source link

Nullable property typehints not always working. #2835

Closed prolic closed 4 years ago

prolic commented 4 years ago

If you have something like this /** @var UserCredentials|null */ it happens that rector does not prepend the ? to the property typehint. Most of the time it works, but sometimes it doesn't. I couldn't see a pattern yet, why it doesn't work in some situations.

Other than that, if the class / interface is already imported, rector should not use the FQCN but the imported one.

Rector: 0.6.14 PHP: 7.4.1 Command: ./vendor/bin/rector process src --set php74

TomasVotruba commented 4 years ago

Demo link is needed to make it fixable: https://getrector.org/demo

prolic commented 4 years ago

That's the problem, it works on most properties but not on all. Therefore I don't know how to provide a demo. In order to reproduce you can checkout https://github.com/prooph/event-store-client/ at commit a3cb683 and try to apply the php74 set yourself.

When I tried to create a script that reproduces the issue, it just worked.

If that's not good enough for you and you want to just close this issue, go ahead. I'll probably use regex for this task next time anyway. But I still like this project very much! Good work!

TomasVotruba commented 4 years ago

Without evidence it's impossible to fix usually. I tried that many times and it often was local Docker, wrong version or PHP or prefixed mixed with other tool etc. It's possible to waste dozens of hours on that.

If you compare it to demo link with real bug, we can fix that in 30-40 minutes on average. Better invest in that.

Closing then thanks to your suggestion.

I'll probably use regex for this task next time anyway. But I still like this project very much! Good work

Yea, it's 99 % tool, we still have to work :D thanks for feedback!