diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/StringConcatenationInLoopsInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/StringConcatenationInLoopsInspection.java index ec656d285666..98210eb7cd6c 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/StringConcatenationInLoopsInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/StringConcatenationInLoopsInspection.java @@ -126,41 +126,13 @@ public class StringConcatenationInLoopsInspection extends BaseInspection { if (variable != null) { PsiLoopStatement commonLoop = getOutermostCommonLoop(expression, variable); - return commonLoop != null && !flowBreaksLoop(PsiTreeUtil.getParentOfType(expression, PsiStatement.class), commonLoop); + return commonLoop != null && !ControlFlowUtils + .flowBreaksLoop(PsiTreeUtil.getParentOfType(expression, PsiStatement.class), commonLoop); } } return !containingStatementExits(expression); } - @Contract("null, _ -> false") - private static boolean flowBreaksLoop(PsiStatement statement, PsiLoopStatement loop) { - if(statement == null || statement == loop) return false; - for(PsiStatement sibling = statement; sibling != null; sibling = PsiTreeUtil.getNextSiblingOfType(sibling, PsiStatement.class)) { - if(sibling instanceof PsiContinueStatement) return false; - if(sibling instanceof PsiThrowStatement || sibling instanceof PsiReturnStatement) return true; - if(sibling instanceof PsiBreakStatement) { - PsiBreakStatement breakStatement = (PsiBreakStatement)sibling; - PsiStatement exitedStatement = breakStatement.findExitedStatement(); - if(exitedStatement == loop) return true; - return flowBreaksLoop(exitedStatement, loop); - } - } - PsiElement parent = statement.getParent(); - if(parent == loop) return false; - if(parent instanceof PsiCodeBlock) { - PsiElement gParent = parent.getParent(); - if(gParent instanceof PsiBlockStatement || gParent instanceof PsiSwitchStatement) { - return flowBreaksLoop((PsiStatement)gParent, loop); - } - return false; - } - if(parent instanceof PsiLabeledStatement || parent instanceof PsiIfStatement || parent instanceof PsiSwitchLabelStatement - || parent instanceof PsiSwitchStatement) { - return flowBreaksLoop((PsiStatement)parent, loop); - } - return false; - } - private PsiLoopStatement getOutermostCommonLoop(PsiExpression expression, PsiVariable variable) { PsiElement stopAt = null; PsiCodeBlock block = getSurroundingBlock(expression); diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ControlFlowUtils.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ControlFlowUtils.java index 2fea7d07dea5..26fc116ab58d 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ControlFlowUtils.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ControlFlowUtils.java @@ -600,6 +600,53 @@ public class ControlFlowUtils { return false; } + /** + * Checks whether control flow after executing given statement will definitely not go into the next iteration of given loop. + * + * @param statement executed statement. It's not checked whether this statement itself breaks the loop. + * @param loop a surrounding loop. Must be parent of statement + * @return true if it can be statically defined that next loop iteration will not be executed. + */ + @Contract("null, _ -> false") + public static boolean flowBreaksLoop(PsiStatement statement, PsiLoopStatement loop) { + if(statement == null || statement == loop) return false; + for (PsiStatement sibling = nextExecutedStatement(statement); sibling != null; sibling = nextExecutedStatement(sibling)) { + if(sibling instanceof PsiContinueStatement) return false; + if(sibling instanceof PsiThrowStatement || sibling instanceof PsiReturnStatement) return true; + if(sibling instanceof PsiBreakStatement) { + PsiBreakStatement breakStatement = (PsiBreakStatement)sibling; + PsiStatement exitedStatement = breakStatement.findExitedStatement(); + if(exitedStatement == loop) return true; + return flowBreaksLoop(exitedStatement, loop); + } + } + return false; + } + + @Nullable + private static PsiStatement nextExecutedStatement(PsiStatement statement) { + PsiStatement next = PsiTreeUtil.getNextSiblingOfType(statement, PsiStatement.class); + while (next instanceof PsiBlockStatement) { + PsiStatement[] statements = ((PsiBlockStatement)next).getCodeBlock().getStatements(); + if (statements.length == 0) break; + next = statements[0]; + } + if (next == null) { + PsiElement parent = statement.getParent(); + if (parent instanceof PsiCodeBlock) { + PsiElement gParent = parent.getParent(); + if (gParent instanceof PsiBlockStatement || gParent instanceof PsiSwitchStatement) { + return nextExecutedStatement((PsiStatement)gParent); + } + } + else if (parent instanceof PsiLabeledStatement || parent instanceof PsiIfStatement || parent instanceof PsiSwitchLabelStatement + || parent instanceof PsiSwitchStatement) { + return nextExecutedStatement((PsiStatement)parent); + } + } + return next; + } + private static class NakedBreakFinder extends JavaRecursiveElementWalkingVisitor { private boolean m_found; diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/string_concatenation_in_loops/StringConcatenationInLoop.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/string_concatenation_in_loops/StringConcatenationInLoop.java index 85473eed3d70..161975019d55 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/string_concatenation_in_loops/StringConcatenationInLoop.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/string_concatenation_in_loops/StringConcatenationInLoop.java @@ -53,6 +53,47 @@ public class StringConcatenationInLoop throw new Error("foo" + i); } } + String s = ""; + for(int i = 0; i < 5; i++) { + if(i > 2) { + s += i; + { + System.out.println(s); + break; + } + } + } + for(int i = 0; i < 5; i++) { + if(i > 2) { + s += i; + { + System.out.println(s); + { + break; + } + } + } + } + for(int i = 0; i < 5; i++) { + if(i > 2) { + s += i; + { + { + System.out.println(s); + } + break; + } + } + } + for(int i = 0; i < 5; i++) { + if(i > 2) { + s += i; + { + System.out.println(s); + } + System.out.println(s); + } + } System.out.println(foo); return foo; }