codeql-coding-standards
codeql-coding-standards copied to clipboard
`A5-2-6`: Exclude cases with the same binary operator
Affected rules
-
A5-2-6
Description
~~It's not well defined by the standard, but I think it's reasonable to exclude ->
and .
when considering whether an operand of a logical operator is binary.~~
Update: We thought the query was incorrectly reporting ->
and .
as binary operators. However, the problem was a confusion over this example:
foo->bar() && foo->baz() && foo->bang()
Where the query is reporting that this should be bracketed like so:
(foo->bar() && foo->baz()) && foo->bang()
This is a grey area in the rule itself - technically the title implies we should report this case. However, the rationale states:
Parentheses are required to add clarity in logical expressions making code easier to review versus code based only C++ operator precedence rules.
Which I think argues against asking for the developers to add unnecessary brackets in this case.
Proposed next steps:
- Exclude cases where the nested binary operator is the same as the parent binary operator.
- Refine the wording of the alert to ensure that it's clear where the brackets should be applied.
Example
foo->bar() && foo->baz() && foo->bang()
Reviewing the query, the description above is not accurate. The query already only reports BinaryOperation
s, which does not include FieldAccess
es (such as ->
and .
). I have investigated whether these could be caused by the use of operator->
, either independently or in conjunction with templates but cannot reproduce the problem.
Issue body updated: the problem has been identified as related to (a) confusion caused by the alert message about where the brackets should apply and (b) an over-zealous rule.
I would like to report a similar over-zealous case when using the same comparison operator:
(a == 0) && (b == 0) // True negative, OK
(a == 0) && (b == 0) && ( c == 0) // Triggers A5-2-6 warning (zealous)
((a == 0) && (b == 0)) && ( c == 0) // Needs to do this to avoid A5-2-6 warning, which alters readability of code
AUTOSAR C++14 is not really clear for this specific case.