C++: New Query `cpp/comma-before-misleading-indentation`

[d10c]

trafficstars

• C++: Initial commit of cpp/comma-before-missing-indentation
• C++: Initial cpp/comma-before-misleading-indentation

[aibaars]

@d10c Could you test this query against git/git to see if there are any false positives. They use the comma operator quite a bit, but mostly in for loop headers.

[MathiasVP]

@d10c Could you test this query against git/git to see if there are any false positives. They use the comma operator quite a bit, but mostly in for loop headers.

git/git is part of the C/C++ DCA suite, so I definitely expect that we'll investigate results on that project before we merge this PR.

[github-actions[bot]]

QHelp previews:

cpp/ql/src/Best Practices/Likely Errors/CommaBeforeMisleadingIndentation.qhelp

Comma before misleading indentation

If the expression after the comma operator starts at an earlier column than the expression before the comma, then this suspicious indentation possibly indicates a logic error, caused by a typo that may escape visual inspection.

WARNING: This query has medium precision because CodeQL currently does not distinguish between tabs and spaces in whitespace. If a file contains mixed tabs and spaces, alerts may highlight code that is correctly indented for one value of tab size but not for other tab sizes.

Recommendation

To ensure that your code is easy to read and review, use standard indentation around the comma operator. Always begin the right-hand-side operand at the same level of indentation (column number) as the left-hand-side operand. This makes it easier for other developers to see the intended behavior of your code.

Use whitespace consistently to communicate your coding intentions. Where possible, avoid mixing tabs and spaces within a file. If you need to mix them, use them consistently.

Example

This example shows three different ways of writing the same code. The first example contains a comma instead of a semicolon which means that the final line is part of the if statement, even though the indentation suggests that it is intended to be separate. The second example looks different but is functionally the same as the first example. It is more likely that the developer intended to write the third example.

/*
 * In this example, the developer intended to use a semicolon but accidentally used a comma:
 */

enum privileges entitlements = NONE;

if (is_admin)
    entitlements = FULL, // BAD

restrict_privileges(entitlements);

/*
 * The use of a comma means that the first example is equivalent to this second example:
 */

enum privileges entitlements = NONE;

if (is_admin) {
    entitlements = FULL;
    restrict_privileges(entitlements);
}

/*
 * The indentation of the first example suggests that the developer probably intended the following code:
 */

enum privileges entitlements = NONE;

if (is_admin)
    entitlements = FULL; // GOOD

restrict_privileges(entitlements);

References

• Wikipedia: Comma operator
• Wikipedia: Indentation style — Tabs, spaces, and size of indentations
• Common Weakness Enumeration: CWE-1078.
• Common Weakness Enumeration: CWE-670.

[d10c]

Thanks for the Doc review @felicitymay! I've made changes based on your suggestions. This PR is now ready for a final review/approval.