diff --git a/java-checks-test-sources/default/src/main/java/checks/SuspiciousListRemove.java b/java-checks-test-sources/default/src/main/java/checks/SuspiciousListRemove.java index 32be8cb4219..4aa0889bcfd 100644 --- a/java-checks-test-sources/default/src/main/java/checks/SuspiciousListRemove.java +++ b/java-checks-test-sources/default/src/main/java/checks/SuspiciousListRemove.java @@ -1,5 +1,6 @@ package checks; +import java.util.ArrayList; import java.util.List; public class SuspiciousListRemove { @@ -66,6 +67,34 @@ void controlFlow2(List list) { } } + void controlFlowReturn(List list) { + for (int i = 0; i < list.size(); i++) { + if (list.get(i).isEmpty()) { + list.remove(i); // Compliant because control flow with "return" + return; + } + } + } + + String controlFlowReturn2(List list) { + for (int i = 0; i < list.size(); i++) { + if (list.get(i).isEmpty()) { + return list.remove(i); // Compliant because control flow with "return" + } + } + return null; + } + + List controlFlowReturn3(List list) { + List deleted = new ArrayList<>(); + for (int i = 0; i < list.size(); i++) { + if (list.get(i).isEmpty()) { + deleted.add(list.remove(i)); // Noncompliant + } + } + return deleted; // "return" is outside the loop + } + void coverage1(List list, int from) { for (this.x = from; this.x < list.size(); this.x++) list.remove(this.x); // Consider only local vars diff --git a/java-checks/src/main/java/org/sonar/java/checks/SuspiciousListRemoveCheck.java b/java-checks/src/main/java/org/sonar/java/checks/SuspiciousListRemoveCheck.java index 4db952a6046..e46c58a4419 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/SuspiciousListRemoveCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/SuspiciousListRemoveCheck.java @@ -35,6 +35,7 @@ import org.sonar.plugins.java.api.tree.ForStatementTree; import org.sonar.plugins.java.api.tree.IdentifierTree; import org.sonar.plugins.java.api.tree.MethodInvocationTree; +import org.sonar.plugins.java.api.tree.ReturnStatementTree; import org.sonar.plugins.java.api.tree.StatementTree; import org.sonar.plugins.java.api.tree.Tree; import org.sonar.plugins.java.api.tree.UnaryExpressionTree; @@ -97,7 +98,7 @@ private static boolean isIncrementingLoop(ForStatementTree forStatementTree, Sym private static class LoopBodyVisitor extends BaseTreeVisitor { private final Symbol counter; private MethodInvocationTree listRemove; - private boolean hasBreakOrContinue; + private boolean hasBreakOrContinueOrReturn; private boolean isCounterAssigned; public LoopBodyVisitor(Symbol counter) { @@ -114,16 +115,22 @@ public void visitMethodInvocation(MethodInvocationTree tree) { @Override public void visitBreakStatement(BreakStatementTree tree) { - hasBreakOrContinue = true; + hasBreakOrContinueOrReturn = true; super.visitBreakStatement(tree); } @Override public void visitContinueStatement(ContinueStatementTree tree) { - hasBreakOrContinue = true; + hasBreakOrContinueOrReturn = true; super.visitContinueStatement(tree); } + @Override + public void visitReturnStatement(ReturnStatementTree tree) { + hasBreakOrContinueOrReturn = true; + super.visitReturnStatement(tree); + } + @Override public void visitAssignmentExpression(AssignmentExpressionTree tree) { if (tree.variable().is(Tree.Kind.IDENTIFIER)) { @@ -141,7 +148,7 @@ public void visitUnaryExpression(UnaryExpressionTree tree) { } boolean hasIssue() { - return listRemove != null && !hasBreakOrContinue && !isCounterAssigned; + return listRemove != null && !hasBreakOrContinueOrReturn && !isCounterAssigned; } }