Merge pull request #4523 from erik-krogh/optionalPromise

Approved by asgerf

javascript/ql/src/Expressions/MissingAwait.ql

@@ -13,20 +13,20 @@ import javascript
 /**
  * Holds if `call` is a call to an `async` function.
  */
-predicate isAsyncCall(DataFlow::CallNode call) {
+predicate isAsyncCall(DataFlow::CallNode call, boolean nullable) {
   // 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())
+  forex(Function callee | call.getACallee() = callee | callee.isAsync()) and
+  if call.asExpr() instanceof OptionalUse then nullable = true else nullable = false
 }
 
 /**
  * Holds if `node` is always a promise.
  */
 predicate isPromise(DataFlow::SourceNode node, boolean nullable) {
-  isAsyncCall(node) and
-  nullable = false
+  isAsyncCall(node, nullable)
   or
-  not isAsyncCall(node) and
+  not isAsyncCall(node, _) and
   node.asExpr().getType() instanceof PromiseType and
   nullable = true
 }

javascript/ql/test/query-tests/Expressions/MissingAwait/MissingAwait.expected

@@ -7,3 +7,4 @@
 | tst.js:22:15:22:19 | thing | Missing await. The value 'thing' is always a promise. |
 | tst.js:25:13:25:17 | thing | Missing await. The value 'thing' is always a promise. |
 | tst.js:48:12:48:16 | thing | Missing await. The value 'thing' is always a promise. |
+| tst.js:71:16:71:25 | getThing() | Missing await. The call to 'getThing' always returns a promise. |

javascript/ql/test/query-tests/Expressions/MissingAwait/tst.js

@@ -65,3 +65,11 @@ function useThingPossiblySync(b) {
 function useThingInVoid() {
     void getThing(); // OK
 }
+
+function useThing() {
+    if (random()) {
+        return getThing() ?? null; // NOT OK
+    } else {
+        return getThing?.() ?? null; // OK
+    }
+}