diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnnotationTypeDeclaration.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnnotationTypeDeclaration.java index 854dfaf029..f73b824f5b 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnnotationTypeDeclaration.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnnotationTypeDeclaration.java @@ -22,4 +22,8 @@ public class ASTAnnotationTypeDeclaration extends AbstractJavaAccessTypeNode { return visitor.visit(this, data); } + public boolean isNested() { + return jjtGetParent() instanceof ASTClassOrInterfaceBodyDeclaration + || jjtGetParent() instanceof ASTAnnotationTypeMemberDeclaration; + } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceDeclaration.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceDeclaration.java index 30d10a5670..8737349eb3 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceDeclaration.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceDeclaration.java @@ -27,7 +27,8 @@ public class ASTClassOrInterfaceDeclaration extends AbstractJavaAccessTypeNode { } public boolean isNested() { - return jjtGetParent() instanceof ASTClassOrInterfaceBodyDeclaration; + return jjtGetParent() instanceof ASTClassOrInterfaceBodyDeclaration + || jjtGetParent() instanceof ASTAnnotationTypeMemberDeclaration; } private boolean isInterface; diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/unusedcode/UnusedModifierRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/unusedcode/UnusedModifierRule.java index fb10b589f8..1d974b1d4d 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/unusedcode/UnusedModifierRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/unusedcode/UnusedModifierRule.java @@ -1,71 +1,118 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ -package net.sourceforge.pmd.lang.java.rule.unusedcode; - -import net.sourceforge.pmd.lang.ast.Node; -import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; -import net.sourceforge.pmd.lang.java.ast.ASTEnumDeclaration; -import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration; -import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; -import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; - -public class UnusedModifierRule extends AbstractJavaRule { - - public Object visit(ASTClassOrInterfaceDeclaration node, Object data) { - if (!node.isNested()) return super.visit(node, data); - - ASTClassOrInterfaceDeclaration parentClassOrInterface = node - .getFirstParentOfType(ASTClassOrInterfaceDeclaration.class); - ASTEnumDeclaration parentEnum = node.getFirstParentOfType(ASTEnumDeclaration.class); - - if (node.isInterface() && node.isPublic()) { - // a public interface - if (parentClassOrInterface != null && parentClassOrInterface.isInterface()) { - // within a interface - addViolation(data, node, getMessage()); - } - } - - if (node.isInterface() && node.isStatic()) { - // a static interface - if (parentClassOrInterface != null || parentEnum != null) { - // within a interface, class or enum - addViolation(data, node, getMessage()); - } - } - - if (!node.isInterface() && (node.isPublic() || node.isStatic())) { - // a public and/or static class - if (parentClassOrInterface != null && parentClassOrInterface.isInterface()) { - // within a interface - addViolation(data, node, getMessage()); - } - } - - return super.visit(node, data); - } - - public Object visit(ASTMethodDeclaration node, Object data) { - if (node.isSyntacticallyPublic() || node.isSyntacticallyAbstract()) { - check(node, data); - } - return super.visit(node, data); - } - - public Object visit(ASTFieldDeclaration node, Object data) { - if (node.isSyntacticallyPublic() || node.isSyntacticallyStatic() || node.isSyntacticallyFinal()) { - check(node, data); - } - return super.visit(node, data); - } - - private void check(Node fieldOrMethod, Object data) { - // third ancestor could be an AllocationExpression - // if this is a method in an anonymous inner class - Node parent = fieldOrMethod.jjtGetParent().jjtGetParent().jjtGetParent(); - if (parent instanceof ASTClassOrInterfaceDeclaration && ((ASTClassOrInterfaceDeclaration) parent).isInterface()) { - addViolation(data, fieldOrMethod); - } - } -} +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.rule.unusedcode; + +import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.java.ast.ASTAnnotationMethodDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTAnnotationTypeDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTEnumDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; + +public class UnusedModifierRule extends AbstractJavaRule { + + @Override + public Object visit(ASTEnumDeclaration node, Object data) { + if (node.isStatic()) { + // a static enum + addViolation(data, node, getMessage()); + } + + return super.visit(node, data); + } + + public Object visit(ASTAnnotationTypeDeclaration node, Object data) { + if (node.isAbstract()) { + // an abstract annotation + addViolation(data, node, getMessage()); + } + + if (!node.isNested()) { + return super.visit(node, data); + } + + Node parent = node.jjtGetParent().jjtGetParent().jjtGetParent(); + boolean isParentInterfaceOrAnnotation = parent instanceof ASTAnnotationTypeDeclaration || + parent instanceof ASTClassOrInterfaceDeclaration && ((ASTClassOrInterfaceDeclaration) parent).isInterface(); + + // a public annotation within an interface or annotation + if (node.isPublic() && isParentInterfaceOrAnnotation) { + addViolation(data, node, getMessage()); + } + + if (node.isStatic()) { + // a static annotation + addViolation(data, node, getMessage()); + } + + return super.visit(node, data); + } + + public Object visit(ASTClassOrInterfaceDeclaration node, Object data) { + if (node.isInterface() && node.isAbstract()) { + // an abstract interface + addViolation(data, node, getMessage()); + } + + if (!node.isNested()) { + return super.visit(node, data); + } + + Node parent = node.jjtGetParent().jjtGetParent().jjtGetParent(); + boolean isParentInterfaceOrAnnotation = parent instanceof ASTAnnotationTypeDeclaration || + parent instanceof ASTClassOrInterfaceDeclaration && ((ASTClassOrInterfaceDeclaration) parent).isInterface(); + + // a public interface within an interface or annotation + if (node.isInterface() && node.isPublic() && isParentInterfaceOrAnnotation) { + addViolation(data, node, getMessage()); + } + + if (node.isInterface() && node.isStatic()) { + // a static interface + addViolation(data, node, getMessage()); + } + + // a public and/or static class within an interface or annotation + if (!node.isInterface() && (node.isPublic() || node.isStatic()) && isParentInterfaceOrAnnotation) { + addViolation(data, node, getMessage()); + } + + return super.visit(node, data); + } + + public Object visit(ASTMethodDeclaration node, Object data) { + if (node.isSyntacticallyPublic() || node.isSyntacticallyAbstract()) { + check(node, data); + } + return super.visit(node, data); + } + + public Object visit(ASTFieldDeclaration node, Object data) { + if (node.isSyntacticallyPublic() || node.isSyntacticallyStatic() || node.isSyntacticallyFinal()) { + check(node, data); + } + return super.visit(node, data); + } + + public Object visit(ASTAnnotationMethodDeclaration node, Object data) { + if (node.isPublic() || node.isAbstract()) { + check(node, data); + } + return super.visit(node, data); + } + + private void check(Node fieldOrMethod, Object data) { + // third ancestor could be an AllocationExpression + // if this is a method in an anonymous inner class + Node parent = fieldOrMethod.jjtGetParent().jjtGetParent().jjtGetParent(); + if (parent instanceof ASTAnnotationTypeDeclaration || + parent instanceof ASTClassOrInterfaceDeclaration + && ((ASTClassOrInterfaceDeclaration) parent).isInterface()) { + addViolation(data, fieldOrMethod); + } + } +} diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/unusedcode/xml/UnusedModifier.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/unusedcode/xml/UnusedModifier.xml index 1937fd763e..51a25296f6 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/unusedcode/xml/UnusedModifier.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/unusedcode/xml/UnusedModifier.xml @@ -293,6 +293,138 @@ public enum TestEnum { ; public interface EnumInnerInterface { } +} + ]]> + + + + Unnecessary public on annotation element + 1 + + + + Unnecessary abstract on annotation element + 1 + + + + Unnecessary final on annotation field + 1 + + + + Unnecessary static on annotation field + 1 + + + + Unnecessary public on annotation field + 1 + + + + Unnecessary public on annotation nested class + 1 + + + + Unnecessary static on annotation nested class + 1 + + + + Unnecessary public on annotation nested interface + 1 + + + + Unnecessary public on annotation nested annotation + 1 + + + + Unnecessary static on annotation nested enum + 1 + + + + Unnecessary static on interface nested enum + 1 + + + + Unnecessary static on class nested enum + 1 + + + + Unnecessary abstract on interface + 1 + + + + Unnecessary abstract on annotation + 1 +