Merge branch 'main' into rust-ti-associated

Author: paldepind

config/sync-files.py

@@ -58,7 +58,19 @@ def file_checksum(filename):
     with open(filename, 'rb') as file_handle:
         return hashlib.sha1(file_handle.read()).hexdigest()
 
-def check_group(group_name, files, master_file_picker, emit_error):
+def accept_prefix(line1, line2):
+    suffix = line2.removeprefix(line1)
+    return not suffix or suffix.lstrip().startswith("//")
+
+def equivalent_lines(lines1, lines2):
+    if len(lines1) != len(lines2):
+        return False
+    for line1, line2 in zip(lines1, lines2):
+        if not accept_prefix(line1, line2) and not accept_prefix(line2, line1):
+            return False
+    return True
+
+def check_group(group_name, files, master_file_picker, emit_error, accept_prefix):
     extant_files = [f for f in files if path.isfile(f)]
     if len(extant_files) == 0:
         emit_error(__file__, 0, "No files found from group '" + group_name + "'.")
@@ -70,11 +82,23 @@ def check_group(group_name, files, master_file_picker, emit_error):
         return
 
     checksums = {file_checksum(f) for f in extant_files}
-
-    if len(checksums) == 1 and len(extant_files) == len(files):
+    same_lengths = len(extant_files) == len(files)
+    if len(checksums) == 1 and same_lengths:
         # All files are present and identical.
         return
 
+    # In this case we also consider files indentical, if
+    # (1) The group only containts two files.
+    # (2) The lines of one file are the same as the lines of another file
+    # modulo comments.
+    if accept_prefix and same_lengths and len(extant_files) == 2:
+        with open(extant_files[0], 'r') as f1:
+            file1_lines = [l.strip('\n\r') for l in f1.readlines()]
+        with open(extant_files[1], 'r') as f2:
+            file2_lines = [l.strip('\n\r') for l in f2.readlines()]
+        if equivalent_lines(file1_lines, file2_lines):
+            return
+
     master_file = master_file_picker(extant_files)
     if master_file is None:
         emit_error(__file__, 0,
@@ -139,9 +163,10 @@ def sync_identical_files(emit_error):
         raise Exception("Bad command line or file not found")
     chdir_repo_root()
     load_if_exists('.', 'config/identical-files.json')
-    file_groups.update(csharp_test_files())
+    for group_name, files in csharp_test_files().items():
+        check_group(group_name, files, master_file_picker, emit_error, True)
     for group_name, files in file_groups.items():
-        check_group(group_name, files, master_file_picker, emit_error)
+        check_group(group_name, files, master_file_picker, emit_error, False)
 
 def main():
     sync_identical_files(emit_local_error)

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

csharp/ql/test/query-tests/API Abuse/NoDisposeCallOnLocalIDisposable/NoDisposeCallOnLocalIDisposable.cs

@@ -114,15 +114,6 @@ void TimerProc(object obj)
     public void Dispose() { }
 }
 
-class Bad
-{
-    long GetLength(string file)
-    {
-        var stream = new FileStream(file, FileMode.Open); // $ Alert
-        return stream.Length;
-    }
-}
-
 static class Extensions
 {
     public static FileStream Fluent(this FileStream fs) => fs;

csharp/ql/test/query-tests/API Abuse/NoDisposeCallOnLocalIDisposable/NoDisposeCallOnLocalIDisposable.expected

@@ -4,4 +4,4 @@
 | NoDisposeCallOnLocalIDisposable.cs:76:25:76:71 | call to method Create | Disposable 'XmlReader' is created but not disposed. |
 | NoDisposeCallOnLocalIDisposable.cs:76:42:76:64 | object creation of type StringReader | Disposable 'StringReader' is created but not disposed. |
 | NoDisposeCallOnLocalIDisposable.cs:104:23:104:38 | object creation of type HttpClient | Disposable 'HttpClient' is created but not disposed. |
-| NoDisposeCallOnLocalIDisposable.cs:121:22:121:56 | object creation of type FileStream | Disposable 'FileStream' is created but not disposed. |
+| NoDisposeCallOnLocalIDisposableBad.cs:8:22:8:56 | object creation of type FileStream | Disposable 'FileStream' is created but not disposed. |

csharp/ql/test/query-tests/API Abuse/NoDisposeCallOnLocalIDisposable/NoDisposeCallOnLocalIDisposableBad.cs

@@ -0,0 +1,11 @@
+using System;
+using System.IO;
+
+class Bad
+{
+    long GetLength(string file)
+    {
+        var stream = new FileStream(file, FileMode.Open); // $ Alert
+        return stream.Length;
+    }
+}

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

@@ -11,6 +11,7 @@
 | 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'. |
 | ConstantCondition.cs:141:22:141:22 | _ | Pattern always matches. |
+| ConstantConditionBad.cs:5:16:5:20 | ... > ... | Condition always evaluates to 'false'. |
 | 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,7 +20,6 @@
 | 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/ConstantConditionBad.cs

@@ -0,0 +1,7 @@
+class Bad
+{
+    public int Max(int a, int b)
+    {
+        return a > a ? a : b; // $ Alert
+    }
+}

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

@@ -25,11 +25,6 @@ public void Foo()
             }
         }
 
-        public int Max(int a, int b)
-        {
-            return a > a ? a : b; // $ Alert
-        }
-
         public int Bar()
         {
             return ZERO;

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<string> M8(IEnumerable<string> source)
@@ -79,8 +79,8 @@ public IEnumerable<string> M9(IEnumerable<string> source)
 
     public void M10(IEnumerable<string> 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,22 +243,22 @@ 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()
     {
         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,18 @@ 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
         {
         }
     }
-}
+}
+
+class StringInterpolation
+{
+    void Pi()
+    {
+        float pi = 3.14159f; // GOOD
+        const int align = 6; // GOOD
+        Console.WriteLine($"Pi, {pi,align:F3}");
+    }
+}

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