Skip to content
This repository has been archived by the owner on Sep 8, 2023. It is now read-only.

Fix deprecated reflection type to string #19

Conversation

mortenscheel
Copy link
Contributor

\ReflectionType::__toString() was deprecated in PHP 7.1.

This PR fixes the issue by using \ReflectionNamedType when the PHP version is >= 7.1.

I've also included a couple of commits with coding standard fixes that are not related to this issue.

Copy link
Owner

@KristofMorva KristofMorva left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution! It seems like a great way to keep backwards compatibility with PHP 7.0 (even if it's EOL).
There were some smaller code style issues that I have highlighted.

You were able to test your changes before submitting, right? Sadly I don't have any environment set up right now.

src/Console/MacrosCommand.php Outdated Show resolved Hide resolved
src/Console/MacrosCommand.php Outdated Show resolved Hide resolved
@mortenscheel
Copy link
Contributor Author

Hi. I've tested it locally in one of my projects, but I'm only able to test with PHP 7.4.
It would be nice to have proper tests, but I don't know if Testbench would work for something like this?

@KristofMorva
Copy link
Owner

Hi. I've tested it locally in one of my projects, but I'm only able to test with PHP 7.4.
It would be nice to have proper tests, but I don't know if Testbench would work for something like this?

Sadly I'm not familiar with the recent tools and practices in PHP so I couldn't say :/
Nevertheless, if you won't find a great way to test it out extensively, we can release a few feature version with this PR so people can try it out.

@mortenscheel
Copy link
Contributor Author

Just a status check. Are you happy with the changes I made?

@KristofMorva
Copy link
Owner

Sorry for the delay. It has been released in https://github.com/KristofMorva/laravel-ide-macros/releases/tag/1.5.0

Thank you for your contribution! :)
Let me know if you experience any issues regarding this solution.

@mortenscheel mortenscheel deleted the fix-deprecated-reflection-type-to-string branch July 16, 2020 07:26
@KristofMorva
Copy link
Owner

Hello @mortenscheel, it seems to break #21. Could you please make a change which does not use \ for built-in types?

@mortenscheel
Copy link
Contributor Author

Hi @KristofMorva
Sorry about that. It's fixed in #22

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants