C#: Exclude more property access expressions from DB quality metric

Author: tamasvajk

csharp/ql/src/Telemetry/DatabaseQuality.qll

@@ -32,27 +32,64 @@ module ReportStats<StatsSig Stats> {
   }
 }
 
-private predicate isNoSetterPropertyCallInConstructor(PropertyCall c) {
-  exists(Property p |
-    p = c.getProperty() and
-    not exists(Setter a | a = p.getAnAccessor()) and
-    c.getEnclosingCallable().(Constructor).getDeclaringType().getASubType*() = p.getDeclaringType()
-  )
-}
-
 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 and
-        not isNoSetterPropertyCallInConstructor(c)
+  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/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; }
+}