Skip to content

Commit 708fca4

Browse files
committed
C#: Update ConstantCondition.ql
1 parent 94deed3 commit 708fca4

File tree

3 files changed

+30
-2
lines changed

3 files changed

+30
-2
lines changed

csharp/ql/src/Bad Practices/Control-Flow/ConstantCondition.ql

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,23 @@ class ConstantBooleanCondition extends ConstantCondition {
3535

3636
override predicate isWhiteListed() {
3737
// E.g. `x ?? false`
38-
this.(BoolLiteral) = any(NullCoalescingExpr nce).getRightOperand()
38+
this.(BoolLiteral) = any(NullCoalescingExpr nce).getRightOperand() or
39+
// No need to flag logical operations when the operands are constant
40+
isConstantCondition(this.(LogicalNotExpr).getOperand(), _) or
41+
this =
42+
any(LogicalAndExpr lae |
43+
isConstantCondition(lae.getAnOperand(), false)
44+
or
45+
isConstantCondition(lae.getLeftOperand(), true) and
46+
isConstantCondition(lae.getRightOperand(), true)
47+
) or
48+
this =
49+
any(LogicalOrExpr loe |
50+
isConstantCondition(loe.getAnOperand(), true)
51+
or
52+
isConstantCondition(loe.getLeftOperand(), false) and
53+
isConstantCondition(loe.getRightOperand(), false)
54+
)
3955
}
4056
}
4157

@@ -51,7 +67,8 @@ class ConstantIfCondition extends ConstantBooleanCondition {
5167
or
5268
// It is a common pattern to use a local constant/constant field to control
5369
// whether code parts must be executed or not
54-
this instanceof AssignableRead
70+
this instanceof AssignableRead and
71+
not this instanceof ParameterRead
5572
}
5673
}
5774

csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,15 @@ string M5(object o)
106106
_ => o.ToString() // GOOD
107107
};
108108
}
109+
110+
void M6(bool b1, bool b2) {
111+
if (!b1)
112+
return;
113+
if (!b2)
114+
return;
115+
if (b1 && b2) // BAD
116+
return;
117+
}
109118
}
110119

111120
class Assertions

csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
| ConstantCondition.cs:66:18:66:18 | 3 | Pattern always matches. |
99
| ConstantCondition.cs:77:18:77:20 | access to type Int32 | Pattern never matches. |
1010
| ConstantCondition.cs:97:13:97:13 | _ | Pattern always matches. |
11+
| ConstantCondition.cs:115:13:115:14 | access to parameter b1 | Condition always evaluates to 'true'. |
12+
| ConstantCondition.cs:115:19:115:20 | access to parameter b2 | Condition always evaluates to 'true'. |
1113
| ConstantConditionBad.cs:5:16:5:20 | ... > ... | Condition always evaluates to 'false'. |
1214
| ConstantConditionalExpressionCondition.cs:11:22:11:34 | ... == ... | Condition always evaluates to 'true'. |
1315
| ConstantConditionalExpressionCondition.cs:12:21:12:25 | false | Condition always evaluates to 'false'. |

0 commit comments

Comments
 (0)