Python: Add (only) basic $HttpResponse tag to other tests files

This seems really nice to me, but you might disagree

python/ql/test/experimental/library-tests/frameworks/flask/ConceptsTest.expected

@@ -1,40 +0,0 @@
-| old_test.py:41:12:41:54 | ControlFlowNode for make_response() | Unexpected result: HttpResponse= |
-| old_test.py:41:12:41:54 | ControlFlowNode for make_response() | Unexpected result: contentType=text/html |
-| old_test.py:41:12:41:54 | ControlFlowNode for make_response() | Unexpected result: responseBody=BinaryExpr |
-| old_test.py:41:12:41:54 | ControlFlowNode for make_response() | Unexpected result: statusCode=200 |
-| old_test.py:46:12:46:62 | ControlFlowNode for make_response() | Unexpected result: HttpResponse= |
-| old_test.py:46:12:46:62 | ControlFlowNode for make_response() | Unexpected result: contentType=text/html |
-| old_test.py:46:12:46:62 | ControlFlowNode for make_response() | Unexpected result: responseBody=BinaryExpr |
-| old_test.py:46:12:46:62 | ControlFlowNode for make_response() | Unexpected result: statusCode=200 |
-| old_test.py:50:12:50:48 | ControlFlowNode for make_response() | Unexpected result: HttpResponse= |
-| old_test.py:50:12:50:48 | ControlFlowNode for make_response() | Unexpected result: contentType=text/html |
-| old_test.py:50:12:50:48 | ControlFlowNode for make_response() | Unexpected result: responseBody=BinaryExpr |
-| old_test.py:50:12:50:48 | ControlFlowNode for make_response() | Unexpected result: statusCode=200 |
-| old_test.py:54:12:54:53 | ControlFlowNode for make_response() | Unexpected result: HttpResponse= |
-| old_test.py:54:12:54:53 | ControlFlowNode for make_response() | Unexpected result: contentType=text/html |
-| old_test.py:54:12:54:53 | ControlFlowNode for make_response() | Unexpected result: responseBody=BinaryExpr |
-| old_test.py:54:12:54:53 | ControlFlowNode for make_response() | Unexpected result: statusCode=200 |
-| old_test.py:60:12:60:62 | ControlFlowNode for make_response() | Unexpected result: HttpResponse= |
-| old_test.py:60:12:60:62 | ControlFlowNode for make_response() | Unexpected result: contentType=text/html |
-| old_test.py:60:12:60:62 | ControlFlowNode for make_response() | Unexpected result: responseBody=Attribute() |
-| old_test.py:60:12:60:62 | ControlFlowNode for make_response() | Unexpected result: statusCode=200 |
-| old_test.py:64:12:64:58 | ControlFlowNode for make_response() | Unexpected result: HttpResponse= |
-| old_test.py:64:12:64:58 | ControlFlowNode for make_response() | Unexpected result: contentType=text/html |
-| old_test.py:64:12:64:58 | ControlFlowNode for make_response() | Unexpected result: responseBody=Attribute() |
-| old_test.py:64:12:64:58 | ControlFlowNode for make_response() | Unexpected result: statusCode=200 |
-| routing_test.py:10:12:10:38 | ControlFlowNode for make_response() | Unexpected result: HttpResponse= |
-| routing_test.py:10:12:10:38 | ControlFlowNode for make_response() | Unexpected result: contentType=text/html |
-| routing_test.py:10:12:10:38 | ControlFlowNode for make_response() | Unexpected result: responseBody="some_route" |
-| routing_test.py:10:12:10:38 | ControlFlowNode for make_response() | Unexpected result: statusCode=200 |
-| routing_test.py:14:12:14:33 | ControlFlowNode for make_response() | Unexpected result: HttpResponse= |
-| routing_test.py:14:12:14:33 | ControlFlowNode for make_response() | Unexpected result: contentType=text/html |
-| routing_test.py:14:12:14:33 | ControlFlowNode for make_response() | Unexpected result: responseBody="index" |
-| routing_test.py:14:12:14:33 | ControlFlowNode for make_response() | Unexpected result: statusCode=200 |
-| routing_test.py:20:12:20:37 | ControlFlowNode for make_response() | Unexpected result: HttpResponse= |
-| routing_test.py:20:12:20:37 | ControlFlowNode for make_response() | Unexpected result: contentType=text/html |
-| routing_test.py:20:12:20:37 | ControlFlowNode for make_response() | Unexpected result: responseBody="later_set" |
-| routing_test.py:20:12:20:37 | ControlFlowNode for make_response() | Unexpected result: statusCode=200 |
-| routing_test.py:27:12:27:40 | ControlFlowNode for make_response() | Unexpected result: HttpResponse= |
-| routing_test.py:27:12:27:40 | ControlFlowNode for make_response() | Unexpected result: contentType=text/html |
-| routing_test.py:27:12:27:40 | ControlFlowNode for make_response() | Unexpected result: responseBody="unkown_route" |
-| routing_test.py:27:12:27:40 | ControlFlowNode for make_response() | Unexpected result: statusCode=200 |

