Apply suggestions from code review

Author: tiferet

javascript/ql/experimental/adaptivethreatmodeling/test/endpoint_large_scale/ContradictoryEndpointCharacteristics.expected

@@ -0,0 +1,2 @@
+erroneousEndpoints
+erroneousConfidences

javascript/ql/experimental/adaptivethreatmodeling/test/endpoint_large_scale/ContradictoryEndpointCharacteristics.ql

@@ -10,6 +10,36 @@ import javascript
 private import experimental.adaptivethreatmodeling.EndpointCharacteristics as EndpointCharacteristics
 private import experimental.adaptivethreatmodeling.EndpointTypes as EndpointTypes
 
+/**
+ * Holds if `characteristic1` and `characteristic2` are among the several pairs of currently known high-confidence
+ * negative characteristics that apply to some known sinks.
+ *
+ * TODO: Experiment with lowering the confidence of `"FileSystemAccess"`, `"DOM"`, `"DatabaseAccess"`, and
+ * `"JQueryArgument"`.
+ */
+private predicate knownContradictoryCharacteristics(
+  EndpointCharacteristics::EndpointCharacteristic characteristic1,
+  EndpointCharacteristics::EndpointCharacteristic characteristic2
+) {
+  characteristic1 != characteristic2 and
+  (
+    characteristic1 = ["TaintedPathSink", "FileSystemAccess"] and
+    characteristic2 = ["TaintedPathSink", "FileSystemAccess"]
+    or
+    characteristic1 = ["DomBasedXssSink", "DOM"] and
+    characteristic2 = ["DomBasedXssSink", "DOM"]
+    or
+    characteristic1 = ["DomBasedXssSink", "JQueryArgument"] and
+    characteristic2 = ["DomBasedXssSink", "JQueryArgument"]
+    or
+    characteristic1 = ["NosqlInjectionSink", "DatabaseAccess"] and
+    characteristic2 = ["NosqlInjectionSink", "DatabaseAccess"]
+    or
+    characteristic1 = ["SqlInjectionSink", "DatabaseAccess"] and
+    characteristic2 = ["SqlInjectionSink", "DatabaseAccess"]
+  )
+}
+
 /**
  * Holds if the given endpoint has a self-contradictory combination of characteristics. Detects errors in our endpoint
  * characteristics. Lists the problematic characterisitics and their implications for all such endpoints, together with
@@ -33,24 +63,7 @@ query predicate erroneousEndpoints(
     confidence > characteristic.mediumConfidence() and
     confidence2 > characteristic2.mediumConfidence() and
     // We currently know of several high-confidence negative characteristics that apply to some known sinks.
-    // TODO: Experiment with lowering the confidence of `"FileSystemAccess"`, `"DOM"`, `"DatabaseAccess"`, and
-    // `"JQueryArgument"`.
-    not (
-      characteristic = ["TaintedPathSink", "FileSystemAccess"] and
-      characteristic2 = ["TaintedPathSink", "FileSystemAccess"]
-      or
-      characteristic = ["DomBasedXssSink", "DOM"] and
-      characteristic2 = ["DomBasedXssSink", "DOM"]
-      or
-      characteristic = ["DomBasedXssSink", "JQueryArgument"] and
-      characteristic2 = ["DomBasedXssSink", "JQueryArgument"]
-      or
-      characteristic = ["NosqlInjectionSink", "DatabaseAccess"] and
-      characteristic2 = ["NosqlInjectionSink", "DatabaseAccess"]
-      or
-      characteristic = ["SqlInjectionSink", "DatabaseAccess"] and
-      characteristic2 = ["SqlInjectionSink", "DatabaseAccess"]
-    )
+    not knownContradictoryCharacteristics(characteristic, characteristic2)
   ) and
   errorMessage = "Endpoint has high-confidence positive indicators for multiple classes"
   or
@@ -65,10 +78,13 @@ query predicate erroneousEndpoints(
     confidence2 > characteristic2.mediumConfidence()
   ) and
   errorMessage = "Endpoint has high-confidence positive and negative indicators for the same class"
-  or
-  // The endpoint's characteristics should not include indicators with confidence outside of [0, 1].
-  characteristic.appliesToEndpoint(endpoint) and
+}
+
+query predicate erroneousConfidences(
+  EndpointCharacteristics::EndpointCharacteristic characteristic, float confidence,
+  string errorMessage
+) {
   characteristic.hasImplications(_, _, confidence) and
   (confidence < 0 or confidence > 1) and
-  errorMessage = "Endpoint has an indicator with confidence outside of [0, 1]"
+  errorMessage = "Characteristic has an indicator with confidence outside of [0, 1]"
 }