SONARJAVA-1586 Execute post statement check only once for SE checks

Author: benzonico

java-frontend/src/main/java/org/sonar/java/se/CheckerDispatcher.java

@@ -33,7 +33,7 @@ public class CheckerDispatcher implements CheckerContext {
   private final JavaFileScannerContext context;
   private final List<SECheck> checks;
   private Tree syntaxNode;
-  private int currentCheckerIndex = 0;
+  private int currentCheckerIndex = -1;
   private boolean transition = false;
 
   public CheckerDispatcher(ExplodedGraphWalker explodedGraphWalker, JavaFileScannerContext context, List<SECheck> checks) {
@@ -58,32 +58,7 @@ public boolean executeCheckPreStatement(Tree syntaxNode) {
 
   public void executeCheckPostStatement(Tree syntaxNode) {
     this.syntaxNode = syntaxNode;
-    ProgramState ps;
-    for (SECheck checker : checks) {
-      ps = checker.checkPostStatement(this, syntaxNode);
-      if (ps == null) {
-        throw new IllegalStateException("Post statement processing is not allowed to generate a sink yet!");
-      }
-      explodedGraphWalker.programState = ps;
-    }
-  }
-
-  private void executePost() {
-    this.transition = false;
-    if (currentCheckerIndex < checks.size()) {
-      explodedGraphWalker.programState = checks.get(currentCheckerIndex).checkPostStatement(this, syntaxNode);
-    } else {
-      if (explodedGraphWalker.programPosition.i< explodedGraphWalker.programPosition.block.elements().size()) {
-        explodedGraphWalker.clearStack(explodedGraphWalker.programPosition.block.elements().get(explodedGraphWalker.programPosition.i));
-      }
-      explodedGraphWalker.enqueue(
-        new ExplodedGraph.ProgramPoint(explodedGraphWalker.programPosition.block, explodedGraphWalker.programPosition.i + 1),
-        explodedGraphWalker.programState, explodedGraphWalker.node.exitPath);
-      return;
-    }
-    if (!transition) {
-      addTransition(explodedGraphWalker.programState);
-    }
+    addTransition(explodedGraphWalker.programState);
   }
 
   @Override
@@ -112,6 +87,24 @@ public void addTransition(ProgramState state) {
     this.transition = true;
   }
 
+  private void executePost() {
+    this.transition = false;
+    if (currentCheckerIndex < checks.size()) {
+      explodedGraphWalker.programState = checks.get(currentCheckerIndex).checkPostStatement(this, syntaxNode);
+    } else {
+      if (explodedGraphWalker.programPosition.i< explodedGraphWalker.programPosition.block.elements().size()) {
+        explodedGraphWalker.clearStack(explodedGraphWalker.programPosition.block.elements().get(explodedGraphWalker.programPosition.i));
+      }
+      explodedGraphWalker.enqueue(
+        new ExplodedGraph.ProgramPoint(explodedGraphWalker.programPosition.block, explodedGraphWalker.programPosition.i + 1),
+        explodedGraphWalker.programState, explodedGraphWalker.node.exitPath);
+      return;
+    }
+    if (!transition) {
+      addTransition(explodedGraphWalker.programState);
+    }
+  }
+
   @Override
   public Object createSink() {
     transition = true;

java-frontend/src/test/java/org/sonar/java/se/CheckerDispatcherTest.java

@@ -0,0 +1,66 @@
+/*
+ * SonarQube Java
+ * Copyright (C) 2012-2016 SonarSource SA
+ * mailto:contact AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.java.se;
+
+import com.google.common.collect.Lists;
+import org.junit.Test;
+import org.sonar.java.cfg.CFG;
+import org.sonar.java.se.checks.NullDereferenceCheck;
+import org.sonar.java.se.checks.SECheck;
+import org.sonar.plugins.java.api.JavaFileScannerContext;
+import org.sonar.plugins.java.api.tree.Tree;
+
+import java.util.List;
+
+import static org.fest.assertions.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+
+public class CheckerDispatcherTest {
+
+  @Test
+  public void test_only_one_execution_of_post_statement_by_check() {
+    List<SECheck> checks = Lists.newArrayList(new NullDereferenceCheck(), new CheckTest(), new CheckTest(), new CheckTest());
+    CheckerDispatcher checkerDispatcher = new CheckerDispatcher(mockExplodedGraphWalker(), mock(JavaFileScannerContext.class), checks);
+    checkerDispatcher.executeCheckPostStatement(mock(Tree.class));
+    for (SECheck check : checks) {
+      if(check instanceof CheckTest) {
+        assertThat(((CheckTest) check).postStatementExecution).isEqualTo(1);
+      }
+    }
+  }
+
+  private static ExplodedGraphWalker mockExplodedGraphWalker() {
+    ExplodedGraphWalker explodedGraphWalker = mock(ExplodedGraphWalker.class);
+    explodedGraphWalker.programPosition = new ExplodedGraph.ProgramPoint(new CFG.Block(1), 0);
+    explodedGraphWalker.programState = mock(ProgramState.class);
+    explodedGraphWalker.node = new ExplodedGraph.Node(explodedGraphWalker.programPosition, explodedGraphWalker.programState);
+    return explodedGraphWalker;
+  }
+
+  private static class CheckTest extends SECheck {
+    int postStatementExecution = 0;
+
+    @Override
+    public ProgramState checkPostStatement(CheckerContext context, Tree syntaxNode) {
+      postStatementExecution++;
+      return mock(ProgramState.class);
+    }
+  }
+}