Merge pull request github#16186 from michaelnebel/csharp/suppressnullablefix

C#: Fix issue with suppress nullable warning directly on a method call.

Author: michaelnebel

csharp/extractor/Semmle.Extraction.CSharp/Entities/CallTypeExtensions.cs

@@ -9,7 +9,7 @@ internal static class CallTypeExtensions
         /// </summary>
         public static ExprKind AdjustKind(this Expression.CallType ct, ExprKind k)
         {
-            if (k == ExprKind.ADDRESS_OF)
+            if (k == ExprKind.ADDRESS_OF || k == ExprKind.SUPPRESS_NULLABLE_WARNING)
             {
                 return k;
             }

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/PostfixUnary.cs

@@ -21,11 +21,11 @@ private PostfixUnary(ExpressionNodeInfo info, ExprKind kind, ExpressionSyntax op
         protected override void PopulateExpression(TextWriter trapFile)
         {
             Create(Context, operand, this, 0);
-            OperatorCall(trapFile, Syntax);
 
             if ((operatorKind == ExprKind.POST_INCR || operatorKind == ExprKind.POST_DECR) &&
                 Kind == ExprKind.OPERATOR_INVOCATION)
             {
+                OperatorCall(trapFile, Syntax);
                 trapFile.mutator_invocation_mode(this, 2);
             }
         }

csharp/ql/lib/change-notes/2024-04-12-suppress-nullable-warning.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Extracting suppress nullable warning expressions did not work when applied directly to a method call (like `System.Console.Readline()!`). This has been fixed.

csharp/ql/test/library-tests/expressions/ConstructorInitializers.expected

@@ -22,6 +22,7 @@
 | file://:0:0:0:0 | Rectangle | expressions.cs:351:18:351:26 | call to constructor Object | file://:0:0:0:0 | Object |
 | file://:0:0:0:0 | Rectangle2 | expressions.cs:361:18:361:27 | call to constructor Object | file://:0:0:0:0 | Object |
 | file://:0:0:0:0 | ReducedClass | ReducedExpression.cs:2:7:2:18 | call to constructor Object | file://:0:0:0:0 | Object |
+| file://:0:0:0:0 | SuppressNullableWarning | expressions.cs:522:11:522:33 | call to constructor Object | file://:0:0:0:0 | Object |
 | file://:0:0:0:0 | TestConversionOperator | expressions.cs:330:11:330:32 | call to constructor Object | file://:0:0:0:0 | Object |
 | file://:0:0:0:0 | TestCreations | expressions.cs:383:18:383:30 | call to constructor Object | file://:0:0:0:0 | Object |
 | file://:0:0:0:0 | TestUnaryOperator | expressions.cs:292:11:292:27 | call to constructor Object | file://:0:0:0:0 | Object |

csharp/ql/test/library-tests/expressions/PrintAst.expected

@@ -2406,3 +2406,26 @@ expressions.cs:
 #  520|           -1: [TypeMention] object
 #  520|       3: [ConstructorInitializer] call to constructor ClassC1
 #  520|         0: [ParameterAccess] access to parameter oc2
+#  522|   24: [Class] SuppressNullableWarning
+#  525|     5: [Method] Api
+#  525|       -1: [TypeMention] object
+#  525|       4: [ObjectCreation] object creation of type Object
+#  525|         0: [TypeMention] object
+#  527|     6: [Method] Test
+#  527|       -1: [TypeMention] Void
+#-----|       2: (Parameters)
+#  527|         0: [Parameter] arg0
+#  527|           -1: [TypeMention] object
+#  528|       4: [BlockStmt] {...}
+#  529|         0: [LocalVariableDeclStmt] ... ...;
+#  529|           0: [LocalVariableDeclAndInitExpr] Object x = ...
+#  529|             -1: [TypeMention] object
+#  529|             0: [LocalVariableAccess] access to local variable x
+#  529|             1: [SuppressNullableWarningExpr] ...!
+#  529|               0: [ParameterAccess] access to parameter arg0
+#  530|         1: [LocalVariableDeclStmt] ... ...;
+#  530|           0: [LocalVariableDeclAndInitExpr] Object y = ...
+#  530|             -1: [TypeMention] object
+#  530|             0: [LocalVariableAccess] access to local variable y
+#  530|             1: [SuppressNullableWarningExpr] ...!
+#  530|               0: [MethodCall] call to method Api

csharp/ql/test/library-tests/expressions/QualifiableExpr.expected

@@ -70,3 +70,4 @@
 | expressions.cs:483:17:483:26 | access to field value | expressions.cs:483:17:483:20 | this access |
 | expressions.cs:488:32:488:39 | access to field value | expressions.cs:488:32:488:33 | access to parameter c1 |
 | expressions.cs:488:43:488:50 | access to field value | expressions.cs:488:43:488:44 | access to parameter c2 |
+| expressions.cs:530:21:530:25 | call to method Api | expressions.cs:530:21:530:25 | this access |

csharp/ql/test/library-tests/expressions/SuppressNullableWarning.expected

@@ -0,0 +1,2 @@
+| expressions.cs:529:21:529:25 | ...! |
+| expressions.cs:530:21:530:26 | ...! |

csharp/ql/test/library-tests/expressions/SuppressNullableWarning.ql

@@ -0,0 +1,3 @@
+import csharp
+
+select any(SuppressNullableWarningExpr e)

csharp/ql/test/library-tests/expressions/expressions.cs

@@ -518,4 +518,16 @@ struct MyInlineArray
     class ClassC1(object oc1) { }
 
     class ClassC2(object oc2) : ClassC1(oc2) { }
+
+    class SuppressNullableWarning
+    {
+
+        public object? Api() => new object();
+
+        public void Test(object? arg0)
+        {
+            var x = arg0!;
+            var y = Api()!;
+        }
+    }
 }

csharp/ql/test/query-tests/Security Features/CWE-089/SqlInjection.cs

@@ -95,6 +95,17 @@ public void GetDataSetByCategory()
                 var result = new DataSet();
                 adapter.Fill(result);
             }
+
+            // BAD: Input from the command line. (also implicitly check flow via suppress nullable warning `!`)
+            using (var connection = new SqlConnection(connectionString))
+            {
+                var queryString = "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='"
+                  + Console.ReadLine()! + "' ORDER BY PRICE";
+                var cmd = new SqlCommand(queryString);
+                var adapter = new SqlDataAdapter(cmd);
+                var result = new DataSet();
+                adapter.Fill(result);
+            }
         }
 
         System.Windows.Forms.TextBox box1;