Merge pull request #18976 from michaelnebel/csharp/constant-condition

C#: Increase precision of `cs/constant-condition`.

Author: michaelnebel

csharp/ql/src/Bad Practices/Control-Flow/ConstantCondition.ql

@@ -119,9 +119,14 @@ class ConstantMatchingCondition extends ConstantCondition {
   }
 
   override predicate isWhiteListed() {
-    exists(SwitchExpr se, int i |
-      se.getCase(i).getPattern() = this.(DiscardExpr) and
+    exists(Switch se, Case c, int i |
+      c = se.getCase(i) and
+      c.getPattern() = this.(DiscardExpr)
+    |
       i > 0
+      or
+      i = 0 and
+      exists(Expr cond | c.getCondition() = cond and not isConstantCondition(cond, true))
     )
     or
     this = any(PositionalPatternExpr ppe).getPattern(_)

csharp/ql/src/change-notes/2025-03-11-constant-condition.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Increase query precision for `cs/constant-condition` and allow the use of discards in switch/case statements and also take the condition (if any) into account.

csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.cs

@@ -35,18 +35,18 @@ class ConstantNullness
 {
     void M1(int i)
     {
-        var j = ((string)null)?.Length; // BAD
-        var s = ((int?)i)?.ToString(); // BAD
+        var j = ((string)null)?.Length; // $ Alert
+        var s = ((int?)i)?.ToString(); // $ Alert
         var k = s?.Length; // GOOD
         k = s?.ToLower()?.Length; // GOOD
     }
 
     void M2(int i)
     {
-        var j = (int?)null ?? 0; // BAD
-        var s = "" ?? "a"; // BAD
-        j = (int?)i ?? 1; // BAD
-        s = ""?.CommaJoinWith(s); // BAD
+        var j = (int?)null ?? 0; // $ Alert
+        var s = "" ?? "a"; // $ Alert
+        j = (int?)i ?? 1; // $ Alert
+        s = ""?.CommaJoinWith(s); // $ Alert
         s = s ?? ""; // GOOD
         s = (i == 0 ? s : null) ?? s; // GOOD
         var k = (i == 0 ? s : null)?.Length; // GOOD
@@ -59,9 +59,9 @@ void M1()
     {
         switch (1 + 2)
         {
-            case 2: // BAD
+            case 2: // $ Alert
                 break;
-            case 3: // BAD
+            case 3: // $ Alert
                 break;
             case int _: // GOOD
                 break;
@@ -72,7 +72,7 @@ void M2(string s)
     {
         switch ((object)s)
         {
-            case int _: // BAD
+            case int _: // $ Alert
                 break;
             case "": // GOOD
                 break;
@@ -92,7 +92,7 @@ string M4(object o)
     {
         return o switch
         {
-            _ => o.ToString() // BAD
+            _ => o.ToString() // $ Alert
         };
     }
 
@@ -111,7 +111,7 @@ void M6(bool b1, bool b2)
             return;
         if (!b2)
             return;
-        if (b1 && b2) // BAD
+        if (b1 && b2) // $ Alert
             return;
     }
 
@@ -124,6 +124,35 @@ string M7(object o)
             _ => "" // GOOD
         };
     }
+
+    string M8(int i)
+    {
+        return i switch
+        {
+            _ when i % 2 == 0 => "even", // GOOD
+            _ => "odd" // GOOD
+        };
+    }
+
+    string M9(int i)
+    {
+        switch (i)
+        {
+            case var _: // $ Alert
+                return "even";
+        }
+    }
+
+    string M10(int i)
+    {
+        switch (i)
+        {
+            case var _ when i % 2 == 0: // GOOD
+                return "even";
+            case var _: // GOOD
+                return "odd";
+        }
+    }
 }
 
 class Assertions

csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.expected

@@ -10,7 +10,7 @@
 | ConstantCondition.cs:95:13:95:13 | _ | Pattern always matches. |
 | ConstantCondition.cs:114:13:114:14 | access to parameter b1 | Condition always evaluates to 'true'. |
 | ConstantCondition.cs:114:19:114:20 | access to parameter b2 | Condition always evaluates to 'true'. |
-| ConstantConditionBad.cs:5:16:5:20 | ... > ... | Condition always evaluates to 'false'. |
+| ConstantCondition.cs:141:22:141:22 | _ | Pattern always matches. |
 | ConstantConditionalExpressionCondition.cs:11:22:11:34 | ... == ... | Condition always evaluates to 'true'. |
 | ConstantConditionalExpressionCondition.cs:12:21:12:25 | false | Condition always evaluates to 'false'. |
 | ConstantConditionalExpressionCondition.cs:13:21:13:30 | ... == ... | Condition always evaluates to 'true'. |
@@ -19,6 +19,7 @@
 | ConstantIfCondition.cs:11:17:11:29 | ... == ... | Condition always evaluates to 'true'. |
 | ConstantIfCondition.cs:14:17:14:21 | false | Condition always evaluates to 'false'. |
 | ConstantIfCondition.cs:17:17:17:26 | ... == ... | Condition always evaluates to 'true'. |
+| ConstantIfCondition.cs:30:20:30:24 | ... > ... | Condition always evaluates to 'false'. |
 | ConstantIsNullOrEmpty.cs:10:21:10:54 | call to method IsNullOrEmpty | Condition always evaluates to 'false'. |
 | ConstantIsNullOrEmpty.cs:46:21:46:46 | call to method IsNullOrEmpty | Condition always evaluates to 'true'. |
 | ConstantIsNullOrEmpty.cs:50:21:50:44 | call to method IsNullOrEmpty | Condition always evaluates to 'true'. |

csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.qlref

@@ -1 +1,2 @@
-Bad Practices/Control-Flow/ConstantCondition.ql
+query: Bad Practices/Control-Flow/ConstantCondition.ql
+postprocess: utils/test/InlineExpectationsTestQuery.ql

csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantConditionBad.cs

@@ -8,10 +8,10 @@ class Main
 
         public void Foo()
         {
-            int i = (ZERO == 1 - 1) ? 0 : 1; // BAD
-            int j = false ? 0 : 1; // BAD
-            int k = " " == " " ? 0 : 1; // BAD
-            int l = (" "[0] == ' ') ? 0 : 1; // BAD: but not flagged
+            int i = (ZERO == 1 - 1) ? 0 : 1; // $ Alert
+            int j = false ? 0 : 1; // $ Alert
+            int k = " " == " " ? 0 : 1; // $ Alert
+            int l = (" "[0] == ' ') ? 0 : 1; // Missing Alert
             int m = Bar() == 0 ? 0 : 1; // GOOD
         }
 
@@ -21,5 +21,4 @@ public int Bar()
         }
 
     }
-
 }

csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantConditionalExpressionCondition.cs

@@ -6,9 +6,9 @@ class Main
     {
         public void M()
         {
-            for (int i = 0; false; i++) // GOOD
+            for (int i = 0; false; i++) // $ Alert
                 ;
-            for (int i = 0; 0 == 1; i++) // BAD
+            for (int i = 0; 0 == 1; i++) // $ Alert
                 ;
             for (; ; ) // GOOD
                 ;

csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantForCondition.cs

@@ -8,23 +8,28 @@ class Main
 
         public void Foo()
         {
-            if (ZERO == 1 - 1)
-            { // BAD
+            if (ZERO == 1 - 1) // $ Alert
+            {
             }
-            if (false)
-            { // BAD
+            if (false) // $ Alert
+            {
             }
-            if (" " == " ")
-            { // BAD
+            if (" " == " ") // $ Alert
+            {
             }
-            if (" "[0] == ' ')
-            { // BAD: but not flagged
+            if (" "[0] == ' ') // Missing Alert
+            {
             }
-            if (Bar() == 0)
-            { // GOOD
+            if (Bar() == 0) // GOOD
+            {
             }
         }
 
+        public int Max(int a, int b)
+        {
+            return a > a ? a : b; // $ Alert
+        }
+
         public int Bar()
         {
             return ZERO;

csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantIfCondition.cs

@@ -7,17 +7,17 @@ internal class Program
         static void Main(string[] args)
         {
             {
-                if (string.IsNullOrEmpty(nameof(args))) // bad: always false
+                if (string.IsNullOrEmpty(nameof(args))) // $ Alert
                 {
                 }
 
                 string? x = null;
-                if (string.IsNullOrEmpty(x)) // would be nice... bad: always true
+                if (string.IsNullOrEmpty(x)) // Missing Alert (always true)
                 {
                 }
 
                 string y = "";
-                if (string.IsNullOrEmpty(y)) // would be nice... bad: always true
+                if (string.IsNullOrEmpty(y)) // Missing Alert (always true)
                 {
                 }
 
@@ -28,12 +28,12 @@ static void Main(string[] args)
                 }
 
                 string z = " ";
-                if (string.IsNullOrEmpty(z)) // would be nice... bad: always false
+                if (string.IsNullOrEmpty(z)) // Missing Alert (always false)
                 {
                 }
 
                 string a = "a";
-                if (string.IsNullOrEmpty(a)) // would be nice... bad: always false
+                if (string.IsNullOrEmpty(a)) // Missing Alert (always false)
                 {
                 }
 
@@ -43,18 +43,18 @@ static void Main(string[] args)
                 {
                 }
 
-                if (string.IsNullOrEmpty(null)) // bad: always true
+                if (string.IsNullOrEmpty(null)) // $ Alert
                 {
                 }
 
-                if (string.IsNullOrEmpty("")) // bad: always true
+                if (string.IsNullOrEmpty("")) // $ Alert
                 {
                 }
 
-                if (string.IsNullOrEmpty(" ")) // bad: always false
+                if (string.IsNullOrEmpty(" ")) // $ Alert
                 {
                 }
             }
         }
     }
-}
+}