python/ql/test/experimental/library-tests/frameworks/flask/ConceptsTest.ql

@@ -1,2 +1,12 @@
 import python
 import experimental.meta.ConceptsTest
+
+class DedicatedFlaskResponseTest extends HttpServerHttpResponseTest {
+  DedicatedFlaskResponseTest() { file.getShortName() = "response_test.py" }
+}
+
+class OtherFlaskResponseTest extends HttpServerHttpResponseTest {
+  OtherFlaskResponseTest() { not this instanceof DedicatedFlaskResponseTest }
+
+  override string getARelevantTag() { result = "HttpResponse" }
+}

python/ql/test/experimental/library-tests/frameworks/flask/old_test.py

@@ -38,30 +38,30 @@ def dangerous2():  # $routeHandler
 @app.route("/unsafe")  # $routeSetup="/unsafe"
 def unsafe():  # $routeHandler
     first_name = request.args.get('name', '')
-    return make_response("Your name is " + first_name)
+    return make_response("Your name is " + first_name)  # $HttpResponse
 
 @app.route("/safe")  # $routeSetup="/safe"
 def safe():  # $routeHandler
     first_name = request.args.get('name', '')
-    return make_response("Your name is " + escape(first_name))
+    return make_response("Your name is " + escape(first_name))  # $HttpResponse
 
 @app.route("/hello/<name>")  # $routeSetup="/hello/<name>"
 def hello(name):  # $routeHandler $routedParameter=name
-    return make_response("Your name is " + name)
+    return make_response("Your name is " + name)  # $HttpResponse
 
 @app.route("/foo/<path:subpath>")  # $routeSetup="/foo/<path:subpath>"
 def foo(subpath):  # $routeHandler $routedParameter=subpath
-    return make_response("The subpath is " + subpath)
+    return make_response("The subpath is " + subpath)  # $HttpResponse
 
 @app.route("/multiple/")  # $routeSetup="/multiple/"
 @app.route("/multiple/foo/<foo>")  # $routeSetup="/multiple/foo/<foo>"
 @app.route("/multiple/bar/<bar>")  # $routeSetup="/multiple/bar/<bar>"
 def multiple(foo=None, bar=None):  # $routeHandler $routedParameter=foo $routedParameter=bar
-    return make_response("foo={!r} bar={!r}".format(foo, bar))
+    return make_response("foo={!r} bar={!r}".format(foo, bar))  # $HttpResponse
 
 @app.route("/complex/<string(length=2):lang_code>")  # $routeSetup="/complex/<string(length=2):lang_code>"
 def complex(lang_code):  # $routeHandler $routedParameter=lang_code
-    return make_response("lang_code {}".format(lang_code))
+    return make_response("lang_code {}".format(lang_code))  # $HttpResponse
 
 if __name__ == "__main__":
     app.run(debug=True)

