Decouple ForStmt from Increment.qll and rewrite getLoopStepOfForStmt

Author: jeongsoolee09

cpp/common/src/codingstandards/cpp/ast/Increment.qll

@@ -1,57 +1,76 @@
-import cpp
-
-abstract class LegacyForLoopUpdateExpression extends Expr {
-  ForStmt forLoop;
-
-  LegacyForLoopUpdateExpression() { this = forLoop.getUpdate().getAChild*() }
-
-  abstract Expr getLoopStep();
-  /* TODO: Complete below and use it for 3-2 */
-  // abstract VariableAccess getUpdatedVariable();
-}
+/**
+ * Provides a library for working with expressions that update the value
+ * of a numeric variable by incrementing or decrementing it by a certain
+ * amount.
+ */
 
-class CrementLegacyForLoopUpdateExpression extends LegacyForLoopUpdateExpression {
-  CrementLegacyForLoopUpdateExpression() { this instanceof CrementOperation }
-
-  override Expr getLoopStep() { none() }
-}
+import cpp
 
-class AssignAddOrSubExpr extends LegacyForLoopUpdateExpression {
+private class AssignAddOrSubExpr extends AssignArithmeticOperation {
   AssignAddOrSubExpr() {
     this instanceof AssignAddExpr or
     this instanceof AssignSubExpr
   }
+}
 
-  override Expr getLoopStep() {
-    result = this.(AssignAddExpr).getRValue() or
-    result = this.(AssignSubExpr).getRValue()
+private class AddOrSubExpr extends BinaryArithmeticOperation {
+  AddOrSubExpr() {
+    this instanceof AddExpr or
+    this instanceof SubExpr
   }
 }
 
-class AddOrSubThenAssignExpr extends LegacyForLoopUpdateExpression {
-  Expr assignRhs;
+/**
+ * An expression that updates a numeric variable by adding to or subtracting
+ * from it a certain amount.
+ */
+abstract class StepCrementUpdateExpr extends Expr {
+  /**
+   * The expression in the abstract syntax tree that represents the amount of
+   * value by which the variable is updated.
+   */
+  abstract Expr getAmountExpr();
+}
+
+/**
+ * An increment or decrement operator application, either postfix or prefix.
+ */
+class PostfixOrPrefixCrementExpr extends CrementOperation, StepCrementUpdateExpr {
+  override Expr getAmountExpr() { none() }
+}
+
+/**
+ * An add-then-assign or subtract-then-assign expression in a shortened form,
+ * i.e. `+=` or `-=`.
+ */
+class AssignAddOrSubUpdateExpr extends AssignAddOrSubExpr, StepCrementUpdateExpr {
+  override Expr getAmountExpr() { result = this.getRValue() }
+}
+
+/**
+ * An add-then-assign expression or a subtract-then-assign expression, i.e.
+ * `x = x + E` or `x = x - E`, where `x` is some variable and `E` an
+ * arbitrary expression.
+ */
+class AddOrSubThenAssignExpr extends AssignExpr, StepCrementUpdateExpr {
+  /** The `x` as in the left-hand side of `x = x + E`. */
+  VariableAccess lvalueVariable;
+  /** The `x + E` as in `x = x + E`. */
+  AddOrSubExpr addOrSubExpr;
+  /** The `E` as in `x = x + E`. */
+  Expr amountExpr;
 
   AddOrSubThenAssignExpr() {
-    this.(AssignExpr).getRValue() = assignRhs and
-    (
-      assignRhs instanceof AddExpr or
-      assignRhs instanceof SubExpr
+    this.getLValue() = lvalueVariable and
+    this.getRValue() = addOrSubExpr and
+    exists(VariableAccess lvalueVariableAsRvalue |
+      lvalueVariableAsRvalue = addOrSubExpr.getAnOperand() and
+      amountExpr = addOrSubExpr.getAnOperand() and
+      lvalueVariableAsRvalue != amountExpr
+    |
+      lvalueVariable.getTarget() = lvalueVariableAsRvalue.(VariableAccess).getTarget()
     )
   }
 
-  override Expr getLoopStep() {
-    (
-      result = assignRhs.(AddExpr).getAnOperand() or
-      result = assignRhs.(SubExpr).getAnOperand()
-    ) and
-    exists(VariableAccess iterationVariableAccess |
-      (
-        iterationVariableAccess = assignRhs.(AddExpr).getAnOperand()
-        or
-        iterationVariableAccess = assignRhs.(SubExpr).getAnOperand()
-      ) and
-      iterationVariableAccess.getTarget() = forLoop.getAnIterationVariable() and
-      result != iterationVariableAccess
-    )
-  }
-}
+  override Expr getAmountExpr() { result = amountExpr }
+}

cpp/misra/src/rules/RULE-9-5-1/LegacyForStatementsShouldBeSimple.ql

@@ -66,23 +66,7 @@ Expr getLoopStepOfForStmt(ForStmt forLoop) {
    */
 
   /* 1. Get the expression `E` when the update expression is `i += E` or `i -= E`. */
-  result = forLoop.getUpdate().getAChild*().(AssignAddOrSubExpr).getLoopStep()
-  or
-  /* 2. Get the expression `E` when the update expression is `i = i + E` or `i = i - E`. */
-  (
-    result = forLoop.getUpdate().getAChild*().(AssignExpr).getRValue().(AddExpr).getAnOperand() or
-    result = forLoop.getUpdate().getAChild*().(AssignExpr).getRValue().(SubExpr).getAnOperand()
-  ) and
-  exists(VariableAccess iterationVariableAccess |
-    (
-      iterationVariableAccess =
-        forLoop.getUpdate().getAChild*().(AssignExpr).getRValue().(AddExpr).getAnOperand() or
-      iterationVariableAccess =
-        forLoop.getUpdate().getAChild*().(AssignExpr).getRValue().(SubExpr).getAnOperand()
-    ) and
-    iterationVariableAccess.getTarget() = getDeclaredVariableInForLoop(forLoop) and
-    result != iterationVariableAccess
-  )
+  result = forLoop.getUpdate().getAChild*().(StepCrementUpdateExpr).getAmountExpr()
 }
 
 /**
@@ -194,7 +178,7 @@ private newtype TAlertType =
   ) {
     loopCounterVariable = getDeclaredVariableInForLoop(forLoop) and
     variableModifiedInExpression(updateExpr, loopCounterVariable.getAnAccess()) and
-    not updateExpr instanceof LegacyForLoopUpdateExpression
+    not updateExpr instanceof StepCrementUpdateExpr
   } or
   /* 4. The type size of the loop counter is smaller than that of the loop bound. */
   TLoopCounterSmallerThanLoopBound(ForStmt forLoop, LegacyForLoopCondition forLoopCondition) {