-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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] Improve ASTMethodDeclaration::isOverridden #3757
Conversation
3bf8847
to
0a9a80e
Compare
Generated by 🚫 Danger |
Add unit tests
600ed97
to
f261eeb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I like this change!
This removes many false positives for the rules, that depend on the overridden flag:
- CommentRequired won't flag methods, that are overridden by default (there is an option to enable this).
- BooleanGetMethodName doesn't flag methods, which are overridden and thus can't be renamed easily (or not at all)
- LooseCoupling doesn't flag methods, whose return type is a concrete collection type, if the method is overridden
- MethodNamingConvention doesn't flag methods, which are overridden and thus can't be renamed easily (or not at all)
- SignatureDeclareThrowsException
- UseVarArgs
I'll merge this later this week.
.../src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateMethodRule.java
Show resolved
Hide resolved
...java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentRequiredRule.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the review @adangel
Describe the PR
Move the logic that's currently in MissingOverrideRule into a separate attribution pass, so that every rule can benefit from it.
Related issues
isOverridden
in ASTMethodDeclaration #3749Ready?
./mvnw clean verify
passes (checked automatically by github actions)