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

Allow for PhpDoc for macros with union types #1148

Merged
merged 15 commits into from
Jul 30, 2021
Merged

Conversation

riesjart
Copy link
Contributor

@riesjart riesjart commented Feb 2, 2021

Summary

When running the IDE helper on macros with PHP 8 union types, an error Call to undefined method ReflectionUnionType::getName()will be thrown. This PR aims to fix that for both the parameter and return types.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • Existing tests have been adapted and/or new tests have been added
  • Add a CHANGELOG.md entry
  • Code style has been fixed via composer fix-style

Copy link
Collaborator

@mfn mfn left a comment

Choose a reason for hiding this comment

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

LGTM, got a question :)

$type = $this->concatReflectionTypes($reflectionType);

/** @psalm-suppress UndefinedClass */
if ($reflectionType && !$reflectionType instanceof \ReflectionUnionType && $reflectionType->allowsNull()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

\ReflectionUnionType

I assume in PHP < 8 this will "just work", because a non-existent class check in instanceof will not trigger any problems?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct!

tests/MacroTest.php Outdated Show resolved Hide resolved
@riesjart riesjart requested a review from mfn July 7, 2021 23:24
@sebastiaanluca
Copy link

Is there anything left to do before merging? We're currently using the patch branch in our project since this bug was breaking the ide-helper package. Thanks!

@barryvdh
Copy link
Owner

@mfn you haven't approved this, but did say LGTM so I guess it's okay with you :)

@barryvdh barryvdh merged commit f4e0fc3 into barryvdh:master Jul 30, 2021
@sebastiaanluca
Copy link

Thanks for merging!

@heiswald
Copy link

heiswald commented Sep 1, 2021

The last release was on April 9th. When will this fix be released? I dont want to use dev-master version.

d3v2a pushed a commit to d3v2a/laravel-ide-helper that referenced this pull request Feb 16, 2024
* Allow for PhpDoc for macros with union types

* Replace null coalescing operator

* Test init PhpDoc for macros with parameter union types

* Allow for PhpDoc for macros with union return types

* Add helper method to Macro class

* Add changelog entry

* Format

* Complement changelog with PR link

* Add missing return

* Fix test for PHP < 8

* Rewrite PHP 8 test using eval()

* Remove obsolete test class

* Suppress Psalm errors for undefined ReflectionUnionType class

* Remove unreachable return statement
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.

5 participants