vimeo / psalm

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

[Enhancement] Emit an error when phpdoc is impossible #5990

Open mr-feek opened 3 years ago

mr-feek commented 3 years ago

https://psalm.dev/r/af5065a4c0

Given that bar has a native return type of string, shouldn't psalm be able to emit an error that the phpdoc is invalid? I'm thinking that phpdoc should only become the source of truth if there are no native typehints or the typehints are castable to each other..

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

I found these snippets:

https://psalm.dev/r/af5065a4c0 ```php bar(); } ``` ``` Psalm output (using commit 38d3b15): ERROR: InvalidReturnStatement - 13:12 - The inferred type 'array' does not match the declared return type 'string' for test ERROR: InvalidReturnType - 11:26 - The declared return type 'string' for test is incorrect, got 'array' ```
weirdan commented 3 years ago

I think it should be consistent with @return: https://psalm.dev/r/cba0b4b5a3, i.e. trust the docblock but emit MismatchingDocblock issue

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

I found these snippets:

https://psalm.dev/r/cba0b4b5a3 ```php bar(); } else { return $foo->baz(); } } ``` ``` Psalm output (using commit 38d3b15): ERROR: InvalidReturnStatement - 17:16 - The inferred type 'array' does not match the declared return type 'string' for test ERROR: InvalidReturnStatement - 19:16 - The inferred type 'array' does not match the declared return type 'string' for test ERROR: InvalidReturnType - 14:26 - The declared return type 'string' for test is incorrect, got 'array' ERROR: MismatchingDocblockReturnType - 10:17 - Docblock has incorrect return type 'array', should be 'string' ```
orklah commented 3 years ago

I think this is a duplicate of https://github.com/vimeo/psalm/issues/5786

weirdan commented 3 years ago

We don't have any inheritance here, so the only possible developer intent is to clarify native return type. This makes it very clear that contradiction is a different problem to LSP violations that linked ticket discusses.