IG: warn in more cases, less false positives

in "Object instantiation inside 'equals()' or 'hashCode()'" inspection
This commit is contained in:
Bas Leijdekkers
2018-06-04 12:54:53 +02:00
parent c11a658aea
commit 55cfa8fa16
3 changed files with 73 additions and 26 deletions
@@ -1,6 +1,8 @@
// Copyright 2000-2017 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
package com.siyeh.ig.performance;
import com.intellij.codeInspection.dataFlow.*;
import com.intellij.codeInspection.dataFlow.rangeSet.LongRangeSet;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
import com.siyeh.InspectionGadgetsBundle;
@@ -13,6 +15,8 @@ import com.siyeh.ig.psiutils.TypeUtils;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
import java.util.List;
/**
* @author Bas Leijdekkers
*/
@@ -45,7 +49,7 @@ public class ObjectInstantiationInEqualsHashCodeInspection extends BaseInspectio
@Override
public void visitExpression(PsiExpression expression) {
if (!ExpressionUtils.isAutoBoxed(expression) || !isInsideEqualsOrHashCode(expression)) {
if (!ExpressionUtils.isAutoBoxed(expression) || isAutoBoxingFromCache(expression) || !isInsideEqualsOrHashCode(expression)) {
return;
}
final PsiType expectedType = ExpectedTypeUtils.findExpectedType(expression, false, true);
@@ -67,24 +71,29 @@ public class ObjectInstantiationInEqualsHashCodeInspection extends BaseInspectio
@Override
public void visitMethodCallExpression(PsiMethodCallExpression expression) {
final PsiReferenceExpression methodExpression = expression.getMethodExpression();
final PsiMethod method = expression.resolveMethod();
if (method == null) {
return;
}
if (method.isVarArgs()) {
List<StandardMethodContract> contracts = JavaMethodContractUtil.getMethodContracts(method);
ContractReturnValue contractValue = JavaMethodContractUtil.getNonFailingReturnValue(contracts);
if (ContractReturnValue.returnNew().equals(contractValue)) {
if (!isInsideEqualsOrHashCode(expression)) {
return;
}
registerError(expression, expression, "varargs call");
registerMethodCallError(expression, expression);
}
else {
final String name = methodExpression.getReferenceName();
if (!"valueOf".equals(name)) {
else if (method.isVarArgs()) {
if (!isInsideEqualsOrHashCode(expression)) {
return;
}
final PsiExpressionList argumentList = expression.getArgumentList();
final PsiExpression[] expressions = argumentList.getExpressions();
registerMethodCallError(expression, expression, "varargs call");
}
else {
if (!"valueOf".equals(method.getName())) {
return;
}
final PsiExpression[] expressions = expression.getArgumentList().getExpressions();
if (expressions.length != 1) {
return;
}
@@ -93,18 +102,41 @@ public class ObjectInstantiationInEqualsHashCodeInspection extends BaseInspectio
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;
if (CommonClassNames.JAVA_LANG_SHORT.equals(qualifiedName) ||
CommonClassNames.JAVA_LANG_INTEGER.equals(qualifiedName) ||
CommonClassNames.JAVA_LANG_LONG.equals(qualifiedName) ||
CommonClassNames.JAVA_LANG_CHARACTER.equals(qualifiedName)) {
if (isAutoBoxingFromCache(expressions[0]) || !isInsideEqualsOrHashCode(expression)) {
return;
}
registerError(expression, expression);
}
if (!isInsideEqualsOrHashCode(expression)) {
return;
}
registerError(expression, expression);
}
}
private static boolean isAutoBoxingFromCache(PsiExpression expression) {
final LongRangeSet range = CommonDataflow.getExpressionFact(expression, DfaFactType.RANGE);
if (range != null && !range.isEmpty() && range.min() >= -128 && range.max() <= 127) {
return true;
}
final Object value = ExpressionUtils.computeConstantExpression(expression);
if (value instanceof Number) {
final Number number = (Number)value;
final int l = number.intValue();
if (l >= -128 && l <= 127) {
return true;
}
}
else if (value instanceof Character) {
final Character character = (Character)value;
final char c = character.charValue();
if (c <= 127) {
return true;
}
}
return false;
}
@Override
public void visitArrayInitializerExpression(PsiArrayInitializerExpression expression) {
if (!(expression.getParent() instanceof PsiVariable)) {
@@ -1,5 +1,6 @@
import java.util.Arrays;
import java.util.Comparator;
import java.util.regex.Pattern;
class ObjectInstantiationInEqualsHashCode {
@@ -45,23 +46,29 @@ class Y {
@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>;
Integer i = 1;
i = <warning descr="Object instantiation inside 'hashCode()' (autoboxing)">128</warning>;
Short.valueOf((short) 1);
<warning descr="Object instantiation inside 'hashCode()'">Short.valueOf((short) 128)</warning>;
Byte.valueOf((byte) 1); // nope
<warning descr="Object instantiation inside 'hashCode()'">Long.valueOf(1)</warning>;
Long.valueOf(1);
<warning descr="Object instantiation inside 'hashCode()'">Long.valueOf(128)</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>;
Character.valueOf('a');
<warning descr="Object instantiation inside 'hashCode()'">Character.valueOf('П')</warning>;
Float.<warning descr="Object instantiation inside 'hashCode()'">valueOf</warning>((float) 1.0);
Double.<warning descr="Object instantiation inside 'hashCode()'">valueOf</warning>(1.0);
<warning descr="Object instantiation inside 'hashCode()' (autoboxing)">i</warning>++;
int j = 1;
j++;
int[] is = <warning descr="Object instantiation inside 'hashCode()'">{j}</warning>;
int[] is = <warning descr="Object instantiation inside 'hashCode()'">{127}</warning>;
Integer.valueOf(is[0]);
int hashCode = 7;
<warning descr="Object instantiation inside 'hashCode()' (varargs call)">java.util.Arrays.asList()</warning>;
java.util.Arrays.<warning descr="Object instantiation inside 'hashCode()'">asList</warning>();
for (Object fooElement : <warning descr="Object instantiation inside 'hashCode()' (iterator)">fooList</warning>) {
hashCode = 31 * hashCode + (fooElement == null ? 0 : fooElement.hashCode());
}
Pattern.<warning descr="Object instantiation inside 'hashCode()'">compile</warning>("regex");
return hashCode;
}
@@ -16,7 +16,9 @@
package com.siyeh.ig.performance;
import com.intellij.codeInspection.InspectionProfileEntry;
import com.intellij.testFramework.LightProjectDescriptor;
import com.siyeh.ig.LightInspectionTestCase;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
/**
@@ -33,4 +35,10 @@ public class ObjectInstantiationInEqualsHashCodeInspectionTest extends LightInsp
protected InspectionProfileEntry getInspection() {
return new ObjectInstantiationInEqualsHashCodeInspection();
}
@NotNull
@Override
protected LightProjectDescriptor getProjectDescriptor() {
return JAVA_9;
}
}