SONARJAVA-5628 In CFG computation, mark semantic incomplete for unexpected break and continue

Instead of causing the rule to crash with a stack trace in the logs.

Author: romainbrenguier

java-checks/src/main/java/org/sonar/java/checks/ParameterReassignedToCheck.java

@@ -77,7 +77,7 @@ public void visitMethod(MethodTree tree) {
 
   @Override
   public void visitCatch(CatchTree tree) {
-    CFG cfg = CFG.buildCFG(tree.block().body(), true);
+    CFG cfg = CFG.buildCFG(tree.block().body());
     Symbol variable = tree.parameter().symbol();
     boolean liveVar = true;
     if(variable.owner().isMethodSymbol()) {
@@ -97,7 +97,7 @@ public void visitCatch(CatchTree tree) {
 
   @Override
   public void visitForEachStatement(ForEachStatement tree) {
-    CFG cfg = CFG.buildCFG(Collections.singletonList(tree), true);
+    CFG cfg = CFG.buildCFG(Collections.singletonList(tree));
     Symbol variable = tree.variable().symbol();
     boolean liveVar = true;
     if(variable.owner().isMethodSymbol()) {

java-checks/src/main/java/org/sonar/java/checks/SwitchCaseWithoutBreakCheck.java

@@ -47,7 +47,7 @@ public List<Tree.Kind> nodesToVisit() {
   public void visitNode(Tree tree) {
     SwitchStatementTree switchStatementTree = (SwitchStatementTree) tree;
     Set<CaseGroupTree> caseGroupTrees = new HashSet<>(switchStatementTree.cases());
-    CFG cfg = CFG.buildCFG(Collections.singletonList(tree), true);
+    CFG cfg = CFG.buildCFG(Collections.singletonList(tree));
     Set<CFG.Block> switchSuccessors = cfg.entryBlock().successors();
 
     Map<CFG.Block, CaseGroupTree> cfgBlockToCaseGroupMap = createMapping(switchSuccessors, caseGroupTrees);

java-checks/src/main/java/org/sonar/java/checks/TryWithResourcesCheck.java

@@ -88,7 +88,7 @@ private static boolean isFollowedByTryWithFinally(Tree tree) {
     }
 
     if (blockParent != null) {
-      CFG cfg = CFG.buildCFG(Collections.singletonList(blockParent), true);
+      CFG cfg = CFG.buildCFG(Collections.singletonList(blockParent));
       if (!cfg.blocks().isEmpty()) {
         return newFollowedByTryStatement(cfg.blocks().get(0));
       }

java-frontend/src/main/java/org/sonar/java/cfg/CFG.java

@@ -97,7 +97,6 @@ public class CFG implements ControlFlowGraph {
 
   private static final Logger LOG = LoggerFactory.getLogger(CFG.class);
 
-  private final boolean ignoreBreakAndContinue;
   @Nullable
   private Symbol.MethodSymbol methodSymbol;
   private Block currentBlock;
@@ -135,12 +134,11 @@ public void addCatch(Type type, Block catchBlock) {
   private Map<String, Block> labelsBreakTarget = new HashMap<>();
   private Map<String, Block> labelsContinueTarget = new HashMap<>();
 
-  private CFG(List<? extends Tree> trees, @Nullable MethodTree tree, boolean ignoreBreakAndContinue) {
+  private CFG(List<? extends Tree> trees, @Nullable MethodTree tree) {
     if (tree != null) {
       methodSymbol = tree.symbol();
       checkSymbolSemantic(methodSymbol, "method definition", tree.simpleName().identifierToken());
     }
-    this.ignoreBreakAndContinue = ignoreBreakAndContinue;
     exitBlocks.add(createBlock());
     currentBlock = createBlock(exitBlock());
     outerTry = new TryStatement();
@@ -493,18 +491,14 @@ private Block createBlock() {
     return result;
   }
 
-  public static CFG buildCFG(List<? extends Tree> trees, boolean ignoreBreak) {
-    return new CFG(trees, null, ignoreBreak);
-  }
-
   public static CFG buildCFG(List<? extends Tree> trees) {
-    return new CFG(trees, null, false);
+    return new CFG(trees, null);
   }
 
   public static CFG build(MethodTree tree) {
     BlockTree block = tree.block();
     Preconditions.checkArgument(block != null, "Cannot build CFG for method with no body.");
-    return new CFG(block.body(), tree, false);
+    return new CFG(block.body(), tree);
   }
 
   private void build(ListTree<? extends Tree> trees) {
@@ -922,9 +916,7 @@ private void buildBreakStatement(BreakStatementTree tree) {
     Block targetBlock = null;
     if (label == null) {
       if (breakTargets.isEmpty()) {
-        if (!ignoreBreakAndContinue) {
-          throw new IllegalStateException("'break' statement not in loop or switch statement");
-        }
+        markSemanticAsIncomplete("'break' statement not in loop or switch statement", tree.firstToken());
       } else {
         targetBlock = breakTargets.getLast();
       }
@@ -953,9 +945,7 @@ private void buildContinueStatement(ContinueStatementTree tree) {
     Block targetBlock = null;
     if (label == null) {
       if (continueTargets.isEmpty()) {
-        if (!ignoreBreakAndContinue) {
-          throw new IllegalStateException("'continue' statement not in loop or switch statement");
-        }
+        markSemanticAsIncomplete("'continue' statement not in loop or switch statement", tree.firstToken());
       } else {
         targetBlock = continueTargets.getLast();
       }

java-frontend/src/test/java/org/sonar/java/cfg/CFGTest.java

@@ -42,7 +42,6 @@
 import org.sonar.plugins.java.api.tree.VariableTree;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.fail;
 import static org.sonar.java.cfg.CFGTestUtils.buildCFG;
 import static org.sonar.java.cfg.CFGTestUtils.buildCFGFromLambda;
 import static org.sonar.plugins.java.api.tree.Tree.Kind.ARRAY_ACCESS_EXPRESSION;
@@ -3278,6 +3277,9 @@ void test_semantic_completeness() {
 
     assertCompleteSemantic("void foo() { Runnable ignoredLambdaContent = () -> { Unknown x; };  }", true);
     assertCompleteSemantic("void foo() { class IgnoredNestedClass { void foo() { Unknown x; } } }", true);
+
+    assertCompleteSemantic("void foo(int b) { break; }", false, "Incomplete Semantic, 'break' statement not in loop or switch statement 'break' line 1 col 29");
+    assertCompleteSemantic("void foo(int b) { continue; }", false, "Incomplete Semantic, 'continue' statement not in loop or switch statement 'continue' line 1 col 29");
   }
 
 
@@ -3300,16 +3302,15 @@ private void build_partial_cfg(String breakOrContinue) {
     CompilationUnitTree cut = JParserTestUtils.parse("class A {" + methodCode + "}");
     MethodTree methodTree = (MethodTree) ((ClassTree) cut.types().get(0)).members().get(0);
     List<StatementTree> body = methodTree.block().body();
-    CFG cfg = CFG.buildCFG(body, true);
+
+    logTester.setLevel(Level.DEBUG);
+    logTester.clear();
+    CFG cfg = CFG.buildCFG(body);
     cfg.setMethodSymbol(methodTree.symbol());
     assertThat(cfg.blocks()).hasSize(5);
     assertThat(cfg.methodSymbol()).isSameAs(methodTree.symbol());
-
-    try {
-      CFG.buildCFG(body, false);
-      fail("IllegalStateException should have been thrown");
-    } catch (IllegalStateException iae) {
-      assertThat(iae).hasMessage("'"+breakOrContinue+"' statement not in loop or switch statement");
-    }
+    assertThat(cfg.hasCompleteSemantic()).isFalse();
+    assertThat(logTester.logs(Level.DEBUG))
+      .containsExactly("Incomplete Semantic, '" + breakOrContinue + "' statement not in loop or switch statement '" + breakOrContinue + "' " + "line 1 col 80");
   }
 }