Skip to content

C#: Reduce location tuple extraction for named types. #20581

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

Merged
michaelnebel merged 10 commits into from
Oct 7, 2025
Merged

C#: Reduce location tuple extraction for named types. #20581

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
f2b45b8
C#: Add type locations test.
michaelnebel Sep 30, 2025
0cd7c37
C#: Avoid extracting duplicate type locations.
michaelnebel Sep 30, 2025
b5592ad
C#: Add partial class example.
michaelnebel Oct 2, 2025
12dc65d
C#: For implicit constructors, pick a unique source location as repor…
michaelnebel Oct 3, 2025
0ba9b80
C#: Add some more partial class related tests.
michaelnebel Oct 3, 2025
7b61a5f
C#: Update some test expected output that uses the location of the im…
michaelnebel Oct 3, 2025
b362b46
C#: Add some examples with nested types.
michaelnebel Oct 6, 2025
6149608
C#: Always use the unbound type declaration location for type location.
michaelnebel Oct 6, 2025
f9d62a0
C#: Narrow the use of unbound declaration locations to nested types.
michaelnebel Oct 7, 2025
584d8c5
C#: Add change-note.
michaelnebel Oct 7, 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
3 changes: 2 additions & 1 deletion csharp/extractor/Semmle.Extraction.CSharp/Entities/Constructor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,8 @@ public override Microsoft.CodeAnalysis.Location? ReportingLocation

if (Symbol.IsImplicitlyDeclared)
{
return ContainingType!.ReportingLocation;
var best = Symbol.Locations.Where(l => l.IsInSource).BestOrDefault();
return best ?? ContainingType!.ReportingLocation;
}

return Symbol.ContainingType.Locations.FirstOrDefault();
Expand Down
13 changes: 8 additions & 5 deletions csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/NamedType.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,15 +111,18 @@ public override IEnumerable<Location> Locations
}
}

private static IEnumerable<Microsoft.CodeAnalysis.Location> GetLocations(INamedTypeSymbol type)
private IEnumerable<Microsoft.CodeAnalysis.Location> GetLocations(INamedTypeSymbol type)
{
return type.Locations
.Where(l => l.IsInMetadata)
.Concat(type.DeclaringSyntaxReferences
var metadataLocations = type.Locations
.Where(l => l.IsInMetadata);
var sourceLocations = type.DeclaringSyntaxReferences
.Select(loc => loc.GetSyntax())
.OfType<CSharpSyntaxNode>()
.Select(l => l.FixedLocation())
);
.Where(Context.IsLocationInContext);

return metadataLocations
.Concat(sourceLocations);
}

public override Microsoft.CodeAnalysis.Location? ReportingLocation => GetLocations(Symbol).BestOrDefault();
Expand Down
7 changes: 5 additions & 2 deletions csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/TypeParameter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,12 @@ public override void Populate(TextWriter trapFile)
var parentNs = Namespace.Create(Context, Symbol.TypeParameterKind == TypeParameterKind.Method ? Context.Compilation.GlobalNamespace : Symbol.ContainingNamespace);
trapFile.parent_namespace(this, parentNs);

foreach (var l in Symbol.Locations)
if (Context.ExtractLocation(Symbol))
{
WriteLocationToTrap(trapFile.type_location, this, Context.CreateLocation(l));
foreach (var l in Symbol.Locations)
{
WriteLocationToTrap(trapFile.type_location, this, Context.CreateLocation(l));
}
}

if (IsSourceDeclaration)
Expand Down
3 changes: 3 additions & 0 deletions csharp/extractor/Semmle.Extraction.CSharp/Extractor/Context.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,9 @@ public bool ExtractLocation(ISymbol symbol) =>
SymbolEqualityComparer.Default.Equals(symbol, symbol.OriginalDefinition) &&
scope.InScope(symbol);

public bool IsLocationInContext(Location location) =>
location.SourceTree == SourceTree;

