diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties index e85d701f15be..e5cc1d224308 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties @@ -1027,6 +1027,7 @@ public.field.accessed.in.synchronized.context.problem.descriptor=Non-private fie field.accessed.synchronized.and.unsynchronized.problem.descriptor=Field #ref is accessed in both synchronized and unsynchronized contexts #loc extended.for.statement.problem.descriptor=Extended #ref statement #loc object.allocation.in.loop.problem.descriptor=Object allocation new #ref() in loop #loc +object.allocation.in.loop.problem.indirect.descriptor=Indirect object allocation via #ref() call in loop #loc instantiating.object.to.get.class.object.problem.descriptor=Instantiating object to get Class object #loc field.may.be.static.problem.descriptor=Field #ref may be 'static' #loc method.may.be.static.problem.descriptor=Method #ref() may be 'static' #loc diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/ObjectAllocationInLoopInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/ObjectAllocationInLoopInspection.java index e0024e078d04..708bd9a43ebf 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/ObjectAllocationInLoopInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/ObjectAllocationInLoopInspection.java @@ -15,14 +15,21 @@ */ package com.siyeh.ig.performance; +import com.intellij.codeInsight.PsiEquivalenceUtil; +import com.intellij.codeInspection.dataFlow.*; import com.intellij.psi.*; import com.intellij.psi.util.PsiTreeUtil; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; import com.siyeh.ig.psiutils.ControlFlowUtils; +import com.siyeh.ig.psiutils.ExpressionUtils; import org.jetbrains.annotations.NotNull; +import java.util.List; + +import static com.intellij.util.ObjectUtils.tryCast; + public class ObjectAllocationInLoopInspection extends BaseInspection { @Override @@ -35,8 +42,10 @@ public class ObjectAllocationInLoopInspection extends BaseInspection { @Override @NotNull protected String buildErrorString(Object... infos) { - return InspectionGadgetsBundle.message( - "object.allocation.in.loop.problem.descriptor"); + boolean direct = (boolean)infos[0]; + return InspectionGadgetsBundle.message(direct + ? "object.allocation.in.loop.problem.descriptor" + : "object.allocation.in.loop.problem.indirect.descriptor"); } @Override @@ -44,90 +53,64 @@ public class ObjectAllocationInLoopInspection extends BaseInspection { return new ObjectAllocationInLoopsVisitor(); } - private static class ObjectAllocationInLoopsVisitor - extends BaseInspectionVisitor { + private static class ObjectAllocationInLoopsVisitor extends BaseInspectionVisitor { + @Override + public void visitMethodCallExpression(PsiMethodCallExpression call) { + super.visitMethodCallExpression(call); + PsiMethod method = call.resolveMethod(); + if (method != null) { + List contracts = JavaMethodContractUtil.getMethodContracts(method); + ContractReturnValue value = JavaMethodContractUtil.getNonFailingReturnValue(contracts); + if (ContractReturnValue.returnNew().equals(value) && isPerformedRepeatedlyInLoop(call)) { + registerMethodCallError(call, false); + } + } + } @Override public void visitNewExpression(@NotNull PsiNewExpression expression) { super.visitNewExpression(expression); - if (!ControlFlowUtils.isInLoop(expression)) { - return; + if (isPerformedRepeatedlyInLoop(expression)) { + registerNewExpressionError(expression, true); } - if (ControlFlowUtils.isInExitStatement(expression)) { - return; - } - final PsiStatement newExpressionStatement = - PsiTreeUtil.getParentOfType(expression, PsiStatement.class); - if (newExpressionStatement == null) { - return; - } - final PsiStatement parentStatement = - PsiTreeUtil.getParentOfType(newExpressionStatement, - PsiStatement.class); - if (!ControlFlowUtils.statementMayCompleteNormally( - parentStatement)) { - return; - } - if (isAllocatedOnlyOnce(expression)) { - return; - } - registerNewExpressionError(expression); } - private static boolean isAllocatedOnlyOnce( - PsiNewExpression expression) { - final PsiElement parent = expression.getParent(); - if (!(parent instanceof PsiAssignmentExpression)) { + private static boolean isPerformedRepeatedlyInLoop(@NotNull PsiExpression expression) { + if (!ControlFlowUtils.isInLoop(expression)) return false; + if (ControlFlowUtils.isInExitStatement(expression)) return false; + final PsiStatement newExpressionStatement = PsiTreeUtil.getParentOfType(expression, PsiStatement.class); + if (newExpressionStatement == null) return false; + final PsiStatement parentStatement = PsiTreeUtil.getParentOfType(newExpressionStatement, PsiStatement.class); + if (!ControlFlowUtils.statementMayCompleteNormally(parentStatement)) return false; + return !isAllocatedOnlyOnce(expression); + } + + private static boolean isAllocatedOnlyOnce(PsiExpression expression) { + final PsiAssignmentExpression assignment = + PsiTreeUtil.getParentOfType(expression, PsiAssignmentExpression.class, true, PsiStatement.class); + if (assignment == null) return false; + final PsiReferenceExpression assignedRef = tryCast(assignment.getLExpression(), PsiReferenceExpression.class); + if (assignedRef == null) return false; + // to support cases like if(foo == null) foo = new Foo(new Bar()); + if (assignment.getRExpression() != expression && + NullnessUtil.getExpressionNullness(assignment.getRExpression()) != Nullness.NOT_NULL) { return false; } - final PsiAssignmentExpression assignmentExpression = - (PsiAssignmentExpression)parent; - final PsiExpression lExpression = - assignmentExpression.getLExpression(); - if (!(lExpression instanceof PsiReferenceExpression)) { - return false; + final PsiIfStatement ifStatement = PsiTreeUtil.getParentOfType(assignment, PsiIfStatement.class); + if (ifStatement == null) return false; + boolean equals; + if (PsiTreeUtil.isAncestor(ifStatement.getThenBranch(), assignment, true)) { + equals = true; } - final PsiIfStatement ifStatement = - PsiTreeUtil.getParentOfType(assignmentExpression, - PsiIfStatement.class); - if (ifStatement == null) { - return false; - } - final PsiExpression condition = ifStatement.getCondition(); - if (!(condition instanceof PsiBinaryExpression)) { - return false; - } - final PsiBinaryExpression binaryExpression = - (PsiBinaryExpression)condition; - if (binaryExpression.getOperationTokenType() != - JavaTokenType.EQEQ) { - return false; - } - final PsiReferenceExpression referenceExpression = - (PsiReferenceExpression)lExpression; - final PsiExpression lhs = binaryExpression.getLOperand(); - final PsiExpression rhs = binaryExpression.getROperand(); - if (lhs instanceof PsiLiteralExpression) { - if (!"null".equals(lhs.getText())) { - return false; - } - if (!(rhs instanceof PsiReferenceExpression)) { - return false; - } - return referenceExpression.getText().equals(rhs.getText()); - } - else if (rhs instanceof PsiLiteralExpression) { - if (!"null".equals(rhs.getText())) { - return false; - } - if (!(lhs instanceof PsiReferenceExpression)) { - return false; - } - return referenceExpression.getText().equals(lhs.getText()); + else if (PsiTreeUtil.isAncestor(ifStatement.getElseBranch(), assignment, true)) { + equals = false; } else { return false; } + final PsiExpression condition = ifStatement.getCondition(); + PsiReferenceExpression nullCheckedRef = ExpressionUtils.getReferenceExpressionFromNullComparison(condition, equals); + return nullCheckedRef != null && PsiEquivalenceUtil.areElementsEquivalent(nullCheckedRef, assignedRef); } } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/object_allocation_in_loop/ObjectAllocationInLoop.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/object_allocation_in_loop/ObjectAllocationInLoop.java index 58581dfe2e3a..65c839548915 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/object_allocation_in_loop/ObjectAllocationInLoop.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/object_allocation_in_loop/ObjectAllocationInLoop.java @@ -1,5 +1,7 @@ package com.siyeh.igtest.performance.object_allocation_in_loop; +import java.util.regex.*; + class ObjectAllocationInLoop { void m() { @@ -7,4 +9,39 @@ class ObjectAllocationInLoop { new Object(); } } + + void m1() { + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < 10; i++) { + if(sb != null) { + sb.append(i); + } else { + sb = new StringBuilder(String.valueOf(i)); + } + } + } + + void m2() { + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < 10; i++) { + if (sb == null) { + sb = new StringBuilder(String.valueOf(i)); + } + else { + sb.append(i); + } + } + } + + boolean checkPatterns(String[] patterns) { + for (String pattern : patterns) { + try { + Pattern.compile(pattern); + } + catch (PatternSyntaxException exception) { + return false; + } + } + return true; + } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/performance/ObjectAllocationInLoopInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/performance/ObjectAllocationInLoopInspectionTest.java index 0fcd297f6771..93d4e223e528 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/performance/ObjectAllocationInLoopInspectionTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/performance/ObjectAllocationInLoopInspectionTest.java @@ -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; /** * @author Bas Leijdekkers @@ -27,5 +29,11 @@ public class ObjectAllocationInLoopInspectionTest extends LightInspectionTestCas return new ObjectAllocationInLoopInspection(); } + @NotNull + @Override + protected LightProjectDescriptor getProjectDescriptor() { + return JAVA_9; + } + public void testObjectAllocationInLoop() { doTest(); } }