PDOStatement conditional fetch/fetchAll return values

[Anders Jenbo (AJenbo)]

No description provided.

[Gleb Sieemshchikov (LolGleb)]

It seems wrong to have two @return tags inside one PhpDoc. It would be better to use different tag like @psalm-return or @phpstan-return. Or just remove the old @return tag

[Anders Jenbo (AJenbo)]

I totally missed that, thanks.

I went with the @phpstan-return option since the conditions is already a fairly big block tacking a long comment at the end seems a bit much.

[Christian Sciberras (uuf6429)]

I suggest copying it for psalm (to avoid preferential treatment) and having a simplified @return for e.g. phpstorm.

I also had the same type of failure with those changes and solved it in my PR here: https://github.com/JetBrains/phpstorm-stubs/pull/1810/changes#diff-41a14d60208b4270d5d5d096abca3eb48024718babf786e207c3ab54592000a8R270 (‎tests/StubsPhpDocTest.php)

[Markus Staab (staabm)]

I suggest copying it for psalm

you should give it a test in psalm before. IIRC psalm (and also PHPStan) pick the others phpdoc (in case no more tools-specific phpdoc exists). so the PR as is, might already work for psalm

[Christian Sciberras (uuf6429)]

I know they support each other, my concern is more about preferential treatment... Unless the project owners consciously decide to go for one tool and not the other officially, we might end up with different contributions using either tool (inconsistently).

[Gleb Sieemshchikov (LolGleb)]

Hi Anders Jenbo (@AJenbo) , сould you please mark this PR as ready for review? It's currently in draft state, which prevents us from merging it

[Christian Sciberras (uuf6429)]

Gleb Sieemshchikov (@LolGleb) there are at least two remaining open points:

1. Tests are failing because @phpstan-return is currently not allowed/expected (can be fixed as per my link above, or if my PR is merged first).
2. I believe we still need a general @return that more tools (such as phpstorm) can understand (instead of the absurdly-generic-to-the-point-of-being-useless @return array).

Then there's the point about having @phpstan- and @psalm- annotations for consistency or preferring one over the other (or none). That's more of a "business decision" - but now is the best time to decide, before we end up with a mishmash that needs to be maintained.

[Anders Jenbo (AJenbo)]

Wow can't belive this was almost a month ago, sorry for the long wait I have just been insanely busy with other things thinking I was going to update this one at the end of the day or next day repeatedly. Well it's done now, except for uuf6429's point 2 which i find contradicts the note about having both the complicated logic and very long comment split appart.