python/ql/test/experimental/library-tests/frameworks/flask/routing_test.py

@@ -7,24 +7,24 @@ app = Flask(__name__)
 SOME_ROUTE = "/some/route"
 @app.route(SOME_ROUTE) # $routeSetup="/some/route"
 def some_route():  # $routeHandler
-    return make_response("some_route")
+    return make_response("some_route")  # $HttpResponse
 
 
 def index():  # $routeHandler
-    return make_response("index")
+    return make_response("index")  # $HttpResponse
 app.add_url_rule('/index', 'index', index)  # $routeSetup="/index"
 
 
 # We don't support this yet, and I think that's OK
 def later_set():  # $f-:routeHandler
-    return make_response("later_set")
+    return make_response("later_set")  # $HttpResponse
 app.add_url_rule('/later-set', 'later_set', view_func=None)  # $routeSetup="/later-set"
 app.view_functions['later_set'] = later_set
 
 
 @app.route(UNKNOWN_ROUTE) # $routeSetup
 def unkown_route(foo, bar):  # $routeHandler $routedParameter=foo $routedParameter=bar
-    return make_response("unkown_route")
+    return make_response("unkown_route")  # $HttpResponse
 
 
 if __name__ == "__main__":

python/ql/test/experimental/meta/ConceptsTest.qll

@@ -144,39 +144,49 @@ class HttpServerRouteSetupTest extends InlineExpectationsTest {
 }
 
 class HttpServerHttpResponseTest extends InlineExpectationsTest {
-  HttpServerHttpResponseTest() { this = "HttpServerHttpResponseTest" }
+  File file;
+
+  HttpServerHttpResponseTest() { this = "HttpServerHttpResponseTest: " + file }
 
   override string getARelevantTag() {
     result in ["HttpResponse", "responseBody", "contentType", "statusCode"]
   }
 
   override predicate hasActualResult(Location location, string element, string tag, string value) {
-    exists(HTTP::Server::HttpResponse response |
-      location = response.getLocation() and
-      element = response.toString() and
-      value = "" and
-      tag = "HttpResponse"
-    )
-    or
-    exists(HTTP::Server::HttpResponse response |
-      location = response.getLocation() and
-      element = response.toString() and
-      value = value_from_expr(response.getBody().asExpr()) and
-      tag = "responseBody"
-    )
-    or
-    exists(HTTP::Server::HttpResponse response |
-      location = response.getLocation() and
-      element = response.toString() and
-      value = response.getContentType() and
-      tag = "contentType"
-    )
-    or
-    exists(HTTP::Server::HttpResponse response |
-      location = response.getLocation() and
-      element = response.toString() and
-      value = response.getStatusCode().toString() and
-      tag = "statusCode"
+    // By adding `file` as a class field, and these two restrictions, it's possible to
+    // say that we only want to check _some_ tags for certain files. This helped make
+    // flask tests more readable since adding full annotations for HttpResponses in the
+    // the tests for routing setup is both annoying and not very useful.
+    location.getFile() = file and
+    tag = getARelevantTag() and
+    (
+      exists(HTTP::Server::HttpResponse response |
+        location = response.getLocation() and
+        element = response.toString() and
+        value = "" and
+        tag = "HttpResponse"
+      )
+      or
+      exists(HTTP::Server::HttpResponse response |
+        location = response.getLocation() and
+        element = response.toString() and
+        value = value_from_expr(response.getBody().asExpr()) and
+        tag = "responseBody"
+      )
+      or
+      exists(HTTP::Server::HttpResponse response |
+        location = response.getLocation() and
+        element = response.toString() and
+        value = response.getContentType() and
+        tag = "contentType"
+      )
+      or
+      exists(HTTP::Server::HttpResponse response |
+        location = response.getLocation() and
+        element = response.toString() and
+        value = response.getStatusCode().toString() and
+        tag = "statusCode"
+      )
     )
   }
 }