Skip to content
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

Detect unused protected methods #144

Closed
wants to merge 3 commits into from
Closed

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Dec 16, 2024

POC for #143

@staabm
Copy link
Contributor Author

staabm commented Dec 16, 2024

I intentionally kept the PR to a very minimum to see whats actually needed.
in case we agree on this change, I would rename a few classes, e.g.
PublicClassMethodMatcher -> UnusedClassMethodMatcher


I will run this POC on a real world codebase in the mid of the week to get a better idea whether we are missing something important

@staabm
Copy link
Contributor Author

staabm commented Jan 4, 2025

@TomasVotruba I think this will work. how could you imagine support for protected methods?

do we need separate rules and collectors for protected stuff or can we change "public" into "not-private"?
would we need separate feature switches in the phpstan config file to enable "protected" analsis?

(I would be fine with only supporting protected methods. I don't need support for protected properties. IMO methods are way more valuable then constants or properties as they contain way more code)

@TomasVotruba
Copy link
Owner

Not sure this is worth it, as provided method are private as in final class. Coincidentally wrote about it here :)
https://tomasvotruba.com/blog/why-final-classes-make-rector-and-phpstan-more-powerful

I'd prefer to keep this package simple, focus on public only and let final handle the private. There PHPStan detect unused methods out of the box.

@staabm staabm closed this Jan 4, 2025
@staabm staabm deleted the prot branch January 4, 2025 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants