vimeo / psalm

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

Closure return type not considered with @template annotation. #7764

Open calvinalkan opened 2 years ago

calvinalkan commented 2 years ago

From my understanding of templates, this should throw an error.

https://psalm.dev/r/fbd9f9957c

Or is this the expected behavior?

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

I found these snippets:

https://psalm.dev/r/fbd9f9957c ```php $id *@param Closure():T $factory * */ public function factory(string $id, Closure $factory) :void { $this->factories[$id] = $factory; } } $c = new Container(); // This is invalid. $c->factory(Foo::class, function() :string { return 'bar'; }); // This should be invalid aswell. $c->factory(Foo::class, function() :Bar { return new Bar(); }); ``` ``` Psalm output (using commit f0b2142): ERROR: InvalidArgument - 29:25 - Argument 2 of Container::factory expects Closure():object, pure-Closure():'bar' provided ```
calvinalkan commented 2 years ago

Its not limited to closures.

https://psalm.dev/r/37921aa516

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

I found these snippets:

https://psalm.dev/r/37921aa516 ```php $class_string * @param T $class */ public function someMethod(string $class_string, object $class) :void { // } } $container = new Container(); $container->someMethod(Foo::class, new Bar()); ``` ``` Psalm output (using commit f0b2142): No issues! ```
AndrolGenhald commented 2 years ago

Templates that are used in multiple parameters will automatically widen to accept the necessary types, so in both cases you really have class-string<Foo|Bar> and Foo|Bar $class. I think the best solution here is to fix Psalm so that using a template as a template constraint doesn't cause that automatic type widening so that you could do this, but someone needs to get around to implementing that.

Sort of related to #7549, I think both issues would be fixed by the solution I'm thinking of.

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

I found these snippets:

https://psalm.dev/r/1c6c2963a3 ```php $class_string * @param T2 $class */ public function someMethod(string $class_string, object $class) :void { // } } $container = new Container(); $container->someMethod(Foo::class, new Bar()); ``` ``` Psalm output (using commit f0b2142): No issues! ```