Merge pull request github#18141 from tamasvajk/fix/db-quality-query

C#: Exclude `get`-only property accesses from `CallTargetStats`

Author: tamasvajk

csharp/ql/src/Telemetry/DatabaseQuality.qll

@@ -35,15 +35,61 @@ module ReportStats<StatsSig Stats> {
 module CallTargetStats implements StatsSig {
   int getNumberOfOk() { result = count(Call c | exists(c.getTarget())) }
 
-  int getNumberOfNotOk() {
-    result =
-      count(Call c |
-        not exists(c.getTarget()) and
-        not c instanceof DelegateCall and
-        not c instanceof DynamicExpr
+  private predicate isNoSetterPropertyCallInConstructor(PropertyCall c) {
+    exists(Property p, Constructor ctor |
+      p = c.getProperty() and
+      not exists(Setter a | a = p.getAnAccessor()) and
+      c.getEnclosingCallable() = ctor and
+      (
+        c.hasThisQualifier()
+        or
+        ctor instanceof StaticConstructor and p.getDeclaringType() = ctor.getDeclaringType()
       )
+    )
   }
 
+  private predicate isNoSetterPropertyInitialization(PropertyCall c) {
+    exists(Property p, AssignExpr assign |
+      p = c.getProperty() and
+      not exists(Setter a | a = p.getAnAccessor()) and
+      assign = c.getParent() and
+      assign.getLValue() = c and
+      assign.getParent() instanceof Property
+    )
+  }
+
+  private predicate isAnonymousObjectMemberDeclaration(PropertyCall c) {
+    exists(Property p, AssignExpr assign |
+      p = c.getProperty() and
+      assign = c.getParent() and
+      assign.getLValue() = c and
+      assign.getParent() instanceof ObjectInitializer and
+      assign.getParent().getParent() instanceof AnonymousObjectCreation
+    )
+  }
+
+  private predicate isInitializedWithCollectionInitializer(PropertyCall c) {
+    exists(Property p, AssignExpr assign |
+      p = c.getProperty() and
+      assign = c.getParent() and
+      assign.getLValue() = c and
+      assign.getRValue() instanceof CollectionInitializer
+    )
+  }
+
+  additional predicate isNotOkCall(Call c) {
+    not exists(c.getTarget()) and
+    not c instanceof DelegateCall and
+    not c instanceof DynamicExpr and
+    not isNoSetterPropertyCallInConstructor(c) and
+    not isNoSetterPropertyInitialization(c) and
+    not isAnonymousObjectMemberDeclaration(c) and
+    not isInitializedWithCollectionInitializer(c) and
+    not c.getParent+() instanceof NameOfExpr
+  }
+
+  int getNumberOfNotOk() { result = count(Call c | isNotOkCall(c)) }
+
   string getOkText() { result = "calls with call target" }
 
   string getNotOkText() { result = "calls with missing call target" }

csharp/ql/src/change-notes/2024-11-28-db-quality-property-access.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* `csharp/diagnostic/database-quality` has been changed to exclude various property access expressions from database quality evaluation. The excluded property access expressions are expected to have no target callables even in manual or autobuilt databases.

csharp/ql/test/query-tests/Telemetry/DatabaseQuality/IsNotOkayCall.expected

@@ -0,0 +1,12 @@
+/**
+ * @id test/missing-call-target
+ * @kind problem
+ * @problem.severity warning
+ */
+
+import csharp
+import Telemetry.DatabaseQuality
+
+from Call call
+where CallTargetStats::isNotOkCall(call)
+select call, "Call without target $@.", call, call.toString()

csharp/ql/test/query-tests/Telemetry/DatabaseQuality/IsNotOkayCall.ql

@@ -0,0 +1,7 @@
+| Quality.cs:8:9:8:19 | access to property MyProperty5 | Call without target $@. | Quality.cs:8:9:8:19 | access to property MyProperty5 | access to property MyProperty5 |
+| Quality.cs:13:9:13:19 | access to property MyProperty1 | Call without target $@. | Quality.cs:13:9:13:19 | access to property MyProperty1 | access to property MyProperty1 |
+| Quality.cs:14:24:14:34 | access to property MyProperty3 | Call without target $@. | Quality.cs:14:24:14:34 | access to property MyProperty3 | access to property MyProperty3 |
+| Quality.cs:15:24:15:34 | access to property MyProperty3 | Call without target $@. | Quality.cs:15:24:15:34 | access to property MyProperty3 | access to property MyProperty3 |
+| Quality.cs:15:24:15:46 | access to property MyProperty2 | Call without target $@. | Quality.cs:15:24:15:46 | access to property MyProperty2 | access to property MyProperty2 |
+| Quality.cs:19:13:19:23 | access to property MyProperty4 | Call without target $@. | Quality.cs:19:13:19:23 | access to property MyProperty4 | access to property MyProperty4 |
+| Quality.cs:24:16:24:26 | access to property MyProperty2 | Call without target $@. | Quality.cs:24:16:24:26 | access to property MyProperty2 | access to property MyProperty2 |

csharp/ql/test/query-tests/Telemetry/DatabaseQuality/NoTarget.expected

@@ -0,0 +1,12 @@
+/**
+ * @id test/missing-call-target
+ * @kind problem
+ * @problem.severity warning
+ */
+
+import csharp
+import Telemetry.DatabaseQuality
+
+from Call call
+where not exists(call.getTarget())
+select call, "Call without target $@.", call, call.toString()

csharp/ql/test/query-tests/Telemetry/DatabaseQuality/NoTarget.ql

@@ -0,0 +1,28 @@
+using System;
+using System.Collections.Generic;
+
+public class Test
+{
+    static Test()
+    {
+        MyProperty5 = 42;
+    }
+
+    public Test()
+    {
+        MyProperty1 = 42;
+        var x = nameof(MyProperty3);
+        var y = nameof(MyProperty3.MyProperty2);
+
+        new Test()
+        {
+            MyProperty4 = { 1, 2, 3 }
+        };
+    }
+
+    public int MyProperty1 { get; }
+    public int MyProperty2 { get; } = 42;
+    public Test MyProperty3 { get; set; }
+    public List<int> MyProperty4 { get; }
+    static int MyProperty5 { get; }
+}