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; + } + } }