-
-
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] Refactor annotation suppression #1927
[java] Refactor annotation suppression #1927
Conversation
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AnnotationSuppressionUtil.java
Show resolved
Hide resolved
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AnnotationSuppressionUtil.java
Show resolved
Hide resolved
* nspmd.lang.java.dfa * nspmd.lang.java.symboltable * Some of nspmd.lang.java.metrics The remaining failures are fixed by #1927
Generated by 🚫 Danger |
@@ -96,8 +96,12 @@ public static boolean importsPackage(ASTCompilationUnit node, String packageName | |||
return false; | |||
} | |||
|
|||
/** | |||
* @deprecated Used in only one deprecated rule, which is removed by pending PR #1772 |
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.
#1772 is now merged… shall this method be removed altogether and deprecated in master?
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.
@oowekyala other than that one doubt this looks mergeable. We should also deprecate |
This removes the need for the Java
CanSuppressWarnings
interface, and decouples the AST from rules.See also this PR comment.
The implementation of the interface was duplicated 4 times, but we can use Annotatable to remove the duplication.
Relevant regression tests are in SuppressWarningsTest, which isn't touched by this PR.
TODO on master:
JavaRuleViolation::isSupressed
: there's a typo in the method name, and it doesn't need to be publicAbstractJavaRule::isSuppressed
. It seems like it's only used by one rule deprecated for removalNotes:
SuppressionGroup("unused")
), which would make the suppression mechanism extensible, and also usable by end-users.