Merge pull request #431 from calumgrant/cs/extractor/fsharp-core

C#: Fix extraction of method signatures

change-notes/1.19/analysis-csharp.md

@@ -24,6 +24,7 @@
 
 * Arguments passed using `in` are now extracted.
 * Fix a bug where the `dynamic` type name was not extracted correctly in certain circumstances.
+* Fixed a bug where method type signatures were extracted incorrectly in some circumstances.
 
 ## Changes to QL libraries
 

csharp/extractor/Semmle.Extraction.CIL/Entities/Method.cs

@@ -27,7 +27,8 @@ namespace Semmle.Extraction.CIL.Entities
 
         public override IEnumerable<Type> TypeParameters => gc.TypeParameters.Concat(declaringType.TypeParameters);
 
-        public override IEnumerable<Type> MethodParameters => genericParams == null ? Enumerable.Empty<Type>() : genericParams;
+        public override IEnumerable<Type> MethodParameters =>
+            genericParams == null ? gc.MethodParameters : gc.MethodParameters.Concat(genericParams);
 
         public int GenericParameterCount => signature.GenericParameterCount;
 
@@ -47,14 +48,14 @@ namespace Semmle.Extraction.CIL.Entities
 
         internal protected Id MakeMethodId(Type parent, Id methodName)
         {
-            var id = signature.ReturnType.MakeId(gc) + space + parent.ShortId + dot + methodName;
+            var id = signature.ReturnType.MakeId(this) + space + parent.ShortId + dot + methodName;
 
             if (signature.GenericParameterCount > 0)
             {
                 id += tick + signature.GenericParameterCount;
             }
 
-            id += open + CIL.Id.CommaSeparatedList(signature.ParameterTypes.Select(p => p.MakeId(gc))) + close;
+            id += open + CIL.Id.CommaSeparatedList(signature.ParameterTypes.Select(p => p.MakeId(this))) + close;
             return id;
         }
 

csharp/extractor/Semmle.Extraction.CIL/Entities/Type.cs

@@ -779,8 +779,8 @@ namespace Semmle.Extraction.CIL.Entities
 
         public override Id MakeId(bool inContext) => elementType.GetId(inContext) + openBracket + rank + closeBracket;
 
-        static readonly StringId openBracket = new StringId("[]");
-        static readonly StringId closeBracket = new StringId("[]");
+        static readonly StringId openBracket = new StringId("[");
+        static readonly StringId closeBracket = new StringId("]");
 
         public override Id Name => elementType.Name + openBracket + closeBracket;
 
@@ -1107,11 +1107,20 @@ namespace Semmle.Extraction.CIL.Entities
         ITypeSignature IConstructedTypeProvider<ITypeSignature>.GetGenericInstantiation(ITypeSignature genericType, ImmutableArray<ITypeSignature> typeArguments) =>
             new Instantiation { genericType = genericType, typeArguments = typeArguments };
 
+        static readonly Id open = Id.Create("{");
+        static readonly Id close = Id.Create("}");
+
         class GenericMethodParameter : ITypeSignature
         {
+            public object innerGc;
             public int index;
             static readonly Id excl = Id.Create("M!");
-            public Id MakeId(GenericContext gc) => excl + index;
+            public Id MakeId(GenericContext outerGc)
+            {
+                if (innerGc != outerGc && innerGc is Method method)
+                    return open + method.Label.Value + close + excl + index;
+                return excl + index;
+            }
         }
 
         class GenericTypeParameter : ITypeSignature
@@ -1122,7 +1131,7 @@ namespace Semmle.Extraction.CIL.Entities
         }
 
         ITypeSignature ISignatureTypeProvider<ITypeSignature, object>.GetGenericMethodParameter(object genericContext, int index) =>
-            new GenericMethodParameter { index = index };
+            new GenericMethodParameter { innerGc = genericContext, index = index };
 
         ITypeSignature ISignatureTypeProvider<ITypeSignature, object>.GetGenericTypeParameter(object genericContext, int index) =>
             new GenericTypeParameter { index = index };

csharp/ql/src/semmle/code/cil/Generics.qll

@@ -45,4 +45,10 @@ class UnboundGenericMethod extends UnboundGeneric, Method { }
 class ConstructedType extends ConstructedGeneric, Type { }
 
 /** A constructed generic method. */
-class ConstructedMethod extends ConstructedGeneric, Method { }
+class ConstructedMethod extends ConstructedGeneric, Method {
+  final override UnboundGenericMethod getUnboundGeneric() { result = getUnboundMethod() }
+
+  final override Location getLocation() {
+    result = getUnboundGeneric().getLocation()
+  }
+}

csharp/ql/test/library-tests/cil/regressions/ConstructedMethods.expected

@@ -0,0 +1,2 @@
+| Methods.dll:0:0:0:0 | Methods.Class1.F | Methods.dll:0:0:0:0 | Methods.Class1.F | Methods.dll:0:0:0:0 | Methods.Class1.G.!0 |
+| Methods.dll:0:0:0:0 | Methods.Class1.F | Methods.dll:0:0:0:0 | Methods.Class1.F | Methods.dll:0:0:0:0 | Methods.Class1.H.!0 |

csharp/ql/test/library-tests/cil/regressions/ConstructedMethods.ql

@@ -0,0 +1,7 @@
+import cil::CIL
+
+from UnboundGenericMethod f, ConstructedMethod fc
+where
+  fc.getUnboundMethod() = f and
+  f.getQualifiedName() = "Methods.Class1.F"
+select f, fc, fc.getTypeArgument(0)

csharp/ql/test/library-tests/cil/regressions/Methods.cs

@@ -0,0 +1,19 @@
+/*
+ * A regression test for the CIL extractor - compiled into Methods.dll
+ * This tests the correct extraction of F<T>, and we should end up with
+ * 2 constructed methods of F<T>.
+ */
+ 
+// semmle-extractor-options: --cil
+
+namespace Methods
+{
+    public class Class1
+    {
+        public T F<T>(T t) { return new T[] { t }[0]; }
+
+        public T G<T>(T t) { return F(t); }
+
+        public T H<T>(T t) { return F(t); }
+    }
+}