Commit 708fca4
File tree
- csharp/ql
- src/Bad Practices/Control-Flow
- test/query-tests/Bad Practices/Control-Flow/ConstantCondition
Lines changed: 19 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -35,7 +35,23 @@ class ConstantBooleanCondition extends ConstantCondition { | |||
| 35 | 35 | ||
| 36 | 36 | override predicate isWhiteListed() { | |
| 37 | 37 | // 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 | + ) | ||
| 39 | 55 | } | |
| 40 | 56 | } | |
| 41 | 57 | ||
@@ -51,7 +67,8 @@ class ConstantIfCondition extends ConstantBooleanCondition { | |||
| 51 | 67 | or | |
| 52 | 68 | // It is a common pattern to use a local constant/constant field to control | |
| 53 | 69 | // whether code parts must be executed or not | |
| 54 | - this instanceof AssignableRead | ||
| 70 | + this instanceof AssignableRead and | ||
| 71 | + not this instanceof ParameterRead | ||
| 55 | 72 | } | |
| 56 | 73 | } | |
| 57 | 74 | ||
Lines changed: 9 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -106,6 +106,15 @@ string M5(object o) | |||
| 106 | 106 | _ => o.ToString() // GOOD | |
| 107 | 107 | }; | |
| 108 | 108 | } | |
| 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 | + } | ||
| 109 | 118 | } | |
| 110 | 119 | ||
| 111 | 120 | class Assertions | |
csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.expected
Lines changed: 2 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -8,6 +8,8 @@ | |||
| 8 | 8 | | ConstantCondition.cs:66:18:66:18 | 3 | Pattern always matches. | | |
| 9 | 9 | | ConstantCondition.cs:77:18:77:20 | access to type Int32 | Pattern never matches. | | |
| 10 | 10 | | 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'. | | ||
| 11 | 13 | | ConstantConditionBad.cs:5:16:5:20 | ... > ... | Condition always evaluates to 'false'. | | |
| 12 | 14 | | ConstantConditionalExpressionCondition.cs:11:22:11:34 | ... == ... | Condition always evaluates to 'true'. | | |
| 13 | 15 | | ConstantConditionalExpressionCondition.cs:12:21:12:25 | false | Condition always evaluates to 'false'. | | |
0 commit comments