Merge pull request #3386 from erik-krogh/lessJQueryChaining

Approved by asgerf

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

@@ -37,12 +37,50 @@ 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 (
+    neverReturnsJQuery(methodName)
+    or
+    methodName = "val" and call.getNumArgument() = 0 // `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(_)
+  )
+}
+
+/**
+ * Holds if a jQuery method named `name` never returns a JQuery object.
+ */
+private predicate neverReturnsJQuery(string name) {
+  forex(ExternalMemberDecl decl |
+    decl.getBaseName() = "jQuery" and
+    decl.getName() = name
+  |
+    not decl
+        .getDocumentation()
+        .getATagByTitle("return")
+        .getType()
+        .getAnUnderlyingType()
+        .hasQualifiedName("jQuery")
+  )
+}
+
 /**
  * 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/jquery-3.2.js

@@ -0,0 +1,65 @@
+/*
+ * Copyright 2017 The Closure Compiler Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * @fileoverview Externs for jQuery 3.1
+ *
+ * Note that some functions use different return types depending on the number
+ * of parameters passed in. In these cases, you may need to annotate the type
+ * of the result in your code, so the JSCompiler understands which type you're
+ * expecting. For example:
+ *    <code>var elt = /** @type {Element} * / (foo.get(0));</code>
+ *
+ * @see http://api.jquery.com/
+ * @externs
+ */
+
+/**
+ * @typedef {(Window|Document|Element|Array<Element>|string|jQuery|
+ *     NodeList)}
+ */
+var jQuerySelector;
+
+/**
+ * @constructor
+ * @param {(jQuerySelector|Object|function())=} arg1
+ * @param {(Element|jQuery|Document|
+ *     Object<string, (string|function(!jQuery.Event))>)=} arg2
+ * @throws {Error} on invalid selector
+ * @return {!jQuery}
+ * @implements {Iterable}
+ */
+function jQuery(arg1, arg2) { };
+
+/**
+ * @const
+ */
+var $ = jQuery;
+
+/**
+ * @param {(string|jQueryAjaxSettings|Object<string,*>)} arg1
+ * @param {(jQueryAjaxSettings|Object<string, *>)=} settings
+ * @return {!jQuery.jqXHR}
+ */
+jQuery.ajax = function (arg1, settings) { };
+
+/**
+ * @param {string} str
+ * @return {string}
+ * @nosideeffects
+ */
+jQuery.trim = function (str) { };
+

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()