From 93ec1f75f735e43bbb25a92cf34a41602f746c2f Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Thu, 29 Sep 2016 12:58:40 +0200 Subject: [PATCH] IG: report more cases of object construction inside equals() and hashCode() methods --- .../siyeh/InspectionGadgetsBundle.properties | 1 + ...stantiationInEqualsHashCodeInspection.java | 64 ++++++++++++++++++- .../ObjectInstantiationInEqualsHashCode.html | 3 +- .../ObjectInstantiationInEqualsHashCode.java | 29 ++++++++- 4 files changed, 94 insertions(+), 3 deletions(-) diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties index 2f213319dd35..1cbbc76cd3ae 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties @@ -2157,6 +2157,7 @@ synchronization.on.get.class.display.name=Synchronization on 'getClass()' synchronization.on.get.class.problem.descriptor=Synchronization on #ref() #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 diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/performance/ObjectInstantiationInEqualsHashCodeInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/performance/ObjectInstantiationInEqualsHashCodeInspection.java index 449b8e0afcca..a8e018ff84c2 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/performance/ObjectInstantiationInEqualsHashCodeInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/performance/ObjectInstantiationInEqualsHashCodeInspection.java @@ -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); diff --git a/plugins/InspectionGadgets/src/inspectionDescriptions/ObjectInstantiationInEqualsHashCode.html b/plugins/InspectionGadgets/src/inspectionDescriptions/ObjectInstantiationInEqualsHashCode.html index 8361d39e41ce..75b171119f68 100644 --- a/plugins/InspectionGadgets/src/inspectionDescriptions/ObjectInstantiationInEqualsHashCode.html +++ b/plugins/InspectionGadgets/src/inspectionDescriptions/ObjectInstantiationInEqualsHashCode.html @@ -2,7 +2,8 @@ Reports construction of (temporary) new objects inside an equals(), hashCode(), compareTo or Comparator.compare() method. -This can cause significant performance problems when for example objects of classes with such constructs are added to a Set or Map, +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 Set or Map, where these methods will be called often. This inspection will not warn when the objects are created in a throw or assert statement. diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/object_instantiation_in_equals_hash_code/ObjectInstantiationInEqualsHashCode.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/object_instantiation_in_equals_hash_code/ObjectInstantiationInEqualsHashCode.java index 843963abde3b..9974a070d8d2 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/object_instantiation_in_equals_hash_code/ObjectInstantiationInEqualsHashCode.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/object_instantiation_in_equals_hash_code/ObjectInstantiationInEqualsHashCode.java @@ -8,7 +8,7 @@ class ObjectInstantiationInEqualsHashCode { public boolean equals(Object o) { ObjectInstantiationInEqualsHashCode other = (ObjectInstantiationInEqualsHashCode)o; - return Arrays.equals(new Object[] {a, b}, new Object[] {other.a, other.b}); + return Arrays.equals(new Object[] {a, b}, new Object[] {other.a, other.b}); } public int hashCode() { @@ -38,4 +38,31 @@ class X implements Comparable, Comparator { new Object(); return 0; } +} +class Y { + + public java.util.List fooList = new java.util.ArrayList<>(); + + @Override + public int hashCode() { + Integer i = 1; + Short.valueOf((short) 1); + Byte.valueOf((byte) 1); // nope + Long.valueOf(1); + Boolean.valueOf(true); // nope + Character.valueOf('a'); + Float.valueOf((float) 1.0); + Double.valueOf(1.0); + i++; + int j = 1; + j++; + int[] is = {j}; + int hashCode = 7; + java.util.Arrays.asList(); + for (Object fooElement : fooList) { + hashCode = 31 * hashCode + (fooElement == null ? 0 : fooElement.hashCode()); + } + return hashCode; + } + } \ No newline at end of file