diff --git a/.vscode/tasks.json b/.vscode/tasks.json index 9b53539c04..74f065ac3b 100644 --- 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 new file mode 100644 index 0000000000..1dda0df93f --- /dev/null +++ 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 new file mode 100644 index 0000000000..ef355c7306 --- /dev/null +++ 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 new file mode 100644 index 0000000000..733c035604 --- /dev/null +++ b/cpp/autosar/test/rules/M5-0-2/InsufficientUseOfParentheses.qlref @@ -0,0 +1 @@ +rules/M5-0-2/InsufficientUseOfParentheses.ql \ No newline at end of file diff --git a/cpp/autosar/test/rules/M5-0-2/test.cpp b/cpp/autosar/test/rules/M5-0-2/test.cpp index 06dab1e64c..d028b632f9 100644 --- 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 } \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/exclusions/cpp/OrderOfEvaluation.qll b/cpp/common/src/codingstandards/cpp/exclusions/cpp/OrderOfEvaluation.qll index 2c7da3d64a..9aa62ad377 100644 --- 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 index c471ca8f48..00ec0dbc65 100644 --- 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."