Fix type inference of array_sum()

[zer0-star]

fix phpstan/phpstan#9808

[zer0-star]

@ondrejmirtes Could you review this PR?

[ondrejmirtes]

Hi, I don't really love that this PR is working with specific scalar values and calls array_sum internally. It misses out on various properties of the type system, like working with IntegerRangeType.

I'd much rather be if for ConstantArrayType objects, like [$one, $two, $three], it tried to do $scope->getType(new Plus($one, new Plus($two, $three))). Because Plus expr handling in MutatingScope already knows how to work with al possible Types, we can just call it here like that.

Because ConstantArrayType contains Type and not Expr, you can take advantage of TypeExpr virtual node to build an Expr out of Type instances.

[ondrejmirtes]

Yes, that's the idea :) And we don't have to limit ourselves to a single constant array, we can union results from all constant arrays. For example when something like array{1}|array{2, 3} is passed in.

[zer0-star]

Thank you very much for your review! I will continue working it in tomorrow.

[phpstan-bot]

This pull request has been marked as ready for review.

[zer0-star]

@ondrejmirtes
Hello, can I ask you a question?
In the test case foo4, the inferred type is BenevolentUnionType, so the result becomes (float|int). Do you think it's needed to fix this?

[ondrejmirtes]

@zer0-star No, that's totally fine and actually a nice side-effect of the changes :)

[zer0-star]

@ondrejmirtes
I think this PR is ready to be reviewed! I'm sorry to rush you, but could you review it for me?

[ondrejmirtes]

Hi, I really appreciate this, especially the thorough test suite :) I wanted to write a simplification and it helped me a lot to get it right.

I also pushed a failing test case - ConstantArrayType might have an optional key. This is how to check for that:

foreach ($constantArray->getValueTypes() as $i => $type) {
    if ($constantArray->isOptionalKey($i)) { ... }
}

[zer0-star]

Thanks so much for your help! I really appreciate it.
I will fix this problem later.

Is there something else to revise?

[ondrejmirtes]

No, once the optional keys are supported, we are good to merge this :)

[zer0-star]

I fixed it! I hope you like this change 🤗

[ondrejmirtes]

I don't really like any instanceof *Type, it's usually a code smell. Explained here: https://phpstan.org/blog/why-is-instanceof-type-wrong-and-getting-deprecated

There should be nothing special about MixedType and there were probably other types that needed a similar treatment. I was also able to get rid of other problems by doing ->toNumber() on the resulting type. See: 291bb57?w=1

I'll merge the PR if the build gets green :) Thank you very much for your cooperation!

[zer0-star]

Thanks! I was happy to contribute to PHPStan.

I learned a lot from your review!