github-forks
/
p3c
mirror of https://github.com/alibaba/p3c.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

fix https://github.com/alibaba/p3c/issues/209

Browse Source
pull/614/head
曾候 5 years ago
parent 20889aba6e
commit 670c4f4ac0
2 changed files with 185 additions and 150 deletions
Show Stats Download Patch File Download Diff File

15
p3c-pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/oop/WrapperTypeEqualityRule.java
Unescape Escape View File

@ -45,8 +45,14 @@ public class WrapperTypeEqualityRule extends AbstractAliRule {
// possible elements around "==" are PrimaryExpression or UnaryExpression(e.g. a == -2) // possible elements around "==" are PrimaryExpression or UnaryExpression(e.g. a == -2)
List<ASTPrimaryExpression> expressions = node.findChildrenOfType(ASTPrimaryExpression.class); List<ASTPrimaryExpression> expressions = node.findChildrenOfType(ASTPrimaryExpression.class);
if (expressions.size() == NumberConstants.INTEGER_SIZE_OR_LENGTH_2) { if (expressions.size() == NumberConstants.INTEGER_SIZE_OR_LENGTH_2) {
if (NodeUtils.isWrapperType(expressions.get(0)) && // PMD can not resolve array length type, but only the
NodeUtils.isWrapperType(expressions.get(1))) { ASTPrimaryExpression left = expressions.get(0);
ASTPrimaryExpression right = expressions.get(1);
boolean bothArrayLength = isArrayLength(left) && isArrayLength(right);
boolean bothWrapperType = NodeUtils.isWrapperType(left) && NodeUtils.isWrapperType(right);
if (!bothArrayLength && bothWrapperType) {
addViolationWithMessage(data, node, "java.oop.WrapperTypeEqualityRule.violation.msg"); addViolationWithMessage(data, node, "java.oop.WrapperTypeEqualityRule.violation.msg");
} }
} }
@ -54,4 +60,9 @@ public class WrapperTypeEqualityRule extends AbstractAliRule {
return super.visit(node, data); return super.visit(node, data);
} }
private boolean isArrayLength(ASTPrimaryExpression expression) {
// assume expression like "x.length" is the length of array, field with name "length" may result in misrecognition
return "length".equals(expression.jjtGetLastToken().getImage())
&& ".".equals(expression.jjtGetFirstToken().getNext().getImage());
}
} }

320
p3c-pmd/src/test/resources/com/alibaba/p3c/pmd/lang/java/rule/oop/xml/WrapperTypeEqualityRule.xml
Unescape Escape View File

