Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C# 13: Partial properties and indexers. #18533

Merged
merged 12 commits into from
Feb 3, 2025
Merged

C# 13: Partial properties and indexers. #18533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
963e658
C#: Add partial properties and indexers examples.
michaelnebel Jan 20, 2025
ff7719f
C#: Updated expected test output.
michaelnebel Jan 20, 2025
b3e56e6
C#: Add dispatch examples for partial properties and indexers.
michaelnebel Jan 21, 2025
e53c750
C#: Update expected output for dispatch tests.
michaelnebel Jan 21, 2025
dae5ca8
C#: Add dataflow examples for partial properties.
michaelnebel Jan 22, 2025
5836c8d
C#: Update expected test output for data flow tests.
michaelnebel Jan 22, 2025
568fd9c
C#: Add some partial tests for Properties and Indexers.
michaelnebel Jan 20, 2025
c481e82
C#: Add indexer data flow test.
michaelnebel Jan 22, 2025
443a2a4
C#: Add indexer data flow test expected output file.
michaelnebel Jan 22, 2025
0453ae8
C#: Use property- and indexer implementation location and extract the…
michaelnebel Jan 20, 2025
fc03c36
C#: Update expected test output.
michaelnebel Jan 20, 2025
501f985
C#: Add change note.
michaelnebel Jan 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions csharp/extractor/Semmle.Extraction.CSharp/Entities/Accessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ protected Accessor(Context cx, IMethodSymbol init, IPropertySymbol property)
return props.SingleOrDefault();
}

public override bool NeedsPopulation =>
base.NeedsPopulation &&
!Symbol.IsPartialDefinition; // Accessors always have an implementing declaration as well.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that aligns with how we extract partial methods; for partial methods we still only extract one entity, but it will have multiple locations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure that I understand the comparison to partial methods. In this case we don't want to extract the methodsymbol used in the declaring defintion (as this is not a callable).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I think we may still want to extract it's location, like we do for partial methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the current implementation mirrors that of partial methods. For methods we might extract multiple method symbols (one for the declaration and one of the implementation), but both method symbols will (only) extract the implementation location as the source location (note the overrides of BodyDeclaringSymbol and ReportingLocation in OrdinaryMethod).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so I double checked, and you are right, sorry for the confusion. We do indeed only extract one location for partial methods, it only for partial classes that we extract multiple locations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree.


