[java] Update UnusedPrivateFieldRule - ignore any annotations#4100
Conversation
revise SpyBean about unusedprivatefield
Generated by 🚫 Danger |
adangel
left a comment
There was a problem hiding this comment.
Thanks for your PR!
Please add a new test case to prove that #4037 is indeed fixed.
However, I think it's time to test another strategy for fixing these issues: Please try out what happens, if this rule just doesn't report any private fields, that has any annotations.
Because otherwise, we will keep adding one annotation after another, until every java framework is represented in this rule.
If this new strategy works and doesn't create too many false positives, then we would also have a fix for #4033 (and possible other issues).
|
Hi, adangel, I have added the test case, could you please help check it? Besides, I think it is inappropriate to dismiss all fields with annotations because many annotations may do not affect the current rule under checking. Fixing these issues is very easy, but dismissing them could lead to many false negatives. Thanks |
|
Thanks for the updated test case.
That's why I wanted to try this out to see the impact. I just did it - see the last regression tester report. It's not too bad IMHO. Static code analysis always needs to balance false positives and false negatives. In that case - the rule is in category "best practices" - there is no big harm in having some false negatives (no program will crash because of an unused private field). Therefore I believe it is more important to reduce false positives than to avoid any false negatives. If the rule is too noisy it may lead to being ignored or disabled and maybe even removed from the ruleset entirely. In the end, the rule might not be used at all anymore. I decided to deprecate the property "ignoreAnnotations". If there are good examples for false negatives, we might think of adding a new property to change the behavior, to deal with some annotations in a special way (e.g. "reportForAnnotations") or have a boolean flag (e.g. "reportRegardlessOfAnnotations"=true) which could be enabled if needed and would report all unused private fields whether annotated or not. Having a more general solution also has the positive side effect, that #4033 is also fixed now. |
[java] Update UnusedPrivateFieldRule - ignore any annotations pmd#4100
Describe the PR
I improve the rule UnusedPrivatefield and make the annotation @spybean can be detected
Related issues
Ready?
./mvnw clean verifypasses (checked automatically by github actions)