Skip to content

Simplify arrow-function return type inference#2873

Closed
staabm wants to merge 1 commit intophpstan:1.10.xfrom
staabm:less-void-keeping
Closed

Simplify arrow-function return type inference#2873
staabm wants to merge 1 commit intophpstan:1.10.xfrom
staabm:less-void-keeping

Conversation

@staabm
Copy link
Copy Markdown
Contributor

@staabm staabm commented Jan 14, 2024

Arrow-functions can't have a void return type

refs #2778 (comment)

Arrow-functions can't have a void return type
$returnType = $arrowScope->getKeepVoidType($node->expr);
$returnType = $arrowScope->getType($node->expr);
if ($node->returnType !== null) {
$returnType = TypehintHelper::decideType($this->getFunctionType($node->returnType, false, false), $returnType);
Copy link
Copy Markdown
Contributor Author

@staabm staabm Jan 14, 2024

Choose a reason for hiding this comment

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

I came to the conclusion that getKeepVoidType might not be necessary because the following IF did not keep void when infered from $node->returnType either

@staabm staabm marked this pull request as ready for review January 14, 2024 18:12
@phpstan-bot
Copy link
Copy Markdown
Collaborator

This pull request has been marked as ready for review.

@staabm
Copy link
Copy Markdown
Contributor Author

staabm commented Jan 14, 2024

//cc @herndlm

@ondrejmirtes
Copy link
Copy Markdown
Member

If no tests were failing before, and this doesn't fix anything, what is the purpose of this change?

@staabm
Copy link
Copy Markdown
Contributor Author

staabm commented Jan 14, 2024

Its a line of code which did not make sense to me while looking thru functionType stuff.

I have no concrete case in mind - feel free to close

@ondrejmirtes
Copy link
Copy Markdown
Member

If it ain't broke don't fix it :)

But when testing a piece of code I found a false negative on 1.10.x:

@staabm staabm deleted the less-void-keeping branch January 14, 2024 19:05
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.

3 participants