diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SingleMethodSingletonRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SingleMethodSingletonRule.java new file mode 100644 index 0000000000..0ab91a1d8c --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SingleMethodSingletonRule.java @@ -0,0 +1,78 @@ +package net.sourceforge.pmd.lang.java.rule.design; + + +import java.util.HashSet; +import java.util.List; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; + +import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBodyDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; +import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; +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.ASTReturnStatement; +import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; + +public class SingleMethodSingletonRule extends AbstractJavaRule { + + private static Map fieldDecls = new HashMap(); + private static Set returnset = new HashSet(); + boolean violation=false; + private static Set methodset = new HashSet(); + @Override + public Object visit(ASTFieldDeclaration node, Object data) { + + if (node.isStatic() && node.isPrivate()) { + ASTVariableDeclaratorId varDeclaratorId=node.getFirstDescendantOfType(ASTVariableDeclaratorId.class); + if(varDeclaratorId!=null){ + String varName=varDeclaratorId.getImage(); + fieldDecls.put(varName, node); + } + } + + return super.visit(node, data); + } + + @Override + public Object visit(ASTCompilationUnit node, Object data){ + violation=false; + fieldDecls.clear(); + returnset.clear(); + methodset.clear(); + return super.visit(node,data); + } + @Override + public Object visit(ASTMethodDeclaration node, Object data) { + + violation=false; + if (node.getResultType().isVoid()) { + return super.visit(node, data); + } + + if ("getInstance".equals(node.getMethodName())) { + + if(!methodset.add(node.getMethodName())){ + violation=true; + } + } + + if(violation){ + addViolation(data, node); + } + return super.visit(node, data); + } + + private String getNameFromPrimaryPrefix(ASTPrimaryPrefix pp) { + if ((pp.jjtGetNumChildren() == 1) + && (pp.jjtGetChild(0) instanceof ASTName)) { + return ((ASTName) pp.jjtGetChild(0)).getImage(); + } + return null; + } +} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SingletonClassReturningNewInstanceRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SingletonClassReturningNewInstanceRule.java new file mode 100644 index 0000000000..3097ab5c60 --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SingletonClassReturningNewInstanceRule.java @@ -0,0 +1,125 @@ +package net.sourceforge.pmd.lang.java.rule.design; + +import java.util.List; + +import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.java.ast.ASTAllocationExpression; +import net.sourceforge.pmd.lang.java.ast.ASTBlockStatement; +import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; +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.ASTReturnStatement; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; + +public class SingletonClassReturningNewInstanceRule extends AbstractJavaRule { + + @Override + public Object visit(ASTMethodDeclaration node, Object data) { + + boolean violation = false; + String localVarName = null; + String returnVariableName = null; + + if (node.getResultType().isVoid()) { + return super.visit(node, data); + } + + if ("getInstance".equals(node.getMethodName())) { + List rsl = node + .findDescendantsOfType(ASTReturnStatement.class); + if (rsl.isEmpty()) { + return super.visit(node, data); + } else { + for(ASTReturnStatement rs : rsl){ + + List pel = rs + .findDescendantsOfType(ASTPrimaryExpression.class); + ASTPrimaryExpression ape = pel.get(0); + if (ape.getFirstDescendantOfType(ASTAllocationExpression.class) != null) { + violation = true; + break; + } + } + } + + /* + * public class Singleton { + * + * private static Singleton m_instance=null; + * + * public static Singleton getInstance() { + * + * Singleton m_instance=null; + * + * if ( m_instance == null ) { + * synchronized(Singleton.class) { + * if(m_instance == null) { + * m_instance = new Singleton(); + * } + * } + * } + * return m_instance; + * } + * } + * + */ + + List ASTBlockStatements = node + .findDescendantsOfType(ASTBlockStatement.class); + returnVariableName = getReturnVariableName(node); + if (ASTBlockStatements.size() != 0) { + for (ASTBlockStatement blockStatement : ASTBlockStatements) { + if (blockStatement + .hasDescendantOfType(ASTLocalVariableDeclaration.class)) { + List lVarList=blockStatement.findDescendantsOfType(ASTLocalVariableDeclaration.class); + if(!lVarList.isEmpty()){ + for(ASTLocalVariableDeclaration localVar : lVarList){ + localVarName = localVar.getVariableName(); + if (returnVariableName != null + && returnVariableName.equals(localVarName)) { + violation = true; + break; + } + } + } + } + } + } + } + if (violation) { + addViolation(data, node); + } + return super.visit(node, data); + } + + private String getReturnVariableName(ASTMethodDeclaration node) { + + List rsl = node + .findDescendantsOfType(ASTReturnStatement.class); + ASTReturnStatement rs = rsl.get(0); + List pel = rs + .findDescendantsOfType(ASTPrimaryExpression.class); + ASTPrimaryExpression ape = pel.get(0); + Node lastChild = ape.jjtGetChild(0); + String returnVariableName = null; + if (lastChild instanceof ASTPrimaryPrefix) { + returnVariableName = getNameFromPrimaryPrefix((ASTPrimaryPrefix) lastChild); + } + /* + * if(lastChild instanceof ASTPrimarySuffix){ returnVariableName = + * getNameFromPrimarySuffix((ASTPrimarySuffix) lastChild); } + */ + return returnVariableName; + + } + + private String getNameFromPrimaryPrefix(ASTPrimaryPrefix pp) { + if ((pp.jjtGetNumChildren() == 1) + && (pp.jjtGetChild(0) instanceof ASTName)) { + return ((ASTName) pp.jjtGetChild(0)).getImage(); + } + return null; + } +} \ No newline at end of file diff --git a/pmd-java/src/main/resources/rulesets/java/design.xml b/pmd-java/src/main/resources/rulesets/java/design.xml index e0b52089b8..d8b16e9b71 100644 --- a/pmd-java/src/main/resources/rulesets/java/design.xml +++ b/pmd-java/src/main/resources/rulesets/java/design.xml @@ -1297,6 +1297,59 @@ public static Foo getFoo() { ]]> + + +Some classes contain overloaded getInstance. The problem with overloaded getInstance methods +is that the instance created using the overloaded method is not cached and so, + for each call and new objects will be created for every invocation. + +2 + + + + + + +Some classes contain overloaded getInstance. The problem with overloaded getInstance methods +is that the instance created using the overloaded method is not cached and so, + for each call and new objects will be created for every invocation. + +2 + + + + + + + 1 + + + + + + + 0 + + + + + \ No newline at end of file diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/SingletonClassReturningNewInstance.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/SingletonClassReturningNewInstance.xml new file mode 100644 index 0000000000..58512635e3 --- /dev/null +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/SingletonClassReturningNewInstance.xml @@ -0,0 +1,64 @@ + + + + + 1 + + + + + + + 1 + + + + + + + 0 + + + + + \ No newline at end of file