From 838e1f756bda1fcb438a2762c034d51903c1bec5 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 30 Sep 2026 10:32:35 +0200 Subject: [PATCH 1/7] C#: Add some tests with incomplete type information. --- .../SimplifyBoolExpr/SimplifyBoolExpr.cs | 3 +++ .../SimplifyBoolExpr/SimplifyBoolExpr.cs | 22 +++++++++++++++++++ .../SimplifyBoolExpr.expected | 6 +++++ 3 files changed, 31 insertions(+) diff --git a/csharp/ql/test/query-tests/Language Abuse/SimplifyBoolExpr/SimplifyBoolExpr.cs b/csharp/ql/test/query-tests/Language Abuse/SimplifyBoolExpr/SimplifyBoolExpr.cs index 175507010a13..68a875383e72 100644 --- a/csharp/ql/test/query-tests/Language Abuse/SimplifyBoolExpr/SimplifyBoolExpr.cs +++ b/csharp/ql/test/query-tests/Language Abuse/SimplifyBoolExpr/SimplifyBoolExpr.cs @@ -44,5 +44,8 @@ void Fn() if (true != false) ; if (true && true) ; if (true || false) ; + + bool? boption = false; + if (boption == true) ; // GOOD. Cant be simplified like a regular bool } } diff --git a/csharp/ql/test/query-tests/standalone/Language Abuse/SimplifyBoolExpr/SimplifyBoolExpr.cs b/csharp/ql/test/query-tests/standalone/Language Abuse/SimplifyBoolExpr/SimplifyBoolExpr.cs index 882ded10f873..c9ce38772a7c 100644 --- a/csharp/ql/test/query-tests/standalone/Language Abuse/SimplifyBoolExpr/SimplifyBoolExpr.cs +++ b/csharp/ql/test/query-tests/standalone/Language Abuse/SimplifyBoolExpr/SimplifyBoolExpr.cs @@ -58,3 +58,25 @@ class IncompleteNestedOperatorTest return lambda() || Local(); } } + +class Test +{ + public class Container + { + public bool? Field; + } + + public void Fn() + { + bool? boption = false; + if (boption == true) ; // GOOD. Can't be simplified like a regular bool + + bool b = false; + if (b ? boption : false) ; // GOOD. Can't be simplified like a regular bool + + // Emulating incomplete type information by not declaring container explicitly. + if (container.Field == true) ; // GOOD. Can't be simplified like a regular bool + + if (b ? container.Field : false) ; // GOOD. Can't be simplified like a regular bool + } +} diff --git a/csharp/ql/test/query-tests/standalone/Language Abuse/SimplifyBoolExpr/SimplifyBoolExpr.expected b/csharp/ql/test/query-tests/standalone/Language Abuse/SimplifyBoolExpr/SimplifyBoolExpr.expected index a0b97ee97862..e1d1dba56bc7 100644 --- a/csharp/ql/test/query-tests/standalone/Language Abuse/SimplifyBoolExpr/SimplifyBoolExpr.expected +++ b/csharp/ql/test/query-tests/standalone/Language Abuse/SimplifyBoolExpr/SimplifyBoolExpr.expected @@ -1,3 +1,9 @@ +#select | SimplifyBoolExpr.cs:10:29:10:56 | !... | The expression '!(A == B)' can be simplified to 'A != B'. | | SimplifyBoolExpr.cs:13:33:13:64 | !... | The expression '!(A == B)' can be simplified to 'A != B'. | | SimplifyBoolExpr.cs:45:115:45:130 | !... | The expression '!(A == B)' can be simplified to 'A != B'. | +| SimplifyBoolExpr.cs:78:13:78:35 | ... == ... | The expression 'A == true' can be simplified to 'A'. | +| SimplifyBoolExpr.cs:80:13:80:39 | ... ? ... : ... | The expression 'A ? B : false' can be simplified to 'A && B'. | +testFailures +| SimplifyBoolExpr.cs:78:13:78:35 | The expression 'A == true' can be simplified to 'A'. | Unexpected result: Alert | +| SimplifyBoolExpr.cs:80:13:80:39 | The expression 'A ? B : false' can be simplified to 'A && B'. | Unexpected result: Alert | From 964adaa448f79e2ecd54768be95a40a44093eec8 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 30 Sep 2026 09:34:28 +0200 Subject: [PATCH 2/7] C#: Simplify to avoid binding sets. --- .../ql/src/Language Abuse/SimplifyBoolExpr.ql | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql b/csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql index 100fa2338110..1bbf3e7cb969 100644 --- a/csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql +++ b/csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql @@ -37,16 +37,6 @@ predicate rewriteBinaryExpr(BinaryOperation op, boolean value, string oldPattern literalChild(op, 1, value) and oldPattern = "A " + op.getOperator() + " " + value } -bindingset[withFalseOperand, withTrueOperand] -predicate rewriteBinaryExpr( - BinaryOperation op, string oldPattern, string withFalseOperand, string withTrueOperand, - string newPattern -) { - rewriteBinaryExpr(op, false, oldPattern) and newPattern = withFalseOperand - or - rewriteBinaryExpr(op, true, oldPattern) and newPattern = withTrueOperand -} - predicate rewriteConditionalExpr(ConditionalExpr cond, string oldPattern, string newPattern) { literalChild(cond, 1, false) and oldPattern = "A ? false : B" and newPattern = "!A && B" or @@ -115,12 +105,20 @@ predicate pushNegation(LogicalNotExpr expr, string oldPattern, string newPattern ) } -predicate rewrite(Expr expr, string oldPattern, string newPattern) { +predicate rewriteBinaryOperation(BinaryOperation op, string oldPattern, string newPattern) { exists(string withFalseOperand, string withTrueOperand | - simplifyBinaryExpr(expr.(BinaryOperation).getOperator(), withFalseOperand, withTrueOperand) + simplifyBinaryExpr(op.getOperator(), withFalseOperand, withTrueOperand) | - rewriteBinaryExpr(expr, oldPattern, withFalseOperand, withTrueOperand, newPattern) + rewriteBinaryExpr(op, false, oldPattern) and + newPattern = withFalseOperand + or + rewriteBinaryExpr(op, true, oldPattern) and + newPattern = withTrueOperand ) +} + +predicate rewrite(Expr expr, string oldPattern, string newPattern) { + rewriteBinaryOperation(expr, oldPattern, newPattern) or rewriteConditionalExpr(expr, oldPattern, newPattern) or From fd116f237804c35cbda6e531fae8fd9490f0527f Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 30 Sep 2026 10:02:39 +0200 Subject: [PATCH 3/7] C#: Re-factor the implementation to use the designated predicates for left/right and branches instead of relying on magic constants. --- .../ql/src/Language Abuse/SimplifyBoolExpr.ql | 73 ++++++++++++++----- 1 file changed, 53 insertions(+), 20 deletions(-) diff --git a/csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql b/csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql index 1bbf3e7cb969..f24426158e3c 100644 --- a/csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql +++ b/csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql @@ -13,46 +13,79 @@ import csharp /** - * Holds if expression `expr` has Boolean `value` at child `child`. - * No other child nodes are boolean literals. + * Holds if the left operand of a binary operation is a Boolean literal with the specified value + * and the right operand is not a Boolean literal. */ -predicate literalChild(Expr expr, int child, boolean value) { - value = expr.getChild(child).(BoolLiteral).getBoolValue() and - forall(int c | c != child | not expr.getChild(c) instanceof BoolLiteral) +predicate binaryLiteralLeft(BinaryOperation op, boolean value) { + value = op.getLeftOperand().(BoolLiteral).getBoolValue() and + not op.getRightOperand() instanceof BoolLiteral } /** - * Expression `expr` has Boolean `value1` at child `child1`, and boolean `value2` at `child2`. - * No other child nodes are boolean literals. + * Holds if the right operand of a binary operation is a Boolean literal with the specified value + * and the left operand is not a Boolean literal. */ -predicate literalChildren(Expr expr, int child1, boolean value1, int child2, boolean value2) { - value1 = expr.getChild(child1).(BoolLiteral).getBoolValue() and - value2 = expr.getChild(child2).(BoolLiteral).getBoolValue() and - forall(int c | c != child1 and c != child2 | not expr.getChild(c) instanceof BoolLiteral) +predicate binaryLiteralRight(BinaryOperation op, boolean value) { + value = op.getRightOperand().(BoolLiteral).getBoolValue() and + not op.getLeftOperand() instanceof BoolLiteral +} + +/** + * Holds if the 'then' branch of a conditional expression is a Boolean literal with the specified value + * and the 'condition' or 'else' branch are not Boolean literals. + */ +predicate conditionalThenLiteral(ConditionalExpr cond, boolean value) { + value = cond.getThen().(BoolLiteral).getBoolValue() and + not cond.getCondition() instanceof BoolLiteral and + not cond.getElse() instanceof BoolLiteral +} + +/** + * Holds if the 'else' branch of a conditional expression is a Boolean literal with the specified value + * and the 'condition' or 'then' branch are not Boolean literals. + */ +predicate conditionalElseLiteral(ConditionalExpr cond, boolean value) { + value = cond.getElse().(BoolLiteral).getBoolValue() and + not cond.getCondition() instanceof BoolLiteral and + not cond.getThen() instanceof BoolLiteral +} + +/** + * Holds if both the 'then' and 'else' branches of a conditional expression are Boolean literals with the specified values + * and the 'condition' branch is not a Boolean literal. + */ +predicate conditionalThenAndElseLiteral(ConditionalExpr cond, boolean thenValue, boolean elseValue) { + thenValue = cond.getThen().(BoolLiteral).getBoolValue() and + elseValue = cond.getElse().(BoolLiteral).getBoolValue() and + not cond.getCondition() instanceof BoolLiteral } predicate rewriteBinaryExpr(BinaryOperation op, boolean value, string oldPattern) { - literalChild(op, 0, value) and oldPattern = value + " " + op.getOperator() + " A" + binaryLiteralLeft(op, value) and oldPattern = value + " " + op.getOperator() + " A" or - literalChild(op, 1, value) and oldPattern = "A " + op.getOperator() + " " + value + binaryLiteralRight(op, value) and oldPattern = "A " + op.getOperator() + " " + value } predicate rewriteConditionalExpr(ConditionalExpr cond, string oldPattern, string newPattern) { - literalChild(cond, 1, false) and oldPattern = "A ? false : B" and newPattern = "!A && B" + conditionalThenLiteral(cond, false) and oldPattern = "A ? false : B" and newPattern = "!A && B" or - literalChild(cond, 1, true) and oldPattern = "A ? true : B" and newPattern = "A || B" + conditionalThenLiteral(cond, true) and oldPattern = "A ? true : B" and newPattern = "A || B" or - literalChild(cond, 2, false) and oldPattern = "A ? B : false" and newPattern = "A && B" + conditionalElseLiteral(cond, false) and oldPattern = "A ? B : false" and newPattern = "A && B" or - literalChild(cond, 2, true) and oldPattern = "A ? B : true" and newPattern = "!A || B" + conditionalElseLiteral(cond, true) and oldPattern = "A ? B : true" and newPattern = "!A || B" or - exists(boolean b | literalChildren(cond, 1, b, 2, b) | + exists(boolean b | conditionalThenAndElseLiteral(cond, b, b) | oldPattern = "A ? " + b + " : " + b and newPattern = b.toString() ) or - literalChildren(cond, 1, true, 2, false) and oldPattern = "A ? true : false" and newPattern = "A" + conditionalThenAndElseLiteral(cond, true, false) and + oldPattern = "A ? true : false" and + newPattern = "A" or - literalChildren(cond, 1, false, 2, true) and oldPattern = "A ? false : true" and newPattern = "!A" + conditionalThenAndElseLiteral(cond, false, true) and + oldPattern = "A ? false : true" and + newPattern = "!A" } predicate negatedOperators(string op, string negated) { From cbf1c4ff5e3ce192eec5d995006f4d5036aba83f Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 30 Sep 2026 10:43:57 +0200 Subject: [PATCH 4/7] C#: Check that both operands are booleans when suggesting re-writes for equality, logical and conditional operations. --- .../ql/src/Language Abuse/SimplifyBoolExpr.ql | 50 +++++++++++-------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql b/csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql index f24426158e3c..c49918ccbff0 100644 --- a/csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql +++ b/csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql @@ -61,31 +61,39 @@ predicate conditionalThenAndElseLiteral(ConditionalExpr cond, boolean thenValue, } predicate rewriteBinaryExpr(BinaryOperation op, boolean value, string oldPattern) { - binaryLiteralLeft(op, value) and oldPattern = value + " " + op.getOperator() + " A" - or - binaryLiteralRight(op, value) and oldPattern = "A " + op.getOperator() + " " + value + op.getLeftOperand().getType() instanceof BoolType and + op.getRightOperand().getType() instanceof BoolType and + ( + binaryLiteralLeft(op, value) and oldPattern = value + " " + op.getOperator() + " A" + or + binaryLiteralRight(op, value) and oldPattern = "A " + op.getOperator() + " " + value + ) } predicate rewriteConditionalExpr(ConditionalExpr cond, string oldPattern, string newPattern) { - conditionalThenLiteral(cond, false) and oldPattern = "A ? false : B" and newPattern = "!A && B" - or - conditionalThenLiteral(cond, true) and oldPattern = "A ? true : B" and newPattern = "A || B" - or - conditionalElseLiteral(cond, false) and oldPattern = "A ? B : false" and newPattern = "A && B" - or - conditionalElseLiteral(cond, true) and oldPattern = "A ? B : true" and newPattern = "!A || B" - or - exists(boolean b | conditionalThenAndElseLiteral(cond, b, b) | - oldPattern = "A ? " + b + " : " + b and newPattern = b.toString() + cond.getThen().getType() instanceof BoolType and + cond.getElse().getType() instanceof BoolType and + ( + conditionalThenLiteral(cond, false) and oldPattern = "A ? false : B" and newPattern = "!A && B" + or + conditionalThenLiteral(cond, true) and oldPattern = "A ? true : B" and newPattern = "A || B" + or + conditionalElseLiteral(cond, false) and oldPattern = "A ? B : false" and newPattern = "A && B" + or + conditionalElseLiteral(cond, true) and oldPattern = "A ? B : true" and newPattern = "!A || B" + or + exists(boolean b | conditionalThenAndElseLiteral(cond, b, b) | + oldPattern = "A ? " + b + " : " + b and newPattern = b.toString() + ) + or + conditionalThenAndElseLiteral(cond, true, false) and + oldPattern = "A ? true : false" and + newPattern = "A" + or + conditionalThenAndElseLiteral(cond, false, true) and + oldPattern = "A ? false : true" and + newPattern = "!A" ) - or - conditionalThenAndElseLiteral(cond, true, false) and - oldPattern = "A ? true : false" and - newPattern = "A" - or - conditionalThenAndElseLiteral(cond, false, true) and - oldPattern = "A ? false : true" and - newPattern = "!A" } predicate negatedOperators(string op, string negated) { From 9ab1f347196006d8940b4cc2ec80f9af97fb20b1 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 30 Sep 2026 10:46:10 +0200 Subject: [PATCH 5/7] C#: Update test expected output. --- .../SimplifyBoolExpr/SimplifyBoolExpr.expected | 6 ------ 1 file changed, 6 deletions(-) diff --git a/csharp/ql/test/query-tests/standalone/Language Abuse/SimplifyBoolExpr/SimplifyBoolExpr.expected b/csharp/ql/test/query-tests/standalone/Language Abuse/SimplifyBoolExpr/SimplifyBoolExpr.expected index e1d1dba56bc7..a0b97ee97862 100644 --- a/csharp/ql/test/query-tests/standalone/Language Abuse/SimplifyBoolExpr/SimplifyBoolExpr.expected +++ b/csharp/ql/test/query-tests/standalone/Language Abuse/SimplifyBoolExpr/SimplifyBoolExpr.expected @@ -1,9 +1,3 @@ -#select | SimplifyBoolExpr.cs:10:29:10:56 | !... | The expression '!(A == B)' can be simplified to 'A != B'. | | SimplifyBoolExpr.cs:13:33:13:64 | !... | The expression '!(A == B)' can be simplified to 'A != B'. | | SimplifyBoolExpr.cs:45:115:45:130 | !... | The expression '!(A == B)' can be simplified to 'A != B'. | -| SimplifyBoolExpr.cs:78:13:78:35 | ... == ... | The expression 'A == true' can be simplified to 'A'. | -| SimplifyBoolExpr.cs:80:13:80:39 | ... ? ... : ... | The expression 'A ? B : false' can be simplified to 'A && B'. | -testFailures -| SimplifyBoolExpr.cs:78:13:78:35 | The expression 'A == true' can be simplified to 'A'. | Unexpected result: Alert | -| SimplifyBoolExpr.cs:80:13:80:39 | The expression 'A ? B : false' can be simplified to 'A && B'. | Unexpected result: Alert | From 9bf56c526c540ce08271eb00fb57418435612af3 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 30 Sep 2026 10:50:40 +0200 Subject: [PATCH 6/7] C#: Add change-note. --- csharp/ql/src/change-notes/2026-09-30-simplify-bool-expr.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 csharp/ql/src/change-notes/2026-09-30-simplify-bool-expr.md diff --git a/csharp/ql/src/change-notes/2026-09-30-simplify-bool-expr.md b/csharp/ql/src/change-notes/2026-09-30-simplify-bool-expr.md new file mode 100644 index 000000000000..ae69a2c8197f --- /dev/null +++ b/csharp/ql/src/change-notes/2026-09-30-simplify-bool-expr.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The `cs/simplifiable-boolean-expression` query now produces fewer false-positive results when type information is incomplete. From 8ebc37c992f2360023d635612aaa22f63da74154 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 30 Sep 2026 11:59:00 +0200 Subject: [PATCH 7/7] C#: Address copilots review comments. --- csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql | 1 + .../Language Abuse/SimplifyBoolExpr/SimplifyBoolExpr.cs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql b/csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql index c49918ccbff0..5656da73af71 100644 --- a/csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql +++ b/csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql @@ -71,6 +71,7 @@ predicate rewriteBinaryExpr(BinaryOperation op, boolean value, string oldPattern } predicate rewriteConditionalExpr(ConditionalExpr cond, string oldPattern, string newPattern) { + cond.getCondition().getType() instanceof BoolType and cond.getThen().getType() instanceof BoolType and cond.getElse().getType() instanceof BoolType and ( diff --git a/csharp/ql/test/query-tests/Language Abuse/SimplifyBoolExpr/SimplifyBoolExpr.cs b/csharp/ql/test/query-tests/Language Abuse/SimplifyBoolExpr/SimplifyBoolExpr.cs index 68a875383e72..388b07a5a8d8 100644 --- a/csharp/ql/test/query-tests/Language Abuse/SimplifyBoolExpr/SimplifyBoolExpr.cs +++ b/csharp/ql/test/query-tests/Language Abuse/SimplifyBoolExpr/SimplifyBoolExpr.cs @@ -46,6 +46,6 @@ void Fn() if (true || false) ; bool? boption = false; - if (boption == true) ; // GOOD. Cant be simplified like a regular bool + if (boption == true) ; // GOOD. Can't be simplified like a regular bool } }