C#: Fix issue with partial method extraction.

csharp/extractor/Semmle.Extraction.CSharp/CodeAnalysisExtensions/SymbolExtensions.cs

@@ -728,6 +728,15 @@ public static bool IsUnboundGenericType(this INamedTypeSymbol type) =>
         public static INamedTypeSymbol? GetNonObjectBaseType(this ITypeSymbol symbol, Context cx) =>
             symbol is ITypeParameterSymbol || SymbolEqualityComparer.Default.Equals(symbol.BaseType, cx.Compilation.ObjectType) ? null : symbol.BaseType;
 
+        public static IMethodSymbol GetBodyDeclaringSymbol(this IMethodSymbol method) =>
+            method.PartialImplementationPart ?? method;
+
+        public static IPropertySymbol GetBodyDeclaringSymbol(this IPropertySymbol property) =>
+            property.PartialImplementationPart ?? property;
+
+        public static IEventSymbol GetBodyDeclaringSymbol(this IEventSymbol symbol) =>
+            symbol.PartialImplementationPart ?? symbol;
+
         [return: NotNullIfNotNull(nameof(symbol))]
         public static IEntity? CreateEntity(this Context cx, ISymbol symbol)
         {

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

@@ -70,7 +70,7 @@ public override void Populate(TextWriter trapFile)
 
             Overrides(trapFile);
 
-            if (Symbol.FromSource() && Block is null)
+            if (Symbol.FromSource() && !HasBody)
             {
                 trapFile.compiler_generated(this);
             }

csharp/extractor/Semmle.Extraction.CSharp/Entities/Base/CachedSymbol.cs

@@ -87,31 +87,32 @@ protected void BindComments()
                 Context.BindComments(this, FullLocation);
         }
 
-        protected virtual T BodyDeclaringSymbol => Symbol;
-
-        public BlockSyntax? Block
+        private static BlockSyntax? GetBlock(T symbol)
         {
-            get
-            {
-                return BodyDeclaringSymbol.DeclaringSyntaxReferences
+            return symbol.DeclaringSyntaxReferences
                     .SelectMany(r => r.GetSyntax().ChildNodes())
                     .OfType<BlockSyntax>()
                     .FirstOrDefault();
-            }
         }
 
-        public ExpressionSyntax? ExpressionBody
+        private static ExpressionSyntax? GetExpressionBody(T symbol)
         {
-            get
-            {
-                return BodyDeclaringSymbol.DeclaringSyntaxReferences
+            return symbol.DeclaringSyntaxReferences
                     .SelectMany(r => r.GetSyntax().ChildNodes())
                     .OfType<ArrowExpressionClauseSyntax>()
                     .Select(arrow => arrow.Expression)
                     .FirstOrDefault();
-            }
         }
 
+        private BlockSyntax? vBlock;
+        public BlockSyntax? Block => vBlock ??= GetBlock(Symbol);
+
+        private ExpressionSyntax? vExpressionBody;
+        public ExpressionSyntax? ExpressionBody => vExpressionBody ??= GetExpressionBody(Symbol);
+
+        private bool? vHasBody;
+        public bool HasBody => vHasBody ??= Block is not null || ExpressionBody is not null;
+
         public virtual bool IsSourceDeclaration => Symbol.IsSourceDeclaration();
 
         public override bool NeedsPopulation => Context.Defines(Symbol);

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

@@ -42,7 +42,7 @@ public override void Populate(TextWriter trapFile)
                 return;
             }
 
