C#: Extract indexer when accessing a span with a range.#21877
C#: Extract indexer when accessing a span with a range.#21877michaelnebel wants to merge 7 commits into
Conversation
4eec760 to
9167814
Compare
There was a problem hiding this comment.
Pull request overview
This PR improves the C# extractor’s handling of element-access expressions that use range syntax (for example, a[0..3]) when Roslyn lowers them into method calls (for example, Slice(...)). The goal is to preserve the source-level “indexer access” shape by still recording an indexer as the call target, improving database-quality telemetry.
Changes:
- Added extractor special-casing to recover an indexer symbol even when Roslyn binds the element access to a method symbol.
- Updated the Telemetry/DatabaseQuality query test inputs and expected outputs to reflect the improved extraction.
- Added a C# library change note documenting the analysis-impacting extraction improvement.
Show a summary per file
| File | Description |
|---|---|
| csharp/ql/test/query-tests/Telemetry/DatabaseQuality/Quality.cs | Updates test code to treat range element access as properly-targeted indexer calls. |
| csharp/ql/test/query-tests/Telemetry/DatabaseQuality/NoTarget.expected | Removes expected “missing call target” results now that targets are extracted. |
| csharp/ql/test/query-tests/Telemetry/DatabaseQuality/IsNotOkayCall.expected | Eliminates expected “not ok” call-target results for the fixed cases. |
| csharp/ql/lib/change-notes/2026-05-21-spanaccess-range.md | Documents the extraction change as a minor analysis improvement. |
| csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/ElementAccess.cs | Implements the new symbol recovery logic for range-based element access. |
Copilot's findings
- Files reviewed: 5/5 changed files
- Comments generated: 1
| if (symbol is IMethodSymbol method) | ||
| { | ||
| var indexers = method | ||
| .ContainingType | ||
| .GetMembers() | ||
| .OfType<IPropertySymbol>() | ||
| .Where(p => p.IsIndexer); | ||
|
|
||
| var intIndexer = indexers | ||
| .Where(i => i.Parameters.Length == 1 && i.Parameters[0].Type.SpecialType == SpecialType.System_Int32) | ||
| .FirstOrDefault(); | ||
|
|
||
| return intIndexer; | ||
| } |
There was a problem hiding this comment.
I think it is reasonable to assume that Roslyn only translates a[range] into a method call for integer range expressions (and thus we implicitly want to extract the indexer with an int parameter).
hvitved
left a comment
There was a problem hiding this comment.
I'm not convinced that extracting the indexer is what we want; what about extracting the Roslyn-resolved targets instead and treating the index expressions as ordinary method calls?
Yes, I also considered that, but thought it would be better to extract something that more closely resembles the syntax instead of what it is being translated into. If I were to query our code and asking for uses of indexers, then I would expect cases like |
9167814 to
ee734c9
Compare
| | Slice.cs:15:22:15:28 | call to method Slice | Slice(int, int) | 0 | 0 | | ||
| | Slice.cs:15:22:15:28 | call to method Slice | Slice(int, int) | 1 | 6 | | ||
| | Slice.cs:16:22:16:28 | call to method Slice | Slice(int, int) | 0 | 7 | | ||
| | Slice.cs:16:22:16:28 | call to method Slice | Slice(int, int) | 1 | access to property Length - 7 | |
There was a problem hiding this comment.
Shouldn't this be Length - 7 + 1?
| | Slice.cs:14:22:14:29 | call to method Slice | Slice(int, int) | 0 | 5 | | ||
| | Slice.cs:14:22:14:29 | call to method Slice | Slice(int, int) | 1 | access to parameter a - 5 | | ||
| | Slice.cs:15:22:15:28 | call to method Slice | Slice(int, int) | 0 | 0 | | ||
| | Slice.cs:15:22:15:28 | call to method Slice | Slice(int, int) | 1 | 6 | |
There was a problem hiding this comment.
I believe the length arguments are correct in the mentioned cases.
The semantics of s[a..b] means: Take the elements from s from index a up to and NOT including the element at index b (semantics of s[..b] is similar).
Example
var s = "Hello";
var sub = s[1..3];then sub is el and it has length 3 - 1 = 2.
| @@ -0,0 +1,22 @@ | |||
| methodCalls | |||
| | Slice.cs:8:20:8:26 | call to method Substring | Substring(int, int) | 0 | 1 | | |||
| | Slice.cs:8:20:8:26 | call to method Substring | Substring(int, int) | 1 | access to parameter a - 1 | | |||
There was a problem hiding this comment.
The second parameter of Substring is also a length, similar to the Slice method.
| | Slice.cs:11:20:11:26 | call to method Substring | Substring(int, int) | 0 | 0 | | ||
| | Slice.cs:11:20:11:26 | call to method Substring | Substring(int, int) | 1 | access to property Length - 4 | | ||
| | Slice.cs:14:22:14:29 | call to method Slice | Slice(int, int) | 0 | 5 | | ||
| | Slice.cs:14:22:14:29 | call to method Slice | Slice(int, int) | 1 | access to parameter a - 5 | |
There was a problem hiding this comment.
Shouldn't this be a - 5 + 1? For example, if a is 6 then the length should be 2.
…guments, when using indexers in conjunction with ranges on spans and strings.
ee734c9 to
8d0575c
Compare
| switch (range.LeftOperand, range.RightOperand) | ||
| { | ||
| case (ExpressionSyntax lsyntax, ExpressionSyntax rsyntax): |
| /// <summary> | ||
| /// Determines whether the given method is a slice method, which is defined as a method with | ||
| /// the name "Slice" or "SubString" and two parameters. | ||
| /// </summary> <param name="method">The method symbol to check.</param> | ||
| /// <returns>True if the method is a slice method, false otherwise.</returns> |
| --- | ||
| category: minorAnalysis | ||
| --- | ||
| * Improved extraction of span range-access expressions (for example, `a[0..3]`). These expressions are now extracted as span `Slice` calls. |
Improves the extraction of span access with range expressions (such as
a[1..3]).Roslyn translates
a[1..3]intoa.Slice(1,2)for spans (and something similar for strings).In this PR we introduce some special handling in the extractor for such cases
awe extract aa.Slicemethod call.a.Slice. Note, that the semantics ofa[1..3]means the elements fromastarting from index1up to but NOT including3. That, is the correspondingSlicecall fora[1..3]isa.Slice(1, 3 - 1).Roslyn translates indexing on spans as follows
a[x..y]is translated toa.Slice(x, y - x).a[..y]is translated toa.Slice(0, y)a[x..]is translated toa.Slice(x, a.Length - x).Furthermore, it is possible to use read from the end as well (e.g.
^2).a[x..^y]translates intoa.Slice(x, a.Length - y - x).and similar for the other constructs (that is, we can replace
^ywithLength - y).This means that synthesizing
Slicecalls in the extractor requires that we synthesize0literal.-expressions..Lengthproperty calls.Similar to span access with range expressions, it is also possible to use range expressions in conjunction with strings. If
sis a string, thens[1..3]translates intos.Substring(1, 3 - 1). This is also handled in this PR.