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] Fix #4224 MissingStaticMethodInNonInstantiatableClass: Exclude lombok's UtilityClass #4228

Merged
merged 3 commits into from
Nov 25, 2022

Conversation

LynnBroe
Copy link
Contributor

Describe the PR

This PR excludes lombok's UtilityClass which create a private method which throw a exception but PMD treats it as a false negative.

Related issues

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
Copy link

pmd-test commented Nov 23, 2022

1 Message
📖 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 7 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 7 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 7 configuration errors.
Full report

Generated by 🚫 Danger

@adangel adangel changed the title Fix #4224 MissingStaticMethodInNonInstantiatableClass : Exclude lombok's UtilityClass [java] Fix #4224 MissingStaticMethodInNonInstantiatableClass : Exclude lombok's UtilityClass Nov 24, 2022
@adangel adangel added this to the 6.52.0 milestone Nov 24, 2022
Copy link
Member

@adangel adangel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@adangel adangel modified the milestones: 6.52.0, 6.53.0 Nov 24, 2022
Copy link
Member

@adangel adangel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, need to revise my review: Now any class that is annotated with UtilityClass is flagged. However, this is wrong. Only classes that don't have methods should be flagged.

E.g. if you add a method in your test class, then the rule should not be triggered anymore. It doesn't matter, whether the method is static or not since the UtilityClass annotation makes all methods static.
Maybe we also need to look at the visibility of the methods - since if all methods are private, then they can't be called as well and the class remains unusable.

@LynnBroe
Copy link
Contributor Author

Thanks, I will recheck my commit

Copy link
Member

@adangel adangel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@adangel adangel changed the title [java] Fix #4224 MissingStaticMethodInNonInstantiatableClass : Exclude lombok's UtilityClass [java] Fix #4224 MissingStaticMethodInNonInstantiatableClass: Exclude lombok's UtilityClass Nov 25, 2022
@adangel adangel modified the milestones: 6.53.0, 6.52.0 Nov 25, 2022
@adangel adangel merged commit 4c38c5d into pmd:master Nov 25, 2022
adangel added a commit to adangel/pmd that referenced this pull request Nov 25, 2022
adangel added a commit to adangel/pmd that referenced this pull request Nov 25, 2022
[java] Fix pmd#4224 MissingStaticMethodInNonInstantiatableClass: Exclude lombok's UtilityClass pmd#4228
@LynnBroe LynnBroe deleted the issue4224 branch November 29, 2022 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[java] MissingStaticMethodInNonInstantiatableClass should consider Lombok's @UtilityClass
3 participants