IG: report more cases of object construction inside equals() and hashCode() methods

This commit is contained in:
Bas Leijdekkers
2016-09-29 16:50:36 +02:00
parent 6f0fe899d4
commit 93ec1f75f7
4 changed files with 94 additions and 3 deletions
@@ -2157,6 +2157,7 @@ synchronization.on.get.class.display.name=Synchronization on 'getClass()'
synchronization.on.get.class.problem.descriptor=Synchronization on <code>#ref()</code> #loc
object.instantiation.inside.equals.or.hashcode.display.name=Object instantiation inside 'equals()' or 'hashCode()'
object.instantiation.inside.equals.or.hashcode.problem.descriptor=Object instantiation inside ''{0}()'' #loc
object.instantiation.inside.equals.or.hashcode.problem.descriptor2=Object instantiation inside ''{0}()'' ({1})#loc
lambda.body.can.be.code.block.name=Lambda body can be code block
lambda.body.can.be.code.block.quickfix=Expand lambda body to {...}
lambda.parameter.type.can.be.specified.name=Lambda parameter type can be specified
@@ -42,6 +42,9 @@ public class ObjectInstantiationInEqualsHashCodeInspection extends BaseInspectio
protected String buildErrorString(Object... infos) {
final PsiMethod method = PsiTreeUtil.getParentOfType((PsiElement)infos[0], PsiMethod.class);
assert method != null;
if (infos.length > 1) {
return InspectionGadgetsBundle.message("object.instantiation.inside.equals.or.hashcode.problem.descriptor2", method.getName(), infos[1]);
}
return InspectionGadgetsBundle.message("object.instantiation.inside.equals.or.hashcode.problem.descriptor", method.getName());
}
@@ -50,9 +53,68 @@ public class ObjectInstantiationInEqualsHashCodeInspection extends BaseInspectio
return new ObjectInstantiationInEqualsHashCodeVisitor();
}
// todo check boxing too
private static class ObjectInstantiationInEqualsHashCodeVisitor extends BaseInspectionVisitor {
@Override
public void visitExpression(PsiExpression expression) {
if (!ExpressionUtils.isAutoBoxed(expression)) {
return;
}
registerError(expression, expression, "autoboxing");
}
@Override
public void visitForeachStatement(PsiForeachStatement statement) {
final PsiExpression iteratedValue = statement.getIteratedValue();
if (iteratedValue == null || iteratedValue.getType() instanceof PsiArrayType) {
return;
}
registerError(iteratedValue, iteratedValue, "iterator");
}
@Override
public void visitMethodCallExpression(PsiMethodCallExpression expression) {
final PsiReferenceExpression methodExpression = expression.getMethodExpression();
final PsiMethod method = expression.resolveMethod();
if (method == null) {
return;
}
if (method.isVarArgs()) {
registerError(expression, expression, "varargs call");
}
else {
final String name = methodExpression.getReferenceName();
if (!"valueOf".equals(name)) {
return;
}
final PsiExpressionList argumentList = expression.getArgumentList();
final PsiExpression[] expressions = argumentList.getExpressions();
if (expressions.length != 1) {
return;
}
final PsiClass aClass = method.getContainingClass();
if (aClass == null) {
return;
}
final String qualifiedName = aClass.getQualifiedName();
if (!CommonClassNames.JAVA_LANG_SHORT.equals(qualifiedName) && !CommonClassNames.JAVA_LANG_INTEGER.equals(qualifiedName) &&
!CommonClassNames.JAVA_LANG_LONG.equals(qualifiedName) && !CommonClassNames.JAVA_LANG_DOUBLE.equals(qualifiedName) &&
!CommonClassNames.JAVA_LANG_FLOAT.equals(qualifiedName) && !CommonClassNames.JAVA_LANG_CHARACTER.equals(qualifiedName)) {
return;
}
registerError(expression, expression);
}
}
@Override
public void visitArrayInitializerExpression(PsiArrayInitializerExpression expression) {
if (!(expression.getParent() instanceof PsiVariable)) {
// new expressions are already reported.
return;
}
registerError(expression, expression);
}
@Override
public void visitPolyadicExpression(PsiPolyadicExpression expression) {
super.visitPolyadicExpression(expression);
@@ -2,7 +2,8 @@
<body>
Reports construction of (temporary) new objects inside an <b>equals()</b>, <b>hashCode()</b>, <b>compareTo</b> or
<b>Comparator.compare()</b> method.
This can cause significant performance problems when for example objects of classes with such constructs are added to a <b>Set</b> or <b>Map</b>,
Besides constructor invocations, new objects can also be created by autoboxing or iterator creation inside a foreach statement.
This can cause performance problems for example when objects are added to a <b>Set</b> or <b>Map</b>,
where these methods will be called often.
This inspection will not warn when the objects are created in a <b>throw</b> or <b>assert</b> statement.
<!-- tooltip end -->
@@ -8,7 +8,7 @@ class ObjectInstantiationInEqualsHashCode {
public boolean equals(Object o) {
ObjectInstantiationInEqualsHashCode other = (ObjectInstantiationInEqualsHashCode)o;
return Arrays.equals(new <warning descr="Object instantiation inside 'equals()'">Object</warning>[] {a, b}, new <warning descr="Object instantiation inside 'equals()'">Object</warning>[] {other.a, other.b});
return Arrays.equals(new <warning descr="Object instantiation inside 'equals()'">Object</warning>[] {a, <warning descr="Object instantiation inside 'equals()' (autoboxing)">b</warning>}, new <warning descr="Object instantiation inside 'equals()'">Object</warning>[] {other.a, <warning descr="Object instantiation inside 'equals()' (autoboxing)">other.b</warning>});
}
public int hashCode() {
@@ -38,4 +38,31 @@ class X implements Comparable<X>, Comparator<String> {
new <warning descr="Object instantiation inside 'compare()'">Object</warning>();
return 0;
}
}
class Y {
public java.util.List<Object> fooList = new java.util.ArrayList<>();
@Override
public int hashCode() {
Integer i = <warning descr="Object instantiation inside 'hashCode()' (autoboxing)">1</warning>;
<warning descr="Object instantiation inside 'hashCode()'">Short.valueOf((short) 1)</warning>;
Byte.valueOf((byte) 1); // nope
<warning descr="Object instantiation inside 'hashCode()'">Long.valueOf(1)</warning>;
Boolean.valueOf(true); // nope
<warning descr="Object instantiation inside 'hashCode()'">Character.valueOf('a')</warning>;
<warning descr="Object instantiation inside 'hashCode()'">Float.valueOf((float) 1.0)</warning>;
<warning descr="Object instantiation inside 'hashCode()'">Double.valueOf(1.0)</warning>;
<warning descr="Object instantiation inside 'hashCode()' (autoboxing)">i</warning>++;
int j = 1;
j++;
int[] is = <warning descr="Object instantiation inside 'hashCode()'">{j}</warning>;
int hashCode = 7;
<warning descr="Object instantiation inside 'hashCode()' (varargs call)">java.util.Arrays.asList()</warning>;
for (Object fooElement : <warning descr="Object instantiation inside 'hashCode()' (iterator)">fooList</warning>) {
hashCode = 31 * hashCode + (fooElement == null ? 0 : fooElement.hashCode());
}
return hashCode;
}
}