Skip to content

Fix 152: Adding query for depencence on operator precedence #548

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .vscode/tasks.json
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@
"Null",
"OperatorInvariants",
"Operators",
"OrderOfEvaluation",
"OutOfBounds",
"Pointers",
"Pointers1",
Expand Down
44 changes: 44 additions & 0 deletions cpp/autosar/src/rules/M5-0-2/InsufficientUseOfParentheses.ql
Original file line number Diff line number Diff line change
@@ -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."
Original file line number Diff line number Diff line change
@@ -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. |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
rules/M5-0-2/InsufficientUseOfParentheses.ql
17 changes: 17 additions & 0 deletions cpp/autosar/test/rules/M5-0-2/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ newtype OrderOfEvaluationQuery =
TOperandsOfALogicalAndOrNotParenthesizedQuery() or
TExplicitConstructionOfUnnamedTemporaryQuery() or
TGratuitousUseOfParenthesesQuery() or
TInsufficientUseOfParenthesesQuery() or
TIncrementAndDecrementOperatorsMixedWithOtherOperatorsInExpressionQuery() or
TAssignmentInSubExpressionQuery()

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 =
Expand Down
12 changes: 12 additions & 0 deletions rule_packages/cpp/OrderOfEvaluation.json
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down