From a9d0de9450f61aee3a533dceecb77b4a376faa83 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 4 Nov 2016 12:41:57 +0100 Subject: [PATCH] Fixes #1532 [java] CloneMethodMustImplementCloneable: Implemented Interface extends Cloneable --- .../CloneMethodMustImplementCloneable.java | 70 ++++++++++++++++--- .../main/resources/rulesets/java/clone.xml | 4 +- .../xml/CloneMethodMustImplementCloneable.xml | 10 +++ .../xml/CloneMethodMustImplementCloneable.xml | 27 +++++++ src/site/markdown/overview/changelog.md | 1 + 5 files changed, 101 insertions(+), 11 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/rules/CloneMethodMustImplementCloneable.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/rules/CloneMethodMustImplementCloneable.java index d48fb0398a..497f15e00a 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/rules/CloneMethodMustImplementCloneable.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/rules/CloneMethodMustImplementCloneable.java @@ -4,13 +4,16 @@ package net.sourceforge.pmd.lang.java.typeresolution.rules; import java.util.Arrays; +import java.util.HashSet; import java.util.List; +import java.util.Set; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.java.ast.ASTBlock; import net.sourceforge.pmd.lang.java.ast.ASTBlockStatement; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType; +import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; import net.sourceforge.pmd.lang.java.ast.ASTExtendsList; import net.sourceforge.pmd.lang.java.ast.ASTFormalParameters; import net.sourceforge.pmd.lang.java.ast.ASTImplementsList; @@ -31,6 +34,13 @@ public class CloneMethodMustImplementCloneable extends AbstractJavaRule { @Override public Object visit(ASTClassOrInterfaceDeclaration node, Object data) { + if (extendsOrImplementsCloneable(node)) { + return data; + } + return super.visit(node, data); + } + + private boolean extendsOrImplementsCloneable(ASTClassOrInterfaceDeclaration node) { ASTImplementsList impl = node.getFirstChildOfType(ASTImplementsList.class); if (impl != null && impl.jjtGetParent().equals(node)) { for (int ix = 0; ix < impl.jjtGetNumChildren(); ix++) { @@ -43,14 +53,16 @@ public class CloneMethodMustImplementCloneable extends AbstractJavaRule { ASTClassOrInterfaceType type = (ASTClassOrInterfaceType) child; if (type.getType() == null) { if ("Cloneable".equals(type.getImage())) { - return data; + return true; } } else if (type.getType().equals(Cloneable.class)) { - return data; + return true; + } else if (Cloneable.class.isAssignableFrom(type.getType())) { + return true; } else { List> implementors = Arrays.asList(type.getType().getInterfaces()); if (implementors.contains(Cloneable.class)) { - return data; + return true; } } } @@ -59,23 +71,21 @@ public class CloneMethodMustImplementCloneable extends AbstractJavaRule { ASTClassOrInterfaceType type = (ASTClassOrInterfaceType) node.jjtGetChild(0).jjtGetChild(0); Class clazz = type.getType(); if (clazz != null && clazz.equals(Cloneable.class)) { - return data; + return true; } while (clazz != null && !Object.class.equals(clazz)) { if (Arrays.asList(clazz.getInterfaces()).contains(Cloneable.class)) { - return data; + return true; } clazz = clazz.getSuperclass(); } } - - return super.visit(node, data); + return false; } @Override public Object visit(ASTMethodDeclaration node, Object data) { - ASTClassOrInterfaceDeclaration classOrInterface = node - .getFirstParentOfType(ASTClassOrInterfaceDeclaration.class); + ASTClassOrInterfaceDeclaration classOrInterface = node.getFirstParentOfType(ASTClassOrInterfaceDeclaration.class); if (classOrInterface != null && //Don't analyze enums, which cannot subclass clone() (node.isFinal() || classOrInterface.isFinal())) { if (node.findDescendantsOfType(ASTBlock.class).size() == 1) { @@ -93,9 +103,51 @@ public class CloneMethodMustImplementCloneable extends AbstractJavaRule { } } } + + // Now check other whether implemented or extended classes are defined inside the same file + if (classOrInterface != null) { + Set classesNames = determineTopLevelCloneableClasses(classOrInterface); + + ASTImplementsList implementsList = classOrInterface.getFirstChildOfType(ASTImplementsList.class); + if (implementsList != null) { + List types = implementsList.findChildrenOfType(ASTClassOrInterfaceType.class); + for (ASTClassOrInterfaceType t : types) { + if (classesNames.contains(t.getImage())) { + return data; + } + } + } + + ASTExtendsList extendsList = classOrInterface.getFirstChildOfType(ASTExtendsList.class); + if (extendsList != null) { + ASTClassOrInterfaceType type = extendsList.getFirstChildOfType(ASTClassOrInterfaceType.class); + if (classesNames.contains(type.getImage())) { + return data; + } + } + } + return super.visit(node, data); } + /** + * Determines all the class/interface declarations inside this compilation unit, + * which implement Cloneable + * @param currentClass the node of the class, that is currently analyzed (inside this compilation unit) + * @return a Set of class/interface names + */ + private Set determineTopLevelCloneableClasses(ASTClassOrInterfaceDeclaration currentClass) { + List classes = currentClass.getFirstParentOfType(ASTCompilationUnit.class) + .findDescendantsOfType(ASTClassOrInterfaceDeclaration.class); + Set classesNames = new HashSet(); + for (ASTClassOrInterfaceDeclaration c : classes) { + if (c != currentClass && extendsOrImplementsCloneable(c)) { + classesNames.add(c.getImage()); + } + } + return classesNames; + } + @Override public Object visit(ASTMethodDeclarator node, Object data) { if (!"clone".equals(node.getImage())) { diff --git a/pmd-java/src/main/resources/rulesets/java/clone.xml b/pmd-java/src/main/resources/rulesets/java/clone.xml index e1ce5bb9d3..c3bf51b8b1 100644 --- a/pmd-java/src/main/resources/rulesets/java/clone.xml +++ b/pmd-java/src/main/resources/rulesets/java/clone.xml @@ -101,8 +101,8 @@ The method clone() should only be implemented if the class implements the Clonea implements @Readonly List<@Readonly T> {} ]]> + + + #1532 [java] CloneMethodMustImplementCloneable: Implemented Interface extends Cloneable - part 1: interface + 0 + + diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/CloneMethodMustImplementCloneable.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/CloneMethodMustImplementCloneable.xml index 2db0d188df..554b3283cb 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/CloneMethodMustImplementCloneable.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/CloneMethodMustImplementCloneable.xml @@ -169,4 +169,31 @@ public enum Foo { public class UnmodifiableList implements @Readonly List<@Readonly T> {} ]]> + + + #1532 [java] CloneMethodMustImplementCloneable: Implemented Interface extends Cloneable - part 1: interface + 0 + + + + + #1532 [java] CloneMethodMustImplementCloneable: Implemented Interface extends Cloneable + 0 + + diff --git a/src/site/markdown/overview/changelog.md b/src/site/markdown/overview/changelog.md index 403b93ccde..c6e2439e84 100644 --- a/src/site/markdown/overview/changelog.md +++ b/src/site/markdown/overview/changelog.md @@ -32,6 +32,7 @@ * java-imports/UnusedImports * [#1529](https://sourceforge.net/p/pmd/bugs/1529/): \[java] UnusedImports: The created rule violation has no class name * java-typeresolution/CloneMethodMustImplementCloneable + * [#1532](https://sourceforge.net/p/pmd/bugs/1532/): \[java] CloneMethodMustImplementCloneable: Implemented Interface extends Cloneable * [#1534](https://sourceforge.net/p/pmd/bugs/1534/): \[java] CloneMethodMustImplementCloneable: ClassCastException with Annotation (java8) * java-typeresolution/SignatureDeclareThrowsException * [#1535](https://sourceforge.net/p/pmd/bugs/1535/): \[java] SignatureDeclareThrowsException: ClassCastException with Annotation