Shared: Also take query ID into account in PathProblemSourceTestInput

Author: hvitved

csharp/ql/test/TestUtilities/inline-tests/PathProblemQuery.expected

@@ -21,3 +21,11 @@ testFailures
 | InlineTests.cs:34:18:34:25 | "Source" | Unexpected result: Source |
 | InlineTests.cs:35:16:35:21 | "Sink" | Unexpected result: Sink |
 | InlineTests.cs:36:13:36:23 | InlineTests.cs:34:18:34:25 | Unexpected result: Alert |
+| InlineTests.cs:58:16:58:21 | "Sink" | Unexpected result: Sink=source2 |
+| InlineTests.cs:58:24:58:60 | // ... | Missing result: Sink[path-problem-query]=source1 |
+| InlineTests.cs:64:13:64:23 | InlineTests.cs:62:18:62:25 | Unexpected result: Alert=source3 |
+| InlineTests.cs:64:26:64:63 | // ... | Missing result: Alert[path-problem-query]=source2 |
+| InlineTests.cs:72:13:72:23 | "Alert:1:0" | Unexpected result: Alert=source5 |
+| InlineTests.cs:72:26:72:63 | // ... | Missing result: Alert[path-problem-query]=source4 |
+| InlineTests.cs:79:16:79:21 | "Sink" | Unexpected result: Sink=sink1 |
+| InlineTests.cs:79:24:79:58 | // ... | Missing result: Sink[path-problem-query]=sink2 |

shared/util/codeql/util/test/InlineExpectationsTest.qll

@@ -717,13 +717,44 @@ module TestPostProcessing {
       )
     }
 
+    private string getTagRegex() {
+      exists(string sourceSinkTags |
+        (
+          getQueryKind() = "problem"
+          or
+          not exists(getSourceTag(_)) and
+          not exists(getSinkTag(_))
+        ) and
+        sourceSinkTags = ""
+        or
+        sourceSinkTags = "|" + getSourceTag(_) + "|" + getSinkTag(_)
+      |
+        result = "(Alert" + sourceSinkTags + ")(\\[(.*)\\])?"
+      )
+    }
+
     /**
      * A configuration for matching `// $ Source=foo` comments against actual
      * path-problem sources.
+     *
+     * Whenever a source is tagged with a value, like `foo`, we will use that
+     * to define the expected tags at the sink and the alert.
      */
     private module PathProblemSourceTestInput implements TestSig {
       string getARelevantTag() { result = getSourceTag(_) }
 
+      bindingset[expectedTag, actualTag]
+      predicate tagMatches(string expectedTag, string actualTag) {
+        actualTag = expectedTag.regexpCapture(getTagRegex(), 1) and
+        (
+          // expected tag is annotated with a query ID
+          getQueryId() = expectedTag.regexpCapture(getTagRegex(), 3)
+          or
+          // expected tag is not annotated with a query ID
+          not exists(expectedTag.regexpCapture(getTagRegex(), 3))
+        )
+      }
+
       bindingset[expectedValue, actualValue]
       predicate valueMatches(string expectedValue, string actualValue) {
         exists(expectedValue) and
@@ -754,28 +785,7 @@ module TestPostProcessing {
       bindingset[result]
       string getARelevantTag() { any() }
 
-      private string getTagRegex() {
-        exists(string sourceSinkTags |
-          getQueryKind() = "problem" and
-          sourceSinkTags = ""
-          or
-          sourceSinkTags = "|" + getSourceTag(_) + "|" + getSinkTag(_)
-        |
-          result = "(Alert" + sourceSinkTags + ")(\\[(.*)\\])?"
-        )
-      }
-
-      bindingset[expectedTag, actualTag]
-      predicate tagMatches(string expectedTag, string actualTag) {
-        actualTag = expectedTag.regexpCapture(getTagRegex(), 1) and
-        (
-          // expected tag is annotated with a query ID
-          getQueryId() = expectedTag.regexpCapture(getTagRegex(), 3)
-          or
-          // expected tag is not annotated with a query ID
-          not exists(expectedTag.regexpCapture(getTagRegex(), 3))
-        )
-      }
+      predicate tagMatches = PathProblemSourceTestInput::tagMatches/2;
 
       bindingset[expectedTag]
       predicate tagIsOptional(string expectedTag) {
@@ -789,64 +799,58 @@ module TestPostProcessing {
         )
       }
 
-      bindingset[expectedValue, actualValue]
-      predicate valueMatches(string expectedValue, string actualValue) {
-        expectedValue = actualValue
-        or
-        actualValue = ""
-      }
-
       private predicate hasPathProblemSource = PathProblemSourceTestInput::hasPathProblemSource/5;
 
-      /**
-       * Gets the expected sink value for result row `row`. This value must
-       * match the value at the corresponding path-problem source (if it is
-       * present).
-       */
-      private string getSinkValue(int row) {
-        exists(Input::Location location, string element, string tag, string val |
-          hasPathProblemSource(row, location, element, tag, val) and
-          result =
-            PathProblemSourceTest::getAMatchingExpectation(location, element, tag, val, false)
-                .getValue()
-        )
-      }
-
       private predicate hasPathProblemSink(
-        int row, Input::Location location, string element, string tag, string value
+        int row, Input::Location location, string element, string tag
       ) {
         getQueryKind() = "path-problem" and
         exists(string loc |
           queryResults("#select", row, 4, loc) and
           queryResults("#select", row, 5, element) and
           tag = getSinkTag(row) and
           Input2::getRelativeUrl(location) = loc
-        |
-          not exists(getSinkValue(row)) and value = ""
-          or
-          value = getSinkValue(row)
         )
       }
 
-      private predicate hasAlert(Input::Location location, string element, string tag, string value) {
+      private predicate hasAlert(int row, Input::Location location, string element, string tag) {
         getQueryKind() = ["problem", "path-problem"] and
-        exists(int row, string loc |
+        exists(string loc |
           queryResults("#select", row, 0, loc) and
           queryResults("#select", row, 2, element) and
           tag = "Alert" and
-          value = "" and
           Input2::getRelativeUrl(location) = loc and
           not hasPathProblemSource(row, location, _, _, _) and
-          not hasPathProblemSink(row, location, _, _, _)
+          not hasPathProblemSink(row, location, _, _)
+        )
+      }
+
+      /**
+       * Gets the expected value for result row `row`, if any. This value must
+       * match the value at the corresponding path-problem source (if it is
+       * present).
+       */
+      private string getValue(int row) {
+        exists(Input::Location location, string element, string tag, string val |
+          hasPathProblemSource(row, location, element, tag, val) and
+          result =
+            PathProblemSourceTest::getAMatchingExpectation(location, element, tag, val, false)
+                .getValue()
         )
       }
 
       predicate hasActualResult(Input::Location location, string element, string tag, string value) {
-        hasPathProblemSource(_, location, element, tag, value)
-        or
-        hasPathProblemSink(_, location, element, tag, value)
-        or
-        hasAlert(location, element, tag, value)
+        exists(int row |
+          hasPathProblemSource(row, location, element, tag, _)
+          or
+          hasPathProblemSink(row, location, element, tag)
+          or
+          hasAlert(row, location, element, tag)
+        |
+          not exists(getValue(row)) and value = ""
+          or
+          value = getValue(row)
+        )
       }
     }