TomasVotruba / unused-public

Find Unused Public Elements in Your Code
https://tomasvotruba.com/blog/can-phpstan-find-dead-public-methods/
MIT License
151 stars 12 forks source link

add phpstan error identifiers #118

Closed pilif closed 5 months ago

pilif commented 5 months ago

For easier handling of ignores, PHPStan 1.11 has added error identifiers.

This PR adds them to the various reports generated by the unused-public extension.

As far as I can tell, the built-in testing framework for rules doesn't yet support error identifiers and thus I can't really provide test-cases for these. Correct me if I'm wrong and I'll gladly add test-cases.

TomasVotruba commented 5 months ago

Thanks for addition! 👍

"Method unused" could suggest its an unused private method. Could you add word "public.*" to make it more clear?

pilif commented 5 months ago

I intentionally chose identifiers that were already known to PHPStan, especially given that they are all semantically matching these rules.

I thought that by using existing identifiers, tools which make use of identifiers would work out of the box and would be able to make use of the existing semantics.

Adding a public.* prefix would a) cause these identifiers to be different in format from all existing ones (by having three segments rather than two like all others) and b) cause them to be distinct from the known ones with known semantics

then you look at the list of known identifiers you will see that many rules share identifiers already and none of the identifiers make a difference depending on visibility of where the issue is raised.

I’m totally willing to make the change and I’m absolutely not in the habit of arguing with maintainers (I am very well aware how busy you are), but I would love to get some insight into the why of your request given the considerations above.

TomasVotruba commented 5 months ago

Using already existing PHPStan identifiers doesn't seem like a good idea. It's like naming a service with same generic like "manager" string, overriding all existing services with same name. Hence the prefix

pilif commented 5 months ago

force pushed to add the prefix.

TomasVotruba commented 5 months ago

Thank you :pray: