[java] Update UnusedPrivateFieldRule - ignore any annotations

[LynnBroe]

Describe the PR

I improve the rule UnusedPrivatefield and make the annotation @SpyBean can be detected

Related issues

• Fixes [java] UnusedPrivateField - false positive with Lombok @ToString.Include #4033
• Fixes [java] UnusedPrivateField - false positive with Spring @SpyBean #4037

Ready?

• Added unit tests for fixed bug/feature
• Passing all unit tests
• Complete build ./mvnw clean verify passes (checked automatically by github actions)
• Added (in-code) documentation (if needed)

[pmd-test]

	1 Message
📖	Compared to master: This changeset changes 0 violations, introduces 0 new violations, 0 new errors and 0 new configuration errors, removes 55 violations, 2 errors and 6 configuration errors. Full report
✅	Compared to master: This changeset changes 0 violations, introduces 0 new violations, 0 new errors and 0 new configuration errors, removes 55 violations, 2 errors and 6 configuration errors. Full report
✅	Compared to master: This changeset changes 0 violations, introduces 0 new violations, 0 new errors and 0 new configuration errors, removes 0 violations, 2 errors and 6 configuration errors. Full report
✅	Compared to master: This changeset changes 0 violations, introduces 15 new violations, 0 new errors and 0 new configuration errors, removes 0 violations, 0 errors and 0 configuration errors. Full report

Generated by 🚫 Danger

[adangel]

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).

[LynnBroe]

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

[adangel]

Thanks for the updated test case.

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

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.