From 2d78db0c9b269b27430f9a557dbee1d74f6854a2 Mon Sep 17 00:00:00 2001
From: Anders Schack-Mulligen <aschackmull@github.com>
Date: Thu, 5 Feb 2026 10:47:11 +0100
Subject: [PATCH] C#: Make a more principled fix in ConstantCondition.

---
 .../Control-Flow/ConstantCondition.ql            |  5 -----
 .../controlflow/queries/ConstantCondition.qll    | 16 +++++++++++++---
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/csharp/ql/src/Bad Practices/Control-Flow/ConstantCondition.ql b/csharp/ql/src/Bad Practices/Control-Flow/ConstantCondition.ql
index 386b238e049c..4e5a2821a794 100644
--- a/csharp/ql/src/Bad Practices/Control-Flow/ConstantCondition.ql	
+++ b/csharp/ql/src/Bad Practices/Control-Flow/ConstantCondition.ql	
@@ -54,11 +54,6 @@ predicate constantGuard(
   Guards::Guards::Guard g, string msg, Guards::Guards::Guard reason, string reasonMsg
 ) {
   ConstCondImpl::problems(g, msg, reason, reasonMsg) and
-  // conditional qualified expressions sit at an akward place in the CFG, which
-  // leads to FPs
-  not g.(QualifiableExpr).getQualifier() = reason and
-  // and if they're extension method calls, the syntactic qualifier is actually argument 0
-  not g.(ExtensionMethodCall).getArgument(0) = reason and
   // if a logical connective is constant, one of its operands is constant, so
   // we report that instead
   not g instanceof LogicalNotExpr and
diff --git a/shared/controlflow/codeql/controlflow/queries/ConstantCondition.qll b/shared/controlflow/codeql/controlflow/queries/ConstantCondition.qll
index d05f2df6eff8..ef3737a5a196 100644
--- a/shared/controlflow/codeql/controlflow/queries/ConstantCondition.qll
+++ b/shared/controlflow/codeql/controlflow/queries/ConstantCondition.qll
@@ -69,6 +69,15 @@ module Make<LocationSig Location, BB::CfgSig<Location> Cfg, InputSig<Cfg::BasicB
   private import Cfg
   private import Input
 
+  /**
+   * Holds if the edge from `bb1` to `bb2` implies both that `def` evaluates
+   * to `v` and that `g` evaluates to `gv`. The latter implication is also a
+   * biimplication in most cases, except when `g` is a short-circuiting
+   * side-effecting construct.
+   *
+   * Checking that `bb1` to `bb2` is a dominating edge is sufficient to ensure
+   * that the biimplication holds.
+   */
   private predicate ssaControlsGuardEdge(
     SsaDefinition def, GuardValue v, Guard g, BasicBlock bb1, BasicBlock bb2, GuardValue gv
   ) {
@@ -81,9 +90,10 @@ module Make<LocationSig Location, BB::CfgSig<Location> Cfg, InputSig<Cfg::BasicB
   private predicate impossibleValue(
     Guard g, GuardValue gv, SsaDefinition def, BasicBlock bb, GuardValue v2
   ) {
-    exists(GuardValue dual, GuardValue v1 |
-      // If `g` in `bb` evaluates to `gv` then `def` has value `v1`,
-      ssaControlsGuardEdge(def, v1, g, bb, _, gv) and
+    exists(GuardValue dual, GuardValue v1, BasicBlock bb2 |
+      // If `g` evaluates to `gv` and its branch edge `(bb, bb2)` is a dominating edge then `def` has value `v1`,
+      ssaControlsGuardEdge(def, v1, g, bb, bb2, gv) and
+      dominatingEdge(bb, bb2) and
       dual = gv.getDualValue() and
       not gv.isThrowsException() and
       not dual.isThrowsException() and