Merge pull request #548 from rvermeulen/rvermeulen/fix-152

Fix 152: Adding query for depencence on operator precedence
github · Mar 10, 2024 · 3734236 · 3734236
2 parents b221f4e + a78dd7a
commit 3734236
Show file tree

Hide file tree

Showing 7 changed files with 100 additions and 0 deletions.
diff --git a/.vscode/tasks.json b/.vscode/tasks.json
@@ -271,6 +271,7 @@
                 "Null",
                 "OperatorInvariants",
                 "Operators",
+                "OrderOfEvaluation",
                 "OutOfBounds",
                 "Pointers",
                 "Pointers1",

diff --git a/cpp/autosar/src/rules/M5-0-2/InsufficientUseOfParentheses.ql b/cpp/autosar/src/rules/M5-0-2/InsufficientUseOfParentheses.ql
@@ -0,0 +1,44 @@
+/**
+ * @id cpp/autosar/insufficient-use-of-parentheses
+ * @name M5-0-2: Limited dependence should be placed on C++ operator precedence rules in expressions
+ * @description The use of parentheses can be used to emphasize precedence and increase code
+ *              readability.
+ * @kind problem
+ * @precision medium
+ * @problem.severity recommendation
+ * @tags external/autosar/id/m5-0-2
+ *       external/autosar/audit
+ *       readability
+ *       external/autosar/allocated-target/implementation
+ *       external/autosar/enforcement/partially-automated
+ *       external/autosar/obligation/advisory
+ */
+
+import cpp
+import codingstandards.cpp.autosar
+import semmle.code.cpp.commons.Assertions
+
+class InsufficientlyParenthesizedExpr extends Expr {
+  InsufficientlyParenthesizedExpr() {
+    // Exclude expressions affected by macros, including assertions, because
+    // it is unclear that the expression must be parenthesized since it seems
+    // to be the top-level expression instead of an operand of a binary or ternary operation.
+    not this.isAffectedByMacro() and
+    (
+      exists(BinaryOperation root, BinaryOperation child | child = this |
+        root.getAnOperand() = child and
+        root.getOperator() != child.getOperator() and
+        not any(ParenthesisExpr pe).getExpr() = child
+      )
+      or
+      exists(ConditionalExpr root, BinaryOperation child | child = this |
+        root.getAnOperand() = child and
+        not any(ParenthesisExpr pe).getExpr() = child
+      )
+    )
+  }
+}
+
+from InsufficientlyParenthesizedExpr e
+where not isExcluded(e, OrderOfEvaluationPackage::insufficientUseOfParenthesesQuery())
+select e, "Dependence on operator precedence rules."
diff --git a/cpp/autosar/test/rules/M5-0-2/InsufficientUseOfParentheses.expected b/cpp/autosar/test/rules/M5-0-2/InsufficientUseOfParentheses.expected
@@ -0,0 +1,8 @@
+| test.cpp:40:8:40:13 | ... * ... | Dependence on operator precedence rules. |
+| test.cpp:41:19:41:24 | ... * ... | Dependence on operator precedence rules. |
+| test.cpp:42:8:42:13 | ... * ... | Dependence on operator precedence rules. |
+| test.cpp:42:17:42:22 | ... * ... | Dependence on operator precedence rules. |
+| test.cpp:48:8:48:15 | ... == ... | Dependence on operator precedence rules. |
+| test.cpp:49:26:49:32 | ... - ... | Dependence on operator precedence rules. |
+| test.cpp:50:8:50:15 | ... == ... | Dependence on operator precedence rules. |
+| test.cpp:50:24:50:30 | ... - ... | Dependence on operator precedence rules. |
diff --git a/cpp/autosar/test/rules/M5-0-2/InsufficientUseOfParentheses.qlref b/cpp/autosar/test/rules/M5-0-2/InsufficientUseOfParentheses.qlref
@@ -0,0 +1 @@
+rules/M5-0-2/InsufficientUseOfParentheses.ql
diff --git a/cpp/autosar/test/rules/M5-0-2/test.cpp b/cpp/autosar/test/rules/M5-0-2/test.cpp
@@ -31,4 +31,21 @@ void f1() {
   int **l7;
   l1 = (*l7)[l2];           // NON_COMPLIANT[FALSE_NEGATIVE]
   char l8 = (char)(l1 + 1); // NON_COMPLIANT[FALSE_NEGATIVE]
+}
+
+void test_insufficient_parentheses() {
+  int l1, l2, l3;
+
+  l1 = (2 * l2) + (3 * l3);  // COMPLIANT
+  l1 = 2 * l2 + (3 * l3);    // NON_COMPLIANT
+  l1 = (2 * l2) + 3 * l3;    // NON_COMPLIANT
+  l1 = 2 * l2 + 3 * l3;      // NON_COMPLIANT
+  l1 = (2 * l2) + l3 + 1;    // COMPLIANT
+  l1 = (l2 + 1) - (l2 + l3); // COMPLIANT
+  l1 = l2 + l3 + 1;          // COMPLIANT
+
+  l1 = (l2 == l3) ? l2 : (l2 - l3); // COMPLIANT
+  l1 = l2 == l3 ? l2 : (l2 - l3);   // NON_COMPLIANT
+  l1 = (l2 == l3) ? l2 : l2 - l3;   // NON_COMPLIANT
+  l1 = l2 == l3 ? l2 : l2 - l3;     // NON_COMPLIANT
 }
