diff --git a/java/java-impl/src/com/intellij/codeInspection/WrapperTypeMayBePrimitiveInspection.java b/java/java-impl/src/com/intellij/codeInspection/WrapperTypeMayBePrimitiveInspection.java index 355221226512..3d7dd90233f1 100644 --- a/java/java-impl/src/com/intellij/codeInspection/WrapperTypeMayBePrimitiveInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/WrapperTypeMayBePrimitiveInspection.java @@ -6,6 +6,7 @@ import com.intellij.codeInspection.dataFlow.NullnessUtil; import com.intellij.openapi.project.Project; import com.intellij.psi.*; import com.intellij.psi.tree.IElementType; +import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiTypesUtil; import com.intellij.psi.util.PsiUtil; import com.intellij.util.ArrayUtil; @@ -53,34 +54,71 @@ public class WrapperTypeMayBePrimitiveInspection extends AbstractBaseJavaLocalIn public void visitLocalVariable(PsiLocalVariable variable) { if (!isBoxedType(variable.getType())) return; PsiExpression initializer = variable.getInitializer(); - if (initializer != null && !isValidRvalue(initializer)) { - return; - } + UnboxingStatistics statistics = new UnboxingStatistics(); + if (initializer != null && !statistics.checkExpression(initializer)) return; + if (ExpressionUtils.isNullLiteral(variable.getInitializer())) return; PsiElement block = PsiUtil.getVariableCodeBlock(variable, null); if (block == null) return; - WrapperTypeCanBePrimitiveDetectingVisitor visitor = new WrapperTypeCanBePrimitiveDetectingVisitor(variable); + WrapperTypeCanBePrimitiveDetectingVisitor visitor = new WrapperTypeCanBePrimitiveDetectingVisitor(variable, statistics); block.accept(visitor); - if (visitor.myBoxingRequired || !visitor.myHasReferences) return; + if (visitor.myBoxingRequired || + !visitor.myHasReferences || + !visitor.myStatistics.primitiveReplacementReducesUnnecessaryOperationCount()) { + return; + } holder.registerProblem(variable.getTypeElement(), InspectionsBundle.message("inspection.wrapper.type.may.be.primitive.name"), new ConvertWrapperTypeToPrimitive()); } }; } - private static boolean isValidRvalue(PsiExpression expression) { - return isNotNull(expression) || expression instanceof PsiMethodCallExpression && VALUE_OF.test((PsiMethodCallExpression)expression); + private static boolean isValueOfCall(PsiExpression expression) { + return expression instanceof PsiMethodCallExpression && VALUE_OF.test((PsiMethodCallExpression)expression); + } + + private static class UnboxingStatistics { + private int myBoxedUnnecessaryOperationCount = 0; + private int myUnboxedUnnecessaryOperationCount = 0; + + /** + * Check, whether expression passed as argument is suitable to be right part of assignment or initializer when variable will be primitive + * Also collect statistics if boxing needed or unboxing needed + * + * @return false if boxing is required anyway + */ + boolean checkExpression(@NotNull PsiExpression expression) { + if (expression.getType() instanceof PsiPrimitiveType && !PsiType.NULL.equals(expression.getType())) { + myBoxedUnnecessaryOperationCount++; + } + else if (!isValueOfCall(expression)) { + if (NullnessUtil.getExpressionNullness(expression) != Nullness.NOT_NULL) { // not safe using with primitive + return false; + } + myUnboxedUnnecessaryOperationCount++; + } + return true; + } + + boolean primitiveReplacementReducesUnnecessaryOperationCount() { + return myUnboxedUnnecessaryOperationCount < myBoxedUnnecessaryOperationCount; + } } private static class WrapperTypeCanBePrimitiveDetectingVisitor extends JavaRecursiveElementWalkingVisitor { private final PsiLocalVariable myVariable; boolean myBoxingRequired = false; boolean myHasReferences = false; + private final @NotNull UnboxingStatistics myStatistics; - public WrapperTypeCanBePrimitiveDetectingVisitor(PsiLocalVariable variable) { + public WrapperTypeCanBePrimitiveDetectingVisitor(PsiLocalVariable variable, + @NotNull UnboxingStatistics statistics) { myVariable = variable; + myStatistics = statistics; } + private static final int IN_LOOP_ASSIGNMENT_OPERATION_MULTIPLIER = 10; + @Override public void visitReferenceExpression(PsiReferenceExpression expression) { super.visitReferenceExpression(expression); @@ -102,43 +140,97 @@ public class WrapperTypeMayBePrimitiveInspection extends AbstractBaseJavaLocalIn PsiMethod method = callExpression.resolveMethod(); if (method == null) return; PsiParameter[] parameters = method.getParameterList().getParameters(); - int parameterIndex = parameters.length > argumentsIndex ? parameters.length - 1 : argumentsIndex; + int parameterIndex = parameters.length < argumentsIndex + 1 ? parameters.length - 1 : argumentsIndex; PsiParameter parameter = parameters[parameterIndex]; - if (parameter.getType() instanceof PsiPrimitiveType) return; - boxingRequired(); + PsiType type = parameter.getType(); + if (type instanceof PsiPrimitiveType) { + myStatistics.myBoxedUnnecessaryOperationCount++; + } + else { + myStatistics.myUnboxedUnnecessaryOperationCount++; + } } else if (parent instanceof PsiAssignmentExpression) { PsiExpression rExpression = ((PsiAssignmentExpression)parent).getRExpression(); if (rExpression == null) return; - if (isValidRvalue(rExpression)) { - return; + if (!myStatistics.checkExpression(rExpression)) { + boxingRequired(); } - boxingRequired(); } else if (parent instanceof PsiSynchronizedStatement) { boxingRequired(); } else if (parent instanceof PsiBinaryExpression) { - PsiBinaryExpression binaryExpression = (PsiBinaryExpression)parent; - IElementType operationTokenType = binaryExpression.getOperationTokenType(); - if (operationTokenType == JavaTokenType.EQEQ || operationTokenType == JavaTokenType.NE) { - PsiExpression other = ExpressionUtils.getOtherOperand(binaryExpression, myVariable); - if (other.getType() instanceof PsiPrimitiveType) return; - boxingRequired(); + checkBinaryExpression((PsiBinaryExpression)parent); + } + else if (parent instanceof PsiReturnStatement) { + PsiMethod method = PsiTreeUtil.getParentOfType(parent, PsiMethod.class, false, PsiLambdaExpression.class); + if (method != null) { + PsiType returnType = method.getReturnType(); + if (returnType != null) { + if (returnType instanceof PsiPrimitiveType) { + myStatistics.myBoxedUnnecessaryOperationCount++; + } else { + myStatistics.myUnboxedUnnecessaryOperationCount++; + } + } } } } + private void checkBinaryExpression(PsiBinaryExpression binaryExpression) { + IElementType operationTokenType = binaryExpression.getOperationTokenType(); + PsiExpression other = ExpressionUtils.getOtherOperand(binaryExpression, myVariable); + PsiType type = other.getType(); + if (operationTokenType == JavaTokenType.EQEQ || operationTokenType == JavaTokenType.NE) { + if (type instanceof PsiPrimitiveType && !PsiType.NULL.equals(type)) { + myStatistics.myBoxedUnnecessaryOperationCount++; + } else { + boxingRequired(); + return; + } + } + + int boxedUnnecessaryOpImpact = 0; + int unboxedUnnecessaryOpImpact = 0; + if (type instanceof PsiPrimitiveType) { + if (PsiType.NULL.equals(type)) { + boxingRequired(); + return; + } + else { + if (ExpressionUtils.isCompoundAssignmentOperation(binaryExpression)) { + boxedUnnecessaryOpImpact += 2; + } + } + } + else if (NullnessUtil.getExpressionNullness(other) == Nullness.NOT_NULL) { + boxedUnnecessaryOpImpact += 3; + unboxedUnnecessaryOpImpact += 3; + } + else { + boxingRequired(); + return; + } + PsiLoopStatement binopLoop = + PsiTreeUtil.getParentOfType(binaryExpression, PsiLoopStatement.class, false, PsiClass.class, PsiLambdaExpression.class); + PsiLoopStatement variableLoop = + PsiTreeUtil.getParentOfType(myVariable, PsiLoopStatement.class, false, PsiClass.class, PsiLambdaExpression.class); + if (binopLoop != null && binopLoop == variableLoop) { + boxedUnnecessaryOpImpact *= IN_LOOP_ASSIGNMENT_OPERATION_MULTIPLIER; + unboxedUnnecessaryOpImpact *= IN_LOOP_ASSIGNMENT_OPERATION_MULTIPLIER; + } + myStatistics.myBoxedUnnecessaryOperationCount += boxedUnnecessaryOpImpact; + myStatistics.myUnboxedUnnecessaryOperationCount += unboxedUnnecessaryOpImpact; + } + + // Strong boxing requirement private void boxingRequired() { myBoxingRequired = true; stopWalking(); } } - private static boolean isNotNull(PsiExpression rExpression) { - return NullnessUtil.getExpressionNullness(rExpression) == Nullness.NOT_NULL; - } - private static boolean isBoxedType(@NotNull PsiType type) { return type.equalsToText(CommonClassNames.JAVA_LANG_BOOLEAN) || type.equalsToText(CommonClassNames.JAVA_LANG_INTEGER) || diff --git a/java/java-tests/testData/inspection/wrapperTypeMayBePrimitive/TypeMayBePrimitive.java b/java/java-tests/testData/inspection/wrapperTypeMayBePrimitive/TypeMayBePrimitive.java index 7d4ef25c493c..f6377d91b981 100644 --- a/java/java-tests/testData/inspection/wrapperTypeMayBePrimitive/TypeMayBePrimitive.java +++ b/java/java-tests/testData/inspection/wrapperTypeMayBePrimitive/TypeMayBePrimitive.java @@ -23,6 +23,7 @@ class TypeMayBePrimitive { Boolean boxNotNeeded; boxNotNeeded = getNotNullBox(); use(boxNotNeeded); + boxNotNeeded |= true; Boolean bool = getBool(); use(bool); @@ -52,7 +53,7 @@ class TypeMayBePrimitive { Integer i2 = 12; boxedParam(i2); - Integer i3 = 12; + Integer i3 = 12; boxedAndPrimitiveParam(i3, i3); } @@ -84,5 +85,17 @@ class TypeMayBePrimitive { Integer i2 = 12; if (i2 == 43) { } + + Boolean b = true; + if (b != null) {} + } + + void varargUse() { + Integer i = 12; + vararg(i, i, i, i); + } + + void vararg(int k, int... i) { + } } \ No newline at end of file diff --git a/java/java-tests/testData/inspection/wrapperTypeMayBePrimitive/afterInteger.java b/java/java-tests/testData/inspection/wrapperTypeMayBePrimitive/afterLong.java similarity index 100% rename from java/java-tests/testData/inspection/wrapperTypeMayBePrimitive/afterInteger.java rename to java/java-tests/testData/inspection/wrapperTypeMayBePrimitive/afterLong.java diff --git a/java/java-tests/testData/inspection/wrapperTypeMayBePrimitive/beforeInteger.java b/java/java-tests/testData/inspection/wrapperTypeMayBePrimitive/beforeLong.java similarity index 100% rename from java/java-tests/testData/inspection/wrapperTypeMayBePrimitive/beforeInteger.java rename to java/java-tests/testData/inspection/wrapperTypeMayBePrimitive/beforeLong.java diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java index e9e5963e5b37..c72b81dd8917 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java @@ -1239,4 +1239,16 @@ public class ExpressionUtils { return true; }); } + + public static boolean isCompoundAssignmentOperation(@NotNull PsiBinaryExpression binExpr) { + IElementType tokenType = binExpr.getOperationTokenType(); + return tokenType == JavaTokenType.PLUSEQ || + tokenType == JavaTokenType.MINUSEQ || + tokenType == JavaTokenType.ASTERISKEQ || + tokenType == JavaTokenType.DIVEQ || + tokenType == JavaTokenType.ANDEQ || + tokenType == JavaTokenType.OREQ || + tokenType == JavaTokenType.PERCEQ || + tokenType == JavaTokenType.XOREQ; + } } \ No newline at end of file