Merge pull request github#13298 from michaelnebel/csharp/paramdefaultimplicitconversion

michaelnebel · web-flow · commit 3a3f9a265583 · 2023-06-02T10:52:24.000+02:00
C#: Extract default parameter values.
diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expression.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expression.cs
@@ -211,6 +211,11 @@ private static bool ContainsPattern(SyntaxNode node) =>
                 return Default.CreateGenerated(cx, parent, childIndex, location, ValueAsString(null));
             }
 
+            if (type.SpecialType is SpecialType.None)
+            {
+                return ImplicitCast.CreateGenerated(cx, parent, childIndex, type, defaultValue, location);
+            }
+
             if (type.SpecialType is SpecialType.System_DateTime)
             {
                 return DateTimeObjectCreation.CreateGenerated(cx, parent, childIndex, type, defaultValue, location);
diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/ImplicitCast.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/ImplicitCast.cs
@@ -1,3 +1,4 @@
+using System.Linq;
 using Microsoft.CodeAnalysis;
 using Semmle.Extraction.Kinds;
 
@@ -11,33 +12,73 @@ public Expression Expr
             private set;
         }
 
-        public ImplicitCast(ExpressionNodeInfo info)
+        private ImplicitCast(ExpressionNodeInfo info)
             : base(new ExpressionInfo(info.Context, info.ConvertedType, info.Location, ExprKind.CAST, info.Parent, info.Child, true, info.ExprValue))
         {
             Expr = Factory.Create(new ExpressionNodeInfo(Context, info.Node, this, 0));
         }
 
-        public ImplicitCast(ExpressionNodeInfo info, IMethodSymbol method)
+        private ImplicitCast(ExpressionNodeInfo info, IMethodSymbol method)
             : base(new ExpressionInfo(info.Context, info.ConvertedType, info.Location, ExprKind.OPERATOR_INVOCATION, info.Parent, info.Child, true, info.ExprValue))
         {
             Expr = Factory.Create(info.SetParent(this, 0));
 
+            AddOperatorCall(method);
+        }
+
+        private ImplicitCast(ExpressionInfo info, IMethodSymbol method, object value) : base(info)
+        {
+            Expr = Literal.CreateGenerated(Context, this, 0, method.Parameters[0].Type, value, info.Location);
+
+            AddOperatorCall(method);
+        }
+
+        private void AddOperatorCall(IMethodSymbol method)
+        {
             var target = Method.Create(Context, method);
-            if (target is not null)
-                Context.TrapWriter.Writer.expr_call(this, target);
+            Context.TrapWriter.Writer.expr_call(this, target);
+        }
+
+        private static IMethodSymbol? GetImplicitConversionMethod(ITypeSymbol type, object value) =>
+            type
+                .GetMembers()
+                .OfType<IMethodSymbol>()
+                .Where(method =>
+                    method.GetName() == "op_Implicit" &&
+                    method.Parameters.Length == 1 &&
+                    method.Parameters[0].Type.Name == value.GetType().Name
+                )
+                .FirstOrDefault();
+
+        // Creates a new generated expression with an implicit cast added, if needed.
+        public static Expression CreateGenerated(Context cx, IExpressionParentEntity parent, int childIndex, ITypeSymbol type, object value,
+            Extraction.Entities.Location location)
+        {
+            ExpressionInfo create(ExprKind kind, string? v) =>
+                new ExpressionInfo(
+                    cx,
+                    AnnotatedTypeSymbol.CreateNotAnnotated(type),
+                    location,
+                    kind,
+                    parent,
+                    childIndex,
+                    true,
+                    v);
+
+            var method = GetImplicitConversionMethod(type, value);
+            if (method is not null)
+            {
+                var info = create(ExprKind.OPERATOR_INVOCATION, null);
+                return new ImplicitCast(info, method, value);
+            }
             else
-                Context.ModelError(info.Node, "Failed to resolve target for operator invocation");
+            {
+                cx.ModelError(location, "Failed to resolve target for implicit operator invocation for a parameter default.");
+                return new Expression(create(ExprKind.UNKNOWN, ValueAsString(value)));
+            }
         }
 