/// <summary>
/// Runs the given action <paramref name="a"/>, guarding for trap duplication
/// based on key <paramref name="key"/>.
Expand Down
4 changes: 4 additions & 0 deletions csharp/ql/lib/change-notes/2025-10-07-entity-locations.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The extraction of location information for named types (classes, structs, etc.) has been optimized. Previously, location information was extracted multiple times for each type when it was declared across multiple files. Now, the extraction context is respected during the extraction phase, ensuring locations are only extracted within the appropriate context. This change should be transparent to end-users but may improve extraction performance in some cases.
2 changes: 2 additions & 0 deletions csharp/ql/lib/semmle/code/csharp/Type.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,8 @@ class NestedType extends ValueOrRefType {
NestedType() { nested_types(this, _, _) }

override ValueOrRefType getDeclaringType() { nested_types(this, result, _) }

override Location getALocation() { type_location(this.getUnboundDeclaration(), result) }
}

/**
Expand Down
8 changes: 4 additions & 4 deletions csharp/ql/test/library-tests/csharp8/NullableRefTypes.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,8 @@ returnTypes
| NullableRefTypes.cs:51:12:51:15 | Q<MyClass> | object |
| NullableRefTypes.cs:51:12:51:15 | Q<MyClass> | object! |
| NullableRefTypes.cs:51:12:51:15 | Q`1 | object! |
| NullableRefTypes.cs:54:11:54:33 | Generic | Void! |
| NullableRefTypes.cs:58:11:58:26 | Generic2 | Void! |
| NullableRefTypes.cs:54:11:54:17 | Generic | Void! |
| NullableRefTypes.cs:58:11:58:18 | Generic2 | Void! |
| NullableRefTypes.cs:67:10:67:21 | GenericFn<MyClass> | Void |
| NullableRefTypes.cs:67:10:67:21 | GenericFn<MyClass> | Void! |
| NullableRefTypes.cs:67:10:67:21 | GenericFn`1 | Void! |
Expand Down Expand Up @@ -271,8 +271,8 @@ expressionTypes
| NullableRefTypes.cs:40:26:40:30 | ref ... | MyClass |
| NullableRefTypes.cs:40:30:40:30 | access to local variable b | MyClass? |
| NullableRefTypes.cs:51:44:51:47 | null | null |
| NullableRefTypes.cs:54:11:54:33 | call to constructor Object | object |
| NullableRefTypes.cs:58:11:58:26 | call to constructor Object | object |
| NullableRefTypes.cs:54:11:54:17 | call to constructor Object | object |
| NullableRefTypes.cs:58:11:58:18 | call to constructor Object | object |
| NullableRefTypes.cs:73:18:73:18 | access to local variable x | MyClass! |
| NullableRefTypes.cs:73:18:73:25 | MyClass x = ... | MyClass! |
| NullableRefTypes.cs:73:22:73:25 | null | null |
Expand Down
8 changes: 4 additions & 4 deletions csharp/ql/test/library-tests/csharp9/withExpr.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ withExpr
| Record.cs:77:21:77:31 | ... with { ... } | Person1 | Record.cs:77:21:77:22 | access to local variable p1 | Record.cs:77:29:77:31 | { ..., ... } | Person1.<Clone>$() |
| Record.cs:84:16:84:33 | ... with { ... } | R1 | Record.cs:84:16:84:16 | access to local variable b | Record.cs:84:23:84:33 | { ..., ... } | R1.<Clone>$() |
withTarget
| Record.cs:75:18:75:47 | ... with { ... } | Record.cs:27:1:27:57 | <Clone>$ | Record.cs:27:1:27:57 | Person1 |
| Record.cs:76:18:76:81 | ... with { ... } | Record.cs:29:1:30:35 | <Clone>$ | Record.cs:29:1:30:35 | Teacher1 |
| Record.cs:77:21:77:31 | ... with { ... } | Record.cs:27:1:27:57 | <Clone>$ | Record.cs:27:1:27:57 | Person1 |
| Record.cs:84:16:84:33 | ... with { ... } | Record.cs:54:1:54:39 | <Clone>$ | Record.cs:54:1:54:39 | R1 |
| Record.cs:75:18:75:47 | ... with { ... } | Record.cs:27:1:27:57 | <Clone>$ | Record.cs:27:15:27:21 | Person1 |
| Record.cs:76:18:76:81 | ... with { ... } | Record.cs:29:1:30:35 | <Clone>$ | Record.cs:29:15:29:22 | Teacher1 |
| Record.cs:77:21:77:31 | ... with { ... } | Record.cs:27:1:27:57 | <Clone>$ | Record.cs:27:15:27:21 | Person1 |
| Record.cs:84:16:84:33 | ... with { ... } | Record.cs:54:1:54:39 | <Clone>$ | Record.cs:54:24:54:25 | R1 |
cloneOverrides
| Person1.<Clone>$() | Student1.<Clone>$() |
| Person1.<Clone>$() | Teacher1.<Clone>$() |
Expand Down
4 changes: 2 additions & 2 deletions csharp/ql/test/library-tests/expressions/ConstructorInitializers.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,6 @@
| file://:0:0:0:0 | TestCreations | expressions.cs:383:18:383:30 | call to constructor Object | file://:0:0:0:0 | Object |
| file://:0:0:0:0 | TestUnaryOperator | expressions.cs:292:11:292:27 | call to constructor Object | file://:0:0:0:0 | Object |
| file://:0:0:0:0 | TupleExprs | expressions.cs:501:11:501:20 | call to constructor Object | file://:0:0:0:0 | Object |
| file://:0:0:0:0 | X | expressions.cs:108:15:108:18 | call to constructor Object | file://:0:0:0:0 | Object |
| file://:0:0:0:0 | X | expressions.cs:108:15:108:15 | call to constructor Object | file://:0:0:0:0 | Object |
| file://:0:0:0:0 | X | expressions.cs:216:18:216:18 | call to constructor Object | file://:0:0:0:0 | Object |
| file://:0:0:0:0 | Y | expressions.cs:104:15:104:21 | call to constructor Object | file://:0:0:0:0 | Object |
| file://:0:0:0:0 | Y | expressions.cs:104:15:104:15 | call to constructor Object | file://:0:0:0:0 | Object |
6 changes: 6 additions & 0 deletions csharp/ql/test/library-tests/locations/Base.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
public abstract class Base<T>
{
public void M() { }

public class InnerBase { }
}
1 change: 1 addition & 0 deletions csharp/ql/test/library-tests/locations/Multiple1.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
public partial class Multiple { }
1 change: 1 addition & 0 deletions csharp/ql/test/library-tests/locations/Multiple2.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
public partial class Multiple { }
8 changes: 8 additions & 0 deletions csharp/ql/test/library-tests/locations/Sub.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
public class Sub : Base<int>
{
public void SubM()
{
M();
var x = new InnerBase();
}
}
31 changes: 31 additions & 0 deletions csharp/ql/test/library-tests/locations/locations.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,12 @@ member_locations
| B.cs:3:14:3:14 | B | B.cs:7:25:7:28 | Item | B.cs:7:25:7:28 | B.cs:7:25:7:28 |
| B.cs:3:14:3:14 | B | B.cs:13:40:13:44 | Event | B.cs:13:40:13:44 | B.cs:13:40:13:44 |
| B.cs:3:14:3:14 | B | B.cs:19:28:19:35 | ToObject | B.cs:19:28:19:35 | B.cs:19:28:19:35 |
| Base.cs:1:23:1:29 | Base<Int32> | Base.cs:3:17:3:17 | M | Base.cs:3:17:3:17 | Base.cs:3:17:3:17 |
| Base.cs:1:23:1:29 | Base<Int32> | Base.cs:5:18:5:26 | InnerBase | Base.cs:5:18:5:26 | Base.cs:5:18:5:26 |
| Base.cs:1:23:1:29 | Base`1 | Base.cs:3:17:3:17 | M | Base.cs:3:17:3:17 | Base.cs:3:17:3:17 |
| Base.cs:1:23:1:29 | Base`1 | Base.cs:5:18:5:26 | InnerBase | Base.cs:5:18:5:26 | Base.cs:5:18:5:26 |
| C.cs:3:7:3:7 | C | C.cs:5:17:5:17 | M | C.cs:5:17:5:17 | C.cs:5:17:5:17 |
| Sub.cs:1:14:1:16 | Sub | Sub.cs:3:17:3:20 | SubM | Sub.cs:3:17:3:20 | Sub.cs:3:17:3:20 |
accessor_location
| A.cs:3:23:3:26 | A<Int32> | A.cs:5:30:5:32 | get_Prop | A.cs:5:30:5:32 | A.cs:5:30:5:32 |
| A.cs:3:23:3:26 | A<Int32> | A.cs:6:41:6:43 | get_Item | A.cs:6:41:6:43 | A.cs:6:41:6:43 |
Expand All @@ -50,3 +55,29 @@ accessor_location
| B.cs:3:14:3:14 | B | B.cs:10:9:10:11 | set_Item | B.cs:10:9:10:11 | B.cs:10:9:10:11 |
| B.cs:3:14:3:14 | B | B.cs:15:9:15:11 | add_Event | B.cs:15:9:15:11 | B.cs:15:9:15:11 |
| B.cs:3:14:3:14 | B | B.cs:16:9:16:14 | remove_Event | B.cs:16:9:16:14 | B.cs:16:9:16:14 |
type_location
| A.cs:3:23:3:26 | A<Int32> | A.cs:3:23:3:26 | A.cs:3:23:3:26 |
| A.cs:3:23:3:26 | A<String> | A.cs:3:23:3:26 | A.cs:3:23:3:26 |
| A.cs:3:23:3:26 | A`1 | A.cs:3:23:3:26 | A.cs:3:23:3:26 |
| A.cs:3:25:3:25 | T | A.cs:3:25:3:25 | A.cs:3:25:3:25 |
| A.cs:12:14:12:15 | A2 | A.cs:12:14:12:15 | A.cs:12:14:12:15 |
| B.cs:3:14:3:14 | B | B.cs:3:14:3:14 | B.cs:3:14:3:14 |
| Base.cs:1:23:1:29 | Base<Int32> | Base.cs:1:23:1:29 | Base.cs:1:23:1:29 |
| Base.cs:1:23:1:29 | Base`1 | Base.cs:1:23:1:29 | Base.cs:1:23:1:29 |
| Base.cs:1:28:1:28 | T | Base.cs:1:28:1:28 | Base.cs:1:28:1:28 |
| Base.cs:5:18:5:26 | InnerBase | Base.cs:5:18:5:26 | Base.cs:5:18:5:26 |
| Base.cs:5:18:5:26 | InnerBase | Base.cs:5:18:5:26 | Base.cs:5:18:5:26 |
| C.cs:3:7:3:7 | C | C.cs:3:7:3:7 | C.cs:3:7:3:7 |
| Multiple1.cs:1:22:1:29 | Multiple | Multiple1.cs:1:22:1:29 | Multiple1.cs:1:22:1:29 |
| Multiple1.cs:1:22:1:29 | Multiple | Multiple2.cs:1:22:1:29 | Multiple2.cs:1:22:1:29 |
| Multiple2.cs:1:22:1:29 | Multiple | Multiple1.cs:1:22:1:29 | Multiple1.cs:1:22:1:29 |
| Multiple2.cs:1:22:1:29 | Multiple | Multiple2.cs:1:22:1:29 | Multiple2.cs:1:22:1:29 |
| Sub.cs:1:14:1:16 | Sub | Sub.cs:1:14:1:16 | Sub.cs:1:14:1:16 |
calltype_location
| A.cs:12:14:12:15 | call to constructor A | A.cs:3:23:3:26 | A<String> | A.cs:3:23:3:26 | A.cs:3:23:3:26 |
| A.cs:32:20:32:24 | object creation of type A2 | A.cs:12:14:12:15 | A2 | A.cs:12:14:12:15 | A.cs:12:14:12:15 |
| B.cs:3:14:3:14 | call to constructor A | A.cs:3:23:3:26 | A<Int32> | A.cs:3:23:3:26 | A.cs:3:23:3:26 |
| C.cs:7:15:7:21 | object creation of type B | B.cs:3:14:3:14 | B | B.cs:3:14:3:14 | B.cs:3:14:3:14 |
| C.cs:9:17:9:24 | object creation of type A2 | A.cs:12:14:12:15 | A2 | A.cs:12:14:12:15 | A.cs:12:14:12:15 |
| Sub.cs:1:14:1:16 | call to constructor Base | Base.cs:1:23:1:29 | Base<Int32> | Base.cs:1:23:1:29 | Base.cs:1:23:1:29 |
| Sub.cs:6:17:6:31 | object creation of type InnerBase | Base.cs:5:18:5:26 | InnerBase | Base.cs:5:18:5:26 | Base.cs:5:18:5:26 |
9 changes: 9 additions & 0 deletions csharp/ql/test/library-tests/locations/locations.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,12 @@ query predicate accessor_location(Type t, Accessor a, SourceLocation l) {
l = a.getLocation() and
not l instanceof EmptyLocation
}

query predicate type_location(Type t, SourceLocation l) {
l = t.getLocation() and not l instanceof EmptyLocation
}

query predicate calltype_location(Call call, Type t, SourceLocation l) {
t = call.getType() and
l = t.getALocation()
}
2 changes: 2 additions & 0 deletions csharp/ql/test/library-tests/partial/Partial1.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,5 @@
| Partial.cs:28:9:28:11 | set_Item |
| Partial.cs:32:15:32:33 | OnePartPartialClass |
| Partial.cs:34:18:34:42 | PartialMethodWithoutBody2 |
| PartialMultipleFiles1.cs:1:22:1:41 | PartialMultipleFiles |
| PartialMultipleFiles2.cs:1:22:1:41 | PartialMultipleFiles |
4 changes: 4 additions & 0 deletions csharp/ql/test/library-tests/partial/PartialConstructors.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
| Partial.cs:1:15:1:26 | TwoPartClass | Partial.cs:1:15:1:26 | {...} |
| Partial.cs:32:15:32:33 | OnePartPartialClass | Partial.cs:32:15:32:33 | {...} |
| Partial.cs:38:7:38:21 | NonPartialClass | Partial.cs:38:7:38:21 | {...} |
| PartialMultipleFiles1.cs:1:22:1:41 | PartialMultipleFiles | PartialMultipleFiles1.cs:1:22:1:41 | {...} |
5 changes: 5 additions & 0 deletions csharp/ql/test/library-tests/partial/PartialConstructors.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import csharp

from Constructor c
where c.getDeclaringType().fromSource()
select c, c.getBody()
1 change: 1 addition & 0 deletions csharp/ql/test/library-tests/partial/PartialMultipleFiles1.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
public partial class PartialMultipleFiles { }
1 change: 1 addition & 0 deletions csharp/ql/test/library-tests/partial/PartialMultipleFiles2.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
public partial class PartialMultipleFiles { }
4 changes: 4 additions & 0 deletions csharp/ql/test/library-tests/partial/PrintAst.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,7 @@ Partial.cs:
# 42| 0: [Parameter] index
# 45| 1: [Parameter] value
# 45| 4: [BlockStmt] {...}
PartialMultipleFiles1.cs:
# 1| [Class] PartialMultipleFiles
PartialMultipleFiles2.cs:
# 1| [Class] PartialMultipleFiles