Add isInvalidLoop predicate that provides a reason

Author: rvermeulen

cpp/common/src/codingstandards/cpp/Loops.qll

@@ -37,13 +37,14 @@ predicate isForLoopWithFloatingPointCounters(ForStmt forLoop, Variable v) {
  * Holds if for loop `forLoop` contains an invalid for loop incrementation.
  * M6-5-2
  */
-predicate isInvalidForLoopIncrementation(ForStmt forLoop, LoopControlVariable v) {
-  v.getAnAccess() = forLoop.getCondition().getAChild*() and
-  exists(VariableAccess va |
-    va = v.getAnAccess() and
-    va = forLoop.getUpdate().getAChild*() and
-    not exists(CrementOperation cop | cop.getOperand() = va) and
-    not exists(Call c | c.getQualifier() = va and c.getTarget() instanceof UserCrementOperator)
+predicate isInvalidForLoopIncrementation(ForStmt forLoop, Variable v, VariableAccess modification) {
+  v = getAnIterationVariable(forLoop) and
+  modification = v.getAnAccess() and
+  modification = forLoop.getUpdate().getAChild*() and
+  modification.isModified() and
+  not exists(CrementOperation cop | cop.getOperand() = modification) and
+  not exists(Call c |
+    c.getQualifier() = modification and c.getTarget() instanceof UserCrementOperator
   ) and
   exists(VariableAccess va | va = forLoop.getCondition().getAChild*() and va = v.getAnAccess() |
     exists(EqualityOperation eop | eop.getAnOperand() = va)
@@ -163,26 +164,72 @@ predicate isLoopControlVarModifiedInLoopExpr(
 predicate isNonBoolLoopControlVar(
   ForStmt forLoop, LoopControlVariable loopControlVariable, VariableAccess loopControlVariableAccess
 ) {
-  // get a loop control variable that is not a loop counter
-  loopControlVariableAccess = loopControlVariable.getVariableAccessInLoop(forLoop) and
-  not loopControlVariable = getAnIterationVariable(forLoop) and
-  loopControlVariableAccess.getEnclosingStmt() = forLoop.getStmt().getAChild*() and
-  // filter only loop control variables that are modified
-  (
-    loopControlVariableAccess.isModified() or
-    loopControlVariableAccess.isAddressOfAccess()
-  ) and
-  // check if the variable type is anything but bool
-  not loopControlVariable.getType() instanceof BoolType
+  exists(Variable loopCounter, ComparisonOperation terminationCheck |
+    loopCounter = getAnIterationVariable(forLoop) and
+    forLoop.getCondition() = terminationCheck.getParent*()
+  |
+    // get a loop control variable that is not a loop counter
+    loopControlVariableAccess = loopControlVariable.getVariableAccessInLoop(forLoop) and
+    not loopControlVariable = getAnIterationVariable(forLoop) and
+    // filter only loop control variables that are modified
+    (
+      loopControlVariableAccess.isModified() or
+      loopControlVariableAccess.isAddressOfAccess()
+    ) and
+    // check if the variable type is anything but bool
+    not loopControlVariable.getType() instanceof BoolType and
+    // check if the control variable is part of the termination check, but is not compared to the loop counter
+    terminationCheck.getAnOperand() = loopControlVariable.getAnAccess().getParent*() and
+    not terminationCheck.getAnOperand() = loopCounter.getAnAccess().getParent*()
+  )
 }
 
-predicate isInvalidLoop(ForStmt forLoop) {
-  isInvalidForLoopIncrementation(forLoop, _) or
-  isForLoopWithMulipleCounters(forLoop) or
-  isForLoopWithFloatingPointCounters(forLoop, _) or
-  isLoopCounterModifiedInCondition(forLoop, _) or
-  isLoopCounterModifiedInStatement(forLoop, _, _) or
-  isIrregularLoopCounterModification(forLoop, _, _) or
-  isLoopControlVarModifiedInLoopExpr(forLoop, _, _) or
-  isNonBoolLoopControlVar(forLoop, _, _)
+predicate isInvalidLoop(ForStmt forLoop) { isInvalidLoop(forLoop, _, _, _) }
+
+predicate isInvalidLoop(ForStmt forLoop, string reason, Locatable reasonLocation, string reasonLabel) {
+  exists(Variable loopCounter |
+    isInvalidForLoopIncrementation(forLoop, loopCounter, reasonLocation) and
+    reason =
+      "it $@ its loop counter '" + loopCounter.getName() +
+        "' with an operation that is not an increment or decrement" and
+    reasonLabel = "updates"
+  )
+  or
+  isForLoopWithMulipleCounters(forLoop) and
+  reason = "it uses multiple loop counters$@" and
+  reasonLabel = "" and
+  reasonLocation.getLocation() instanceof UnknownExprLocation
+  or
+  isForLoopWithFloatingPointCounters(forLoop, reasonLocation) and
+  reason = "it uses a loop counter '$@' of type floating-point" and
+  reasonLabel = reasonLocation.(Variable).getName()
+  or
+  isLoopCounterModifiedInCondition(forLoop, reasonLocation) and
+  reason =
+    "it $@ the loop counter '" + reasonLocation.(VariableAccess).getTarget().getName() +
+      "' in the condition" and
+  reasonLabel = "updates"
+  or
+  exists(Variable loopCounter |
+    isLoopCounterModifiedInStatement(forLoop, loopCounter, reasonLocation) and
+    reason = "it $@ the loop counter '" + loopCounter.getName() + "' in the body of the loop" and
+    reasonLabel = "updates"
+  )
+  or
+  exists(Variable loopCounter |
+    isIrregularLoopCounterModification(forLoop, loopCounter, reasonLocation) and
+    reason = "it $@ the loop counter '" + loopCounter.getName() + "' irregularly" and
+    reasonLabel = "updates"
+  )
+  or
+  exists(Variable loopControlVariable |
+    isLoopControlVarModifiedInLoopExpr(forLoop, loopControlVariable, reasonLocation) and
+    reason =
+      "it updates $@, a loop control variable other than the loop counter, in the update expression of the loop" and
+    reasonLabel = loopControlVariable.getName()
+  )
+  or
+  isNonBoolLoopControlVar(forLoop, reasonLocation, _) and
+  reason = "its $@ is not a boolean" and
+  reasonLabel = "loop control variable"
 }