diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/ElementAccess.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/ElementAccess.cs index 345e691a8a85..6f8321321855 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/ElementAccess.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/ElementAccess.cs @@ -1,4 +1,5 @@ using System.IO; +using System.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp.Syntax; using Semmle.Extraction.Kinds; @@ -17,6 +18,35 @@ protected ElementAccess(ExpressionNodeInfo info, ExpressionSyntax qualifier, Bra private readonly ExpressionSyntax qualifier; private readonly BracketedArgumentListSyntax argumentList; + + private IPropertySymbol? GetIndexerSymbol() + { + var symbol = Context.GetSymbolInfo(base.Syntax).Symbol; + + if (symbol is IPropertySymbol { IsIndexer: true } indexer) + return indexer; + + // In some cases, Roslyn translates the use of range expressions directly into method calls. + // E.g. `a[0..3]` is translated into `a.Slice(0, 3)`, if `a` is a `Span`. + // In this case, we want to populate the indexer access as normal (as this reflects the source code more accurately). + if (symbol is IMethodSymbol method) + { + var indexers = method + .ContainingType + .GetMembers() + .OfType() + .Where(p => p.IsIndexer); + + var intIndexer = indexers + .Where(i => i.Parameters.Length == 1 && i.Parameters[0].Type.SpecialType == SpecialType.System_Int32) + .FirstOrDefault(); + + return intIndexer; + } + + return null; + } + protected override void PopulateExpression(TextWriter trapFile) { if (Kind == ExprKind.POINTER_INDIRECTION) @@ -32,9 +62,8 @@ protected override void PopulateExpression(TextWriter trapFile) Create(Context, qualifier, this, -1); PopulateArguments(trapFile, argumentList, 0); - var symbolInfo = Context.GetSymbolInfo(base.Syntax); - - if (symbolInfo.Symbol is IPropertySymbol indexer) + var indexer = GetIndexerSymbol(); + if (indexer is not null) { trapFile.expr_access(this, Indexer.Create(Context, indexer)); } diff --git a/csharp/ql/lib/change-notes/2026-05-21-spanaccess-range.md b/csharp/ql/lib/change-notes/2026-05-21-spanaccess-range.md new file mode 100644 index 000000000000..b7bc97b9a94e --- /dev/null +++ b/csharp/ql/lib/change-notes/2026-05-21-spanaccess-range.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Improved extraction of span range-access expressions (for example, `a[0..3]`) so the indexer is recorded as the call target. diff --git a/csharp/ql/lib/change-notes/2026-05-22-property-indexer-partial-override.md b/csharp/ql/lib/change-notes/2026-05-22-property-indexer-partial-override.md new file mode 100644 index 000000000000..4be78a49c1f0 --- /dev/null +++ b/csharp/ql/lib/change-notes/2026-05-22-property-indexer-partial-override.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Improved property and indexer call target resolution for partially overridden properties and indexers. diff --git a/csharp/ql/lib/semmle/code/csharp/Property.qll b/csharp/ql/lib/semmle/code/csharp/Property.qll index c9a338d0359f..50b151694dea 100644 --- a/csharp/ql/lib/semmle/code/csharp/Property.qll +++ b/csharp/ql/lib/semmle/code/csharp/Property.qll @@ -57,6 +57,34 @@ class DeclarationWithGetSetAccessors extends DeclarationWithAccessors, TopLevelE /** Gets the `set` accessor of this declaration, if any. */ Setter getSetter() { result = this.getAnAccessor() } + /** Gets the target `get` accessor of this declaration, if any. */ + private Getter getFirstGetter() { + if exists(this.getGetter()) + then result = this.getGetter() + else result = this.getOverridee().getFirstGetter() + } + + /** Gets the target accessor of this declaration when used in a read context, if any. */ + Accessor getReadTarget() { result = this.getFirstGetter() } + + /** Gets the target `set` accessor of this declaration, if any. */ + private Setter getFirstSetter() { + if exists(this.getSetter()) + then result = this.getSetter() + else result = this.getOverridee().getFirstSetter() + } + + /** Gets the target accessor of this declaration when used in a write context, if any. */ + Accessor getWriteTarget() { + result = this.getFirstSetter() + or + result = + any(Getter g | + g = this.getFirstGetter() and + g.getAnnotatedReturnType().isRef() + ) + } + override DeclarationWithGetSetAccessors getOverridee() { result = DeclarationWithAccessors.super.getOverridee() } diff --git a/csharp/ql/lib/semmle/code/csharp/exprs/Call.qll b/csharp/ql/lib/semmle/code/csharp/exprs/Call.qll index a358e73970c1..941f6666b281 100644 --- a/csharp/ql/lib/semmle/code/csharp/exprs/Call.qll +++ b/csharp/ql/lib/semmle/code/csharp/exprs/Call.qll @@ -762,20 +762,12 @@ class AccessorCall extends Call, QualifiableExpr, @call_access_expr { */ class PropertyCall extends AccessorCall, PropertyAccessExpr { override Accessor getReadTarget() { - this instanceof AssignableRead and result = this.getProperty().getGetter() + this instanceof AssignableRead and result = this.getProperty().getReadTarget() } override Accessor getWriteTarget() { this instanceof AssignableWrite and - exists(Property p | p = this.getProperty() | - result = p.getSetter() - or - result = - any(Getter g | - g = p.getGetter() and - g.getAnnotatedReturnType().isRef() - ) - ) + result = this.getProperty().getWriteTarget() } override Expr getArgument(int i) { @@ -806,20 +798,12 @@ class PropertyCall extends AccessorCall, PropertyAccessExpr { */ class IndexerCall extends AccessorCall, IndexerAccessExpr { override Accessor getReadTarget() { - this instanceof AssignableRead and result = this.getIndexer().getGetter() + this instanceof AssignableRead and result = this.getIndexer().getReadTarget() } override Accessor getWriteTarget() { this instanceof AssignableWrite and - exists(Indexer i | i = this.getIndexer() | - result = i.getSetter() - or - result = - any(Getter g | - g = i.getGetter() and - g.getAnnotatedReturnType().isRef() - ) - ) + result = this.getIndexer().getWriteTarget() } override Expr getArgument(int i) { diff --git a/csharp/ql/src/Telemetry/DatabaseQuality.qll b/csharp/ql/src/Telemetry/DatabaseQuality.qll index ad7ac682bf5c..a26993905dee 100644 --- a/csharp/ql/src/Telemetry/DatabaseQuality.qll +++ b/csharp/ql/src/Telemetry/DatabaseQuality.qll @@ -63,7 +63,7 @@ module CallTargetStats implements StatsSig { additional predicate isNotOkCall(Call c) { not exists(c.getTarget()) and - not c instanceof DelegateCall and + not c instanceof DelegateLikeCall and not c instanceof DynamicExpr and not isNoSetterPropertyCallInConstructor(c) and not isNoSetterPropertyInitialization(c) and diff --git a/csharp/ql/test/library-tests/properties/PrintAst.expected b/csharp/ql/test/library-tests/properties/PrintAst.expected index ef482ed33d06..d07cc484c716 100644 --- a/csharp/ql/test/library-tests/properties/PrintAst.expected +++ b/csharp/ql/test/library-tests/properties/PrintAst.expected @@ -293,3 +293,69 @@ properties.cs: # 160| 0: [LocalVariableAccess] access to local variable x # 160| 1: [PropertyCall] access to property Prop # 160| -1: [LocalVariableAccess] access to local variable s +# 164| 13: [Class] BaseClass +# 166| 6: [Property] Value +# 166| -1: [TypeMention] int +# 168| 3: [Getter] get_Value +# 168| 4: [BlockStmt] {...} +# 168| 0: [ReturnStmt] return ...; +# 168| 0: [FieldAccess] access to field Value.field +# 169| 4: [Setter] set_Value +#-----| 2: (Parameters) +# 169| 0: [Parameter] value +# 169| 4: [BlockStmt] {...} +# 169| 0: [ExprStmt] ...; +# 169| 0: [AssignExpr] ... = ... +# 169| 0: [FieldAccess] access to field Value.field +# 169| 1: [ParameterAccess] access to parameter value +# 166| 7: [Field] Value.field +# 173| 14: [Class] DerivedClass1 +#-----| 3: (Base types) +# 173| 0: [TypeMention] BaseClass +# 175| 6: [Property] Value +# 175| -1: [TypeMention] int +# 177| 3: [Getter] get_Value +# 177| 4: [BlockStmt] {...} +# 177| 0: [ReturnStmt] return ...; +# 177| 0: [IntLiteral] 20 +# 181| 15: [Class] DerivedClass2 +#-----| 3: (Base types) +# 181| 0: [TypeMention] BaseClass +# 183| 16: [Class] TestPartialPropertyOverride +# 185| 6: [Method] M +# 185| -1: [TypeMention] Void +# 186| 4: [BlockStmt] {...} +# 187| 0: [LocalVariableDeclStmt] ... ...; +# 187| 0: [LocalVariableDeclAndInitExpr] DerivedClass1 d1 = ... +# 187| -1: [TypeMention] DerivedClass1 +# 187| 0: [LocalVariableAccess] access to local variable d1 +# 187| 1: [ObjectCreation] object creation of type DerivedClass1 +# 187| 0: [TypeMention] DerivedClass1 +# 188| 1: [ExprStmt] ...; +# 188| 0: [AssignExpr] ... = ... +# 188| 0: [PropertyCall] access to property Value +# 188| -1: [LocalVariableAccess] access to local variable d1 +# 188| 1: [IntLiteral] 11 +# 189| 2: [LocalVariableDeclStmt] ... ...; +# 189| 0: [LocalVariableDeclAndInitExpr] Int32 test1 = ... +# 189| -1: [TypeMention] int +# 189| 0: [LocalVariableAccess] access to local variable test1 +# 189| 1: [PropertyCall] access to property Value +# 189| -1: [LocalVariableAccess] access to local variable d1 +# 191| 3: [LocalVariableDeclStmt] ... ...; +# 191| 0: [LocalVariableDeclAndInitExpr] DerivedClass2 d2 = ... +# 191| -1: [TypeMention] DerivedClass2 +# 191| 0: [LocalVariableAccess] access to local variable d2 +# 191| 1: [ObjectCreation] object creation of type DerivedClass2 +# 191| 0: [TypeMention] DerivedClass2 +# 192| 4: [ExprStmt] ...; +# 192| 0: [AssignExpr] ... = ... +# 192| 0: [PropertyCall] access to property Value +# 192| -1: [LocalVariableAccess] access to local variable d2 +# 192| 1: [IntLiteral] 12 +# 193| 5: [LocalVariableDeclStmt] ... ...; +# 193| 0: [LocalVariableDeclAndInitExpr] Int32 test2 = ... +# 193| -1: [TypeMention] int +# 193| 0: [LocalVariableAccess] access to local variable test2 +# 193| 1: [PropertyCall] access to property Value +# 193| -1: [LocalVariableAccess] access to local variable d2 diff --git a/csharp/ql/test/library-tests/properties/Properties17.expected b/csharp/ql/test/library-tests/properties/Properties17.expected index 74efae145f78..7e031d39aaff 100644 --- a/csharp/ql/test/library-tests/properties/Properties17.expected +++ b/csharp/ql/test/library-tests/properties/Properties17.expected @@ -1,4 +1,5 @@ | Prop.field | +| Value.field | | caption | | next | | x | diff --git a/csharp/ql/test/library-tests/properties/Properties19.expected b/csharp/ql/test/library-tests/properties/Properties19.expected index 7c027119067b..0c2ba9c8ceb3 100644 --- a/csharp/ql/test/library-tests/properties/Properties19.expected +++ b/csharp/ql/test/library-tests/properties/Properties19.expected @@ -6,3 +6,7 @@ | properties.cs:71:28:71:28 | Y | properties.cs:83:39:83:44 | access to property Y | properties.cs:74:13:74:15 | set_Y | | properties.cs:146:24:146:27 | Prop | properties.cs:159:13:159:18 | access to property Prop | properties.cs:148:13:148:15 | get_Prop | | properties.cs:146:24:146:27 | Prop | properties.cs:160:21:160:26 | access to property Prop | properties.cs:148:13:148:15 | get_Prop | +| properties.cs:166:28:166:32 | Value | properties.cs:192:13:192:20 | access to property Value | properties.cs:169:13:169:15 | set_Value | +| properties.cs:166:28:166:32 | Value | properties.cs:193:25:193:32 | access to property Value | properties.cs:168:13:168:15 | get_Value | +| properties.cs:175:29:175:33 | Value | properties.cs:188:13:188:20 | access to property Value | properties.cs:169:13:169:15 | set_Value | +| properties.cs:175:29:175:33 | Value | properties.cs:189:25:189:32 | access to property Value | properties.cs:177:13:177:15 | get_Value | diff --git a/csharp/ql/test/library-tests/properties/properties.cs b/csharp/ql/test/library-tests/properties/properties.cs index 391245e3497e..f2f72638838c 100644 --- a/csharp/ql/test/library-tests/properties/properties.cs +++ b/csharp/ql/test/library-tests/properties/properties.cs @@ -160,4 +160,37 @@ public void M() var x = s.Prop; } } + + public class BaseClass + { + public virtual int Value + { + get { return field; } + set { field = value; } + } + } + + public class DerivedClass1 : BaseClass + { + public override int Value + { + get { return 20; } + } + } + + public class DerivedClass2 : BaseClass { } + + public class TestPartialPropertyOverride + { + public void M() + { + var d1 = new DerivedClass1(); + d1.Value = 11; + var test1 = d1.Value; + + var d2 = new DerivedClass2(); + d2.Value = 12; + var test2 = d2.Value; + } + } } diff --git a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/IsNotOkayCall.expected b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/IsNotOkayCall.expected index dcdb8b09058c..e69de29bb2d1 100644 --- a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/IsNotOkayCall.expected +++ b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/IsNotOkayCall.expected @@ -1,2 +0,0 @@ -| Quality.cs:26:19:26:26 | access to indexer | Call without target $@. | Quality.cs:26:19:26:26 | access to indexer | access to indexer | -| Quality.cs:29:21:29:27 | access to indexer | Call without target $@. | Quality.cs:29:21:29:27 | access to indexer | access to indexer | diff --git a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/NoTarget.expected b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/NoTarget.expected index a76dd08cdb6b..b96815507f14 100644 --- a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/NoTarget.expected +++ b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/NoTarget.expected @@ -7,7 +7,5 @@ | Quality.cs:20:13:20:23 | access to property MyProperty6 | Call without target $@. | Quality.cs:20:13:20:23 | access to property MyProperty6 | access to property MyProperty6 | | Quality.cs:23:9:23:14 | access to event Event1 | Call without target $@. | Quality.cs:23:9:23:14 | access to event Event1 | access to event Event1 | | Quality.cs:23:9:23:30 | delegate call | Call without target $@. | Quality.cs:23:9:23:30 | delegate call | delegate call | -| Quality.cs:26:19:26:26 | access to indexer | Call without target $@. | Quality.cs:26:19:26:26 | access to indexer | access to indexer | -| Quality.cs:29:21:29:27 | access to indexer | Call without target $@. | Quality.cs:29:21:29:27 | access to indexer | access to indexer | | Quality.cs:38:16:38:26 | access to property MyProperty2 | Call without target $@. | Quality.cs:38:16:38:26 | access to property MyProperty2 | access to property MyProperty2 | | Quality.cs:50:20:50:26 | object creation of type T | Call without target $@. | Quality.cs:50:20:50:26 | object creation of type T | object creation of type T | diff --git a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/Quality.cs b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/Quality.cs index e10ce10f6c4b..648083edad8d 100644 --- a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/Quality.cs +++ b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/Quality.cs @@ -23,10 +23,10 @@ public Test() Event1.Invoke(this, 5); var str = "abcd"; - var sub = str[..3]; // TODO: this is not an indexer call, but rather a `str.Substring(0, 3)` call. + var sub = str[..3]; Span sp = null; - var slice = sp[..3]; // TODO: this is not an indexer call, but rather a `sp.Slice(0, 3)` call. + var slice = sp[..3]; Span guidBytes = stackalloc byte[16]; guidBytes[08] = 1;