diff --git a/cpp/common/src/codingstandards/cpp/exclusions/cpp/OrderOfEvaluation.qll b/cpp/common/src/codingstandards/cpp/exclusions/cpp/OrderOfEvaluation.qll
@@ -8,6 +8,7 @@ newtype OrderOfEvaluationQuery =
   TOperandsOfALogicalAndOrNotParenthesizedQuery() or
   TExplicitConstructionOfUnnamedTemporaryQuery() or
   TGratuitousUseOfParenthesesQuery() or
+  TInsufficientUseOfParenthesesQuery() or
   TIncrementAndDecrementOperatorsMixedWithOtherOperatorsInExpressionQuery() or
   TAssignmentInSubExpressionQuery()
 
@@ -50,6 +51,15 @@ predicate isOrderOfEvaluationQueryMetadata(
   ruleId = "M5-0-2" and
   category = "advisory"
   or
+  query =
+    // `Query` instance for the `insufficientUseOfParentheses` query
+    OrderOfEvaluationPackage::insufficientUseOfParenthesesQuery() and
+  queryId =
+    // `@id` for the `insufficientUseOfParentheses` query
+    "cpp/autosar/insufficient-use-of-parentheses" and
+  ruleId = "M5-0-2" and
+  category = "advisory"
+  or
   query =
     // `Query` instance for the `incrementAndDecrementOperatorsMixedWithOtherOperatorsInExpression` query
     OrderOfEvaluationPackage::incrementAndDecrementOperatorsMixedWithOtherOperatorsInExpressionQuery() and
@@ -98,6 +108,13 @@ module OrderOfEvaluationPackage {
       TQueryCPP(TOrderOfEvaluationPackageQuery(TGratuitousUseOfParenthesesQuery()))
   }
 
+  Query insufficientUseOfParenthesesQuery() {
+    //autogenerate `Query` type
+    result =
+      // `Query` type for `insufficientUseOfParentheses` query
+      TQueryCPP(TOrderOfEvaluationPackageQuery(TInsufficientUseOfParenthesesQuery()))
+  }
+
   Query incrementAndDecrementOperatorsMixedWithOtherOperatorsInExpressionQuery() {
     //autogenerate `Query` type
     result =

diff --git a/rule_packages/cpp/OrderOfEvaluation.json b/rule_packages/cpp/OrderOfEvaluation.json
@@ -90,6 +90,18 @@
             "external/autosar/audit",
             "readability"
           ]
+        },
+        {
+          "description": "The use of parentheses can be used to emphasize precedence and increase code readability.",
+          "kind": "problem",
+          "name": "Limited dependence should be placed on C++ operator precedence rules in expressions",
+          "precision": "medium",
+          "severity": "recommendation",
+          "short_name": "InsufficientUseOfParentheses",
+          "tags": [
+            "external/autosar/audit",
+            "readability"
+          ]
         }
       ],
       "title": "Limited dependence should be placed on C++ operator precedence rules in expressions."