Skip to content

Commit a567709

Browse files
d10cfelicitymaygeoffw0
authored
Apply suggestions from code review
Co-authored-by: Felicity Chapman <[email protected]> Co-authored-by: Geoffrey White <[email protected]>
1 parent ec2549a commit a567709

File tree

3 files changed

+12
-9
lines changed

3 files changed

+12
-9
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Here, the comma should have been a semicolon:
2+
* In this example, the developer intended to use a semicolon but accidentally used a comma:
33
*/
44

55
enum privileges entitlements = NONE;
@@ -10,7 +10,7 @@ if (is_admin)
1010
restrict_privileges(entitlements);
1111

1212
/*
13-
* This is misleading, because the code is unexpectedly equivalent to:
13+
* The use of a comma means that the first example is equivalent to this second example:
1414
*/
1515

1616
enum privileges entitlements = NONE;
@@ -21,7 +21,7 @@ if (is_admin) {
2121
}
2222

2323
/*
24-
* Whereas the following code was probably intended:
24+
* The indentation of the first example suggests that the developer probably intended the following code:
2525
*/
2626

2727
enum privileges entitlements = NONE;

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,26 +5,29 @@
55

66
<overview>
77
<p>
8-
If the expression to the right of a comma operator starts at an earlier column than the expression to the left, then
8+
If the expression after the comma operator starts at an earlier column than the expression before the comma, then
99
this suspicious indentation possibly indicates a logic error, caused by a typo that may escape visual inspection.
1010
</p>
1111
<warning>
1212
This query has medium precision because CodeQL currently does not distinguish between tabs and spaces in whitespace.
13-
Alerts may therefore flag code that appears readable for one value of tab size but not another.
13+
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.
1414
</warning>
1515
</overview>
1616

1717
<recommendation>
1818
<p>
19-
Use standard indentation around the comma operator: begin the right-hand-side operand at the same level of
20-
indentation (column number) as the left-hand-side operand.
19+
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
20+
indentation (column number) as the left-hand-side operand. This makes it easier for other developers to see the intended behavior of your code.
2121
</p>
2222
<p>
23-
When it comes to whitespace, either do not mix tabs and spaces, or mix them consistently.
23+
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.
2424
</p>
2525
</recommendation>
2626

2727
<example>
28+
<p>
29+
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 <code>if</code> statement, even though the indentation suggests that it is intended to be separate. The second example looks different put is functionally the same as the first example. It is more likely that the developer intended to write the third example.
30+
</p>
2831
<sample src="CommaBeforeMisleadingIndentation.cpp" />
2932
</example>
3033

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
* @name Comma before misleading indentation
3-
* @description The expressions before and after the comma operator can be misread because of an unusual difference in start columns.
3+
* @description If expressions before and after a comma operator use different indentation, it is easy to misread the purpose of the code.
44
* @kind problem
55
* @id cpp/comma-before-misleading-indentation
66
* @problem.severity warning

0 commit comments

Comments
 (0)