-        /// <summary>
-        /// Creates a new expression, adding casts as required.
-        /// </summary>
-        /// <param name="cx">The extraction context.</param>
-        /// <param name="node">The expression node.</param>
-        /// <param name="parent">The parent of the expression.</param>
-        /// <param name="child">The child number.</param>
-        /// <param name="type">A type hint.</param>
-        /// <returns>A new expression.</returns>
+        // Creates a new expression, adding casts as required.
         public static Expression Create(ExpressionNodeInfo info)
         {
             var resolvedType = info.ResolvedType;
diff --git a/csharp/extractor/Semmle.Extraction.CSharp/SymbolExtensions.cs b/csharp/extractor/Semmle.Extraction.CSharp/SymbolExtensions.cs
@@ -25,7 +25,7 @@ public AnnotatedTypeSymbol(ITypeSymbol? symbol, NullableAnnotation nullability)
             Nullability = nullability;
         }
 
-        public static AnnotatedTypeSymbol? CreateNotAnnotated(ITypeSymbol symbol) =>
+        public static AnnotatedTypeSymbol? CreateNotAnnotated(ITypeSymbol? symbol) =>
             symbol is null ? (AnnotatedTypeSymbol?)null : new AnnotatedTypeSymbol(symbol, NullableAnnotation.None);
     }
 
diff --git a/csharp/ql/test/library-tests/parameters/Parameters.cs b/csharp/ql/test/library-tests/parameters/Parameters.cs
@@ -25,7 +25,14 @@ public class Parameters
     public void M17([Optional, DefaultParameterValue(null)] object arg7) => throw null;
     public void M18([Optional, DefaultParameterValue(3)] int? arg8) => throw null;
     public void M19([Optional, DecimalConstant(1, 0, 0, 0, 103)] decimal arg9) => throw null;
+    public void M20([Optional, DefaultParameterValue(7)] MyStruct arg10) => throw null;
+    public void M21([Optional, DefaultParameterValue("mystring")] MyStruct arg10) => throw null;
 
-    public struct MyStruct { }
+    public struct MyStruct
+    {
+        public static implicit operator MyStruct(int i) => new MyStruct();
+        public static implicit operator MyStruct(string s) => new MyStruct();
+
+    }
     public enum MyEnum { A = 1, B = 2 }
 }
diff --git a/csharp/ql/test/library-tests/parameters/Parameters.cs_ b/csharp/ql/test/library-tests/parameters/Parameters.cs_
@@ -25,7 +25,14 @@ public class ParametersDll
     public void M17([Optional, DefaultParameterValue(null)] object arg7) => throw null;
     public void M18([Optional, DefaultParameterValue(3)] int? arg8) => throw null;
     public void M19([Optional, DecimalConstant(1, 0, 0, 0, 103)] decimal arg9) => throw null;
+    public void M20([Optional, DefaultParameterValue(7)] MyStruct arg10) => throw null;
+    public void M21([Optional, DefaultParameterValue("mystring")] MyStruct arg10) => throw null;
 
-    public struct MyStruct { }
+    public struct MyStruct
+    {
+        public static implicit operator MyStruct(int i) => new MyStruct();
+        public static implicit operator MyStruct(string s) => new MyStruct();
+
+    }
     public enum MyEnum { A = 1, B = 2 }
 }
diff --git a/csharp/ql/test/library-tests/parameters/Parameters.dll b/csharp/ql/test/library-tests/parameters/Parameters.dll
diff --git a/csharp/ql/test/library-tests/parameters/Parameters.expected b/csharp/ql/test/library-tests/parameters/Parameters.expected
@@ -5,12 +5,16 @@ noDefaultValue
 | Parameters.cs:8:17:8:18 | M2 | Parameters.cs:8:24:8:24 | a | 0 |
 | Parameters.cs:12:17:12:18 | M6 | Parameters.cs:12:29:12:30 | s1 | 0 |
 | Parameters.cs:13:17:13:18 | M7 | Parameters.cs:13:27:13:28 | e1 | 0 |
+| Parameters.cs:33:32:33:39 | implicit conversion | Parameters.cs:33:54:33:54 | i | 0 |
+| Parameters.cs:34:32:34:39 | implicit conversion | Parameters.cs:34:57:34:57 | s | 0 |
 | Parameters.dll:0:0:0:0 | M1 | Parameters.dll:0:0:0:0 | a | 0 |
 | Parameters.dll:0:0:0:0 | M1 | Parameters.dll:0:0:0:0 | b | 1 |
 | Parameters.dll:0:0:0:0 | M1 | Parameters.dll:0:0:0:0 | c | 2 |
 | Parameters.dll:0:0:0:0 | M2 | Parameters.dll:0:0:0:0 | a | 0 |
 | Parameters.dll:0:0:0:0 | M6 | Parameters.dll:0:0:0:0 | s1 | 0 |
 | Parameters.dll:0:0:0:0 | M7 | Parameters.dll:0:0:0:0 | e1 | 0 |
