phpstan-src icon indicating copy to clipboard operation
phpstan-src copied to clipboard

add failing tests

Open rajyan opened this issue 3 years ago • 3 comments

Just to start with https://github.com/phpstan/phpstan/issues/6402

I'm not sure all these test are solvable or not.

rajyan avatar May 19 '22 13:05 rajyan

Hi, I have ideas about this, but it needs a much bigger picture - I'll introduce to the whole picture in this comment :)

So, there are some related bugs/feature requests:

  • The linked issue https://github.com/phpstan/phpstan/issues/6402 - shouldn't report an error but currently does
  • Being able to detect when the readonly property is really assigned multiple times (like inside a while loop) - currently not reported
  • Being able to detect when the readonly property is only assigned in an if but not else - currently not reported
  • Being able to detect unused variables (it was written/created but later it's not read)
  • Being able to detect unused function/method parameters
  • Verify that function body really does what its conditional return type says
  • Verify that function body really does what its @phpstan-assert says (https://github.com/phpstan/phpstan-src/pull/1317)

What all of these have in common is that you need to construct a graph of nodes with relations between them - where values are written and when they are read. Psalm has some of these features and this article (https://psalm.dev/articles/better-unused-variable-detection) talks about it.

I've kindly asked @arnaud-lb to work on this and right now it's in a form of this package: https://github.com/arnaud-lb/php-sema which should somehow be adopted in PHPStan. Not sure if we can use it directly, most likely we'll adapt the algorithms and apply them here.

The priority for me is to get rid of the false positives about readonly properties. At the same time we should be able to detect more situations where readonly property assignment is missing or there is a risk of multiple assignments.

After that is all figured out and released, we can work on the dead variable rules etc. :)

Are you interested in continuing this work? Thanks :)

ondrejmirtes avatar May 19 '22 15:05 ondrejmirtes

Wow! The picture was much bigger than I thought. This sounds really interesting too. I'm currently working on https://github.com/phpstan/phpstan-src/pull/1270#issuecomment-1115079016 mainly, I think I can look into this one after it :smile:

rajyan avatar May 20 '22 00:05 rajyan

Oh wow, you're definitely not picking the easy stuff 😊 I'm looking forward to that too! Thank you very much.

And remember - more smaller PRs is better than a single huge one 😊

ondrejmirtes avatar May 20 '22 02:05 ondrejmirtes

Hi, I'm cleaning up old and stale PRs. I don't think we need to keep this one open :)

ondrejmirtes avatar Oct 16 '22 10:10 ondrejmirtes