hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-12 02:24:00 +02:00
Code Issues Packages Projects Releases Wiki Activity

C#: Simplify the ConstantCondition query.

Browse Source
This commit is contained in:
Anders Schack-Mulligen
2026-03-27 15:08:21 +01:00
parent 898d12b0be
commit 056be6d504
11 changed files with 51 additions and 120 deletions
Download Patch File Download Diff File Expand all files Collapse all files

113
csharp/ql/src/Bad Practices/Control-Flow/ConstantCondition.ql
View File

@@ -41,7 +41,9 @@ module ConstCondInput implements ConstCond::InputSig<ControlFlow::BasicBlock> {
module ConstCondImpl = ConstCond::Make<Location, Cfg, ConstCondInput>;
predicate nullCheck(Expr e, boolean direct) {
exists(QualifiableExpr qe | qe.isConditional() and qe.getQualifier() = e and direct = true)
exists(QualifiableExpr qe | qe.isConditional() and direct = true |
qe.getQualifier() = e or qe.(ExtensionMethodCall).getArgument(0) = e
)
or
exists(NullCoalescingOperation nce | nce.getLeftOperand() = e and direct = true)
or
@@ -108,57 +110,14 @@ class ConstantGuard extends ConstantCondition {
class ConstantBooleanCondition extends ConstantCondition {
boolean b;
ConstantBooleanCondition() { isConstantCondition(this, b) }
ConstantBooleanCondition() { isConstantComparison(this, b) }
override string getMessage() { result = "Condition always evaluates to '" + b + "'." }
override predicate isWhiteListed() {
// E.g. `x ?? false`
this.(BoolLiteral) = any(NullCoalescingOperation nce).getRightOperand() or
// No need to flag logical operations when the operands are constant
isConstantCondition(this.(LogicalNotExpr).getOperand(), _) or
this =
any(LogicalAndExpr lae |
isConstantCondition(lae.getAnOperand(), false)
or
isConstantCondition(lae.getLeftOperand(), true) and
isConstantCondition(lae.getRightOperand(), true)
) or
this =
any(LogicalOrExpr loe |
isConstantCondition(loe.getAnOperand(), true)
or
isConstantCondition(loe.getLeftOperand(), false) and
isConstantCondition(loe.getRightOperand(), false)
)
}
}
/** A constant condition in an `if` statement or a conditional expression. */
class ConstantIfCondition extends ConstantBooleanCondition {
ConstantIfCondition() {
this = any(IfStmt is).getCondition().getAChildExpr*() or
this = any(ConditionalExpr ce).getCondition().getAChildExpr*()
}
override predicate isWhiteListed() {
ConstantBooleanCondition.super.isWhiteListed()
or
// It is a common pattern to use a local constant/constant field to control
// whether code parts must be executed or not
this instanceof AssignableRead and
not this instanceof ParameterRead
}
}
/** A constant loop condition. */
class ConstantLoopCondition extends ConstantBooleanCondition {
ConstantLoopCondition() { this = any(LoopStmt ls).getCondition() }
override predicate isWhiteListed() {
// Clearly intentional infinite loops are allowed
this.(BoolLiteral).getBoolValue() = true
}
private Expr getQualifier(QualifiableExpr e) {
// `e.getQualifier()` does not work for calls to extension methods
result = e.getChildExpr(-1)
}
/** A constant nullness condition. */
@@ -166,14 +125,23 @@ class ConstantNullnessCondition extends ConstantCondition {
boolean b;
ConstantNullnessCondition() {
forex(ControlFlow::Node cfn | cfn = this.getAControlFlowNode() |
exists(ControlFlow::NullnessSuccessor t, ControlFlow::Node s |
s = cfn.getASuccessorByType(t)
|
b = t.getValue() and
not s.isJoin()
) and
strictcount(ControlFlow::SuccessorType t | exists(cfn.getASuccessorByType(t))) = 1
nullCheck(this, true) and
exists(Expr stripped | stripped = this.(Expr).stripCasts() |
stripped.getType() =
any(ValueType t |
not t instanceof NullableType and
// Extractor bug: the type of `x?.Length` is reported as `int`, but it should
// be `int?`
not getQualifier*(stripped).(QualifiableExpr).isConditional()
) and
b = false
or
stripped instanceof NullLiteral and
b = true
or
stripped.hasValue() and
not stripped instanceof NullLiteral and
b = false
)
}
@@ -184,39 +152,6 @@ class ConstantNullnessCondition extends ConstantCondition {
}
}
/** A constant matching condition. */
class ConstantMatchingCondition extends ConstantCondition {
boolean b;
ConstantMatchingCondition() {
this instanceof Expr and
forex(ControlFlow::Node cfn | cfn = this.getAControlFlowNode() |
exists(ControlFlow::MatchingSuccessor t | exists(cfn.getASuccessorByType(t)) |
b = t.getValue()
) and
strictcount(ControlFlow::SuccessorType t | exists(cfn.getASuccessorByType(t))) = 1
)
}
override predicate isWhiteListed() {
exists(Switch se, Case c, int i |
c = se.getCase(i) and
c.getPattern() = this.(DiscardExpr)
|
i > 0
or
i = 0 and
exists(Expr cond | c.getCondition() = cond and not isConstantCondition(cond, true))
)
or
this = any(PositionalPatternExpr ppe).getPattern(_)
}
override string getMessage() {
if b = true then result = "Pattern always matches." else result = "Pattern never matches."
}
}
from ConstantCondition c, string msg, Guards::Guards::Guard reason, string reasonMsg
where
msg = c.getMessage() and

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

@@ -59,9 +59,9 @@ class ConstantMatching
{
switch (1 + 2)
{
case 2: // $ Alert
case 2: // Intentionally missing Alert
break;
case 3: // $ Alert
case 3: // Intentionally missing Alert
break;
case int _: // GOOD
break;
@@ -72,7 +72,7 @@ class ConstantMatching
{
switch ((object)s)
{
case int _: // $ Alert
case int _: // Intentionally missing Alert
break;
case "": // GOOD
break;
@@ -92,7 +92,7 @@ class ConstantMatching
{
return o switch
{
_ => o.ToString() // $ Alert
_ => o.ToString() // GOOD, catch-all pattern is fine
};
}
@@ -138,7 +138,7 @@ class ConstantMatching
{
switch (i)
{
case var _: // $ Alert
case var _: // GOOD, catch-all pattern is fine
return "even";
}
}

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

@@ -4,28 +4,18 @@
| ConstantCondition.cs:47:17:47:18 | "" | Expression is never 'null'. | ConstantCondition.cs:47:17:47:18 | "" | dummy |
| ConstantCondition.cs:48:13:48:19 | (...) ... | Expression is never 'null'. | ConstantCondition.cs:48:13:48:19 | (...) ... | dummy |
| ConstantCondition.cs:49:13:49:14 | "" | Expression is never 'null'. | ConstantCondition.cs:49:13:49:14 | "" | dummy |
| ConstantCondition.cs:62:18:62:18 | 2 | Pattern never matches. | ConstantCondition.cs:62:18:62:18 | 2 | dummy |
| ConstantCondition.cs:64:18:64:18 | 3 | Pattern always matches. | ConstantCondition.cs:64:18:64:18 | 3 | dummy |
| ConstantCondition.cs:75:18:75:20 | access to type Int32 | Pattern never matches. | ConstantCondition.cs:75:18:75:20 | access to type Int32 | dummy |
| ConstantCondition.cs:95:13:95:13 | _ | Pattern always matches. | ConstantCondition.cs:95:13:95:13 | _ | dummy |
| ConstantCondition.cs:114:13:114:14 | access to parameter b1 | Condition is always true because of $@. | ConstantCondition.cs:110:14:110:15 | access to parameter b1 | access to parameter b1 |
| ConstantCondition.cs:114:19:114:20 | access to parameter b2 | Condition is always true because of $@. | ConstantCondition.cs:112:14:112:15 | access to parameter b2 | access to parameter b2 |
| ConstantCondition.cs:141:22:141:22 | _ | Pattern always matches. | ConstantCondition.cs:141:22:141:22 | _ | dummy |
| ConstantConditionBad.cs:5:16:5:20 | ... > ... | Condition always evaluates to 'false'. | ConstantConditionBad.cs:5:16:5:20 | ... > ... | dummy |
| ConstantConditionalExpressionCondition.cs:11:22:11:34 | ... == ... | Condition always evaluates to 'true'. | ConstantConditionalExpressionCondition.cs:11:22:11:34 | ... == ... | dummy |
| ConstantConditionalExpressionCondition.cs:12:21:12:25 | false | Condition always evaluates to 'false'. | ConstantConditionalExpressionCondition.cs:12:21:12:25 | false | dummy |
| ConstantConditionalExpressionCondition.cs:13:21:13:30 | ... == ... | Condition always evaluates to 'true'. | ConstantConditionalExpressionCondition.cs:13:21:13:30 | ... == ... | dummy |
| ConstantForCondition.cs:9:29:9:33 | false | Condition always evaluates to 'false'. | ConstantForCondition.cs:9:29:9:33 | false | dummy |
| ConstantDoCondition.cs:15:22:15:34 | ... == ... | Condition always evaluates to 'true'. | ConstantDoCondition.cs:15:22:15:34 | ... == ... | dummy |
| ConstantDoCondition.cs:32:22:32:31 | ... == ... | Condition always evaluates to 'true'. | ConstantDoCondition.cs:32:22:32:31 | ... == ... | dummy |
| ConstantForCondition.cs:11:29:11:34 | ... == ... | Condition always evaluates to 'false'. | ConstantForCondition.cs:11:29:11:34 | ... == ... | dummy |
| ConstantIfCondition.cs:11:17:11:29 | ... == ... | Condition always evaluates to 'true'. | ConstantIfCondition.cs:11:17:11:29 | ... == ... | dummy |
| ConstantIfCondition.cs:14:17:14:21 | false | Condition always evaluates to 'false'. | ConstantIfCondition.cs:14:17:14:21 | false | dummy |
| ConstantIfCondition.cs:17:17:17:26 | ... == ... | Condition always evaluates to 'true'. | ConstantIfCondition.cs:17:17:17:26 | ... == ... | dummy |
| ConstantIsNullOrEmpty.cs:10:21:10:54 | call to method IsNullOrEmpty | Condition always evaluates to 'false'. | ConstantIsNullOrEmpty.cs:10:21:10:54 | call to method IsNullOrEmpty | dummy |
| ConstantIsNullOrEmpty.cs:46:21:46:46 | call to method IsNullOrEmpty | Condition always evaluates to 'true'. | ConstantIsNullOrEmpty.cs:46:21:46:46 | call to method IsNullOrEmpty | dummy |
| ConstantIsNullOrEmpty.cs:50:21:50:44 | call to method IsNullOrEmpty | Condition always evaluates to 'true'. | ConstantIsNullOrEmpty.cs:50:21:50:44 | call to method IsNullOrEmpty | dummy |
| ConstantIsNullOrEmpty.cs:54:21:54:45 | call to method IsNullOrEmpty | Condition always evaluates to 'false'. | ConstantIsNullOrEmpty.cs:54:21:54:45 | call to method IsNullOrEmpty | dummy |
| ConstantIfCondition.cs:35:20:35:25 | ... >= ... | Condition always evaluates to 'true'. | ConstantIfCondition.cs:35:20:35:25 | ... >= ... | dummy |
| ConstantNullCoalescingLeftHandOperand.cs:11:24:11:34 | access to constant NULL_OBJECT | Expression is never 'null'. | ConstantNullCoalescingLeftHandOperand.cs:11:24:11:34 | access to constant NULL_OBJECT | dummy |
| ConstantNullCoalescingLeftHandOperand.cs:12:24:12:27 | null | Expression is always 'null'. | ConstantNullCoalescingLeftHandOperand.cs:12:24:12:27 | null | dummy |
| ConstantWhileCondition.cs:12:20:12:32 | ... == ... | Condition always evaluates to 'true'. | ConstantWhileCondition.cs:12:20:12:32 | ... == ... | dummy |
| ConstantWhileCondition.cs:16:20:16:24 | false | Condition always evaluates to 'false'. | ConstantWhileCondition.cs:16:20:16:24 | false | dummy |
| ConstantWhileCondition.cs:24:20:24:29 | ... == ... | Condition always evaluates to 'true'. | ConstantWhileCondition.cs:24:20:24:29 | ... == ... | dummy |

2
csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantConditionalExpressionCondition.cs
View File

@@ -9,7 +9,7 @@ namespace ConstantConditionalExpression
public void Foo()
{
int i = (ZERO == 1 - 1) ? 0 : 1; // $ Alert
int j = false ? 0 : 1; // $ Alert
int j = false ? 0 : 1; // GOOD, literal false is likely intentional
int k = " " == " " ? 0 : 1; // $ Alert
int l = (" "[0] == ' ') ? 0 : 1; // Missing Alert
int m = Bar() == 0 ? 0 : 1; // GOOD

4
csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantDoCondition.cs
View File

@@ -12,7 +12,7 @@ namespace ConstantDoCondition
do
{
break;
} while (ZERO == 1 - 1); // BAD
} while (ZERO == 1 - 1); // $ Alert
do
{
break;
@@ -29,7 +29,7 @@ namespace ConstantDoCondition
do
{
break;
} while (" " == " "); // BAD
} while (" " == " "); // $ Alert
do
{
break;

2
csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantForCondition.cs
View File

@@ -6,7 +6,7 @@ namespace ConstantForCondition
{
public void M()
{
for (int i = 0; false; i++) // $ Alert
for (int i = 0; false; i++) // GOOD, literal false is likely intentional
;
for (int i = 0; 0 == 1; i++) // $ Alert
;

7
csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantIfCondition.cs
View File

@@ -11,7 +11,7 @@ namespace ConstantIfCondition
if (ZERO == 1 - 1) // $ Alert
{
}
if (false) // $ Alert
if (false) // GOOD
{
}
if (" " == " ") // $ Alert
@@ -30,6 +30,11 @@ namespace ConstantIfCondition
return ZERO;
}
public void UnsignedCheck(byte n)
{
while (n >= 0) { n--; } // $ Alert
}
}
}

11
csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantIsNullOrEmpty.cs
View File

@@ -7,7 +7,10 @@ namespace ConstantIsNullOrEmpty
static void Main(string[] args)
{
{
if (string.IsNullOrEmpty(nameof(args))) // $ Alert
// All of the IsNullOrEmpty constant checks have been descoped
// from the query as it didn't seem worth the effort to keep them.
if (string.IsNullOrEmpty(nameof(args))) // Missing Alert (always false)
{
}
@@ -43,15 +46,15 @@ namespace ConstantIsNullOrEmpty
{
}
if (string.IsNullOrEmpty(null)) // $ Alert
if (string.IsNullOrEmpty(null)) // Missing Alert
{
}
if (string.IsNullOrEmpty("")) // $ Alert
if (string.IsNullOrEmpty("")) // Missing Alert
{
}
if (string.IsNullOrEmpty(" ")) // $ Alert
if (string.IsNullOrEmpty(" ")) // Missing Alert
{
}
}

2
csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantWhileCondition.cs
View File

@@ -13,7 +13,7 @@ namespace ConstantWhileCondition
{
break;
}
while (false) // $ Alert
while (false) // Silly, but likely intentional
{
break;
}

2
csharp/ql/test/query-tests/standalone/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.cs
View File

@@ -12,7 +12,7 @@ class ConstantMatching
void M1()
{
var c1 = new C1();
if (c1.Prop is int) // $ Alert
if (c1.Prop is int) // Descoped, no longer reported by the query.
{
}

2
csharp/ql/test/query-tests/standalone/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.expected
View File

@@ -1,2 +0,0 @@
| ConstantCondition.cs:15:13:15:26 | ... is ... | Condition always evaluates to 'false'. | ConstantCondition.cs:15:13:15:26 | ... is ... | dummy |
| ConstantCondition.cs:15:24:15:26 | access to type Int32 | Pattern never matches. | ConstantCondition.cs:15:24:15:26 | access to type Int32 | dummy |