+| Parameters.dll:0:0:0:0 | implicit conversion | Parameters.dll:0:0:0:0 | i | 0 |
+| Parameters.dll:0:0:0:0 | implicit conversion | Parameters.dll:0:0:0:0 | s | 0 |
 withDefaultValue
 | Parameters.cs:8:17:8:18 | M2 | Parameters.cs:8:34:8:34 | b | 1 | Parameters.cs:8:38:8:41 | null | null |
 | Parameters.cs:8:17:8:18 | M2 | Parameters.cs:8:51:8:51 | c | 2 | Parameters.cs:8:55:8:70 | "default string" | default string |
@@ -39,6 +43,8 @@ withDefaultValue
 | Parameters.cs:25:17:25:19 | M17 | Parameters.cs:25:68:25:71 | arg7 | 0 | Parameters.cs:25:21:25:71 | default | null |
 | Parameters.cs:26:17:26:19 | M18 | Parameters.cs:26:63:26:66 | arg8 | 0 | Parameters.cs:26:21:26:66 | 3 | 3 |
 | Parameters.cs:27:17:27:19 | M19 | Parameters.cs:27:74:27:77 | arg9 | 0 | Parameters.cs:27:21:27:77 | 10.3 | 10.3 |
+| Parameters.cs:28:17:28:19 | M20 | Parameters.cs:28:67:28:71 | arg10 | 0 | Parameters.cs:28:21:28:71 | call to operator implicit conversion | - |
+| Parameters.cs:29:17:29:19 | M21 | Parameters.cs:29:76:29:80 | arg10 | 0 | Parameters.cs:29:21:29:80 | call to operator implicit conversion | - |
 | Parameters.dll:0:0:0:0 | M2 | Parameters.dll:0:0:0:0 | b | 1 | Parameters.dll:0:0:0:0 | default | null |
 | Parameters.dll:0:0:0:0 | M2 | Parameters.dll:0:0:0:0 | c | 2 | Parameters.dll:0:0:0:0 | "default string" | default string |
 | Parameters.dll:0:0:0:0 | M3 | Parameters.dll:0:0:0:0 | a | 0 | Parameters.dll:0:0:0:0 | 1 | 1 |
@@ -66,8 +72,15 @@ withDefaultValue
 | Parameters.dll:0:0:0:0 | M17 | Parameters.dll:0:0:0:0 | arg7 | 0 | Parameters.dll:0:0:0:0 | default | null |
 | Parameters.dll:0:0:0:0 | M18 | Parameters.dll:0:0:0:0 | arg8 | 0 | Parameters.dll:0:0:0:0 | 3 | 3 |
 | Parameters.dll:0:0:0:0 | M19 | Parameters.dll:0:0:0:0 | arg9 | 0 | Parameters.dll:0:0:0:0 | 10.3 | 10.3 |
+| Parameters.dll:0:0:0:0 | M20 | Parameters.dll:0:0:0:0 | arg10 | 0 | Parameters.dll:0:0:0:0 | call to operator implicit conversion | - |
+| Parameters.dll:0:0:0:0 | M21 | Parameters.dll:0:0:0:0 | arg10 | 0 | Parameters.dll:0:0:0:0 | call to operator implicit conversion | - |
 dateTimeDefaults
 | Parameters.cs:22:17:22:19 | M14 | Parameters.cs:22:64:22:67 | arg4 | Parameters.cs:22:21:22:67 | object creation of type DateTime | DateTime(long) | 14 |
 | Parameters.cs:23:17:23:19 | M15 | Parameters.cs:23:68:23:71 | arg5 | Parameters.cs:23:21:23:71 | object creation of type DateTime | DateTime(long) | 10001 |
 | Parameters.dll:0:0:0:0 | M14 | Parameters.dll:0:0:0:0 | arg4 | Parameters.dll:0:0:0:0 | object creation of type DateTime | DateTime(long) | 14 |
 | Parameters.dll:0:0:0:0 | M15 | Parameters.dll:0:0:0:0 | arg5 | Parameters.dll:0:0:0:0 | object creation of type DateTime | DateTime(long) | 10001 |