@ -3,151 +3,175 @@
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests https://pmd.sourceforge.io/rule-tests_1_0_0.xsd"> xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests https://pmd.sourceforge.io/rule-tests_1_0_0.xsd">
<code-fragment id="wrap-type-not-use-equal"> <code-fragment id="wrap-type-not-use-equal">
<![CDATA[ <![CDATA[
public class Foo { public class Foo {
private static final int x = 3; private static final int x = 3;
public void bar() { public void bar() {
Integer a = 2; Integer a = 2;
int b = 2; int b = 2;
Integer c = 3; Integer c = 3;
if (a == b) { // OK if (a == b) { // OK
// do nothing // do nothing
} }
if (a == c) { // BAD if (a == c) { // BAD
// do nothing // do nothing
} }
if (a != null) { // OK if (a != null) { // OK
// do nothing // do nothing
} }
if (a == -6) { // OK if (a == -6) { // OK
// do nothing // do nothing
} }
if (a == x) { // OK if (a == x) { // OK
// do nothing // do nothing
} }
if (a == Integer.MAX_VALUE) { // BAD if (a == Integer.MAX_VALUE) { // BAD
// do nothing // do nothing
} }
// PMD can not resolve type of Inner.FLAG // PMD can not resolve type of Inner.FLAG
if (a == Inner.FLAG) { if (a == Inner.FLAG) {
// do nothing // do nothing
} }
// After upgrade pmd version, can resolve // After upgrade pmd version, can resolve
if (a == Integer.valueOf("2")) { //BAD if (a == Integer.valueOf("2")) { //BAD
// do nothing // do nothing
} }
if (a == new Integer("2")) { // BAD if (a == new Integer("2")) { // BAD
// do nothing // do nothing
} }
} }
private static class Inner { private static class Inner {
public static final Integer FLAG = 3; public static final Integer FLAG = 3;
} }
} }
]]> ]]>
</code-fragment> </code-fragment>
<test-code> <test-code>
<description>compare wrapper type objects without equals</description> <description>compare wrapper type objects without equals</description>
<expected-problems>4</expected-problems> <expected-problems>4</expected-problems>
<expected-linenumbers>11,23,31,34</expected-linenumbers> <expected-linenumbers>11,23,31,34</expected-linenumbers>
<code-ref id="wrap-type-not-use-equal" /> <code-ref id="wrap-type-not-use-equal" />
</test-code> </test-code>
<!-- ====================================================================== --> <!-- ====================================================================== -->
<code-fragment id="wrong-result-fix"> <code-fragment id="wrong-result-fix">
<![CDATA[ <![CDATA[
package com.alibaba.p3c.pmd.lang.java.rule.concurrent; package com.alibaba.p3c.pmd.lang.java.rule.concurrent;
import net.sourceforge.pmd.lang.java.ast.ASTImportDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTImportDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTName; import net.sourceforge.pmd.lang.java.ast.ASTName;
import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression; import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression;
import net.sourceforge.pmd.lang.java.ast.Token; import net.sourceforge.pmd.lang.java.ast.Token;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import java.util.HashSet; import java.util.HashSet;
import java.util.Set; import java.util.Set;
import java.util.concurrent.Executors; import java.util.concurrent.Executors;
public class ThreadPoolCreationRule extends AbstractJavaRule { public class ThreadPoolCreationRule extends AbstractJavaRule {
private static final String DOT = "."; private static final String DOT = ".";
private static final String COLON = ";"; private static final String COLON = ";";
private static final String NEW = "new"; private static final String NEW = "new";
private static final String EXECUTORS_NEW = Executors.class.getSimpleName() + DOT + NEW; private static final String EXECUTORS_NEW = Executors.class.getSimpleName() + DOT + NEW;
private static final String FULL_EXECUTORS_NEW = Executors.class.getName() + DOT + NEW; private static final String FULL_EXECUTORS_NEW = Executors.class.getName() + DOT + NEW;
private static final String BRACKETS = "()"; private static final String BRACKETS = "()";
private boolean executorsUsed; private boolean executorsUsed;
private Set<String> importedExecutorsMethods = new HashSet<>(); private Set<String> importedExecutorsMethods = new HashSet<>();
@Override @Override
public Object visit(ASTPrimaryExpression node, Object data) { public Object visit(ASTPrimaryExpression node, Object data) {
if (!executorsUsed && importedExecutorsMethods.isEmpty()) { if (!executorsUsed && importedExecutorsMethods.isEmpty()) {
return super.visit(node, data); return super.visit(node, data);
} }
Token initToken = (Token) node.jjtGetFirstToken(); Token initToken = (Token) node.jjtGetFirstToken();
if (!checkInitStatement(initToken)) { if (!checkInitStatement(initToken)) {
addViolation(data, node); addViolation(data, node);
} }
return super.visit(node, data); return super.visit(node, data);
} }
private boolean checkInitStatement(Token token) { private boolean checkInitStatement(Token token) {
String fullAssignStatement = getFullAssignStatement(token); String fullAssignStatement = getFullAssignStatement(token);
if (fullAssignStatement.startsWith(EXECUTORS_NEW)) { if (fullAssignStatement.startsWith(EXECUTORS_NEW)) {
return false; return false;
} }
if (!fullAssignStatement.startsWith(NEW) && !fullAssignStatement.startsWith(FULL_EXECUTORS_NEW)) { if (!fullAssignStatement.startsWith(NEW) && !fullAssignStatement.startsWith(FULL_EXECUTORS_NEW)) {
return true; return true;
} }
// in case of lambda // in case of lambda
int index = fullAssignStatement.indexOf(BRACKETS); int index = fullAssignStatement.indexOf(BRACKETS);
if (index == -1) { if (index == -1) {
return true; return true;
} }
fullAssignStatement = fullAssignStatement.substring(0, index); fullAssignStatement = fullAssignStatement.substring(0, index);
// avoid java.util.concurrent.Executors.newxxxx // avoid java.util.concurrent.Executors.newxxxx
if (importedExecutorsMethods.contains(fullAssignStatement)) { if (importedExecutorsMethods.contains(fullAssignStatement)) {
return false; return false;
} }
// static import // static import
return !importedExecutorsMethods.contains(Executors.class.getName() + DOT + fullAssignStatement); return !importedExecutorsMethods.contains(Executors.class.getName() + DOT + fullAssignStatement);
} }
private String getFullAssignStatement(final Token token) { private String getFullAssignStatement(final Token token) {
if (token == null) { if (token == null) {
return ""; return "";
} }
StringBuilder sb = new StringBuilder(48); StringBuilder sb = new StringBuilder(48);
Token next = token; Token next = token;
while (next.next != null && !COLON.equals(next.image)) { while (next.next != null && !COLON.equals(next.image)) {
sb.append(next.image); sb.append(next.image);
next = next.next; next = next.next;
} }
return sb.toString(); return sb.toString();
} }
@Override @Override
public Object visit(ASTImportDeclaration node, Object data) { public Object visit(ASTImportDeclaration node, Object data) {
ASTName name = node.getFirstChildOfType(ASTName.class); ASTName name = node.getFirstChildOfType(ASTName.class);
// in case of static import // in case of static import
executorsUsed = executorsUsed || name.getType() == Executors.class; executorsUsed = executorsUsed || name.getType() == Executors.class;
if (name.getImage().startsWith(Executors.class.getName() + DOT)) { if (name.getImage().startsWith(Executors.class.getName() + DOT)) {
importedExecutorsMethods.add(name.getImage()); importedExecutorsMethods.add(name.getImage());
} }
return super.visit(node, data); return super.visit(node, data);
} }
} }
]]> ]]>
</code-fragment> </code-fragment>
<test-code> <test-code>
<description>bugfix</description> <description>bugfix</description>
<expected-problems>0</expected-problems> <expected-problems>0</expected-problems>
<code-ref id="wrong-result-fix" /> <code-ref id="wrong-result-fix" />
</test-code> </test-code>
</test-data>
<!-- ====================================================================== -->
<code-fragment id="array-length-equals">
<![CDATA[
public class Test {
public void foo(){
Integer[] a;
Integer[] b;
if (a.length == b.length) {
return;
};
}
}
]]>
</code-fragment>
<test-code>
<description>array length equals</description>
<expected-problems>0</expected-problems>
<code-ref id="array-length-equals"/>
</test-code>
<!-- ====================================================================== -->
</test-data>
Write Preview
Loading…
Cancel
Save