diff --git a/csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql b/csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql index 8fea284ae317..100fa2338110 100644 --- a/csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql +++ b/csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql @@ -75,6 +75,23 @@ predicate negatedOperators(string op, string negated) { negatedOperators(negated, op) } +/** Holds if replacing `expr` with `operatorName` could call an enclosing operator. */ +private predicate couldCallEnclosingOperator(LogicalNotExpr expr, string operatorName) { + exists(BinaryOperation binary, Operator enclosingOperator | + binary = expr.getOperand() and + enclosingOperator = expr.getEnclosingCallable().getEnclosingCallable*() and + enclosingOperator.getName() = operatorName and + binary + .getLeftOperand() + .getType() + .isImplicitlyConvertibleTo(enclosingOperator.getParameter(0).getType()) and + binary + .getRightOperand() + .getType() + .isImplicitlyConvertibleTo(enclosingOperator.getParameter(1).getType()) + ) +} + predicate simplifyBinaryExpr(string op, string withFalseOperand, string withTrueOperand) { op = "==" and withTrueOperand = "A" and withFalseOperand = "!A" or @@ -90,7 +107,8 @@ predicate pushNegation(LogicalNotExpr expr, string oldPattern, string newPattern or exists(string oldOperator, string newOperator | oldOperator = expr.getOperand().(BinaryOperation).getOperator() and - negatedOperators(oldOperator, newOperator) + negatedOperators(oldOperator, newOperator) and + not couldCallEnclosingOperator(expr, newOperator) | oldPattern = "!(A " + oldOperator + " B)" and newPattern = "A " + newOperator + " B" diff --git a/csharp/ql/src/change-notes/2026-08-13-simplifiable-operator-recursion.md b/csharp/ql/src/change-notes/2026-08-13-simplifiable-operator-recursion.md new file mode 100644 index 000000000000..177df03a1b55 --- /dev/null +++ b/csharp/ql/src/change-notes/2026-08-13-simplifiable-operator-recursion.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The `cs/simplifiable-boolean-expression` query no longer suggests replacing a negated comparison when the replacement could recursively call an enclosing user-defined operator in `build-mode: none` databases. 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 new file mode 100644 index 000000000000..882ded10f873 --- /dev/null +++ b/csharp/ql/test/query-tests/standalone/Language Abuse/SimplifyBoolExpr/SimplifyBoolExpr.cs @@ -0,0 +1,60 @@ +using System; + +// The missing matching operators deliberately model an incompletely compiled database. +class IncompleteOperatorTest +{ + // GOOD: Rewriting this as `left != right` could recursively call this operator. + public static bool operator !=(IncompleteOperatorTest left, IncompleteOperatorTest right) + { + // BAD: Rewriting this built-in comparison cannot call the enclosing operator. + bool valuesDiffer = !(left.Value == right.Value); // $ Alert + + // BAD: Explicitly converting both operands prevents a call to the enclosing operator. + bool referencesDiffer = !((object)left == (object)right); // $ Alert + + return valuesDiffer && referencesDiffer && !(left == right); + } + + // GOOD: Rewriting this as `left >= right` could recursively call this operator. + public static bool operator >=(IncompleteOperatorTest left, IncompleteOperatorTest right) => !(left < right); + + int Value { get; } +} + +class IncompleteReverseOperatorTest +{ + // GOOD: Rewriting this as `left == right` could recursively call this operator. + public static bool operator ==(IncompleteReverseOperatorTest left, IncompleteReverseOperatorTest right) => !(left != right); + + // GOOD: Rewriting this as `left < right` could recursively call this operator. + public static bool operator <(IncompleteReverseOperatorTest left, IncompleteReverseOperatorTest right) => !(left >= right); +} + +class IncompleteGreaterOperatorTest +{ + // GOOD: Rewriting this as `left <= right` could recursively call this operator. + public static bool operator <=(IncompleteGreaterOperatorTest left, IncompleteGreaterOperatorTest right) => !(left > right); + + // GOOD: Rewriting this as `left > right` could recursively call this operator. + public static bool operator >(IncompleteGreaterOperatorTest left, IncompleteGreaterOperatorTest right) => !(left <= right); +} + +class IncompleteDifferentOperatorTest +{ + // BAD: The suggested operator differs from the enclosing operator. + public static bool operator >(IncompleteDifferentOperatorTest left, IncompleteDifferentOperatorTest right) => !(left == right); // $ Alert +} + +class IncompleteNestedOperatorTest +{ + public static bool operator !=(IncompleteNestedOperatorTest left, IncompleteNestedOperatorTest right) + { + // GOOD: The replacement could call the enclosing operator from this lambda. + Func lambda = () => !(left == right); + + // GOOD: The replacement could call the enclosing operator from this local function. + bool Local() => !(left == right); + + return lambda() || Local(); + } +} 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 new file mode 100644 index 000000000000..a0b97ee97862 --- /dev/null +++ b/csharp/ql/test/query-tests/standalone/Language Abuse/SimplifyBoolExpr/SimplifyBoolExpr.expected @@ -0,0 +1,3 @@ +| 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'. | diff --git a/csharp/ql/test/query-tests/standalone/Language Abuse/SimplifyBoolExpr/SimplifyBoolExpr.qlref b/csharp/ql/test/query-tests/standalone/Language Abuse/SimplifyBoolExpr/SimplifyBoolExpr.qlref new file mode 100644 index 000000000000..222fc236213b --- /dev/null +++ b/csharp/ql/test/query-tests/standalone/Language Abuse/SimplifyBoolExpr/SimplifyBoolExpr.qlref @@ -0,0 +1,2 @@ +query: Language Abuse/SimplifyBoolExpr.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/csharp/ql/test/query-tests/standalone/Language Abuse/SimplifyBoolExpr/options b/csharp/ql/test/query-tests/standalone/Language Abuse/SimplifyBoolExpr/options new file mode 100644 index 000000000000..7ba3811b2afb --- /dev/null +++ b/csharp/ql/test/query-tests/standalone/Language Abuse/SimplifyBoolExpr/options @@ -0,0 +1 @@ +semmle-extractor-options: --standalone