phoedos/pmd
1
1
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[java] Revamp UnusedModifier

Browse Source 
- This covers a ton of missing cases
 - Fixes #246
 - Fixes #247
 - Fixes #248
 - Notice `isNested` has been improved on `ASTClassOrInterfaceDeclaration`
    to cover being nested in an annotation definition
This commit is contained in:
Juan Martín Sotuyo Dodero
2017-02-08 14:55:00 -03:00
committed by Andreas Dangel
parent e16ee448da
commit 1fb05d995f
4 changed files with 256 additions and 72 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnnotationTypeDeclaration.java
View File

@@ -22,4 +22,8 @@ public class ASTAnnotationTypeDeclaration extends AbstractJavaAccessTypeNode {
return visitor.visit(this, data);
}
public boolean isNested() {
return jjtGetParent() instanceof ASTClassOrInterfaceBodyDeclaration
|| jjtGetParent() instanceof ASTAnnotationTypeMemberDeclaration;
}
}

3
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceDeclaration.java
View File

@@ -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;

189
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/unusedcode/UnusedModifierRule.java
View File

@@ -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);
}
}
}

132
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/unusedcode/xml/UnusedModifier.xml
View File

@@ -293,6 +293,138 @@ public enum TestEnum {
;
public interface EnumInnerInterface {
}
}
]]></code>
</test-code>
<test-code>
<description>Unnecessary public on annotation element</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public @interface TestAnnotation {
public String message();
}
]]></code>
</test-code>
<test-code>
<description>Unnecessary abstract on annotation element</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public @interface TestAnnotation {
abstract String message();
}
]]></code>
</test-code>
<test-code>
<description>Unnecessary final on annotation field</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public @interface TestAnnotation {
final String message = "MESSAGE";
}
]]></code>
</test-code>
<test-code>
<description>Unnecessary static on annotation field</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public @interface TestAnnotation {
static String message = "MESSAGE";
}
]]></code>
</test-code>
<test-code>
<description>Unnecessary public on annotation field</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public @interface TestAnnotation {
public String message = "MESSAGE";
}
]]></code>
</test-code>
<test-code>
<description>Unnecessary public on annotation nested class</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public @interface TestAnnotation {
public class Inner {
}
}
]]></code>
</test-code>
<test-code>
<description>Unnecessary static on annotation nested class</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public @interface TestAnnotation {
static class Inner {
}
}
]]></code>
</test-code>
<test-code>
<description>Unnecessary public on annotation nested interface</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public @interface TestAnnotation {
public interface Inner {
}
}
]]></code>
</test-code>
<test-code>
<description>Unnecessary public on annotation nested annotation</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public @interface TestAnnotation {
public @interface Inner {
}
}
]]></code>
</test-code>
<test-code>
<description>Unnecessary static on annotation nested enum</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public @interface TestAnnotation {
public static enum Inner {
}
}
]]></code>
</test-code>
<test-code>
<description>Unnecessary static on interface nested enum</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public interface TestInterface {
public static enum Inner {
}
}
]]></code>
</test-code>
<test-code>
<description>Unnecessary static on class nested enum</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public class TestClass {
public static enum Inner {
}
}
]]></code>
</test-code>
<test-code>
<description>Unnecessary abstract on interface</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public abstract interface TestInterface {
}
]]></code>
</test-code>
<test-code>
<description>Unnecessary abstract on annotation</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public abstract @interface TestAnnotation {
}
]]></code>
</test-code>