JS: Add query for missing await

This commit is contained in:
Asger F
2019-11-27 14:50:37 +00:00
parent 3d8c35e523
commit a30f991b5e
5 changed files with 183 additions and 0 deletions

View File

@@ -0,0 +1,65 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>
In JavaScript, <code>async</code> functions always return a Promise object.
</p>
<p>
PLACEHOLDER
</p>
</overview>
<recommendation>
<p>
PLACEHOLDER
</p>
</recommendation>
<example>
<p>
PLACEHOLDER
</p>
<sample src="examples/ImplicitOperandConversion.js" />
<p>
PLACEHOLDER
</p>
<p>
PLACEHOLDER
</p>
<sample src="examples/ImplicitOperandConversionGood.js" />
<p>
PLACEHOLDER
</p>
<sample src="examples/ImplicitOperandConversion2.js" />
<p>
PLACEHOLDER
</p>
<sample src="examples/ImplicitOperandConversion2Good.js" />
<p>
PLACEHOLDER
</p>
<sample src="examples/ImplicitOperandConversion2Good2.js" />
</example>
<references>
<li>Ecma International, <i>ECMAScript Language Definition</i>, 5.1 Edition, Section 9. ECMA, 2011.</li>
</references>
</qhelp>

View File

@@ -0,0 +1,68 @@
/**
* @name Missing await
* @description Using a promise without awaiting its result can cause unexpected behavior.
* @kind problem
* @problem.severity warning
* @id js/missing-await
* @tags correctness
* @precision high
*/
import javascript
predicate isAsyncCall(DataFlow::CallNode call) {
forex(Function callee | call.getACallee() = callee | callee.isAsync())
}
/**
* Holds if `node` is always a promise.
*/
predicate isPromise(DataFlow::SourceNode node, boolean nullable) {
isAsyncCall(node) and
nullable = false
or
not isAsyncCall(node) and
node.asExpr().getType() instanceof PromiseType and
nullable = true
}
/**
* Holds the result of `e` is used in a way that doesn't make sense for Promise objects.
*/
predicate isBadPromiseContext(Expr expr) {
exists(BinaryExpr binary |
expr = binary.getAnOperand() and
not binary instanceof LogicalExpr and
not binary instanceof InstanceofExpr
)
or
expr = any(LogicalBinaryExpr e).getLeftOperand()
or
expr = any(UnaryExpr e).getOperand()
or
expr = any(UpdateExpr e).getOperand()
}
string tryGetPromiseExplanation(Expr e) {
result = "The value '" + e.(VarAccess).getName() + "' is always a promise."
or
result = "The call to '" + e.(CallExpr).getCalleeName() + "' always returns a promise."
}
string getPromiseExplanation(Expr e) {
result = tryGetPromiseExplanation(e)
or
not exists(tryGetPromiseExplanation(e)) and
result = "This value is always a promise."
}
from Expr expr, boolean nullable
where
isBadPromiseContext(expr) and
isPromise(expr.flow().getImmediatePredecessor*(), nullable) and
(
nullable = false
or
expr.inNullSensitiveContext()
)
select expr, "Missing await. " + getPromiseExplanation(expr)

View File

@@ -0,0 +1,2 @@
| tst.js:8:9:8:13 | thing | Missing await. The value 'thing' is always a promise. |
| tst.js:32:12:32:16 | thing | Missing await. The value 'thing' is always a promise. |

View File

@@ -0,0 +1 @@
Expressions/MissingAwait.ql

View File

@@ -0,0 +1,47 @@
async function getThing() {
return something();
}
function useThing() {
let thing = getThing();
if (thing === undefined) {} // NOT OK
if (thing == null) {} // NOT OK
return thing + "bar"; // NOT OK
}
async function useThingCorrectly() {
let thing = await getThing();
if (thing === undefined) {} // OK
if (thing == null) {} // OK
return thing + "bar"; // OK
}
async function useThingCorrectly2() {
let thing = getThing();
if (await thing === undefined) {} // OK
if (await thing == null) {} // OK
return thing + "bar"; // NOT OK
}
function getThingSync() {
return something();
}
function useThingPossiblySync(b) {
let thing = b ? getThing() : getThingSync();
if (thing === undefined) {} // OK
if (thing == null) {} // OK
return thing + "bar"; // NOT OK - but we don't flag it
}