-            if (MakeSynthetic)
+            if (MakeSyntheticBody)
             {
                 // Create a synthetic empty body for primary and default constructors.
                 Statements.SyntheticEmptyBlock.Create(Context, this, 0, Location);
@@ -60,7 +60,7 @@ protected override void ExtractInitializers(TextWriter trapFile)
             // Do not extract initializers for constructed types.
             // Extract initializers for constructors with a body, primary constructors
             // and default constructors for classes and structs declared in source code.
-            if (Block is null && ExpressionBody is null && !MakeSynthetic || Context.OnlyScaffold)
+            if (!HasBody && !MakeSyntheticBody || Context.OnlyScaffold)
             {
                 return;
             }
@@ -211,7 +211,7 @@ Symbol.ContainingType.TypeKind is TypeKind.Class or TypeKind.Struct &&
         /// </summary>
         private bool IsBestSourceLocation => ReportingLocation is not null && Context.IsLocationInContext(ReportingLocation);
 
-        private bool MakeSynthetic => (IsPrimary || (IsDefault && IsBestSourceLocation)) && !Context.OnlyScaffold;
+        private bool MakeSyntheticBody => (IsPrimary || (IsDefault && IsBestSourceLocation)) && !Context.OnlyScaffold;
 
         [return: NotNullIfNotNull(nameof(constructor))]
         public static new Constructor? Create(Context cx, IMethodSymbol? constructor)

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

@@ -11,10 +11,6 @@ internal class Event : CachedSymbol<IEventSymbol>
         private Event(Context cx, IEventSymbol init)
             : base(cx, init) { }
 
-        protected override IEventSymbol BodyDeclaringSymbol => Symbol.PartialImplementationPart ?? Symbol;
-
-        public override Microsoft.CodeAnalysis.Location? ReportingLocation => BodyDeclaringSymbol.Locations.BestOrDefault();
-
         public override void WriteId(EscapingTextWriter trapFile)
         {
             trapFile.WriteSubId(ContainingType!);
@@ -31,8 +27,8 @@ public override void Populate(TextWriter trapFile)
             var type = Type.Create(Context, Symbol.Type);
             trapFile.events(this, Symbol.GetName(), ContainingType!, type.TypeRef, Create(Context, Symbol.OriginalDefinition));
 
-            var adder = BodyDeclaringSymbol.AddMethod;
-            var remover = BodyDeclaringSymbol.RemoveMethod;
+            var adder = Symbol.AddMethod;
+            var remover = Symbol.RemoveMethod;
 
             if (adder is not null)
                 Method.Create(Context, adder);
@@ -76,7 +72,7 @@ public override void Populate(TextWriter trapFile)
             }
         }
 
-        public static Event Create(Context cx, IEventSymbol symbol) => EventFactory.Instance.CreateEntityFromSymbol(cx, symbol);
+        public static Event Create(Context cx, IEventSymbol symbol) => EventFactory.Instance.CreateEntityFromSymbol(cx, symbol.GetBodyDeclaringSymbol());
 
         private class EventFactory : CachedEntityFactory<IEventSymbol, Event>
         {

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

@@ -59,7 +59,7 @@ public override void Populate(TextWriter trapFile)
 
             Overrides(trapFile);
 
-            if (Symbol.FromSource() && Block is null)
+            if (Symbol.FromSource() && !HasBody)
             {
                 trapFile.compiler_generated(this);
             }

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

@@ -20,8 +20,8 @@ public override void Populate(TextWriter trapFile)
             var type = Type.Create(Context, Symbol.Type);
             trapFile.indexers(this, Symbol.GetName(useMetadataName: true), ContainingType!, type.TypeRef, OriginalDefinition);
 
-            var getter = BodyDeclaringSymbol.GetMethod;
-            var setter = BodyDeclaringSymbol.SetMethod;
+            var getter = Symbol.GetMethod;
+            var setter = Symbol.SetMethod;
 
             if (getter is null && setter is null)
                 Context.ModelError(Symbol, "No indexer accessor defined");
@@ -81,7 +81,7 @@ public override void Populate(TextWriter trapFile)
                 TypeMention.Create(Context, syntax.Type, this, type);
         }
 
-        public static new Indexer Create(Context cx, IPropertySymbol prop) => IndexerFactory.Instance.CreateEntityFromSymbol(cx, prop);
+        public static new Indexer Create(Context cx, IPropertySymbol prop) => IndexerFactory.Instance.CreateEntityFromSymbol(cx, prop.GetBodyDeclaringSymbol());
 
         public override void WriteId(EscapingTextWriter trapFile)
         {

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

@@ -85,7 +85,7 @@ protected virtual void PopulateMethodBody(TextWriter trapFile)
                        else
                            Expression.Create(Context, expr!, this, 0);
 
-                       NumberOfLines(trapFile, BodyDeclaringSymbol, this);
+                       NumberOfLines(trapFile, Symbol, this);
                    });
             }
         }

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

@@ -14,14 +14,12 @@ protected OrdinaryMethod(Context cx, IMethodSymbol init)
 
         public override string Name => Symbol.GetName();
 
-        protected override IMethodSymbol BodyDeclaringSymbol => Symbol.PartialImplementationPart ?? Symbol;
-
         public IMethodSymbol SourceDeclaration => Symbol.OriginalDefinition;
 
         public override Microsoft.CodeAnalysis.Location ReportingLocation =>
             IsCompilerGeneratedDelegate()
                 ? Symbol.ContainingType.GetSymbolLocation()
