Merge branch 'bug-1532' into pmd/5.3.x

This commit is contained in:
Andreas Dangel
2016-11-04 12:46:05 +01:00
5 changed files with 101 additions and 11 deletions

View File

@ -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<Class<?>> 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<String> classesNames = determineTopLevelCloneableClasses(classOrInterface);
ASTImplementsList implementsList = classOrInterface.getFirstChildOfType(ASTImplementsList.class);
if (implementsList != null) {
List<ASTClassOrInterfaceType> 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<String> determineTopLevelCloneableClasses(ASTClassOrInterfaceDeclaration currentClass) {
List<ASTClassOrInterfaceDeclaration> classes = currentClass.getFirstParentOfType(ASTCompilationUnit.class)
.findDescendantsOfType(ASTClassOrInterfaceDeclaration.class);
Set<String> classesNames = new HashSet<String>();
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())) {

View File

@ -101,8 +101,8 @@ The method clone() should only be implemented if the class implements the Clonea
<value>
<![CDATA[
//ClassOrInterfaceDeclaration
[not(./ImplementsList/ClassOrInterfaceType
[@Image='Cloneable'])]
[not(./ExtendsList/ClassOrInterfaceType[@Image='Cloneable'])]
[not(./ImplementsList/ClassOrInterfaceType[@Image='Cloneable'])]
/ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration
[MethodDeclaration
[MethodDeclarator[@Image

View File

@ -91,4 +91,14 @@ throw new CloneNotSupportedException();
public class UnmodifiableList<T> implements @Readonly List<@Readonly T> {}
]]></code>
</test-code>
<test-code>
<description>#1532 [java] CloneMethodMustImplementCloneable: Implemented Interface extends Cloneable - part 1: interface</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
interface TestInterface extends Cloneable {
TestInterface clone();
}
]]></code>
</test-code>
</test-data>

View File

@ -169,4 +169,31 @@ public enum Foo {
public class UnmodifiableList<T> implements @Readonly List<@Readonly T> {}
]]></code>
</test-code>
<test-code>
<description>#1532 [java] CloneMethodMustImplementCloneable: Implemented Interface extends Cloneable - part 1: interface</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
interface TestInterface extends Cloneable {
TestInterface clone();
}
]]></code>
</test-code>
<test-code>
<description>#1532 [java] CloneMethodMustImplementCloneable: Implemented Interface extends Cloneable</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
interface TestInterface extends Cloneable {
TestInterface clone();
}
class CloneableClass implements TestInterface {
@Override // creates a warning though CloneableClass is actually implementing Cloneable
public CloneableClass clone() {
// clone implementation
}
}
]]></code>
</test-code>
</test-data>

View File

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