diff --git a/p3c-pmd/.gitignore b/p3c-pmd/.gitignore index ddb1f85c18436c4e4a1ce9fdee71381482b9b308..f9481d30deb3be45c9fd2b5389f5df1110f3267e 100644 --- a/p3c-pmd/.gitignore +++ b/p3c-pmd/.gitignore @@ -69,6 +69,7 @@ configuration/** # sass gitignore# .sass-cache .idea +.vscode # tcc_coverage coverage.ec @@ -88,3 +89,4 @@ hsf.configuration/ #hsf test *.instance +/target/ diff --git a/p3c-pmd/pom.xml b/p3c-pmd/pom.xml index 18c8bbec999a36be6ce59a975a17a4bf0330e63b..b75e58c1a212c85b6c84c03d02490c6996301dc1 100644 --- a/p3c-pmd/pom.xml +++ b/p3c-pmd/pom.xml @@ -9,7 +9,7 @@ com.alibaba.p3c p3c-pmd - 2.0.0 + 2.0.1 jar p3c-pmd @@ -17,6 +17,7 @@ 6.15.0 1.8 1.3.2 + 1.3.50 Alibaba Java Coding Guidelines PMD implementations https://github.com/alibaba/p3c @@ -99,6 +100,11 @@ javax.annotation-api ${annotation.version} + + org.jetbrains.kotlin + kotlin-stdlib-jdk8 + ${kotlin.version} + @@ -142,6 +148,37 @@ + + kotlin-maven-plugin + org.jetbrains.kotlin + ${kotlin.version} + + + compile + + compile + + + + ${project.basedir}/src/main/kotlin + ${project.basedir}/src/main/java + + + + + test-compile + + test-compile + + + + ${project.basedir}/src/test/kotlin + ${project.basedir}/src/test/java + + + + + org.apache.maven.plugins maven-compiler-plugin @@ -151,6 +188,32 @@ ${maven.compiler.target} UTF-8 + + + + default-compile + none + + + + default-testCompile + none + + + java-compile + compile + + compile + + + + java-test-compile + test-compile + + testCompile + + + maven-assembly-plugin @@ -190,20 +253,38 @@ - + + maven-assembly-plugin + 3.0.0 + + + jar-with-dependencies + + + + + make-assembly + package + + single + + + + + + org.apache.maven.plugins + maven-gpg-plugin + 1.6 + + + sign-artifacts + verify + + sign + + + + diff --git a/p3c-pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/concurrent/AvoidCallStaticSimpleDateFormatRule.java b/p3c-pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/concurrent/AvoidCallStaticSimpleDateFormatRule.java index 4a59a9320425410f41aa193990f456630dec30a1..6a6efea4487f369d3c33d33284535daaf43bf2e3 100644 --- a/p3c-pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/concurrent/AvoidCallStaticSimpleDateFormatRule.java +++ b/p3c-pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/concurrent/AvoidCallStaticSimpleDateFormatRule.java @@ -19,7 +19,6 @@ import java.text.SimpleDateFormat; import java.util.HashSet; import java.util.Set; import java.util.Stack; -import java.util.concurrent.locks.Lock; import com.alibaba.p3c.pmd.lang.java.rule.AbstractAliRule; @@ -37,6 +36,10 @@ import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId; import net.sourceforge.pmd.lang.java.ast.AbstractJavaNode; import net.sourceforge.pmd.lang.java.ast.Token; +import static com.alibaba.p3c.pmd.lang.java.rule.util.NodeUtils.isLockNode; +import static com.alibaba.p3c.pmd.lang.java.rule.util.NodeUtils.isLockStatementExpression; +import static com.alibaba.p3c.pmd.lang.java.rule.util.NodeUtils.isUnLockStatementExpression; + /** * [Mandatory] SimpleDataFormat is unsafe, do not define it as a static variable. * If have to, lock or DateUtils class must be used. @@ -46,8 +49,6 @@ import net.sourceforge.pmd.lang.java.ast.Token; */ public class AvoidCallStaticSimpleDateFormatRule extends AbstractAliRule { private static final String FORMAT_METHOD_NAME = "format"; - private static final String LOCK_NAME = "lock"; - private static final String UN_LOCK_NAME = "unlock"; @Override public Object visit(ASTMethodDeclaration node, Object data) { @@ -101,7 +102,9 @@ public class AvoidCallStaticSimpleDateFormatRule extends AbstractAliRule { // add lock node stack.push(flowNode.getNode()); return; - } else if (isUnLockStatementExpression(statementExpression)) { + } + + if (isUnLockStatementExpression(statementExpression)) { // remove element in lock block while (!stack.isEmpty()) { Node node = stack.pop(); @@ -132,16 +135,10 @@ public class AvoidCallStaticSimpleDateFormatRule extends AbstractAliRule { return name.getImage(); } - private boolean isLockNode(Node node) { - if (!(node instanceof ASTStatementExpression)) { - return false; - } - ASTStatementExpression statementExpression = (ASTStatementExpression)node; - return isLockStatementExpression(statementExpression); - } - - private boolean isStaticSimpleDateFormatCall(ASTPrimaryExpression primaryExpression, - Set localSimpleDateFormatNames) { + private boolean isStaticSimpleDateFormatCall( + ASTPrimaryExpression primaryExpression, + Set localSimpleDateFormatNames + ) { if (primaryExpression.jjtGetNumChildren() == 0) { return false; } @@ -149,6 +146,9 @@ public class AvoidCallStaticSimpleDateFormatRule extends AbstractAliRule { if (name == null || name.getType() != SimpleDateFormat.class) { return false; } + if (name.getNameDeclaration() == null || name.getNameDeclaration().getName() == null) { + return false; + } if (localSimpleDateFormatNames.contains(name.getNameDeclaration().getName())) { return false; } @@ -161,20 +161,4 @@ public class AvoidCallStaticSimpleDateFormatRule extends AbstractAliRule { return FORMAT_METHOD_NAME.equals(token.image); } - private boolean isLockStatementExpression(ASTStatementExpression statementExpression) { - return isLockTypeAndMethod(statementExpression, LOCK_NAME); - } - - private boolean isUnLockStatementExpression(ASTStatementExpression statementExpression) { - return isLockTypeAndMethod(statementExpression, UN_LOCK_NAME); - } - - private boolean isLockTypeAndMethod(ASTStatementExpression statementExpression, String methodName) { - ASTName name = statementExpression.getFirstDescendantOfType(ASTName.class); - if (name == null || name.getType() == null || !Lock.class.isAssignableFrom(name.getType())) { - return false; - } - Token token = (Token)name.jjtGetLastToken(); - return methodName.equals(token.image); - } } diff --git a/p3c-pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/concurrent/AvoidManuallyCreateThreadRule.java b/p3c-pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/concurrent/AvoidManuallyCreateThreadRule.java index 1ad3a8c235fea801b11009169ea45b6d71b15259..ad11f3771b90443183c511f27552857eac81ba30 100644 --- a/p3c-pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/concurrent/AvoidManuallyCreateThreadRule.java +++ b/p3c-pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/concurrent/AvoidManuallyCreateThreadRule.java @@ -19,8 +19,8 @@ import java.util.List; import java.util.concurrent.ThreadFactory; import com.alibaba.p3c.pmd.lang.java.rule.AbstractAliRule; - import com.alibaba.p3c.pmd.lang.java.rule.util.NodeUtils; + import net.sourceforge.pmd.lang.java.ast.ASTAllocationExpression; import net.sourceforge.pmd.lang.java.ast.ASTBlockStatement; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; diff --git a/p3c-pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/naming/LowerCamelCaseVariableNamingRule.java b/p3c-pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/naming/LowerCamelCaseVariableNamingRule.java index b723ff26333d22dd615cc1ccf9199af52998e693..e009beff43046050b1642df62c084b7244c5ceb1 100644 --- a/p3c-pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/naming/LowerCamelCaseVariableNamingRule.java +++ b/p3c-pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/naming/LowerCamelCaseVariableNamingRule.java @@ -15,6 +15,8 @@ */ package com.alibaba.p3c.pmd.lang.java.rule.naming; +import java.util.regex.Pattern; + import com.alibaba.p3c.pmd.I18nResources; import com.alibaba.p3c.pmd.lang.java.rule.AbstractAliRule; import com.alibaba.p3c.pmd.lang.java.util.StringAndCharConstants; @@ -27,8 +29,6 @@ import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclarator; import net.sourceforge.pmd.lang.java.ast.ASTTypeDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId; -import java.util.regex.Pattern; - /** * [Mandatory] Method names, parameter names, member variable names, and local variable names should be written in * lowerCamelCase. @@ -39,7 +39,6 @@ import java.util.regex.Pattern; public class LowerCamelCaseVariableNamingRule extends AbstractAliRule { private static final String MESSAGE_KEY_PREFIX = "java.naming.LowerCamelCaseVariableNamingRule.violation.msg"; - private Pattern pattern = Pattern.compile("^[a-z][a-z0-9]*([A-Z][a-z0-9]+)*(DO|DTO|VO|DAO|BO|DOList|DTOList|VOList|DAOList|BOList|X|Y|Z|UDF|UDAF|[A-Z])?$"); @Override diff --git a/p3c-pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/oop/WrapperTypeEqualityRule.java b/p3c-pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/oop/WrapperTypeEqualityRule.java index 1936940bce17ac7398215e45871ba4e212711788..b1a555ff3b821e4215060de099f82c15d1491515 100644 --- a/p3c-pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/oop/WrapperTypeEqualityRule.java +++ b/p3c-pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/oop/WrapperTypeEqualityRule.java @@ -65,4 +65,5 @@ public class WrapperTypeEqualityRule extends AbstractAliRule { return "length".equals(expression.jjtGetLastToken().getImage()) && ".".equals(expression.jjtGetFirstToken().getNext().getImage()); } + } diff --git a/p3c-pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/set/ClassCastExceptionWithToArrayRule.java b/p3c-pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/set/ClassCastExceptionWithToArrayRule.java index df2d8909f92dcf752b4d3569427a57a216a0068f..53455d41a59e9056d691011ce43114e9f29da780 100644 --- a/p3c-pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/set/ClassCastExceptionWithToArrayRule.java +++ b/p3c-pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/set/ClassCastExceptionWithToArrayRule.java @@ -71,7 +71,7 @@ public class ClassCastExceptionWithToArrayRule extends AbstractAliRule { if (childName.endsWith(".toArray") && suffix.getArgumentCount() == 0 && primarySuffixs.size() == 1) { addViolationWithMessage(data, item, - "java.set.ClassCastExceptionWithSubListToArrayListRule.violation.msg", + "java.set.ClassCastExceptionWithToArrayRule.violation.msg", new Object[] {childName}); } } diff --git a/p3c-pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/util/NodeUtils.java b/p3c-pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/util/NodeUtils.java index cc480e092ba14e2c064af5daadb03fb0947abf4e..2582e59c2188388e949c0a97fc55b4779432c32c 100644 --- a/p3c-pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/util/NodeUtils.java +++ b/p3c-pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/util/NodeUtils.java @@ -15,10 +15,15 @@ */ package com.alibaba.p3c.pmd.lang.java.rule.util; +import java.util.concurrent.locks.Lock; + import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTName; import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression; +import net.sourceforge.pmd.lang.java.ast.ASTStatementExpression; import net.sourceforge.pmd.lang.java.ast.AbstractJavaAccessTypeNode; +import net.sourceforge.pmd.lang.java.ast.Token; import net.sourceforge.pmd.lang.java.typeresolution.TypeHelper; /** @@ -26,6 +31,10 @@ import net.sourceforge.pmd.lang.java.typeresolution.TypeHelper; * @date 2016/11/16 */ public class NodeUtils { + public static final String LOCK_NAME = "lock"; + public static final String LOCK_INTERRUPTIBLY_NAME = "lockInterruptibly"; + public static final String UN_LOCK_NAME = "unlock"; + public static boolean isParentOrSelf(Node descendant, Node ancestor) { if (descendant == ancestor) { return true; @@ -64,4 +73,29 @@ public class NodeUtils { public static Class getNodeType(AbstractJavaAccessTypeNode node) { return node == null ? null : node.getType(); } + + public static boolean isLockStatementExpression(ASTStatementExpression statementExpression) { + return isLockTypeAndMethod(statementExpression, LOCK_NAME); + } + + public static boolean isUnLockStatementExpression(ASTStatementExpression statementExpression) { + return isLockTypeAndMethod(statementExpression, UN_LOCK_NAME); + } + + private static boolean isLockTypeAndMethod(ASTStatementExpression statementExpression, String methodName) { + ASTName name = statementExpression.getFirstDescendantOfType(ASTName.class); + if (name == null || name.getType() == null || !Lock.class.isAssignableFrom(name.getType())) { + return false; + } + Token token = (Token)name.jjtGetLastToken(); + return methodName.equals(token.image); + } + + public static boolean isLockNode(Node node) { + if (!(node instanceof ASTStatementExpression)) { + return false; + } + ASTStatementExpression statementExpression = (ASTStatementExpression)node; + return isLockStatementExpression(statementExpression); + } } diff --git a/p3c-pmd/src/main/kotlin/com/alibaba/p3c/pmd/lang/java/rule/concurrent/LockShouldWithTryFinallyRule.kt b/p3c-pmd/src/main/kotlin/com/alibaba/p3c/pmd/lang/java/rule/concurrent/LockShouldWithTryFinallyRule.kt new file mode 100644 index 0000000000000000000000000000000000000000..fe2a74ccd4012498c5a70d847cd65c3522592906 --- /dev/null +++ b/p3c-pmd/src/main/kotlin/com/alibaba/p3c/pmd/lang/java/rule/concurrent/LockShouldWithTryFinallyRule.kt @@ -0,0 +1,120 @@ +package com.alibaba.p3c.pmd.lang.java.rule.concurrent + +import com.alibaba.p3c.pmd.lang.java.rule.AbstractAliRule +import com.alibaba.p3c.pmd.lang.java.rule.util.NodeUtils.LOCK_INTERRUPTIBLY_NAME +import com.alibaba.p3c.pmd.lang.java.rule.util.NodeUtils.LOCK_NAME +import com.alibaba.p3c.pmd.lang.java.rule.util.NodeUtils.UN_LOCK_NAME +import net.sourceforge.pmd.lang.java.ast.ASTBlock +import net.sourceforge.pmd.lang.java.ast.ASTBlockStatement +import net.sourceforge.pmd.lang.java.ast.ASTFinallyStatement +import net.sourceforge.pmd.lang.java.ast.ASTName +import net.sourceforge.pmd.lang.java.ast.ASTStatementExpression +import net.sourceforge.pmd.lang.java.ast.ASTTryStatement +import net.sourceforge.pmd.lang.java.ast.AbstractJavaNode +import java.util.concurrent.locks.Lock + +/** + * @author caikang + * @date 2019/09/29 + */ +open class LockShouldWithTryFinallyRule : AbstractAliRule() { + + override fun visit(node: ASTBlock, data: Any): Any? { + checkBlock(node, data) + return super.visit(node, data) + } + + private fun checkBlock(block: ASTBlock, data: Any) { + val statements = block.findChildrenOfType(ASTBlockStatement::class.java) + if (statements.isNullOrEmpty()) { + return + } + var lockExpression: ASTStatementExpression? = null + for (statement in statements) { + if (lockExpression != null) { + // check try finally + val tryStatement = findNodeByXpath( + statement, XPATH_TRY_STATEMENT, + ASTTryStatement::class.java + ) + if (!checkTryStatement(tryStatement)) { + addLockViolation(data, lockExpression) + } + lockExpression = null + continue + } + // find lock expression + val expression = findNodeByXpath( + statement, XPATH_LOCK_STATEMENT, + ASTStatementExpression::class.java + ) ?: continue + + if (!expression.isLock) { + continue + } + val astName = expression.getFirstDescendantOfType(ASTName::class.java) + val lockMethod = astName?.image?.let { + it.endsWith(".$LOCK_NAME") || it.endsWith(".$LOCK_INTERRUPTIBLY_NAME") + } ?: false + if (!lockMethod) { + continue + } + lockExpression = expression + } + lockExpression?.let { + addLockViolation(data, it) + } + } + + private fun addLockViolation(data: Any, lockExpression: ASTStatementExpression) { + addViolationWithMessage( + data, lockExpression, + "java.concurrent.LockShouldWithTryFinallyRule.violation.msg", + arrayOf(getExpressName(lockExpression)) + ) + } + + private fun checkTryStatement(tryStatement: ASTTryStatement?): Boolean { + if (tryStatement == null) { + return false + } + val finallyStatement = tryStatement.getFirstChildOfType(ASTFinallyStatement::class.java) ?: return false + val statementExpression = findNodeByXpath( + finallyStatement, + XPATH_UNLOCK_STATEMENT, ASTStatementExpression::class.java + ) ?: return false + + if (!statementExpression.isLock) { + return false + } + val astName = statementExpression.getFirstDescendantOfType(ASTName::class.java) ?: return false + return astName.image?.endsWith(".$UN_LOCK_NAME") ?: false + } + + private fun findNodeByXpath(statement: AbstractJavaNode, xpath: String, clazz: Class): T? { + val nodes = statement.findChildNodesWithXPath(xpath) + if (nodes == null || nodes.isEmpty()) { + return null + } + val node = nodes[0] + return if (!clazz.isAssignableFrom(node.javaClass)) { + null + } else clazz.cast(node) + } + + private fun getExpressName(statementExpression: ASTStatementExpression): String { + val name = statementExpression.getFirstDescendantOfType(ASTName::class.java) + return name.image + } + + companion object { + private const val XPATH_LOCK_STATEMENT = "Statement/StatementExpression" + private const val XPATH_UNLOCK_STATEMENT = "Block/BlockStatement/Statement/StatementExpression" + private const val XPATH_TRY_STATEMENT = "Statement/TryStatement" + } + + private val ASTStatementExpression?.isLock: Boolean + get() = this?.type?.let { + Lock::class.java.isAssignableFrom(it) + } ?: false +} diff --git a/p3c-pmd/src/main/resources/messages.xml b/p3c-pmd/src/main/resources/messages.xml index b5b1cba95e9ef828509fc3d9a2dd603f892188b1..848f3edd735d8f6175852d7d1c48bc2056821fdd 100644 --- a/p3c-pmd/src/main/resources/messages.xml +++ b/p3c-pmd/src/main/resources/messages.xml @@ -178,6 +178,15 @@ + + + + + + diff --git a/p3c-pmd/src/main/resources/messages_en.xml b/p3c-pmd/src/main/resources/messages_en.xml index 4666a7f33d8f4ce41b13666edb01288f06e4bcf7..9491ff065d978bd08f49bb687ae57e5b8cfd2bc6 100644 --- a/p3c-pmd/src/main/resources/messages_en.xml +++ b/p3c-pmd/src/main/resources/messages_en.xml @@ -180,7 +180,15 @@ Note: Below are the problems created by usage of Executors for thread pool creat - + + + + + + diff --git a/p3c-pmd/src/main/resources/rulesets/java/ali-concurrent.xml b/p3c-pmd/src/main/resources/rulesets/java/ali-concurrent.xml index fc0ab14ea9db42094e97f45da38ddd378fd75d53..d4ba1c819bcd603a8c5f1f539fffa747e5a865f3 100644 --- a/p3c-pmd/src/main/resources/rulesets/java/ali-concurrent.xml +++ b/p3c-pmd/src/main/resources/rulesets/java/ali-concurrent.xml @@ -286,4 +286,44 @@ Positive example 2： ]]> + + java.concurrent.LockShouldWithTryFinallyRule.rule.desc + 1 + + + + + + + diff --git a/p3c-pmd/src/test/java/com/alibaba/p3c/pmd/lang/java/rule/concurrent/ConcurrentRuleTest.java b/p3c-pmd/src/test/java/com/alibaba/p3c/pmd/lang/java/rule/concurrent/ConcurrentRuleTest.java index 6cff269a00645efcd001cc18d036836c1b4bde66..7b2b8799d5e02e10896b4a30f2f485b3111fb459 100644 --- a/p3c-pmd/src/test/java/com/alibaba/p3c/pmd/lang/java/rule/concurrent/ConcurrentRuleTest.java +++ b/p3c-pmd/src/test/java/com/alibaba/p3c/pmd/lang/java/rule/concurrent/ConcurrentRuleTest.java @@ -37,5 +37,6 @@ public class ConcurrentRuleTest extends SimpleAggregatorTst { addRule(RULE_NAME, "ThreadLocalShouldRemoveRule"); addRule(RULE_NAME, "AvoidConcurrentCompetitionRandomRule"); addRule(RULE_NAME, "CountDownShouldInFinallyRule"); + addRule(RULE_NAME, "LockShouldWithTryFinallyRule"); } } diff --git a/p3c-pmd/src/test/resources/com/alibaba/p3c/pmd/lang/java/rule/concurrent/xml/LockShouldWithTryFinallyRule.xml b/p3c-pmd/src/test/resources/com/alibaba/p3c/pmd/lang/java/rule/concurrent/xml/LockShouldWithTryFinallyRule.xml new file mode 100644 index 0000000000000000000000000000000000000000..683b343c50c86d0db56219c9eb32446ae97328da --- /dev/null +++ b/p3c-pmd/src/test/resources/com/alibaba/p3c/pmd/lang/java/rule/concurrent/xml/LockShouldWithTryFinallyRule.xml @@ -0,0 +1,43 @@ + + + + LockShouldWithTryFinallyRule + 2 + 15,20 + + +