Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Improved property and indexer call target resolution for partially overridden properties and indexers.
14 changes: 14 additions & 0 deletions csharp/ql/lib/semmle/code/csharp/Property.qll
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,20 @@ 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. */
Getter getFirstGetter() {
if exists(this.getGetter())
then result = this.getGetter()
else result = this.getOverridee().getFirstGetter()
}

/** Gets the target `set` accessor of this declaration, if any. */
Setter getFirstSetter() {
if exists(this.getSetter())
then result = this.getSetter()
else result = this.getOverridee().getFirstSetter()
}

override DeclarationWithGetSetAccessors getOverridee() {
result = DeclarationWithAccessors.super.getOverridee()
}
Expand Down
8 changes: 4 additions & 4 deletions csharp/ql/lib/semmle/code/csharp/exprs/Call.qll
Original file line number Diff line number Diff line change
Expand Up @@ -762,11 +762,11 @@ 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().getFirstGetter()
}

override Accessor getWriteTarget() {
this instanceof AssignableWrite and result = this.getProperty().getSetter()
this instanceof AssignableWrite and result = this.getProperty().getFirstSetter()
}

override Expr getArgument(int i) {
Expand Down Expand Up @@ -797,11 +797,11 @@ 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().getFirstGetter()
}

override Accessor getWriteTarget() {
this instanceof AssignableWrite and result = this.getIndexer().getSetter()
this instanceof AssignableWrite and result = this.getIndexer().getFirstSetter()
}

override Expr getArgument(int i) {
Expand Down
2 changes: 1 addition & 1 deletion csharp/ql/src/Telemetry/DatabaseQuality.qll
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
66 changes: 66 additions & 0 deletions csharp/ql/test/library-tests/properties/PrintAst.expected
Original file line number Diff line number Diff line change
Expand Up @@ -246,3 +246,69 @@ properties.cs:
# 133| 0: [FieldAccess] access to field Prop.field
# 133| 1: [ParameterAccess] access to parameter value
# 130| 7: [Field] Prop.field
# 137| 11: [Class] BaseClass
# 139| 6: [Property] Value
# 139| -1: [TypeMention] int
# 141| 3: [Getter] get_Value
# 141| 4: [BlockStmt] {...}
# 141| 0: [ReturnStmt] return ...;
# 141| 0: [FieldAccess] access to field Value.field
# 142| 4: [Setter] set_Value
#-----| 2: (Parameters)
# 142| 0: [Parameter] value
# 142| 4: [BlockStmt] {...}
# 142| 0: [ExprStmt] ...;
# 142| 0: [AssignExpr] ... = ...
# 142| 0: [FieldAccess] access to field Value.field
# 142| 1: [ParameterAccess] access to parameter value
# 139| 7: [Field] Value.field
# 146| 12: [Class] DerivedClass1
#-----| 3: (Base types)
# 146| 0: [TypeMention] BaseClass
# 148| 6: [Property] Value
# 148| -1: [TypeMention] int
# 150| 3: [Getter] get_Value
# 150| 4: [BlockStmt] {...}
# 150| 0: [ReturnStmt] return ...;
# 150| 0: [IntLiteral] 20
# 154| 13: [Class] DerivedClass2
#-----| 3: (Base types)
# 154| 0: [TypeMention] BaseClass
# 155| 14: [Class] Test2
# 157| 6: [Method] M
# 157| -1: [TypeMention] Void
# 158| 4: [BlockStmt] {...}
# 159| 0: [LocalVariableDeclStmt] ... ...;
# 159| 0: [LocalVariableDeclAndInitExpr] DerivedClass1 d1 = ...
# 159| -1: [TypeMention] DerivedClass1
# 159| 0: [LocalVariableAccess] access to local variable d1
# 159| 1: [ObjectCreation] object creation of type DerivedClass1
# 159| 0: [TypeMention] DerivedClass1
# 160| 1: [ExprStmt] ...;
# 160| 0: [AssignExpr] ... = ...
# 160| 0: [PropertyCall] access to property Value
# 160| -1: [LocalVariableAccess] access to local variable d1
# 160| 1: [IntLiteral] 11
# 161| 2: [LocalVariableDeclStmt] ... ...;
# 161| 0: [LocalVariableDeclAndInitExpr] Int32 test1 = ...
# 161| -1: [TypeMention] int
# 161| 0: [LocalVariableAccess] access to local variable test1
# 161| 1: [PropertyCall] access to property Value
# 161| -1: [LocalVariableAccess] access to local variable d1
# 163| 3: [LocalVariableDeclStmt] ... ...;
# 163| 0: [LocalVariableDeclAndInitExpr] DerivedClass2 d2 = ...
# 163| -1: [TypeMention] DerivedClass2
# 163| 0: [LocalVariableAccess] access to local variable d2
# 163| 1: [ObjectCreation] object creation of type DerivedClass2
# 163| 0: [TypeMention] DerivedClass2
# 164| 4: [ExprStmt] ...;
# 164| 0: [AssignExpr] ... = ...
# 164| 0: [PropertyCall] access to property Value
# 164| -1: [LocalVariableAccess] access to local variable d2
# 164| 1: [IntLiteral] 12
# 165| 5: [LocalVariableDeclStmt] ... ...;
# 165| 0: [LocalVariableDeclAndInitExpr] Int32 test2 = ...
# 165| -1: [TypeMention] int
# 165| 0: [LocalVariableAccess] access to local variable test2
# 165| 1: [PropertyCall] access to property Value
# 165| -1: [LocalVariableAccess] access to local variable d2
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
| Prop.field |
| Value.field |
| caption |
| next |
| y |
Expand Down
10 changes: 10 additions & 0 deletions csharp/ql/test/library-tests/properties/Properties19.expected
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
| properties.cs:29:13:29:28 | access to property Caption | properties.cs:17:13:17:15 | set_Caption |
| properties.cs:30:24:30:39 | access to property Caption | properties.cs:15:13:15:15 | get_Caption |
| properties.cs:61:13:61:13 | access to property X | properties.cs:57:37:57:39 | set_X |
| properties.cs:62:13:62:13 | access to property Y | properties.cs:58:37:58:39 | set_Y |
| properties.cs:82:46:82:51 | access to property X | properties.cs:70:32:70:34 | get_X |
| properties.cs:83:39:83:44 | access to property Y | properties.cs:74:13:74:15 | set_Y |
| properties.cs:160:13:160:20 | access to property Value | properties.cs:142:13:142:15 | set_Value |
| properties.cs:161:25:161:32 | access to property Value | properties.cs:150:13:150:15 | get_Value |
| properties.cs:164:13:164:20 | access to property Value | properties.cs:142:13:142:15 | set_Value |
| properties.cs:165:25:165:32 | access to property Value | properties.cs:141:13:141:15 | get_Value |
5 changes: 5 additions & 0 deletions csharp/ql/test/library-tests/properties/Properties19.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import csharp

from PropertyCall pc, Accessor a
where a = pc.getTarget() and a.fromSource()
select pc, a
32 changes: 32 additions & 0 deletions csharp/ql/test/library-tests/properties/properties.cs
Original file line number Diff line number Diff line change
Expand Up @@ -133,4 +133,36 @@ public object Prop
set { field = value; }
}
}

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 Test2
{
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;
}
}
}
Loading