Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,14 @@ namespace Semmle.Extraction.CSharp.Entities
{
internal abstract class CachedSymbol<T> : CachedEntity<T> where T : class, ISymbol
{
private readonly Lazy<BlockSyntax?> blockLazy;
private readonly Lazy<ExpressionSyntax?> expressionBodyLazy;

protected CachedSymbol(Context cx, T init)
: base(cx, init)
{
blockLazy = new Lazy<BlockSyntax?>(() => GetBlock(Symbol));
expressionBodyLazy = new Lazy<ExpressionSyntax?>(() => GetExpressionBody(Symbol));
}

public virtual Type? ContainingType => Symbol.ContainingType is not null
Expand Down Expand Up @@ -87,31 +92,30 @@ 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();
}
}

public BlockSyntax? Block => blockLazy.Value;

public ExpressionSyntax? ExpressionBody => expressionBodyLazy.Value;

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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
}
Expand Down Expand Up @@ -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)
Expand Down
10 changes: 3 additions & 7 deletions csharp/extractor/Semmle.Extraction.CSharp/Entities/Event.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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!);
Expand All @@ -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);
Expand Down Expand Up @@ -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>
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()) &&
Expand Down Expand Up @@ -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>
Expand Down
10 changes: 3 additions & 7 deletions csharp/extractor/Semmle.Extraction.CSharp/Entities/Property.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: fix
---
* Fixed an issue where the body of a partial member could be extracted twice. When both a *defining* and an *implementing* declaration exist, only the *implementing* declaration is now extracted.
Original file line number Diff line number Diff line change
@@ -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 |
12 changes: 12 additions & 0 deletions csharp/ql/test/library-tests/dataflow/methods/MethodFlow.ql
Original file line number Diff line number Diff line change
@@ -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()
26 changes: 26 additions & 0 deletions csharp/ql/test/library-tests/dataflow/methods/Methods.cs
Original file line number Diff line number Diff line change
@@ -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;
}
15 changes: 8 additions & 7 deletions csharp/ql/test/library-tests/partial/MethodIsPartial.expected
Original file line number Diff line number Diff line change
@@ -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 |
5 changes: 5 additions & 0 deletions csharp/ql/test/library-tests/partial/Partial.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
partial class TwoPartClass
{
partial void PartialMethodWithBody1();
public partial object PartialMethodWithBody2(object obj);
partial void PartialMethodWithoutBody1();
public void Method2() { }
// Declaring declaration.
Expand All @@ -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.
Expand Down
29 changes: 15 additions & 14 deletions csharp/ql/test/library-tests/partial/Partial1.expected
Original file line number Diff line number Diff line change
@@ -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 |
Loading