Merge pull request #22345 from theinfosecguy/csharp-simplifiable-operator-recursion

C#: Avoid recursive operator simplification

Author: michaelnebel

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"

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.

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<bool> lambda = () => !(left == right);
+
+        // GOOD: The replacement could call the enclosing operator from this local function.
+        bool Local() => !(left == right);
+
+        return lambda() || Local();
+    }
+}

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'. |

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

csharp/ql/test/query-tests/standalone/Language Abuse/SimplifyBoolExpr/options

@@ -0,0 +1 @@
+semmle-extractor-options: --standalone