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

pmd: fix #1005 False + for ConstructorCallsOverridableMethod - overloaded methods

Browse Source 
It won't be perfect, but for constructor calls and method calls a
simple check of the argument types is done. Primitive types are
distinguished, reference types not.
In order to support all cases, full symbol resolution is needed.
This commit is contained in:
Andreas Dangel
2013-03-14 20:48:05 +01:00
parent 3b14b3f5cb
commit 8eac048c03
3 changed files with 98 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
pmd/etc/changelog.txt
View File

@ -1,6 +1,7 @@
????? ??, 2013 - 5.0.3:
Fixed bug 1002: False +: FinalFieldCouldBeStatic on inner class
Fixed bug 1005: False + for ConstructorCallsOverridableMethod - overloaded methods
Fixed bug 1064: Exception running PrematureDeclaration
Fixed bug 1068: CPD fails on broken symbolic links
Fixed bug 1073: Hard coded violation messages CommentSize

92
pmd/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ConstructorCallsOverridableMethodRule.java
View File

@ -13,12 +13,15 @@ import java.util.Set;
import java.util.TreeMap;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.ast.ASTArgumentList;
import net.sourceforge.pmd.lang.java.ast.ASTArguments;
import net.sourceforge.pmd.lang.java.ast.ASTBooleanLiteral;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit;
import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTEnumDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTExplicitConstructorInvocation;
import net.sourceforge.pmd.lang.java.ast.ASTFormalParameter;
import net.sourceforge.pmd.lang.java.ast.ASTLiteral;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclarator;
@ -26,6 +29,9 @@ import net.sourceforge.pmd.lang.java.ast.ASTName;
import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression;
import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix;
import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix;
import net.sourceforge.pmd.lang.java.ast.ASTPrimitiveType;
import net.sourceforge.pmd.lang.java.ast.ASTReferenceType;
import net.sourceforge.pmd.lang.java.ast.ASTType;
import net.sourceforge.pmd.lang.java.ast.AccessNode;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
@ -159,14 +165,16 @@ public final class ConstructorCallsOverridableMethodRule extends AbstractJavaRul
private List<String> referenceNames;
private List<String> qualifierNames;
private int argumentSize;
private List<String> argumentTypes;
private boolean superCall;
private MethodInvocation(ASTPrimaryExpression ape, List<String> qualifierNames, List<String> referenceNames, String name, int argumentSize, boolean superCall) {
private MethodInvocation(ASTPrimaryExpression ape, List<String> qualifierNames, List<String> referenceNames, String name, int argumentSize, List<String> argumentTypes, boolean superCall) {
this.ape = ape;
this.qualifierNames = qualifierNames;
this.referenceNames = referenceNames;
this.name = name;
this.argumentSize = argumentSize;
this.argumentTypes = argumentTypes;
this.superCall = superCall;
}
@ -182,6 +190,10 @@ public final class ConstructorCallsOverridableMethodRule extends AbstractJavaRul
return argumentSize;
}
public List<String> getArgumentTypes() {
return argumentTypes;
}
public List<String> getReferenceNames() {
return referenceNames;//new ArrayList(variableNames);
}
@ -208,6 +220,7 @@ public final class ConstructorCallsOverridableMethodRule extends AbstractJavaRul
String methodName = null;
ASTArguments args = (ASTArguments) lastNode.jjtGetChild(0);
int numOfArguments = args.getArgumentCount();
List<String> argumentTypes = ConstructorCallsOverridableMethodRule.getArgumentTypes(args);
boolean superFirst = false;
int thisIndex = -1;
@ -342,7 +355,8 @@ public final class ConstructorCallsOverridableMethodRule extends AbstractJavaRul
}
}
}
meth = new MethodInvocation(node, packagesAndClasses, varNames, methodName, numOfArguments, superFirst);
meth = new MethodInvocation(node, packagesAndClasses, varNames, methodName, numOfArguments, argumentTypes, superFirst);
// meth.show();
}
}
return meth;
@ -362,6 +376,8 @@ public final class ConstructorCallsOverridableMethodRule extends AbstractJavaRul
}
System.out.println(" </References>");
System.out.println(" <Name>" + getName() + "</Name>");
System.out.println(" <ArgumentCount>" + getArgumentCount() + "</ArgumentCount>");
System.out.println(" <ArgumentTypes>" + getArgumentTypes() + "</ArgumentTypes>");
System.out.println("</MethodInvocation>");
}
}
@ -370,6 +386,7 @@ public final class ConstructorCallsOverridableMethodRule extends AbstractJavaRul
private ASTExplicitConstructorInvocation eci;
private String name;
private int count = 0;
private List<String> argumentTypes = new ArrayList<String>();
public ConstructorInvocation(ASTExplicitConstructorInvocation eci) {
this.eci = eci;
@ -377,6 +394,7 @@ public final class ConstructorCallsOverridableMethodRule extends AbstractJavaRul
if (!l.isEmpty()) {
ASTArguments aa = l.get(0);
count = aa.getArgumentCount();
argumentTypes = ConstructorCallsOverridableMethodRule.getArgumentTypes(aa);
}
name = eci.getImage();
}
@ -389,6 +407,10 @@ public final class ConstructorCallsOverridableMethodRule extends AbstractJavaRul
return count;
}
public List<String> getArgumentTypes() {
return argumentTypes;
}
public String getName() {
return name;
}
@ -424,7 +446,7 @@ public final class ConstructorCallsOverridableMethodRule extends AbstractJavaRul
}
}
private final class ConstructorHolder {
private static final class ConstructorHolder {
private ASTConstructorDeclaration cd;
private boolean dangerous;
private ConstructorInvocation ci;
@ -585,7 +607,9 @@ public final class ConstructorCallsOverridableMethodRule extends AbstractJavaRul
if (h.isDangerous()) {
String methName = h.getASTMethodDeclarator().getImage();
int count = h.getASTMethodDeclarator().getParameterCount();
if (methName.equals(meth.getName()) && meth.getArgumentCount() == count) {
List<String> parameterTypes = getMethodDeclaratorParameterTypes(h.getASTMethodDeclarator());
if (methName.equals(meth.getName()) && meth.getArgumentCount() == count
&& parameterTypes.equals(meth.getArgumentTypes())) {
addViolation(data, meth.getASTPrimaryExpression(), "method '" + h.getCalled() + "'");
}
}
@ -638,8 +662,10 @@ public final class ConstructorCallsOverridableMethodRule extends AbstractJavaRul
if (h3.isDangerous()) {
String matchMethodName = h3.getASTMethodDeclarator().getImage();
int matchMethodParamCount = h3.getASTMethodDeclarator().getParameterCount();
List<String> parameterTypes = getMethodDeclaratorParameterTypes(h3.getASTMethodDeclarator());
//System.out.println("matching " + matchMethodName + " to " + meth.getName());
if (matchMethodName.equals(meth.getName()) && matchMethodParamCount == meth.getArgumentCount()) {
if (matchMethodName.equals(meth.getName()) && matchMethodParamCount == meth.getArgumentCount()
&& parameterTypes.equals(meth.getArgumentTypes())) {
h.setDangerous();
h.setCalledMethod(matchMethodName);
found = true;
@ -675,13 +701,14 @@ public final class ConstructorCallsOverridableMethodRule extends AbstractJavaRul
if (h.isDangerous()) {
String matchName = h.getASTMethodDeclarator().getImage();
int matchParamCount = h.getASTMethodDeclarator().getParameterCount();
if (methName.equals(matchName) && methArgCount == matchParamCount) {
List<String> parameterTypes = getMethodDeclaratorParameterTypes(h.getASTMethodDeclarator());
if (methName.equals(matchName) && methArgCount == matchParamCount
&& parameterTypes.equals(meth.getArgumentTypes())) {
ch.setDangerous(true);
//System.out.println("evaluateDangerOfConstructors1 setting dangerous constructor with " + ch.getASTConstructorDeclaration().getParameterCount() + " params");
break;
}
}
}
}
}
@ -712,7 +739,8 @@ public final class ConstructorCallsOverridableMethodRule extends AbstractJavaRul
ConstructorHolder h2 = innerConstIter.next();
if (h2.isDangerous()) {
int matchConstArgCount = h2.getASTConstructorDeclaration().getParameterCount();
if (matchConstArgCount == cCount) {
List<String> parameterTypes = getMethodDeclaratorParameterTypes(h2.getASTConstructorDeclaration());
if (matchConstArgCount == cCount && parameterTypes.equals(calledC.getArgumentTypes())) {
ch.setDangerous(true);
found = true;
//System.out.println("evaluateDangerOfConstructors2 setting dangerous constructor with " + ch.getASTConstructorDeclaration().getParameterCount() + " params");
@ -887,4 +915,52 @@ public final class ConstructorCallsOverridableMethodRule extends AbstractJavaRul
return name;
}
private static List<String> getMethodDeclaratorParameterTypes(Node methodOrConstructorDeclarator) {
List<ASTFormalParameter> parameters = methodOrConstructorDeclarator.findChildrenOfType(ASTFormalParameter.class);
List<String> parameterTypes = new ArrayList<String>();
if (parameters != null) {
for (ASTFormalParameter p : parameters) {
ASTType type = p.getFirstChildOfType(ASTType.class);
if (type.jjtGetChild(0) instanceof ASTPrimitiveType) {
parameterTypes.add(type.jjtGetChild(0).getImage());
} else if (type.jjtGetChild(0) instanceof ASTReferenceType) {
parameterTypes.add("ref");
} else {
parameterTypes.add("<unkown>");
}
}
}
return parameterTypes;
}
private static List<String> getArgumentTypes(ASTArguments args) {
List<String> argumentTypes = new ArrayList<String>();
ASTArgumentList argumentList = args.getFirstChildOfType(ASTArgumentList.class);
if (argumentList != null) {
for (int a = 0; a < argumentList.jjtGetNumChildren(); a++) {
Node expression = argumentList.jjtGetChild(a);
ASTPrimaryPrefix arg = expression.getFirstDescendantOfType(ASTPrimaryPrefix.class);
String type = "<unknown>";
if (arg.jjtGetChild(0) instanceof ASTLiteral) {
ASTLiteral lit = (ASTLiteral) arg.jjtGetChild(0);
if (lit.isCharLiteral()) {
type = "char";
} else if (lit.isFloatLiteral()) {
type = "float";
} else if (lit.isIntLiteral()) {
type = "int";
} else if (lit.isStringLiteral()) {
type = "String";
} else if (lit.jjtGetChild(0) instanceof ASTBooleanLiteral) {
type = "boolean";
}
} else if (arg.jjtGetChild(0) instanceof ASTName) {
// ASTName n = (ASTName)arg.jjtGetChild(0);
type = "ref";
}
argumentTypes.add(type);
}
}
return argumentTypes;
}
}

13
pmd/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/ConstructorCallsOverridableMethod.xml
View File

@ -204,6 +204,19 @@ public class Test {
System.out.println(": "+new JuniorClass());
}
}
]]></code>
</test-code>
<test-code>
<description>bug #1005 False + for ConstructorCallsOverridableMethod - overloaded methods</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
public Foo() {
bar(true);
}
public final void bar(boolean b) {}
public void bar(String o) {}
}
]]></code>
</test-code>