vimeo / psalm

A static analysis tool for finding errors in PHP applications
https://psalm.dev
MIT License
5.54k stars 660 forks source link

Templated types do not support unions as type parameters #6795

Open tux-rampage opened 2 years ago

tux-rampage commented 2 years ago

Not sure if this worked before. When declaring a templated type with a union type parameter, psalm reports an incorrect type mismatch, but it works when writing each template variant verbosively as union type:

https://psalm.dev/r/55488318f8

psalm-github-bot[bot] commented 2 years ago

I found these snippets:

https://psalm.dev/r/55488318f8 ```php */ class SomeSpecific implements SomeGeneric { public function provideSomething(): Something { return new Something(); } } /** * @param SomeGeneric $instance */ function takesSomeSpecifics(SomeGeneric $instance): void { } /** * @param SomeGeneric|SomeGeneric $instance */ function takesSomeSpecificsVerbose(SomeGeneric $instance): void { } takesSomeSpecifics(new SomeSpecific()); takesSomeSpecificsVerbose(new SomeSpecific()); ``` ``` Psalm output (using commit 96ae8e7): INFO: UnusedParam - 31:41 - Param $instance is never referenced in this method INFO: UnusedParam - 38:48 - Param $instance is never referenced in this method ERROR: InvalidArgument - 42:20 - Argument 1 of takesSomeSpecifics expects SomeGeneric, SomeSpecific provided ```
tux-rampage commented 2 years ago

It seems that this works when declaring the Template parameter covariant. Not sure if that is correct, but I thought a union is something different than inheritance, or am i getting something wrong?

https://psalm.dev/r/2798c4a6e7

psalm-github-bot[bot] commented 2 years ago

I found these snippets:

https://psalm.dev/r/2798c4a6e7 ```php */ class SomeSpecific implements SomeGeneric { public function provideSomething(): Something { return new Something(); } } /** * @param SomeGeneric $instance */ function takesSomeSpecifics(SomeGeneric $instance): void { } /** * @param SomeGeneric|SomeGeneric $instance */ function takesSomeSpecificsVerbose(SomeGeneric $instance): void { } takesSomeSpecifics(new SomeSpecific()); takesSomeSpecificsVerbose(new SomeSpecific()); ``` ``` Psalm output (using commit 96ae8e7): INFO: UnusedParam - 31:41 - Param $instance is never referenced in this method INFO: UnusedParam - 38:48 - Param $instance is never referenced in this method ```
weirdan commented 2 years ago

Say you have a Box<T>, with a method like replaceContents(T $contents). If we allowed passing a Box<Chocolates> to a method accepting Box<Chocolates|Flowers> it could have replaced chocolates with flowers, while the code outside would still believe it had a box of chocolates.

Variance is not about just inheritance, it is about type relations e.g. subtypes.

tux-rampage commented 2 years ago

@weirdan Thanks for taking time to explain. I reviewed the psalm docs about covariance and it seems to be the correct use case here.

Anyways I've stumbled over another issue with the following snippet: https://psalm.dev/r/26590496a3

I know this is because of:

But @template-covariant doesn't get rid of all errors – if you add it to the first example, you get a new error (...) – complaining that you're attempting to use a covariant template parameter for function input. That’s no good, as it means you're likely altering the collection somehow (which is, again, a violation).

Is there a way to Type the affected callable parameter covariant-friendly, so that psalm errors when the wrong closure type is passed?

Or as a different and better approach: Can the transactional method somehow be typed that it must not change the class state (so that it doesn't mater whether the closure mutates its input or not).

psalm-github-bot[bot] commented 2 years ago

I found these snippets:

https://psalm.dev/r/26590496a3 ```php counter++; } } /** * @implements SomeGeneric */ class SomeSpecific implements SomeGeneric { public function provideSomething(): Something { return new Something(); } public function transactional(callable $txn): void { $txn(new Something()); } } /** * @param SomeGeneric $instance */ function takesSomeSpecifics(SomeGeneric $instance): void { // This is OK and type-valid $instance->transactional(function(Something|SomethingElse $value): void { $value->increment(); }); // This is Bad and should error $instance->transactional(function(Something|SomethingElse &$value): void { $value = new SomethingElse(); }); } takesSomeSpecifics(new SomeSpecific()); ``` ``` Psalm output (using commit e6dccaa): ERROR: InvalidTemplateParam - 14:15 - Template param T of SomeGeneric is marked covariant and cannot be used here ```