JS: QHelp and a bit of qldoc

asger-semmle · asger-semmle · commit eb82b17f16e0 · 2019-12-12T15:40:41.000Z
diff --git a/javascript/ql/src/Expressions/MissingAwait.qhelp b/javascript/ql/src/Expressions/MissingAwait.qhelp
@@ -4,62 +4,45 @@
 <qhelp>
 <overview>
 <p>
-In JavaScript, <code>async</code> functions always return a Promise object.
-</p>
-
-<p>
-PLACEHOLDER
+In JavaScript, <code>async</code> functions always return a promise object.
+To obtain the underlying value of the promise, the `await` operator or a call to `then` should be used.
+Attempting to use a Promise object instead of its underlying value can lead to expected behavior.
 </p>
 
 </overview>
 <recommendation>
 
 <p>
-PLACEHOLDER
+Use the `await` operator to get the value contained in the promise.
+Alternatively, call `then` on the promise and use the value passed to the callback.
 </p>
 
 </recommendation>
 <example>
 
 <p>
-PLACEHOLDER
-</p>
-
-<sample src="examples/ImplicitOperandConversion.js" />
-
-<p>
-PLACEHOLDER
-</p>
-
-<p>
-PLACEHOLDER
+In the following example, the <code>getData</code> function returns a promise,
+and the caller checks if the returned promise is <code>null</code>:
 </p>
 
-<sample src="examples/ImplicitOperandConversionGood.js" />
+<sample src="examples/MissingAwait.js" />
 
 <p>
-PLACEHOLDER
+However, the null check does not work as expected. The <code>return null</code> statement
+on line 2 actually returns a <em>promise</em> containing the <code>null</code> value.
+Since the promise object itself is not equal to <code>null</code>, the error check is bypassed.
 </p>
 
-<sample src="examples/ImplicitOperandConversion2.js" />
-
 <p>
-PLACEHOLDER
+The issue can be corrected by inserting <code>await</code> before the promise:
 </p>
 
-<sample src="examples/ImplicitOperandConversion2Good.js" />
-
-<p>
-PLACEHOLDER
-</p>
-
-<sample src="examples/ImplicitOperandConversion2Good2.js" />
+<sample src="examples/MissingAwaitGood.js" />
 
 </example>
 <references>
-
-
-<li>Ecma International, <i>ECMAScript Language Definition</i>, 5.1 Edition, Section 9. ECMA, 2011.</li>
-
+<li>MDN: <a href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Using_promises">Using promises</a></li>
+<li>MDN: <a href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/async_function">Async functions</a></li>
+<li>MDN: <a href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/await">Await operator</a></li>
 </references>
 </qhelp>
diff --git a/javascript/ql/src/Expressions/MissingAwait.ql b/javascript/ql/src/Expressions/MissingAwait.ql
@@ -1,6 +1,6 @@
 /**
  * @name Missing await
- * @description Using a promise without awaiting its result can cause unexpected behavior.
+ * @description Using a promise without awaiting its result can lead to unexpected behavior.
  * @kind problem
  * @problem.severity warning
  * @id js/missing-await
@@ -10,7 +10,12 @@
 
 import javascript
 
+/**
+ * Holds if `call` is a call to an `async` function.
+ */
 predicate isAsyncCall(DataFlow::CallNode call) {
+  // If a callee is known, and all known callees are async, assume all
+  // possible callees are async.
   forex(Function callee | call.getACallee() = callee | callee.isAsync())
 }
 
diff --git a/javascript/ql/src/Expressions/examples/MissingAwait.js b/javascript/ql/src/Expressions/examples/MissingAwait.js
@@ -0,0 +1,14 @@
+async function getData(id) {
+  let req = await fetch(`https://example.com/data?id=${id}`);
+  if (!req.ok) return null;
+  return req.json();
+}
+
+async function showData(id) {
+  let data = getData(id);
+  if (data == null) {
+    console.warn("No data for: " + id);
+    return;
+  }
+  // ...
+}
diff --git a/javascript/ql/src/Expressions/examples/MissingAwaitGood.js b/javascript/ql/src/Expressions/examples/MissingAwaitGood.js
@@ -0,0 +1,14 @@
+async function getData(id) {
+  let req = await fetch(`https://example.com/data?id=${id}`);
+  if (!req.ok) return null;
+  return req.json();
+}
+
+async function showData(id) {
+  let data = await getData(id);
+  if (data == null) {
+    console.warn("No data for: " + id);
+    return;
+  }
+  // ...
+}
