[java] Fix #559: Improve UseUtilityClassRule to trigger also on static members#6638
Conversation
… static member variables or a mix of static member variables and static methods.
|
Compared to main: (comment created at 2026-05-14 14:55:43+00:00 for c49b909) |
adangel
left a comment
There was a problem hiding this comment.
Thanks for the PR!
I generally agree with you, that it makes sense to improve the rule UseUtilityClass.
However, we should then also do the following:
- Update title and description of #559 to make this: "UseUtilityClass: False negative for constant only classes"
- Update the rule documentation to reflect this change (it only talks about static methods currently...)
- Consider providing different rule violation messages, depending on the case. Currently, we issue "All methods are static. Consider adding a private no-args constructor to prevent instantiation." for the new violation, which not always makes sense. For the new case, better would be something like "All fields are static. Consider ...". Maybe we could even add the class name in the message.
# Conflicts: # pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/UseUtilityClass.xml
|
Thanks for the review! I changed the rule documentation, examples, and error message to talk about "members" instead of "methods". I remembered that nested classes are a thing, and added support for them, even if they are rare. But since a class with any combination of static variables/static methods/static nested classes can now trigger this, I opted to keep the error message the same in all cases.
|
…/design/xml/UseUtilityClass.xml
adangel
left a comment
There was a problem hiding this comment.
Thanks!
FYI: I've updated #559 to remove the label "new:rule" and add the label "a:false-negative". I've also added a note to the description of that issue. I hope, you have all the permissions to do that yourself in the future - if not, we need to check github roles/permissions.
Long title: [java] Fix #559: Improve UseUtilityClassRule to trigger also on classes that have only static member variables, only static nested classes, or any mix of static member variables, static methods and static nested classes.
Describe the PR
While the issue asked for a new rule, I think this fits very well into the existing rule. Also did some minor refactoring to the existing code.
Related issues
Ready?
./mvnw clean verifypasses (checked automatically by github actions)