From 49ecff3292c9cad2ffc6101f721942d8907cc2be Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Thu, 13 Mar 2025 16:32:37 +0100 Subject: [PATCH 1/4] C#: Add cs/useless-assignment-to-local to the CCR suite. --- csharp/ql/src/codeql-suites/csharp-code-quality.qls | 1 + 1 file changed, 1 insertion(+) diff --git a/csharp/ql/src/codeql-suites/csharp-code-quality.qls b/csharp/ql/src/codeql-suites/csharp-code-quality.qls index b7b533773810..a7cfd44d7348 100644 --- a/csharp/ql/src/codeql-suites/csharp-code-quality.qls +++ b/csharp/ql/src/codeql-suites/csharp-code-quality.qls @@ -12,3 +12,4 @@ - cs/constant-condition - cs/useless-gethashcode-call - cs/non-short-circuit + - cs/useless-assignment-to-local From 2b88600f0fb6680d7f8e3f0d82c825753f49d0fa Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 14 Mar 2025 15:19:09 +0100 Subject: [PATCH 2/4] C#: Re-factor cs/useless-assignment-to-local tests to use inline test framework. --- .../DeadStoreOfLocal/DeadStoreOfLocal.cs | 50 +++++++++---------- .../DeadStoreOfLocal/DeadStoreOfLocal.qlref | 3 +- .../DeadStoreOfLocal/DeadStoreOfLocalBad.cs | 10 ++-- 3 files changed, 32 insertions(+), 31 deletions(-) diff --git a/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.cs b/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.cs index 941981f6d6c2..1b40bf9c3d77 100644 --- a/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.cs +++ b/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.cs @@ -9,14 +9,14 @@ public class DeadStoreOfLocal public int M1() { - int x = M2(); // BAD + int x = M2(); // $ Alert return (x = 1) + x; // GOOD } public int M2() { int x = 1; // GOOD - return x + (x = 1); // BAD + return x + (x = 1); // $ Alert } public int M3() @@ -41,19 +41,19 @@ public int M4() public void M5() { - int x = M3(); // BAD + int x = M3(); // $ Alert } public void M6() { int x = 42; - x += 1; // BAD + x += 1; // $ Alert } public void M7() { int x = 42; - x++; // BAD + x++; // $ Alert } public IEnumerable M8(IEnumerable source) @@ -79,8 +79,8 @@ public IEnumerable M9(IEnumerable source) public void M10(IEnumerable source) { - foreach (var val in source) - { // BAD + foreach (var val in source) // $ Alert + { } } } @@ -98,10 +98,10 @@ public void F() message = "Unsuccessful completion"; // GOOD: Used in finally Process(); info2 = "Finishing"; // GOOD: Used in exception handler - extra = "Dead store here"; // BAD: Dead store + extra = "Dead store here"; // $ Alert Dead store Process(); message = "Successful completion"; // GOOD: Used in finally - info1 = "Used in handler"; // BAD: Used in handler, but not a reachable handler + info1 = "Used in handler"; // $ Alert Used in handler, but not a reachable handler } catch (SystemException ex) { @@ -139,7 +139,7 @@ public void FinallyFlow2() { Process(); } - catch (Exception ex) // BAD + catch (Exception ex) // $ Alert { Console.WriteLine("Stage " + stage); stage = 3; // GOOD: Used in finally @@ -157,7 +157,7 @@ public class OutParam public void Test() { int x; - Fn(out x); // BAD + Fn(out x); // Missing Alert Fn(out _); // GOOD } @@ -194,7 +194,7 @@ void M1() void M2() { - var x = M6(); // BAD [FALSE NEGATIVE] + var x = M6(); // Missing Alert Action a = () => { x = 1; // GOOD @@ -208,7 +208,7 @@ void M3() int x; Action a = () => { - x = 1; // BAD [FALSE NEGATIVE] + x = 1; // Missing Alert }; a(); } @@ -230,7 +230,7 @@ void M4() void M5() { - int x = 0; // BAD: NOT DETECTED + int x = 0; // Missing Alert. Action a = () => { x = 1; // GOOD @@ -243,14 +243,14 @@ int M6() { fn(() => { - int y = M6(); // BAD + int y = M6(); // $ Alert return (y = 1) + y; // GOOD }); int captured = 0; // GOOD: Variable captured variable fn(() => { return captured; }); - return captured = 1; // BAD: NOT DETECTED + return captured = 1; // Missing Alert. } void M7() @@ -258,7 +258,7 @@ void M7() var y = 12; // GOOD: Not a dead store (used in delegate) fn(() => { - var x = y; // BAD: Dead store in lambda + var x = y; // $ Alert Dead store in lambda return 0; }); } @@ -297,8 +297,8 @@ void Test() { // GOOD Console.WriteLine($"int {i1}"); } - else if (o is var v1) - { // BAD + else if (o is var v1) // $ Alert + { } switch (o) @@ -311,7 +311,7 @@ void Test() case int i3: // GOOD Console.WriteLine($"int {i3}"); break; - case var v2: // BAD + case var v2: // $ Alert break; default: Console.WriteLine("Something else"); @@ -328,7 +328,7 @@ void M() Use(x); Use(b); Use(s); - (x, (b, s)) = GetTuple(); // BAD: `b` + (x, (b, s)) = GetTuple(); // $ Alert on `b` Use(x); Use(s); (x, (_, s)) = GetTuple(); // GOOD @@ -369,7 +369,7 @@ string M3() string M4() { - var s = M3(); // BAD + var s = M3(); // $ Alert s = ""; return s; } @@ -395,7 +395,7 @@ string M7(bool b) { var s = ""; if (b) - s = "abc"; // BAD + s = "abc"; // $ Alert if (!b) return s; return null; @@ -469,8 +469,8 @@ public static void M() using var x = new System.IO.FileStream("", System.IO.FileMode.Open); // GOOD using var _ = new System.IO.FileStream("", System.IO.FileMode.Open); // GOOD - using (var y = new System.IO.FileStream("", System.IO.FileMode.Open)) // BAD + using (var y = new System.IO.FileStream("", System.IO.FileMode.Open)) // $ Alert { } } -} \ No newline at end of file +} diff --git a/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.qlref b/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.qlref index 51cf4454da28..cfc0aaa4eef7 100644 --- a/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.qlref +++ b/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.qlref @@ -1 +1,2 @@ -Dead Code/DeadStoreOfLocal.ql +query: Dead Code/DeadStoreOfLocal.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocalBad.cs b/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocalBad.cs index c1b7918e62bd..059fe30b7725 100644 --- a/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocalBad.cs +++ b/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocalBad.cs @@ -4,7 +4,7 @@ class Bad { double ParseInt(string s) { - var success = int.TryParse(s, out int i); + var success = int.TryParse(s, out int i); // $ Alert return i; } @@ -20,7 +20,7 @@ double ParseDouble(string s) { return double.Parse(s); } - catch (FormatException e) + catch (FormatException e) // $ Alert { return double.NaN; } @@ -29,14 +29,14 @@ double ParseDouble(string s) int Count(string[] ss) { int count = 0; - foreach (var s in ss) + foreach (var s in ss) // $ Alert count++; return count; } string IsInt(object o) { - if (o is int i) + if (o is int i) // $ Alert return "yes"; else return "no"; @@ -46,7 +46,7 @@ string IsString(object o) { switch (o) { - case string s: + case string s: // $ Alert return "yes"; default: return "no"; From dd1fbd28beab41658693c6356181a975c7c890e2 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Thu, 20 Mar 2025 11:42:11 +0100 Subject: [PATCH 3/4] C#: Add string interpolation examples to cs/useless-assignment-to-local. --- .../Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.cs b/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.cs index 1b40bf9c3d77..3e4e5fbeeb2e 100644 --- a/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.cs +++ b/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.cs @@ -474,3 +474,13 @@ public static void M() } } } + +class StringInterpolation +{ + void Pi() + { + float pi = 3.14159f; // GOOD + const int align = 6; // GOOD + Console.WriteLine($"Pi, {pi,align:F3}"); + } +} From 70a174ad5a4de2fe6ddc4a8ea3d5970afa7d5f15 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 4 Apr 2025 11:47:46 +0200 Subject: [PATCH 4/4] C#: Address review comments. --- .../Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.cs b/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.cs index 3e4e5fbeeb2e..04d8e20c09e5 100644 --- a/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.cs +++ b/csharp/ql/test/query-tests/Dead Code/DeadStoreOfLocal/DeadStoreOfLocal.cs @@ -157,7 +157,7 @@ public class OutParam public void Test() { int x; - Fn(out x); // Missing Alert + Fn(out x); // $ MISSING: Alert Fn(out _); // GOOD } @@ -194,7 +194,7 @@ void M1() void M2() { - var x = M6(); // Missing Alert + var x = M6(); // $ MISSING: Alert Action a = () => { x = 1; // GOOD @@ -208,7 +208,7 @@ void M3() int x; Action a = () => { - x = 1; // Missing Alert + x = 1; // $ MISSING: Alert }; a(); } @@ -230,7 +230,7 @@ void M4() void M5() { - int x = 0; // Missing Alert. + int x = 0; // $ MISSING: Alert Action a = () => { x = 1; // GOOD @@ -250,7 +250,7 @@ int M6() int captured = 0; // GOOD: Variable captured variable fn(() => { return captured; }); - return captured = 1; // Missing Alert. + return captured = 1; // $ MISSING: Alert } void M7()