+implicitConversionDefaults
+| Parameters.cs:28:17:28:19 | M20 | Parameters.cs:28:67:28:71 | arg10 | Parameters.cs:28:21:28:71 | call to operator implicit conversion | Parameters.cs:28:21:28:71 | 7 | Int32 | 7 |
+| Parameters.cs:29:17:29:19 | M21 | Parameters.cs:29:76:29:80 | arg10 | Parameters.cs:29:21:29:80 | call to operator implicit conversion | Parameters.cs:29:21:29:80 | "mystring" | String | mystring |
+| Parameters.dll:0:0:0:0 | M20 | Parameters.dll:0:0:0:0 | arg10 | Parameters.dll:0:0:0:0 | call to operator implicit conversion | Parameters.dll:0:0:0:0 | 7 | Int32 | 7 |
+| Parameters.dll:0:0:0:0 | M21 | Parameters.dll:0:0:0:0 | arg10 | Parameters.dll:0:0:0:0 | call to operator implicit conversion | Parameters.dll:0:0:0:0 | "mystring" | String | mystring |
diff --git a/csharp/ql/test/library-tests/parameters/Parameters.ql b/csharp/ql/test/library-tests/parameters/Parameters.ql
@@ -16,23 +16,33 @@ query predicate noDefaultValue(Parameterizable container, Parameter p, int i) {
   not compilerGeneratedAttribute(container)
 }
 
-query predicate withDefaultValue(Parameterizable container, Parameter p, int i, Expr e, string value) {
+private predicate defaultValue(Parameterizable container, Parameter p, int i, Expr e) {
   fromTestLocation(container) and
   p.hasDefaultValue() and
   container.getParameter(i) = p and
-  p.getDefaultValue() = e and
+  p.getDefaultValue() = e
+}
+
+query predicate withDefaultValue(Parameterizable container, Parameter p, int i, Expr e, string value) {
+  defaultValue(container, p, i, e) and
   (if exists(e.getValue()) then value = e.getValue() else value = "-") and
   not compilerGeneratedAttribute(container)
 }
 
 query predicate dateTimeDefaults(
   Parameterizable container, Parameter p, ObjectCreation o, string constructor, string value
 ) {
-  fromTestLocation(container) and
-  p.hasDefaultValue() and
-  container.getAParameter() = p and
-  p.getDefaultValue() = o and
+  defaultValue(container, p, _, o) and
   o.getTarget().toStringWithTypes() = constructor and
   o.getAnArgument().getValue() = value and
   not compilerGeneratedAttribute(container)
 }
+
+query predicate implicitConversionDefaults(
+  Parameterizable container, Parameter p, OperatorCall o, Expr e, string type, string value
+) {
+  defaultValue(container, p, _, o) and
+  o.getAnArgument() = e and
+  type = e.getType().toString() and
+  value = e.getValue()
+}

Original file line number	Diff line number	Diff line change
`@@ -211,6 +211,11 @@ private static bool ContainsPattern(SyntaxNode node) =>`
`211`	`211`	`return Default.CreateGenerated(cx, parent, childIndex, location, ValueAsString(null));`
`212`	`212`	`}`
`213`	`213`
	`214`	`+ if (type.SpecialType is SpecialType.None)`
	`215`	`+ {`
	`216`	`+ return ImplicitCast.CreateGenerated(cx, parent, childIndex, type, defaultValue, location);`
	`217`	`+ }`
	`218`	`+`
`214`	`219`	`if (type.SpecialType is SpecialType.System_DateTime)`
`215`	`220`	`{`
`216`	`221`	`return DateTimeObjectCreation.CreateGenerated(cx, parent, childIndex, type, defaultValue, location);`
Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,7 @@ public AnnotatedTypeSymbol(ITypeSymbol? symbol, NullableAnnotation nullability)`
`25`	`25`	`Nullability = nullability;`
`26`	`26`	`}`
`27`	`27`
`28`		`- public static AnnotatedTypeSymbol? CreateNotAnnotated(ITypeSymbol symbol) =>`
	`28`	`+ public static AnnotatedTypeSymbol? CreateNotAnnotated(ITypeSymbol? symbol) =>`
`29`	`29`	`symbol is null ? (AnnotatedTypeSymbol?)null : new AnnotatedTypeSymbol(symbol, NullableAnnotation.None);`
`30`	`30`	`}`
`31`	`31`