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

pmd: fix #938 False positive on LooseCoupling for overriding methods

Browse Source
This commit is contained in:
Andreas Dangel committed 2013-03-30 15:04:47 +01:00
1 parent 5d1d82919c
commit f0d8759b79
8 files changed
+91 -1
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

pmd/etc/changelog.txt
+1
View File
@@ -15,6 +15,7 @@ New Java rule:
????? ??, 2013 - 5.0.3:
Fixed bug 938: False positive on LooseCoupling for overriding methods
Fixed bug 940: False positive on UnsynchronizedStaticDateFormatter
Fixed bug 942: CheckResultSet False Positive and Negative
Fixed bug 943: PreserveStackTrace false positive if a StringBuffer exists
pmd/src/main/java/net/sourceforge/pmd/lang/java/rule/coupling/LooseCouplingRule.java
+22 -1
View File
@@ -4,9 +4,13 @@
package net.sourceforge.pmd.lang.java.rule.coupling;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.ast.ASTAnnotation;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBodyDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType;
import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTFormalParameter;
import net.sourceforge.pmd.lang.java.ast.ASTMarkerAnnotation;
import net.sourceforge.pmd.lang.java.ast.ASTName;
import net.sourceforge.pmd.lang.java.ast.ASTResultType;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.util.CollectionUtil;
@@ -22,11 +26,28 @@ public class LooseCouplingRule extends AbstractJavaRule {
// });
public Object visit(ASTClassOrInterfaceType node, Object data) {
Node parent = node.jjtGetParent().jjtGetParent().jjtGetParent();
if (methodHasOverride(node)) {
return data;
}
Node parent = node.getNthParent(3);
String typeName = node.getImage();
if (CollectionUtil.isCollectionType(typeName, false) && (parent instanceof ASTFieldDeclaration || parent instanceof ASTFormalParameter || parent instanceof ASTResultType)) {
addViolation(data, node, typeName);
}
return data;
}
private boolean methodHasOverride(Node node) {
ASTClassOrInterfaceBodyDeclaration method = node.getFirstParentOfType(ASTClassOrInterfaceBodyDeclaration.class);
if (method.jjtGetNumChildren() > 0 && method.jjtGetChild(0) instanceof ASTAnnotation) {
ASTMarkerAnnotation marker = method.getFirstDescendantOfType(ASTMarkerAnnotation.class);
if (marker != null && marker.getFirstChildOfType(ASTName.class) != null) {
ASTName name = marker.getFirstChildOfType(ASTName.class);
if ("Override".equals(name.getImage())) {
return true;
}
}
}
return false;
}
}
pmd/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java
+8
View File
@@ -663,6 +663,14 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter {
}
}
}
if (myType == null && qualifiedName != null && !qualifiedName.contains(".")) {
// try again with java.lang....
try {
myType = pmdClassLoader.loadClass("java.lang." + qualifiedName);
} catch (Exception e) {
// ignored
}
}
if (myType != null) {
node.setType(myType);
}
pmd/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/rules/LooseCoupling.java
+22
View File
@@ -4,9 +4,13 @@
package net.sourceforge.pmd.lang.java.typeresolution.rules;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.ast.ASTAnnotation;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBodyDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType;
import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTFormalParameter;
import net.sourceforge.pmd.lang.java.ast.ASTMarkerAnnotation;
import net.sourceforge.pmd.lang.java.ast.ASTName;
import net.sourceforge.pmd.lang.java.ast.ASTResultType;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.util.CollectionUtil;
@@ -18,6 +22,9 @@ public class LooseCoupling extends AbstractJavaRule {
@Override
public Object visit(ASTClassOrInterfaceType node, Object data) {
if (methodHasOverride(node)) {
return data;
}
Node parent = node.getNthParent(3);
Class<?> clazzType = node.getType();
boolean isType = CollectionUtil.isCollectionType(clazzType, false);
@@ -27,4 +34,19 @@ public class LooseCoupling extends AbstractJavaRule {
}
return data;
}
private boolean methodHasOverride(Node node) {
ASTClassOrInterfaceBodyDeclaration method = node.getFirstParentOfType(ASTClassOrInterfaceBodyDeclaration.class);
if (method.jjtGetNumChildren() > 0 && method.jjtGetChild(0) instanceof ASTAnnotation) {
ASTMarkerAnnotation marker = method.getFirstDescendantOfType(ASTMarkerAnnotation.class);
if (marker != null && marker.getFirstChildOfType(ASTName.class) != null) {
ASTName name = marker.getFirstChildOfType(ASTName.class);
System.out.println(name.getType());
if (name.getType() == Override.class) {
return true;
}
}
}
return false;
}
}
pmd/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java
+6
View File
@@ -20,6 +20,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTFormalParameter;
import net.sourceforge.pmd.lang.java.ast.ASTImportDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTLiteral;
import net.sourceforge.pmd.lang.java.ast.ASTName;
import net.sourceforge.pmd.lang.java.ast.ASTNullLiteral;
import net.sourceforge.pmd.lang.java.ast.ASTReferenceType;
import net.sourceforge.pmd.lang.java.ast.ASTStatementExpression;
@@ -31,6 +32,7 @@ import net.sourceforge.pmd.lang.java.ast.TypeNode;
import net.sourceforge.pmd.lang.java.typeresolution.ClassTypeResolver;
import net.sourceforge.pmd.typeresolution.testdata.AnonymousInnerClass;
import net.sourceforge.pmd.typeresolution.testdata.ArrayListFound;
import net.sourceforge.pmd.typeresolution.testdata.DefaultJavaLangImport;
import net.sourceforge.pmd.typeresolution.testdata.ExtraTopLevelClass;
import net.sourceforge.pmd.typeresolution.testdata.InnerClass;
import net.sourceforge.pmd.typeresolution.testdata.Literals;
@@ -65,6 +67,10 @@ public class ClassTypeResolverTest {
assertEquals(ArrayList.class, acu.getFirstDescendantOfType(ASTVariableDeclaratorId.class).getType());
assertEquals(ArrayList.class, acu.getFirstDescendantOfType(ASTVariableDeclarator.class).getType());
assertEquals(ArrayList.class, acu.getFirstDescendantOfType(ASTFieldDeclaration.class).getType());
acu = parseAndTypeResolveForClass(DefaultJavaLangImport.class);
assertEquals(String.class, acu.getFirstDescendantOfType(ASTClassOrInterfaceType.class).getType());
assertEquals(Override.class, acu.findDescendantsOfType(ASTName.class).get(1).getType());
}
@Test
pmd/src/test/java/net/sourceforge/pmd/typeresolution/testdata/DefaultJavaLangImport.java
Vendored
+8
View File
@@ -0,0 +1,8 @@
package net.sourceforge.pmd.typeresolution.testdata;
public class DefaultJavaLangImport {
@Override
public String toString() {
return "foo";
}
}
pmd/src/test/resources/net/sourceforge/pmd/lang/java/rule/coupling/xml/LooseCoupling.xml
+11
View File
@@ -124,4 +124,15 @@ public class Foo {
}
]]></code>
</test-code>
<test-code>
<description>#938 False positive on LooseCoupling for overriding methods</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Test {
@Override
public LinkedHashMap findGetters() {}
}
]]></code>
</test-code>
</test-data>
pmd/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/LooseCoupling.xml
+13
View File
@@ -145,4 +145,17 @@ public class Foo {
}
]]></code>
</test-code>
<test-code>
<description>#938 False positive on LooseCoupling for overriding methods</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import java.util.LinkedHashMap;
public class Test {
@Override
public LinkedHashMap findGetters() {}
}
]]></code>
</test-code>
</test-data>