-                : BodyDeclaringSymbol.GetSymbolLocation();
+                : Symbol.GetSymbolLocation();
 
         public override bool NeedsPopulation =>
             (base.NeedsPopulation || IsCompilerGeneratedDelegate()) &&
@@ -77,7 +75,7 @@ Symbol.ContainingType is INamedTypeSymbol nt &&
                 cx.ExtractionContext.Logger.LogWarning("Reduced extension method symbols should not be directly extracted.");
             }
 
-            return OrdinaryMethodFactory.Instance.CreateEntityFromSymbol(cx, method);
+            return OrdinaryMethodFactory.Instance.CreateEntityFromSymbol(cx, method.GetBodyDeclaringSymbol());
         }
 
         private class OrdinaryMethodFactory : CachedEntityFactory<IMethodSymbol, OrdinaryMethod>

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

@@ -21,10 +21,6 @@ protected Property(Context cx, IPropertySymbol init)
 
         private Type Type => type.Value;
 
-        protected override IPropertySymbol BodyDeclaringSymbol => Symbol.PartialImplementationPart ?? Symbol;
-
-        public override Microsoft.CodeAnalysis.Location? ReportingLocation => BodyDeclaringSymbol.Locations.BestOrDefault();
-
         public override void WriteId(EscapingTextWriter trapFile)
         {
             trapFile.WriteSubId(Type);
@@ -46,8 +42,8 @@ public override void Populate(TextWriter trapFile)
             var type = Type;
             trapFile.properties(this, Symbol.GetName(), ContainingType!, type.TypeRef, Create(Context, Symbol.OriginalDefinition));
 
-            var getter = BodyDeclaringSymbol.GetMethod;
-            var setter = BodyDeclaringSymbol.SetMethod;
+            var getter = Symbol.GetMethod;
+            var setter = Symbol.SetMethod;
 
             if (getter is not null)
                 Method.Create(Context, getter);
@@ -132,7 +128,7 @@ public static Property Create(Context cx, IPropertySymbol prop)
         {
             var isIndexer = prop.IsIndexer || prop.Parameters.Any();
 
-            return isIndexer ? Indexer.Create(cx, prop) : PropertyFactory.Instance.CreateEntityFromSymbol(cx, prop);
+            return isIndexer ? Indexer.Create(cx, prop) : PropertyFactory.Instance.CreateEntityFromSymbol(cx, prop.GetBodyDeclaringSymbol());
         }
 
         private class PropertyFactory : CachedEntityFactory<IPropertySymbol, Property>

csharp/ql/test/library-tests/dataflow/methods/MethodFlow.expected

@@ -0,0 +1,40 @@
+models
+edges
+| Methods.cs:8:48:8:48 | o : Object | Methods.cs:10:16:10:16 | access to parameter o : Object | provenance |  |
+| Methods.cs:8:48:8:48 | o : Object | Methods.cs:10:16:10:16 | access to parameter o : Object | provenance |  |
+| Methods.cs:17:13:17:13 | access to local variable o : Object | Methods.cs:19:38:19:38 | access to local variable o : Object | provenance |  |
+| Methods.cs:17:13:17:13 | access to local variable o : Object | Methods.cs:19:38:19:38 | access to local variable o : Object | provenance |  |
+| Methods.cs:17:17:17:33 | call to method Source<Object> : Object | Methods.cs:17:13:17:13 | access to local variable o : Object | provenance |  |
+| Methods.cs:17:17:17:33 | call to method Source<Object> : Object | Methods.cs:17:13:17:13 | access to local variable o : Object | provenance |  |
+| Methods.cs:19:13:19:18 | access to local variable result : Object | Methods.cs:20:14:20:19 | access to local variable result | provenance |  |
+| Methods.cs:19:13:19:18 | access to local variable result : Object | Methods.cs:20:14:20:19 | access to local variable result | provenance |  |
+| Methods.cs:19:22:19:39 | call to method PartialMethod : Object | Methods.cs:19:13:19:18 | access to local variable result : Object | provenance |  |
+| Methods.cs:19:22:19:39 | call to method PartialMethod : Object | Methods.cs:19:13:19:18 | access to local variable result : Object | provenance |  |
+| Methods.cs:19:38:19:38 | access to local variable o : Object | Methods.cs:8:48:8:48 | o : Object | provenance |  |
+| Methods.cs:19:38:19:38 | access to local variable o : Object | Methods.cs:8:48:8:48 | o : Object | provenance |  |
+| Methods.cs:19:38:19:38 | access to local variable o : Object | Methods.cs:19:22:19:39 | call to method PartialMethod : Object | provenance |  |
+| Methods.cs:19:38:19:38 | access to local variable o : Object | Methods.cs:19:22:19:39 | call to method PartialMethod : Object | provenance |  |
+nodes
+| Methods.cs:8:48:8:48 | o : Object | semmle.label | o : Object |
+| Methods.cs:8:48:8:48 | o : Object | semmle.label | o : Object |
+| Methods.cs:10:16:10:16 | access to parameter o : Object | semmle.label | access to parameter o : Object |
+| Methods.cs:10:16:10:16 | access to parameter o : Object | semmle.label | access to parameter o : Object |
+| Methods.cs:17:13:17:13 | access to local variable o : Object | semmle.label | access to local variable o : Object |
+| Methods.cs:17:13:17:13 | access to local variable o : Object | semmle.label | access to local variable o : Object |
+| Methods.cs:17:17:17:33 | call to method Source<Object> : Object | semmle.label | call to method Source<Object> : Object |
+| Methods.cs:17:17:17:33 | call to method Source<Object> : Object | semmle.label | call to method Source<Object> : Object |
+| Methods.cs:19:13:19:18 | access to local variable result : Object | semmle.label | access to local variable result : Object |
+| Methods.cs:19:13:19:18 | access to local variable result : Object | semmle.label | access to local variable result : Object |
+| Methods.cs:19:22:19:39 | call to method PartialMethod : Object | semmle.label | call to method PartialMethod : Object |
+| Methods.cs:19:22:19:39 | call to method PartialMethod : Object | semmle.label | call to method PartialMethod : Object |
+| Methods.cs:19:38:19:38 | access to local variable o : Object | semmle.label | access to local variable o : Object |
+| Methods.cs:19:38:19:38 | access to local variable o : Object | semmle.label | access to local variable o : Object |
+| Methods.cs:20:14:20:19 | access to local variable result | semmle.label | access to local variable result |
+| Methods.cs:20:14:20:19 | access to local variable result | semmle.label | access to local variable result |
+subpaths
+| Methods.cs:19:38:19:38 | access to local variable o : Object | Methods.cs:8:48:8:48 | o : Object | Methods.cs:10:16:10:16 | access to parameter o : Object | Methods.cs:19:22:19:39 | call to method PartialMethod : Object |
+| Methods.cs:19:38:19:38 | access to local variable o : Object | Methods.cs:8:48:8:48 | o : Object | Methods.cs:10:16:10:16 | access to parameter o : Object | Methods.cs:19:22:19:39 | call to method PartialMethod : Object |
+testFailures
+#select
+| Methods.cs:20:14:20:19 | access to local variable result | Methods.cs:17:17:17:33 | call to method Source<Object> : Object | Methods.cs:20:14:20:19 | access to local variable result | $@ | Methods.cs:17:17:17:33 | call to method Source<Object> : Object | call to method Source<Object> : Object |
+| Methods.cs:20:14:20:19 | access to local variable result | Methods.cs:17:17:17:33 | call to method Source<Object> : Object | Methods.cs:20:14:20:19 | access to local variable result | $@ | Methods.cs:17:17:17:33 | call to method Source<Object> : Object | call to method Source<Object> : Object |

csharp/ql/test/library-tests/dataflow/methods/MethodFlow.ql

@@ -0,0 +1,12 @@
+/**
+ * @kind path-problem
+ */
+
+import csharp
+import utils.test.InlineFlowTest
+import DefaultFlowTest
+import PathGraph
+
+from PathNode source, PathNode sink
+where flowPath(source, sink)
+select sink, source, sink, "$@", source, source.toString()

csharp/ql/test/library-tests/dataflow/methods/Methods.cs

@@ -0,0 +1,26 @@
+public partial class Partial
+{
+    public partial object PartialMethod(object o);
+}
+
+public partial class Partial
+{
+    public partial object PartialMethod(object o)
+    {
+        return o;
+    }
+}
+public class C
+{
+    public void M()
+    {
+        var o = Source<object>(1);
+        var p = new Partial();
+        var result = p.PartialMethod(o);
+        Sink(result); // $ hasValueFlow=1
+    }
+
+    public static void Sink(object o) { }
+
+    static T Source<T>(object source) => throw null;
+}

csharp/ql/test/library-tests/partial/MethodIsPartial.expected

@@ -1,7 +1,8 @@
-| Partial.cs:6:18:6:42 | PartialMethodWithoutBody1 | true |
-| Partial.cs:7:17:7:23 | Method2 | false |
-| Partial.cs:18:18:18:39 | PartialMethodWithBody1 | true |
-| Partial.cs:19:17:19:23 | Method3 | false |
-| Partial.cs:41:18:41:42 | PartialMethodWithoutBody2 | true |
-| Partial.cs:42:17:42:23 | Method4 | false |
-| Partial.cs:47:17:47:23 | Method5 | false |
+| Partial.cs:7:18:7:42 | PartialMethodWithoutBody1 | true |
+| Partial.cs:8:17:8:23 | Method2 | false |
+| Partial.cs:19:18:19:39 | PartialMethodWithBody1 | true |
+| Partial.cs:20:27:20:48 | PartialMethodWithBody2 | true |
+| Partial.cs:24:17:24:23 | Method3 | false |
+| Partial.cs:46:18:46:42 | PartialMethodWithoutBody2 | true |
+| Partial.cs:47:17:47:23 | Method4 | false |
+| Partial.cs:52:17:52:23 | Method5 | false |

csharp/ql/test/library-tests/partial/Partial.cs

@@ -3,6 +3,7 @@
 partial class TwoPartClass
 {
     partial void PartialMethodWithBody1();
+    public partial object PartialMethodWithBody2(object obj);
     partial void PartialMethodWithoutBody1();
     public void Method2() { }
     // Declaring declaration.
@@ -16,6 +17,10 @@ public void Method2() { }
 partial class TwoPartClass
 {
     partial void PartialMethodWithBody1() { }
+    public partial object PartialMethodWithBody2(object obj)
+    {
+        return obj;
+    }
     public void Method3() { }
     private object _backingField;
     // Implementation declaration.

csharp/ql/test/library-tests/partial/Partial1.expected

@@ -1,17 +1,18 @@
 | Partial.cs:3:15:3:26 | TwoPartClass |
-| Partial.cs:6:18:6:42 | PartialMethodWithoutBody1 |
-| Partial.cs:16:15:16:26 | TwoPartClass |
-| Partial.cs:18:18:18:39 | PartialMethodWithBody1 |
-| Partial.cs:22:27:22:42 | PartialProperty1 |
-| Partial.cs:24:9:24:11 | get_PartialProperty1 |
-| Partial.cs:25:9:25:11 | set_PartialProperty1 |
-| Partial.cs:29:27:29:30 | Item |
-| Partial.cs:31:9:31:11 | get_Item |
-| Partial.cs:32:9:32:11 | set_Item |
-| Partial.cs:36:39:36:51 | PartialEvent1 |
-| Partial.cs:36:55:36:57 | add_PartialEvent1 |
-| Partial.cs:36:63:36:68 | remove_PartialEvent1 |
-| Partial.cs:39:15:39:33 | OnePartPartialClass |
-| Partial.cs:41:18:41:42 | PartialMethodWithoutBody2 |
+| Partial.cs:7:18:7:42 | PartialMethodWithoutBody1 |
+| Partial.cs:17:15:17:26 | TwoPartClass |
+| Partial.cs:19:18:19:39 | PartialMethodWithBody1 |
+| Partial.cs:20:27:20:48 | PartialMethodWithBody2 |
+| Partial.cs:27:27:27:42 | PartialProperty1 |
+| Partial.cs:29:9:29:11 | get_PartialProperty1 |
+| Partial.cs:30:9:30:11 | set_PartialProperty1 |
+| Partial.cs:34:27:34:30 | Item |
+| Partial.cs:36:9:36:11 | get_Item |
+| Partial.cs:37:9:37:11 | set_Item |
+| Partial.cs:41:39:41:51 | PartialEvent1 |
+| Partial.cs:41:55:41:57 | add_PartialEvent1 |
+| Partial.cs:41:63:41:68 | remove_PartialEvent1 |
+| Partial.cs:44:15:44:33 | OnePartPartialClass |
+| Partial.cs:46:18:46:42 | PartialMethodWithoutBody2 |
 | PartialMultipleFiles1.cs:1:22:1:41 | PartialMultipleFiles |
 | PartialMultipleFiles2.cs:1:22:1:41 | PartialMultipleFiles |