remove spurious jQuery objects

Author: erik-krogh

javascript/ql/src/semmle/javascript/frameworks/jQuery.qll

@@ -37,12 +37,45 @@ private class OrdinaryJQueryObject extends JQueryObjectInternal {
   OrdinaryJQueryObject() {
     exists(JQuery::MethodCall jq |
       this.flow().getALocalSource() = jq and
-      // `jQuery.val()` does _not_ return a jQuery object
-      jq.getMethodName() != "val"
+      returnsAJqueryObject(jq, jq.getMethodName())
     )
   }
 }
 
+/**
+ * Holds if the jQuery method call `call`, with name `methodName`, returns a JQuery object.
+ * 
+ * The `call` parameter has type `DataFlow::CallNode` instead of `JQuery::MethodCall` to avoid non-monotonic recursion. 
+ * The not is placed inside the predicate to avoid non-monotonic recursion. 
+ */
+bindingset[methodName, call]
+private predicate returnsAJqueryObject(DataFlow::CallNode call, string methodName) {
+  not (
+    methodName = "val" // `jQuery.val()`
+    or
+    methodName = ["html", "text"] and call.getNumArgument() = 0 // `jQuery.html()`/`jQuery.text()`
+    or
+    // `jQuery.attr(key)`/`jQuery.prop(key)`
+    methodName = ["attr", "prop"] and
+    call.getNumArgument() = 1 and
+    call.getArgument(0).mayHaveStringValue(_)
+    or
+    // `jQuery.data()`
+    methodName = "data" and call.getNumArgument() = 0
+    or
+    // `jQuery.data(key)`
+    methodName = "data" and call.getNumArgument() = 1 and call.getArgument(0).mayHaveStringValue(_)
+    or
+    methodName = ["Event", "Deferred"] // $.Event / $.Deferred
+    or
+    methodName = "trim" // $.trim()
+    or
+    // `$.ajax`, and related methods. 
+    // note: there are 2 different `get` methods, and none of them return a jQuery object.
+    methodName = ["ajax", "get", "getJSON", "getScript", "post", "load"]
+  )
+}
+
 /**
  * DEPRECATED. Use `JQuery::MethodCall` instead.
  *

javascript/ql/test/library-tests/frameworks/jQuery/JQueryMethodCall.expected

@@ -17,3 +17,15 @@ WARNING: Type JQueryMethodCall has been deprecated and may be removed in future
 | tst.js:5:1:5:15 | window.jQuery() | $ |
 | tst.js:6:1:6:32 | angular ... .attr() | attr |
 | tst.js:7:1:7:14 | $("<br>", doc) | $ |
+| tst.js:11:1:11:8 | $("foo") | $ |
+| tst.js:11:1:11:15 | $("foo").html() | html |
+| tst.js:12:1:12:8 | $("foo") | $ |
+| tst.js:12:1:12:20 | $("foo").html("foo") | html |
+| tst.js:12:1:12:31 | $("foo" ... query() | isJquery |
+| tst.js:13:1:13:8 | $("foo") | $ |
+| tst.js:13:1:13:17 | $("foo").data({}) | data |
+| tst.js:13:1:13:28 | $("foo" ... query() | isJquery |
+| tst.js:15:1:15:8 | $("foo") | $ |
+| tst.js:15:1:15:20 | $("foo").attr("bar") | attr |
+| tst.js:17:1:17:8 | $.trim() | trim |
+| tst.js:18:1:18:10 | $.ajax({}) | ajax |

javascript/ql/test/library-tests/frameworks/jQuery/interpretsArgumentAsHtml.expected

@@ -8,3 +8,8 @@ WARNING: Type JQueryMethodCall has been deprecated and may be removed in future
 | tst2.js:2:1:2:7 | jq("a") | tst2.js:2:4:2:6 | "a" |
 | tst.js:3:1:3:9 | $("<a/>") | tst.js:3:3:3:8 | "<a/>" |
 | tst.js:7:1:7:14 | $("<br>", doc) | tst.js:7:3:7:8 | "<br>" |
+| tst.js:11:1:11:8 | $("foo") | tst.js:11:3:11:7 | "foo" |
+| tst.js:12:1:12:8 | $("foo") | tst.js:12:3:12:7 | "foo" |
+| tst.js:12:1:12:20 | $("foo").html("foo") | tst.js:12:15:12:19 | "foo" |
+| tst.js:13:1:13:8 | $("foo") | tst.js:13:3:13:7 | "foo" |
+| tst.js:15:1:15:8 | $("foo") | tst.js:15:3:15:7 | "foo" |

javascript/ql/test/library-tests/frameworks/jQuery/tst.js

@@ -5,3 +5,14 @@ window.$();
 window.jQuery();
 angular.element("<div/>").attr()
 $("<br>", doc);
+
+
+
+$("foo").html().doesNotReturnJquery();
+$("foo").html("foo").isJquery();
+$("foo").data({}).isJquery();
+
+$("foo").attr("bar").doesNotReturnJquery();
+
+$.trim().doesNotReturnJquery();
+$.ajax({}).doesNotReturnJquery()