Merge pull request #2175 from aschackmull/java/continue-in-false-loop

Java: Port C++ query cpp/continue-in-false-loop to Java.

Author: yh-semmle

change-notes/1.23/analysis-java.md

@@ -2,6 +2,12 @@
 
 The following changes in version 1.23 affect Java analysis in all applications.
 
+## New queries
+
+| **Query**                   | **Tags**  | **Purpose**                                                        |
+|-----------------------------|-----------|--------------------------------------------------------------------|
+| Continue statement that does not continue (`java/continue-in-false-loop`) | correctness | Finds `continue` statements in `do { ... } while (false)` loops. |
+
 ## Changes to existing queries
 
 | **Query**                    | **Expected impact**    | **Change**                        |

java/ql/src/Likely Bugs/Statements/ContinueInFalseLoop.qhelp

@@ -0,0 +1,25 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+
+<overview>
+<p>A <code>continue</code> statement only re-runs the loop if the loop condition is true. Therefore using <code>continue</code> in a loop with a constant false condition will never cause the loop body to be re-run, which is misleading.
+</p>
+
+</overview>
+<recommendation>
+
+<p>Replace the <code>continue</code> statement with a <code>break</code> statement if the intent is to break from the loop.
+</p>
+
+</recommendation>
+
+<references>
+<li>
+Java Language Specification:
+<a href="http://docs.oracle.com/javase/specs/jls/se8/html/jls-14.html#jls-14.13">14.13 The do Statement</a>.
+</li>
+</references>
+</qhelp>

java/ql/src/Likely Bugs/Statements/ContinueInFalseLoop.ql

@@ -0,0 +1,21 @@
+/**
+ * @name Continue statement that does not continue
+ * @description A 'continue' statement only re-runs the loop if the
+ *              loop-condition is true. Therefore using 'continue' in a loop
+ *              with a constant false condition is misleading and usually a
+ *              bug.
+ * @kind problem
+ * @id java/continue-in-false-loop
+ * @problem.severity warning
+ * @precision high
+ * @tags correctness
+ */
+
+import java
+
+from DoStmt do, ContinueStmt continue
+where
+  do.getCondition().(BooleanLiteral).getBooleanValue() = false and
+  continue.(JumpStmt).getTarget() = do
+select continue, "This 'continue' never re-runs the loop - the $@ is always false.",
+  do.getCondition(), "loop condition"

java/ql/test/query-tests/ContinueInFalseLoop/A.java

@@ -0,0 +1,84 @@
+public class A {
+
+  public interface Cond {
+    boolean cond();
+  }
+
+  void test1(int x, Cond c) {
+    int i;
+
+    // --- do loops ---
+
+    do {
+      if (c.cond())
+        continue; // BAD
+      if (c.cond())
+        break;
+    } while (false);
+
+    do {
+      if (c.cond())
+        continue;
+      if (c.cond())
+        break;
+    } while (true);
+
+    do {
+      if (c.cond())
+        continue;
+      if (c.cond())
+        break;
+    } while (c.cond());
+
+    // --- while, for loops ---
+
+    while (false) {
+      if (c.cond())
+        continue; // GOOD [never reached, if the condition changed so it was then the result would no longer apply]
+      if (c.cond())
+        break;
+    }
+
+    for (i = 0; false; i++) {
+      if (c.cond())
+        continue; // GOOD [never reached, if the condition changed so it was then the result would no longer apply]
+      if (c.cond())
+        break;
+    }
+
+    // --- nested loops ---
+
+    do {
+      do {
+        if (c.cond())
+          continue; // BAD
+        if (c.cond())
+          break;
+      } while (false);
+      if (c.cond())
+        break;
+    } while (true);
+
+    do {
+      do {
+        if (c.cond())
+          continue; // GOOD
+        if (c.cond())
+          break;
+      } while (true);
+    } while (false);
+
+    do {
+      switch (x) {
+      case 1:
+        // do [1]
+        break; // break out of the switch
+      default:
+        // do [2]
+        // break out of the loop entirely, skipping [3]
+        continue; // BAD; labelled break is better
+      };
+      // do [3]
+    } while (false);
+  }
+}

java/ql/test/query-tests/ContinueInFalseLoop/ContinueInFalseLoop.expected

@@ -0,0 +1,3 @@
+| A.java:14:9:14:17 | stmt | This 'continue' never re-runs the loop - the $@ is always false. | A.java:17:14:17:18 | false | loop condition |
+| A.java:54:11:54:19 | stmt | This 'continue' never re-runs the loop - the $@ is always false. | A.java:57:16:57:20 | false | loop condition |
+| A.java:79:9:79:17 | stmt | This 'continue' never re-runs the loop - the $@ is always false. | A.java:82:14:82:18 | false | loop condition |

java/ql/test/query-tests/ContinueInFalseLoop/ContinueInFalseLoop.qlref

@@ -0,0 +1 @@
+Likely Bugs/Statements/ContinueInFalseLoop.ql