Skip to content

A5-2-6: Exclude cases with the same binary operator #231

Closed
@lcartey

Description

@lcartey

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()

Metadata

Metadata

Assignees

Labels

Difficulty-LowA false positive or false negative report which is expected to take <1 day effort to addressImpact-Highfalse positive/false negativeAn issue related to observed false positives or false negatives.top-25-fpsuser-reportIssue reported by an end user of CodeQL Coding Standards

Type

No type

Projects

Status

Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions