Skip to content

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Jan 17, 2023

@staabm staabm marked this pull request as draft January 17, 2023 09:18
$unionType = TypeCombinator::union(...$types);
$specifiedTypes = $this->typeSpecifier->create($exprArg, $unionType, $context, false, $scope);

if ($context->true() && $exprArg instanceof Cast) {
Copy link
Contributor Author

@staabm staabm Jan 17, 2023

Choose a reason for hiding this comment

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

my initial attempt was to do this directly within the TypeSpecifier ... but I then got the feeling that the type narrowing of a cast-expression is not a concept which can be applied to all specifying extensions.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, well, is_string((string) $int) is always true, but it doesn't make $int a string :D

@staabm staabm marked this pull request as ready for review January 17, 2023 16:19
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

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

Also please test the negative context.

$castOriginType = TypeCombinator::intersect($scope->getType($exprArg->expr), $unionType);

$specifiedTypes = $specifiedTypes->unionWith(
$this->typeSpecifier->create($exprArg->expr, $castOriginType, $context, false, $scope)
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure you can't just use $unionType instead of $castOriginType here in the 2nd arg?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point. fixed.

@ondrejmirtes ondrejmirtes merged commit 279c781 into phpstan:1.9.x Jan 17, 2023
@ondrejmirtes
Copy link
Member

Thank you.

1 similar comment
@ondrejmirtes
Copy link
Member

Thank you.

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