public override void Populate(TextWriter trapFile)
{
PopulateMethod(trapFile);
Expand Down
8 changes: 4 additions & 4 deletions csharp/extractor/Semmle.Extraction.CSharp/Entities/Indexer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,16 @@ public override void Populate(TextWriter trapFile)
foreach (var l in Locations)
trapFile.indexer_location(this, l);

var getter = Symbol.GetMethod;
var setter = Symbol.SetMethod;
var getter = BodyDeclaringSymbol.GetMethod;
var setter = BodyDeclaringSymbol.SetMethod;

if (getter is null && setter is null)
Context.ModelError(Symbol, "No indexer accessor defined");

if (!(getter is null))
if (getter is not null)
Method.Create(Context, getter);

if (!(setter is null))
if (setter is not null)
Method.Create(Context, setter);

for (var i = 0; i < Symbol.Parameters.Length; ++i)
Expand Down
12 changes: 8 additions & 4 deletions csharp/extractor/Semmle.Extraction.CSharp/Entities/Property.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ 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();

michaelnebel marked this conversation as resolved.
Show resolved Hide resolved
public override void WriteId(EscapingTextWriter trapFile)
{
trapFile.WriteSubId(Type);
Expand All @@ -43,13 +47,13 @@ public override void Populate(TextWriter trapFile)
var type = Type;
trapFile.properties(this, Symbol.GetName(), ContainingType!, type.TypeRef, Create(Context, Symbol.OriginalDefinition));

var getter = Symbol.GetMethod;
var setter = Symbol.SetMethod;
var getter = BodyDeclaringSymbol.GetMethod;
var setter = BodyDeclaringSymbol.SetMethod;

if (!(getter is null))
if (getter is not null)
Method.Create(Context, getter);

if (!(setter is null))
if (setter is not null)
Method.Create(Context, setter);

var declSyntaxReferences = IsSourceDeclaration ?
Expand Down
4 changes: 4 additions & 0 deletions csharp/ql/lib/change-notes/2025-01-22-partial-members.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* C# 13: Added support for partial properties and indexers.
38 changes: 38 additions & 0 deletions csharp/ql/test/library-tests/dataflow/fields/D.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,41 @@ public static void Sink(object o) { }

static T Source<T>(object source) => throw null;
}

public partial class DPartial
{
private object _backingField;
public partial object PartialProp1
{
get { return _backingField; }
set { _backingField = value; }
}

public partial object PartialProp2
{
get { return null; }
set { }
}
}

public partial class DPartial
{
public partial object PartialProp1 { get; set; }
public partial object PartialProp2 { get; set; }

public void M()
{
var o = Source<object>(1);

var d = new DPartial();
d.PartialProp1 = o;
d.PartialProp2 = o;

Sink(d.PartialProp1); // $ hasValueFlow=1
Sink(d.PartialProp2); // no flow
}

public static void Sink(object o) { }

static T Source<T>(object source) => throw null;
}
56 changes: 56 additions & 0 deletions csharp/ql/test/library-tests/dataflow/fields/FieldFlow.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,30 @@ edges
| D.cs:47:14:47:14 | access to local variable d : D [field trivialPropField] : Object | D.cs:14:9:14:11 | this : D [field trivialPropField] : Object | provenance | |
| D.cs:47:14:47:14 | access to local variable d : D [field trivialPropField] : Object | D.cs:47:14:47:26 | access to property ComplexProp | provenance | |
| D.cs:47:14:47:14 | access to local variable d : D [field trivialPropField] : Object | D.cs:47:14:47:26 | access to property ComplexProp | provenance | |
| D.cs:60:9:60:11 | this : DPartial [field _backingField] : Object | D.cs:60:22:60:34 | this access : DPartial [field _backingField] : Object | provenance | |
| D.cs:60:9:60:11 | this : DPartial [field _backingField] : Object | D.cs:60:22:60:34 | this access : DPartial [field _backingField] : Object | provenance | |
| D.cs:60:22:60:34 | this access : DPartial [field _backingField] : Object | D.cs:60:22:60:34 | access to field _backingField : Object | provenance | |
| D.cs:60:22:60:34 | this access : DPartial [field _backingField] : Object | D.cs:60:22:60:34 | access to field _backingField : Object | provenance | |
| D.cs:61:9:61:11 | value : Object | D.cs:61:31:61:35 | access to parameter value : Object | provenance | |
| D.cs:61:9:61:11 | value : Object | D.cs:61:31:61:35 | access to parameter value : Object | provenance | |
| D.cs:61:15:61:27 | [post] this access : DPartial [field _backingField] : Object | D.cs:61:9:61:11 | this [Return] : DPartial [field _backingField] : Object | provenance | |
| D.cs:61:15:61:27 | [post] this access : DPartial [field _backingField] : Object | D.cs:61:9:61:11 | this [Return] : DPartial [field _backingField] : Object | provenance | |
| D.cs:61:31:61:35 | access to parameter value : Object | D.cs:61:15:61:27 | [post] this access : DPartial [field _backingField] : Object | provenance | |
| D.cs:61:31:61:35 | access to parameter value : Object | D.cs:61:15:61:27 | [post] this access : DPartial [field _backingField] : Object | provenance | |
| D.cs:78:13:78:13 | access to local variable o : Object | D.cs:81:26:81:26 | access to local variable o : Object | provenance | |
| D.cs:78:13:78:13 | access to local variable o : Object | D.cs:81:26:81:26 | access to local variable o : Object | provenance | |
| D.cs:78:17:78:33 | call to method Source<Object> : Object | D.cs:78:13:78:13 | access to local variable o : Object | provenance | |
| D.cs:78:17:78:33 | call to method Source<Object> : Object | D.cs:78:13:78:13 | access to local variable o : Object | provenance | |
| D.cs:81:9:81:9 | [post] access to local variable d : DPartial [field _backingField] : Object | D.cs:84:14:84:14 | access to local variable d : DPartial [field _backingField] : Object | provenance | |
| D.cs:81:9:81:9 | [post] access to local variable d : DPartial [field _backingField] : Object | D.cs:84:14:84:14 | access to local variable d : DPartial [field _backingField] : Object | provenance | |
| D.cs:81:26:81:26 | access to local variable o : Object | D.cs:61:9:61:11 | value : Object | provenance | |
| D.cs:81:26:81:26 | access to local variable o : Object | D.cs:61:9:61:11 | value : Object | provenance | |
| D.cs:81:26:81:26 | access to local variable o : Object | D.cs:81:9:81:9 | [post] access to local variable d : DPartial [field _backingField] : Object | provenance | |
| D.cs:81:26:81:26 | access to local variable o : Object | D.cs:81:9:81:9 | [post] access to local variable d : DPartial [field _backingField] : Object | provenance | |
| D.cs:84:14:84:14 | access to local variable d : DPartial [field _backingField] : Object | D.cs:60:9:60:11 | this : DPartial [field _backingField] : Object | provenance | |
| D.cs:84:14:84:14 | access to local variable d : DPartial [field _backingField] : Object | D.cs:60:9:60:11 | this : DPartial [field _backingField] : Object | provenance | |
| D.cs:84:14:84:14 | access to local variable d : DPartial [field _backingField] : Object | D.cs:84:14:84:27 | access to property PartialProp1 | provenance | |
| D.cs:84:14:84:14 | access to local variable d : DPartial [field _backingField] : Object | D.cs:84:14:84:27 | access to property PartialProp1 | provenance | |
| E.cs:8:29:8:29 | o : Object | E.cs:11:21:11:21 | access to parameter o : Object | provenance | |
| E.cs:8:29:8:29 | o : Object | E.cs:11:21:11:21 | access to parameter o : Object | provenance | |
| E.cs:11:9:11:11 | [post] access to local variable ret : S [field Field] : Object | E.cs:12:16:12:18 | access to local variable ret : S [field Field] : Object | provenance | |
Expand Down Expand Up @@ -1745,6 +1769,32 @@ nodes
| D.cs:47:14:47:14 | access to local variable d : D [field trivialPropField] : Object | semmle.label | access to local variable d : D [field trivialPropField] : Object |
| D.cs:47:14:47:26 | access to property ComplexProp | semmle.label | access to property ComplexProp |
| D.cs:47:14:47:26 | access to property ComplexProp | semmle.label | access to property ComplexProp |
| D.cs:60:9:60:11 | this : DPartial [field _backingField] : Object | semmle.label | this : DPartial [field _backingField] : Object |
| D.cs:60:9:60:11 | this : DPartial [field _backingField] : Object | semmle.label | this : DPartial [field _backingField] : Object |
| D.cs:60:22:60:34 | access to field _backingField : Object | semmle.label | access to field _backingField : Object |
| D.cs:60:22:60:34 | access to field _backingField : Object | semmle.label | access to field _backingField : Object |
| D.cs:60:22:60:34 | this access : DPartial [field _backingField] : Object | semmle.label | this access : DPartial [field _backingField] : Object |
| D.cs:60:22:60:34 | this access : DPartial [field _backingField] : Object | semmle.label | this access : DPartial [field _backingField] : Object |
| D.cs:61:9:61:11 | this [Return] : DPartial [field _backingField] : Object | semmle.label | this [Return] : DPartial [field _backingField] : Object |
| D.cs:61:9:61:11 | this [Return] : DPartial [field _backingField] : Object | semmle.label | this [Return] : DPartial [field _backingField] : Object |
| D.cs:61:9:61:11 | value : Object | semmle.label | value : Object |
| D.cs:61:9:61:11 | value : Object | semmle.label | value : Object |
| D.cs:61:15:61:27 | [post] this access : DPartial [field _backingField] : Object | semmle.label | [post] this access : DPartial [field _backingField] : Object |
| D.cs:61:15:61:27 | [post] this access : DPartial [field _backingField] : Object | semmle.label | [post] this access : DPartial [field _backingField] : Object |
| D.cs:61:31:61:35 | access to parameter value : Object | semmle.label | access to parameter value : Object |
| D.cs:61:31:61:35 | access to parameter value : Object | semmle.label | access to parameter value : Object |
| D.cs:78:13:78:13 | access to local variable o : Object | semmle.label | access to local variable o : Object |
| D.cs:78:13:78:13 | access to local variable o : Object | semmle.label | access to local variable o : Object |
| D.cs:78:17:78:33 | call to method Source<Object> : Object | semmle.label | call to method Source<Object> : Object |
| D.cs:78:17:78:33 | call to method Source<Object> : Object | semmle.label | call to method Source<Object> : Object |
| D.cs:81:9:81:9 | [post] access to local variable d : DPartial [field _backingField] : Object | semmle.label | [post] access to local variable d : DPartial [field _backingField] : Object |
| D.cs:81:9:81:9 | [post] access to local variable d : DPartial [field _backingField] : Object | semmle.label | [post] access to local variable d : DPartial [field _backingField] : Object |
| D.cs:81:26:81:26 | access to local variable o : Object | semmle.label | access to local variable o : Object |
| D.cs:81:26:81:26 | access to local variable o : Object | semmle.label | access to local variable o : Object |
| D.cs:84:14:84:14 | access to local variable d : DPartial [field _backingField] : Object | semmle.label | access to local variable d : DPartial [field _backingField] : Object |
| D.cs:84:14:84:14 | access to local variable d : DPartial [field _backingField] : Object | semmle.label | access to local variable d : DPartial [field _backingField] : Object |
| D.cs:84:14:84:27 | access to property PartialProp1 | semmle.label | access to property PartialProp1 |
| D.cs:84:14:84:27 | access to property PartialProp1 | semmle.label | access to property PartialProp1 |
| E.cs:8:29:8:29 | o : Object | semmle.label | o : Object |
| E.cs:8:29:8:29 | o : Object | semmle.label | o : Object |
| E.cs:11:9:11:11 | [post] access to local variable ret : S [field Field] : Object | semmle.label | [post] access to local variable ret : S [field Field] : Object |
Expand Down Expand Up @@ -2582,6 +2632,10 @@ subpaths
| D.cs:45:14:45:14 | access to local variable d : D [field trivialPropField] : Object | D.cs:8:9:8:11 | this : D [field trivialPropField] : Object | D.cs:8:22:8:42 | access to field trivialPropField : Object | D.cs:45:14:45:26 | access to property TrivialProp |
| D.cs:47:14:47:14 | access to local variable d : D [field trivialPropField] : Object | D.cs:14:9:14:11 | this : D [field trivialPropField] : Object | D.cs:14:22:14:42 | access to field trivialPropField : Object | D.cs:47:14:47:26 | access to property ComplexProp |
| D.cs:47:14:47:14 | access to local variable d : D [field trivialPropField] : Object | D.cs:14:9:14:11 | this : D [field trivialPropField] : Object | D.cs:14:22:14:42 | access to field trivialPropField : Object | D.cs:47:14:47:26 | access to property ComplexProp |
| D.cs:81:26:81:26 | access to local variable o : Object | D.cs:61:9:61:11 | value : Object | D.cs:61:9:61:11 | this [Return] : DPartial [field _backingField] : Object | D.cs:81:9:81:9 | [post] access to local variable d : DPartial [field _backingField] : Object |
| D.cs:81:26:81:26 | access to local variable o : Object | D.cs:61:9:61:11 | value : Object | D.cs:61:9:61:11 | this [Return] : DPartial [field _backingField] : Object | D.cs:81:9:81:9 | [post] access to local variable d : DPartial [field _backingField] : Object |
| D.cs:84:14:84:14 | access to local variable d : DPartial [field _backingField] : Object | D.cs:60:9:60:11 | this : DPartial [field _backingField] : Object | D.cs:60:22:60:34 | access to field _backingField : Object | D.cs:84:14:84:27 | access to property PartialProp1 |
| D.cs:84:14:84:14 | access to local variable d : DPartial [field _backingField] : Object | D.cs:60:9:60:11 | this : DPartial [field _backingField] : Object | D.cs:60:22:60:34 | access to field _backingField : Object | D.cs:84:14:84:27 | access to property PartialProp1 |
| E.cs:23:25:23:25 | access to local variable o : Object | E.cs:8:29:8:29 | o : Object | E.cs:12:16:12:18 | access to local variable ret : S [field Field] : Object | E.cs:23:17:23:26 | call to method CreateS : S [field Field] : Object |
| E.cs:23:25:23:25 | access to local variable o : Object | E.cs:8:29:8:29 | o : Object | E.cs:12:16:12:18 | access to local variable ret : S [field Field] : Object | E.cs:23:17:23:26 | call to method CreateS : S [field Field] : Object |
| E.cs:55:29:55:33 | access to local variable taint : Object | E.cs:43:46:43:46 | o : Object | E.cs:43:36:43:36 | s [Return] : RefS [field RefField] : Object | E.cs:55:23:55:26 | [post] access to local variable refs : RefS [field RefField] : Object |
Expand Down Expand Up @@ -2690,6 +2744,8 @@ testFailures
| D.cs:46:14:46:31 | access to field trivialPropField | D.cs:43:32:43:48 | call to method Source<Object> : Object | D.cs:46:14:46:31 | access to field trivialPropField | $@ | D.cs:43:32:43:48 | call to method Source<Object> : Object | call to method Source<Object> : Object |
| D.cs:47:14:47:26 | access to property ComplexProp | D.cs:43:32:43:48 | call to method Source<Object> : Object | D.cs:47:14:47:26 | access to property ComplexProp | $@ | D.cs:43:32:43:48 | call to method Source<Object> : Object | call to method Source<Object> : Object |
| D.cs:47:14:47:26 | access to property ComplexProp | D.cs:43:32:43:48 | call to method Source<Object> : Object | D.cs:47:14:47:26 | access to property ComplexProp | $@ | D.cs:43:32:43:48 | call to method Source<Object> : Object | call to method Source<Object> : Object |
| D.cs:84:14:84:27 | access to property PartialProp1 | D.cs:78:17:78:33 | call to method Source<Object> : Object | D.cs:84:14:84:27 | access to property PartialProp1 | $@ | D.cs:78:17:78:33 | call to method Source<Object> : Object | call to method Source<Object> : Object |
| D.cs:84:14:84:27 | access to property PartialProp1 | D.cs:78:17:78:33 | call to method Source<Object> : Object | D.cs:84:14:84:27 | access to property PartialProp1 | $@ | D.cs:78:17:78:33 | call to method Source<Object> : Object | call to method Source<Object> : Object |
| E.cs:24:14:24:20 | access to field Field | E.cs:22:17:22:33 | call to method Source<Object> : Object | E.cs:24:14:24:20 | access to field Field | $@ | E.cs:22:17:22:33 | call to method Source<Object> : Object | call to method Source<Object> : Object |
| E.cs:24:14:24:20 | access to field Field | E.cs:22:17:22:33 | call to method Source<Object> : Object | E.cs:24:14:24:20 | access to field Field | $@ | E.cs:22:17:22:33 | call to method Source<Object> : Object | call to method Source<Object> : Object |
| E.cs:57:14:57:26 | access to field RefField | E.cs:54:21:54:37 | call to method Source<Object> : Object | E.cs:57:14:57:26 | access to field RefField | $@ | E.cs:54:21:54:37 | call to method Source<Object> : Object | call to method Source<Object> : Object |
Expand Down
Loading