diff --git a/csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql b/csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql index 100fa2338110..5656da73af71 100644 --- a/csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql +++ b/csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql @@ -13,56 +13,88 @@ 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 } -predicate rewriteBinaryExpr(BinaryOperation op, boolean value, string oldPattern) { - literalChild(op, 0, value) and oldPattern = value + " " + op.getOperator() + " A" - or - literalChild(op, 1, value) and oldPattern = "A " + op.getOperator() + " " + value +/** + * 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 } -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 +/** + * 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) { + 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) { - literalChild(cond, 1, false) and oldPattern = "A ? false : B" and newPattern = "!A && B" - or - literalChild(cond, 1, true) and oldPattern = "A ? true : B" and newPattern = "A || B" - or - literalChild(cond, 2, false) and oldPattern = "A ? B : false" and newPattern = "A && B" - or - literalChild(cond, 2, true) and oldPattern = "A ? B : true" and newPattern = "!A || B" - or - exists(boolean b | literalChildren(cond, 1, b, 2, b) | - oldPattern = "A ? " + b + " : " + b and newPattern = b.toString() + cond.getCondition().getType() instanceof BoolType and + 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 - literalChildren(cond, 1, true, 2, false) and oldPattern = "A ? true : false" and newPattern = "A" - or - literalChildren(cond, 1, false, 2, true) and oldPattern = "A ? false : true" and newPattern = "!A" } predicate negatedOperators(string op, string negated) { @@ -115,12 +147,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 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. 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..388b07a5a8d8 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. Can't 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 + } +}