diff --git a/csharp/ql/src/Bad Practices/Control-Flow/ConstantCondition.ql b/csharp/ql/src/Bad Practices/Control-Flow/ConstantCondition.ql index a0542a947356..88d938e399fe 100644 --- a/csharp/ql/src/Bad Practices/Control-Flow/ConstantCondition.ql +++ b/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(_) diff --git a/csharp/ql/src/change-notes/2025-03-11-constant-condition.md b/csharp/ql/src/change-notes/2025-03-11-constant-condition.md new file mode 100644 index 000000000000..2c9e50136af0 --- /dev/null +++ b/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. diff --git a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.cs b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.cs index 9e7386149a40..0445e152ec72 100644 --- a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.cs +++ b/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 diff --git a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.expected b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.expected index 397d77531b29..9e0e69edb904 100644 --- a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.expected +++ b/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'. | diff --git a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.qlref b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.qlref index 1fa68b335bbb..6692217230e0 100644 --- a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.qlref +++ b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.qlref @@ -1 +1,2 @@ -Bad Practices/Control-Flow/ConstantCondition.ql \ No newline at end of file +query: Bad Practices/Control-Flow/ConstantCondition.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantConditionBad.cs b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantConditionBad.cs deleted file mode 100644 index bd1e44b346b5..000000000000 --- a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantConditionBad.cs +++ /dev/null @@ -1,7 +0,0 @@ -class Bad -{ - public int Max(int a, int b) - { - return a > a ? a : b; - } -} diff --git a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantConditionalExpressionCondition.cs b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantConditionalExpressionCondition.cs index 1ee318becbd2..4cd56232627d 100644 --- a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantConditionalExpressionCondition.cs +++ b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantConditionalExpressionCondition.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() } } - } diff --git a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantForCondition.cs b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantForCondition.cs index 7ccb9ac86c74..2da0589d1827 100644 --- a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantForCondition.cs +++ b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantForCondition.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 ; diff --git a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantIfCondition.cs b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantIfCondition.cs index 44869e51af0b..146dbcf56611 100644 --- a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantIfCondition.cs +++ b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantIfCondition.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; diff --git a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantIsNullOrEmpty.cs b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantIsNullOrEmpty.cs index 5cad2e818abe..01e8353a20f4 100644 --- a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantIsNullOrEmpty.cs +++ b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantIsNullOrEmpty.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 { } } } } -} \ No newline at end of file +} diff --git a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantNullCoalescingLeftHandOperand.cs b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantNullCoalescingLeftHandOperand.cs index fa2ee7d00b0f..6901daf643ed 100644 --- a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantNullCoalescingLeftHandOperand.cs +++ b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantNullCoalescingLeftHandOperand.cs @@ -8,8 +8,8 @@ class Main public void Foo() { - object i = NULL_OBJECT ?? ""; // BAD - object j = null ?? ""; // BAD + object i = NULL_OBJECT ?? ""; // $ Alert + object j = null ?? ""; // $ Alert object k = Bar() ?? ""; // GOOD } diff --git a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantWhileCondition.cs b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantWhileCondition.cs index 64dc8150d56e..59575e0de45e 100644 --- a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantWhileCondition.cs +++ b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantWhileCondition.cs @@ -9,28 +9,28 @@ class Main public void Foo() { - while (ZERO == 1 - 1) - { // BAD + while (ZERO == 1 - 1) // $ Alert + { break; } - while (false) - { // GOOD + while (false) // $ Alert + { break; } - while (true) - { // GOOD + while (true) // GOOD + { break; } - while (" " == " ") - { // BAD + while (" " == " ") // $ Alert + { break; } - while (" "[0] == ' ') - { // BAD: but not flagged + while (" "[0] == ' ') // Missing Alert + { break; } - while (Bar() == 0) - { // GOOD + while (Bar() == 0) // GOOD + { break; } }