Merge pull request #18735 from asgerf/inline-test-non-location

asgerf · web-flow · commit 654c6bfec70a · 2025-02-12T10:30:50.000+01:00
Test: Support arbitrary locations in inline test post-processor
diff --git a/javascript/ql/test/query-tests/AngularJS/DuplicateDependency/DuplicateDependency.expected b/javascript/ql/test/query-tests/AngularJS/DuplicateDependency/DuplicateDependency.expected
@@ -2,4 +2,4 @@
 | duplicates.js:6:33:6:56 | functio ... up2b){} | This function has a duplicate dependency $@. | duplicates.js:6:24:6:30 | 'dup2a' | dup2a |
 | duplicates.js:7:33:7:56 | functio ... up3b){} | This function has a duplicate dependency $@. | duplicates.js:7:24:7:30 | 'dup3b' | dup3b |
 | duplicates.js:8:43:8:78 | functio ... up4C){} | This function has a duplicate dependency $@. | duplicates.js:8:35:8:40 | 'dup4' | dup4 |
-| duplicates.js:15:35:15:112 | functio ...       } | This function has a duplicate dependency $@. | duplicates.js:15:25:15:32 | 'dup11a' | dup11a |
+| duplicates.js:15:35:15:113 | functio ...       } | This function has a duplicate dependency $@. | duplicates.js:15:25:15:32 | 'dup11a' | dup11a |
diff --git a/javascript/ql/test/query-tests/AngularJS/DuplicateDependency/DuplicateDependency.qlref b/javascript/ql/test/query-tests/AngularJS/DuplicateDependency/DuplicateDependency.qlref
@@ -1 +1,2 @@
-AngularJS/DuplicateDependency.ql
+query: AngularJS/DuplicateDependency.ql
+postprocess: utils/test/InlineExpectationsTestQuery.ql
diff --git a/javascript/ql/test/query-tests/AngularJS/DuplicateDependency/duplicates.js b/javascript/ql/test/query-tests/AngularJS/DuplicateDependency/duplicates.js
@@ -1,18 +1,18 @@
 (function(){
-    function f(){}
-    f.$inject = ['dup5', 'dup5']; // NOT OK
+    function f(){} // $ Alert
+    f.$inject = ['dup5', 'dup5'];
     angular.module('myModule', [])
-        .run(['dup1a', 'dup1a', function(dup1a, dup1a){}]) // OK (flagged by js/duplicate-parameter-name)
-        .run(['dup2a', 'dup2a', function(dup2a, dup2b){}]) // NOT OK
-        .run(['dup3b', 'dup3b', function(dup3a, dup3b){}]) // NOT OK
-        .run(['dup4', 'notDup4A', 'dup4', function(notDup4B, dup4, notDup4C){}]) // NOT OK
+        .run(['dup1a', 'dup1a', function(dup1a, dup1a){}]) // OK - flagged by js/duplicate-parameter-name
+        .run(['dup2a', 'dup2a', function(dup2a, dup2b){}]) // $ Alert
+        .run(['dup3b', 'dup3b', function(dup3a, dup3b){}]) // $ Alert
+        .run(['dup4', 'notDup4A', 'dup4', function(notDup4B, dup4, notDup4C){}]) // $ Alert
         .run(f)
-        .run(function(dup6, dup6){})// OK (flagged by js/duplicate-parameter-name)
-        .run(function(notDup7a, notDup7b){}) // OK
-        .run(['notDup8a', 'notDup8b', function(notDup8a, notDup8b){}]) // OK
-        .run(['notDup9a', 'notDup9b', function(notDup9c, notDup9d){}]) // OK
-        .run(['dup10a', 'dup10a', 'dup10a', function(dup10a, dup10a, dup10a){}]) // OK (flagged by js/duplicate-parameter-name)
-        .run(['dup11a', 'dup11a', function(dup11a, dup11b){ // NOT OK (alert formatting for multi-line function)
+        .run(function(dup6, dup6){})// OK - flagged by js/duplicate-parameter-name
+        .run(function(notDup7a, notDup7b){})
+        .run(['notDup8a', 'notDup8b', function(notDup8a, notDup8b){}])
+        .run(['notDup9a', 'notDup9b', function(notDup9c, notDup9d){}])
+        .run(['dup10a', 'dup10a', 'dup10a', function(dup10a, dup10a, dup10a){}]) // OK - flagged by js/duplicate-parameter-name
+        .run(['dup11a', 'dup11a', function(dup11a, dup11b){ // $ Alert - alert formatting for multi-line function
         }])
     ;
 })();
diff --git a/shared/util/codeql/util/test/InlineExpectationsTest.qll b/shared/util/codeql/util/test/InlineExpectationsTest.qll
@@ -643,21 +643,112 @@ module TestPostProcessing {
 
   module Make<InlineExpectationsTestSig Input, InputSig<Input> Input2> {
     private import InlineExpectationsTest as InlineExpectationsTest
-    private import InlineExpectationsTest::Make<Input>
+
+    bindingset[loc]
+    private predicate parseLocationString(
+      string loc, string relativePath, int sl, int sc, int el, int ec
+    ) {
+      relativePath = loc.splitAt(":", 0) and
+      sl = loc.splitAt(":", 1).toInt() and
+      sc = loc.splitAt(":", 2).toInt() and
+      el = loc.splitAt(":", 3).toInt() and
+      ec = loc.splitAt(":", 4).toInt()
+    }
+
+    pragma[nomagic]
+    private string getRelativePathTo(string absolutePath) {
+      exists(Input::Location loc |
+        loc.hasLocationInfo(absolutePath, _, _, _, _) and
+        parseLocationString(Input2::getRelativeUrl(loc), result, _, _, _, _)
+      )
+    }
+
+    private newtype TTestLocation =
+      MkInputLocation(Input::Location loc) or
+      MkResultLocation(string relativePath, int sl, int sc, int el, int ec) {
+        exists(string data |
+          queryResults(_, _, _, data) and
+          parseLocationString(data, relativePath, sl, sc, el, ec) and
+          not Input2::getRelativeUrl(_) = data // avoid duplicate locations
+        )
+      }
+
+    /**
+     * A location that is either an `Input::Location` or a location from an alert.
+     *
+     * We use this location type to support queries that select a location that does not correspond
+     * to an instance of `Input::Location`.
+     */
+    abstract private class TestLocationImpl extends TTestLocation {
+      string getAbsoluteFile() { this.hasLocationInfo(result, _, _, _, _) }
+
+      int getStartLine() { this.hasLocationInfo(_, result, _, _, _) }
+
+      int getStartColumn() { this.hasLocationInfo(_, _, result, _, _) }
+
+      int getEndLine() { this.hasLocationInfo(_, _, _, result, _) }
+
+      int getEndColumn() { this.hasLocationInfo(_, _, _, _, result) }
+
+      abstract string getRelativeUrl();
+
+      final string toString() { result = this.getRelativeUrl() }
+
+      abstract predicate hasLocationInfo(string file, int sl, int sc, int el, int ec);
+    }
+
+    private class LocationFromResult extends TestLocationImpl, MkResultLocation {
+      override string getRelativeUrl() {
+        exists(string file, int sl, int sc, int el, int ec |
+          this = MkResultLocation(file, sl, sc, el, ec) and
+          result = file + ":" + sl + ":" + sc + ":" + el + ":" + ec
+        )
+      }
+
+      override predicate hasLocationInfo(string file, int sl, int sc, int el, int ec) {
+        this = MkResultLocation(getRelativePathTo(file), sl, sc, el, ec)
+      }
+    }
+
+    private class LocationFromInput extends TestLocationImpl, MkInputLocation {
+      private Input::Location loc;
+
+      LocationFromInput() { this = MkInputLocation(loc) }
+
+      override string getRelativeUrl() { result = Input2::getRelativeUrl(loc) }
+
+      override predicate hasLocationInfo(string file, int sl, int sc, int el, int ec) {
+        loc.hasLocationInfo(file, sl, sc, el, ec)
+      }
+    }
+
+    final class TestLocation = TestLocationImpl;
+
+    module TestImpl2 implements InlineExpectationsTestSig {
+      final class Location = TestLocation;
+
+      final private class ExpectationCommentFinal = Input::ExpectationComment;
+
+      class ExpectationComment extends ExpectationCommentFinal {
+        Location getLocation() { result = MkInputLocation(super.getLocation()) }
+      }
+    }
+
+    private import InlineExpectationsTest::Make<TestImpl2>
 
     /** Holds if the given locations refer to the same lines, but possibly with different column numbers. */
     bindingset[loc1, loc2]
     pragma[inline_late]
-    private predicate sameLineInfo(Input::Location loc1, Input::Location loc2) {
+    private predicate sameLineInfo(TestLocation loc1, TestLocation loc2) {
       exists(string file, int line1, int line2 |
         loc1.hasLocationInfo(file, line1, _, line2, _) and
         loc2.hasLocationInfo(file, line1, _, line2, _)
       )
     }
 
     pragma[nomagic]
-    private predicate mainQueryResult(int row, int column, Input::Location loc) {
-      queryResults(mainResultSet(), row, column, Input2::getRelativeUrl(loc))
+    private predicate mainQueryResult(int row, int column, TestLocation loc) {
+      queryResults(mainResultSet(), row, column, loc.getRelativeUrl())
     }
 
     /**
@@ -668,7 +759,7 @@ module TestPostProcessing {
      */
     private string getSourceTag(int row) {
       getQueryKind() = "path-problem" and
-      exists(Input::Location sourceLoc, Input::Location selectLoc |
+      exists(TestLocation sourceLoc, TestLocation selectLoc |
         mainQueryResult(row, 0, selectLoc) and
         mainQueryResult(row, 2, sourceLoc) and
         if sameLineInfo(selectLoc, sourceLoc) then result = "Alert" else result = "Source"
@@ -733,7 +824,7 @@ module TestPostProcessing {
       }
 
       additional predicate hasPathProblemSource(
-        int row, Input::Location location, string element, string tag, string value
+        int row, TestLocation location, string element, string tag, string value
       ) {
         getQueryKind() = "path-problem" and
         mainQueryResult(row, 2, location) and
@@ -742,7 +833,7 @@ module TestPostProcessing {
         value = ""
       }
 
-      predicate hasActualResult(Input::Location location, string element, string tag, string value) {
+      predicate hasActualResult(TestLocation location, string element, string tag, string value) {
         hasPathProblemSource(_, location, element, tag, value)
       }
     }
@@ -770,15 +861,15 @@ module TestPostProcessing {
       private predicate hasPathProblemSource = PathProblemSourceTestInput::hasPathProblemSource/5;
 
       private predicate hasPathProblemSink(
-        int row, Input::Location location, string element, string tag
+        int row, TestLocation location, string element, string tag
       ) {
         getQueryKind() = "path-problem" and
         mainQueryResult(row, 4, location) and
         queryResults(mainResultSet(), row, 5, element) and
         tag = getSinkTag(row)
       }
 
-      private predicate hasAlert(int row, Input::Location location, string element, string tag) {
+      private predicate hasAlert(int row, TestLocation location, string element, string tag) {
         getQueryKind() = ["problem", "path-problem"] and
         mainQueryResult(row, 0, location) and
         queryResults(mainResultSet(), row, 2, element) and
@@ -793,15 +884,15 @@ module TestPostProcessing {
        * present).
        */
       private string getValue(int row) {
-        exists(Input::Location location, string element, string tag, string val |
+        exists(TestLocation 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) {
+      predicate hasActualResult(TestLocation location, string element, string tag, string value) {
         exists(int row |
           hasPathProblemSource(row, location, element, tag, _)
           or
@@ -840,7 +931,7 @@ module TestPostProcessing {
         rankedTestFailures(row, f) and
         f = MkTestFailure(fl, message)
       |
-        column = 0 and data = Input2::getRelativeUrl(fl.getLocation())
+        column = 0 and data = fl.getLocation().getRelativeUrl()
         or
         column = 1 and data = fl.toString()
         or

Original file line number	Diff line number	Diff line change
`@@ -1 +1,2 @@`
`1`		`-AngularJS/DuplicateDependency.ql`
	`1`	`+query: AngularJS/DuplicateDependency.ql`
	`2`	`+postprocess: utils/test/InlineExpectationsTestQuery.ql`