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

[java] UnusedPrivateField doesn't find annotated unused private fields anymore #4166

Closed
phoenix384 opened this issue Oct 18, 2022 · 3 comments · Fixed by #4260
Closed

[java] UnusedPrivateField doesn't find annotated unused private fields anymore #4166

phoenix384 opened this issue Oct 18, 2022 · 3 comments · Fixed by #4260
Assignees
Labels
a:false-negative PMD doesn't flag a problematic piece of code an:enhancement An improvement on existing features / rules
Milestone

Comments

@phoenix384
Copy link

phoenix384 commented Oct 18, 2022

Rule: UnusedPrivateField

With PMD 6.50 the rule UnusedPrivateField was changed to completely ignore any private field with an annotation (see #4100).
But what if the previous behaviour was totally ok? How to restore it? In my case I do want to find unused private fields even with annotation.

If you need a use case for that: Java Selenium Tests
There you annotate WebElements with the locator that points to the corresponding Element on the web page.

It looks like this:

@FindBy(id = "whateverID")
private WebElement myElement;

A class like this could have a bunch of WebElements and if some of them are unused, you don't see it.
If I don't use myElement, I want to remove it and I want PMD to find it.

Maybe at least a kind of whitelist could be added to configure PMD to find fields even if they have annotations that are on this list.

@phoenix384 phoenix384 added the an:enhancement An improvement on existing features / rules label Oct 18, 2022
@adangel
Copy link
Member

adangel commented Oct 18, 2022

Thanks for sharing your use case.

As written already in the comment #4100 (comment):

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.

We can easily add either "reportForAnnotations" or "reportRegardlessOfAnnotations". What I want to avoid however is, that we start maintaining now a whitelist for Java frameworks in PMD (as we did before with the blacklist).

Would that work for you - given the rule has such a property - that you configure the rule to your needs?
Would you rather have a simple flag ("reportRegardlessOfAnnotations") or a more fine granular property ("reportForAnnotations") where you can list individual annotation classes?

@adangel adangel changed the title UnusedPrivateField - How to find unused private fields with annotations now? [java] UnusedPrivateField - How to find unused private fields with annotations now? Oct 18, 2022
@adangel adangel added the needs:user-input Maintainers are waiting for feedback from author label Oct 20, 2022
@adangel adangel added this to the 6.53.0 milestone Nov 17, 2022
@adangel adangel self-assigned this Dec 1, 2022
@adangel
Copy link
Member

adangel commented Dec 8, 2022

I'll add a new property annotations (like in https://pmd.github.io/latest/pmd_rules_java_errorprone.html#missingstaticmethodinnoninstantiatableclass) which can be configured for this use case.
Maybe we can use org.openqa.selenium.support.FindBy as a default value.

@adangel adangel removed the needs:user-input Maintainers are waiting for feedback from author label Dec 8, 2022
@adangel adangel changed the title [java] UnusedPrivateField - How to find unused private fields with annotations now? [java] UnusedPrivateField doesn't find annotated unused private fields anymore Dec 8, 2022
@adangel adangel added the a:false-negative PMD doesn't flag a problematic piece of code label Dec 8, 2022
adangel added a commit to adangel/pmd that referenced this issue Dec 8, 2022
@phoenix384
Copy link
Author

Thank you. That works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:false-negative PMD doesn't flag a problematic piece of code an:enhancement An improvement on existing features / rules
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants