benevolent union docs

[staabm]

the article at https://phpstan.org/blog/union-types-vs-intersection-types describes union and intersection types.

phpstan also has a benevolent union type - which I don't know what it specifics are.

it would be great if the article could mention the benevolent union type and would describe what the difference is between benevolent union and 'regular union'. maybe even show it with a small example?

if someone could give me some hints about the differences, I would also try to provide a PR to enhance the article.

[ondrejmirtes]

PHPStan's BenevolentUnionType is mostly an implementation detail that's supposed to make some situations less annoying. It can't be expressed in a PHPDoc and it isn't notable to most users.

[staabm]

could you give me a example in which the types make a difference, just for my understanding?

I see commits like phpstan/phpstan-src@a9881e3 which I cannot read from what the actual end-user impact is

[orklah]

IIRC, it allows downcasting (or coercion) from an union into a subset of this union without errors.

so for example, if you have this:

function a(): string{
    return strstr(...params...);
}

If the return of strstr is in a benevolent union type, it will be allowed, even though strstr can return false. If it's a standard union, it will be seen as an error.

Psalm has something vaguely similar with ignoreInternalFunctionFalseReturn/ignoreInternalFunctionNullReturn

[ondrejmirtes]

The most notable example are array keys: https://phpstan.org/r/014f63ed-ee22-441c-b9db-bb80eae35d0c

[hemberger]

I'd like to write a test case to check that a PHPStan extension is handling benevolent unions correctly (i.e. an issue with using isSuperTypeOf vs. accepts). Since there is no way to express a benevolent union in a PHPDoc, it seems like it is not possible to test this behavior in a RuleTestCase context. I could use a type that I know is implicitly a benevolent union, such as array-key, but this seems like an unsafe assumption to make (and doesn't let me pick what the benevolent union contains). Is there a better way to do this? Thanks!

[ondrejmirtes]

@hemberger I think the best way is to write a test as if it was a normal situation in code that usually happens. If you're handling benevolent unions in your extension, you must have an existing scenario in mind :) Also, once you create a benevolent union, make sure to also assert it with assertType() before working with it further - it has parentheses, (int|string) for example.

[zonuexe]

Presumably date_sun_info() was merged as one of the use cases where the use of benevolent union is justified. phpstan/phpstan-src#2947

This function can potentially return false, but in normal use it only returns only an array.
It is nonsense to write $result = date_sun_info(); assert($result !== false); in the return value of such a function every time.

[zonuexe]

There are several standard PHP functions that may be worth replacing with benevolent unions, one of which is file_get_contents().

$data = unserialize(file_get_contents('/serialized.data'));

https://phpstan.org/r/a26c7f19-15f4-472e-ab9c-1e1a6423d6b1

The return type of file_get_contents() is string|false, so PHPStan will warn you if pass the file contents as-is to a string function. Passing false to a string function will cause a runtime error if the deployment fails or if the file_exists() guard is missing.

From another perspective, you can say that it is the programmer's responsibility to always write !== false assertions when manipulating files, such as Java's checked exceptions. However, file_get_contents() raises an E_WARNING error, so framework-based applications with exception handlers set up will never return false. The !== false assertion is just redundant code.

Static analysis cannot assume that there is a custom handler that converts errors into exceptions and raises them. Therefore, carelessly expanding benevolent unions can reduce the runtime safety of PHP code.

For such cases, Safe PHP is an option to simplify code and flag potential errors as @throws.

[zonuexe]

On the other hand, I feel that there is some validity in replacing the return value of a function that causes errors with __benevolent<>. This is a tradeoff between code simplicity and runtime safety.

[zonuexe]

I have submitted a PR to consider and add functions where adding __benevolent<> does not compromise the runtime safety of the code.

phpstan/phpstan-src#2986