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

Fix array_sum return type #494

Merged
merged 1 commit into from
Apr 15, 2021
Merged

Conversation

VincentLanglet
Copy link
Contributor

No description provided.

@@ -36,12 +36,20 @@ public function getTypeFromFunctionCall(FunctionReflection $functionReflection,
}

if ($arrayType->isIterableAtLeastOnce()->yes()) {
return TypeCombinator::intersect($itemType, new UnionType([new IntegerType(), new FloatType()]));
if ($itemType instanceof IntegerType || $itemType instanceof FloatType) {
Copy link
Member

Choose a reason for hiding this comment

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

Better comparison would be:

(new UnionType([new IntegerType(), new FloatType()]))->isSuperTypeOf($itemType)->yes()

return new UnionType([new IntegerType(), new FloatType()]);
}

if ($itemType instanceof IntegerType || $itemType instanceof FloatType) {
Copy link
Member

Choose a reason for hiding this comment

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

dtto

}

return TypeCombinator::union(
new ConstantIntegerType(0),
TypeCombinator::intersect($itemType, new UnionType([new IntegerType(), new FloatType()]))
new UnionType([new IntegerType(), new FloatType()])
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it makes sense to modify this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For an array of string, $itemType is a stringType.
So TypeCombinator::intersect($itemType, new UnionType([new IntegerType(), new FloatType()]) is nothing.
That's why the dynamic return type return new ConstantIntegerType(0).

The intersect seems to be the reason of the bug.

Copy link
Member

Choose a reason for hiding this comment

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

But on the other hand, this intersect allows us to specify that when we only have array<int>, the result is going to be int.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't this handle by the previous check

if ($intUnionFloat->isSuperTypeOf($itemType)->yes()) {
    return TypeCombinator::union(new ConstantIntegerType(0), $itemType);
}

?

@VincentLanglet VincentLanglet marked this pull request as ready for review April 15, 2021 13:31
@ondrejmirtes
Copy link
Member

Awesome! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants