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

added $kind param to TypeMapperInterface->mapToPHPStanPhpDocTypeNode() #306

Merged
merged 11 commits into from
Jun 26, 2021

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Jun 26, 2021

which allows to differentiate the returned type based on the used location.

this is required to add a proper @return never handling as we are building it in #296

which allows to differentiate the returned type based on the used location
Comment on lines 29 to 31
if ($kind === 'return') {
return new IdentifierTypeNode('never');
}
Copy link
Contributor Author

@staabm staabm Jun 26, 2021

Choose a reason for hiding this comment

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

this line implements the actual motivation of this PR - everything else is just a mechanical change because the interface changed

@staabm
Copy link
Contributor Author

staabm commented Jun 26, 2021

After a few iterations I realized that a TypeKind enum exists. after using it things got easier in a few spots.

@TomasVotruba this PR is ready to review

@staabm staabm marked this pull request as ready for review June 26, 2021 19:50
@staabm
Copy link
Contributor Author

staabm commented Jun 26, 2021

@TomasVotruba this is ready for another round

@TomasVotruba
Copy link
Member

Thank you 👍

I was looking for a test for NeverTypeMapper, but I guess it's in one of your other PRs :)

Lets ship it!

@TomasVotruba TomasVotruba merged commit 7fde365 into rectorphp:main Jun 26, 2021
@staabm staabm deleted the new-kind branch June 26, 2021 20:14
@staabm
Copy link
Contributor Author

staabm commented Jun 26, 2021

I was looking for a test for NeverTypeMapper, but I guess it's in one of your other PRs :)

yep, this is coming with #296

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.

2 participants