PHPStan loses type resolution when creating a union of int and numeric-string

[Ocramius]

Bug report

When using azjezz/psl, the Psl\Type\union() is used to create a union type out of two (or more) Psl\Type\TypeInterface<T> instances.

Roughly: function union(TypeInterface<T>, TypeInterface<U>): TypeInterface<T|U>.

This works fine, but PHPStan seems to have trouble in computing union types for somehow close types like int and numeric-string, leading to inference of int|string rather than int|numeric-string.

Similarly, it cannot compute 0 and positive-int, leading to int being inferred, rather than 0|int<1, max> or int<0, max>.

Code snippet that reproduces the problem

https://phpstan.org/r/749e7db4-8d56-4d2d-9faa-1d3d32790ba0

Expected output

• union(int, numeric-string) should become int|numeric-string (currently produces int|string)
• union(0, int<1, max>) should become 0|int<1, max> (currently produces int)

Did PHPStan help you today? Did it make you happy in any way?

Thanks for your continued work on this: makes PHP bearable :-)

[ondrejmirtes]

We've talked about this in person last year at phpDay :) The problem is that generics generalize types too much.

When we initially implemented them, we decided that:

/**
 * @template T
 * @param T $p
 * @return T
 */

Should turn foo(1) into int, not let it still be 1.

Since then we mostly changed our minds on this topic and the types shouldn't be touched at all. But there's one problem. new Collection([1, 2, 3]) should probably be Collection<int> in order to be useful in the real world.

There's a nice proposal to solve this while keeping the types the same and not generalize them: #6732 (comment)

Unfortunately I'm a bit worried about the complexity and haven't yet had the courage to try to tackle it :)

[Ocramius]

@ondrejmirtes worth splitting the problem in two parts, IMO:

• 0 (literal) + positive-int can be left out of the discussion, for now: we can re-report it elsewhere (Bidirectional type narrowing for generic types (would close an obscure hole in type checking!) #6732 valid place)
• int + numeric-string is currently problematic: it leads to int|string rather than int|numeric-string, so something else is happening there. This one is probably the more helpful scenario.

[ondrejmirtes]

Yeah because numeric-string is generalized into string :)

[Ocramius]

That seems wrong: the numeric-string is not inferred by a given value, but rather by an explicit declaration.

	/** @return TypeInterface<numeric-string> */
	function numeric_string() { throw new \Exception('irrelevant'); }

I would understand if the numeric-string type came from an input value (inferred). but it is a static declaration here 🤔

[ondrejmirtes]

PHPStan does not differentiate a source of a type like that, maybe there could be a half-step like that 🤔

[Ocramius]

Yeah, I think that is basically an "anchor" that needs to be considered a constraint, IMO.

In this case, numeric-string is a smaller set of values than string, so it is more restrictive.

If PHPStan tries to widen the types, it cannot widen further than what is explicitly declared, IMO. In this case, numeric-string is the "widest" type, IMO.

[ondrejmirtes]

Last night I came up with the idea that we mostly shouldn't generalize the generic type variables, except when they're in object generics, like Foo<int>.

Here's the resulting PR: phpstan/phpstan-src#2818

We can't do this for objects, because I want new Collection([1, 2, 3]) to become Collection<int>. This could be improved in the future thanks to this suggestion: #6732 (comment) (but it's pretty complex to implement so for now it has to wait).

The new behaviour now only applies to bleeding edge (https://phpstan.org/blog/what-is-bleeding-edge) so definitely enable it to get the taste of the future 👍

[Ocramius]

@ondrejmirtes not 100% sure about the rationale, but happy that you got an epiphany on this: it is something you've been stuck with for well over a year :-)

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.