Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

add failing tests #1330

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
rajyan wants to merge 1 commit into phpstan:1.7.x from rajyan:fix/issue-6402
Closed

add failing tests #1330

rajyan wants to merge 1 commit into phpstan:1.7.x from rajyan:fix/issue-6402

Conversation

@rajyan
Copy link
Contributor

@rajyan rajyan commented May 19, 2022

Just to start with phpstan/phpstan#6402

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

Copy link
Member

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 False positive "Readonly property is already assigned" in if-else phpstan#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 (PHPDoc-based type narrowing #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 :)

rajyan reacted with thumbs up emoji rajyan, MisatoTremor, fluffycondor, and janedbal reacted with rocket emoji

Copy link
Contributor Author

rajyan commented May 20, 2022

Wow! The picture was much bigger than I thought.
This sounds really interesting too.
I'm currently working on #1270 (comment) mainly, I think I can look into this one after it 😄

rvanlaak reacted with thumbs up emoji

Copy link
Member

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 😊

rajyan and rvanlaak reacted with thumbs up emoji rajyan reacted with laugh emoji

Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /