hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-28 18:25:24 +02:00
Code Issues Packages Projects Releases Wiki Activity

address review feedback

Browse Source
This commit is contained in:
Erik Krogh Kristensen
2019-10-21 20:21:41 +02:00
parent 28056791a5
commit 2e0244cda6
5 changed files with 28 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
javascript/ql/src/Expressions/ExprHasNoEffect.ql
View File

@@ -14,6 +14,7 @@
import javascript
import ExprHasNoEffect
import semmle.javascript.RestrictedLocations
from Expr e

1
javascript/ql/src/Expressions/ExprHasNoEffect.qll
View File

@@ -5,7 +5,6 @@
import javascript
import DOMProperties
import semmle.javascript.frameworks.xUnit
import semmle.javascript.RestrictedLocations
/**
* Holds if `e` appears in a syntactic context where its value is discarded.

25
javascript/ql/src/Statements/UseOfReturnlessFunction.ql
View File

@@ -82,8 +82,19 @@ predicate alwaysThrows(Function f) {
)
}
/**
* Holds if the last statement in the function is flagged by the js/useless-expression query.
*/
predicate alwaysHasNoEffect(Function f) {
exists(ReachableBasicBlock entry, DataFlow::Node noEffect |
entry = f.getEntryBB() and
hasNoEffect(noEffect.asExpr()) and
entry.dominates(noEffect.getBasicBlock())
)
}
predicate callToVoidFunction(DataFlow::CallNode call, Function func) {
not call.isIndefinite(_) and
not call.isIncomplete() and
func = call.getACallee() and
forall(Function f | f = call.getACallee() |
returnsVoid(f) and not isStub(f) and not alwaysThrows(f)
@@ -113,6 +124,11 @@ DataFlow::SourceNode array(DataFlow::TypeTracker t) {
DataFlow::SourceNode array() { result = array(DataFlow::TypeTracker::end()) }
/**
* Holds if `call` is an Array or Lodash method accepting a callback `func`,
* where the `call` expects a callback that returns an expression,
* but `func` does return a value.
*/
predicate voidArrayCallback(DataFlow::CallNode call, Function func) {
hasNonVoidCallbackMethod(call.getCalleeName()) and
func = call.getAnArgument().getALocalSource().asExpr() and
@@ -132,15 +148,14 @@ where
(
callToVoidFunction(call, func) and
msg = "the $@ does not return anything, yet the return value is used." and
name = "function " + call.getCalleeName()
name = func.describe()
or
voidArrayCallback(call, func) and
msg = "the $@ does not return anything, yet the return value from the call to " + call.getCalleeName() + "() is used." and
msg = "the $@ does not return anything, yet the return value from the call to " + call.getCalleeName() + " is used." and
name = "callback function"
) and
not benignContext(call.asExpr()) and
// Avoid double reporting from js/useless-expression
not hasNoEffect(func.getBodyStmt(func.getNumBodyStmt() - 1).(ExprStmt).getExpr()) and
not alwaysHasNoEffect(func) and
// anonymous one-shot closure. Those are used in weird ways and we ignore them.
not oneshotClosure(call.asExpr())
select

8
javascript/ql/test/query-tests/Statements/UseOfReturnlessFunction/UseOfReturnlessFunction.expected
View File

@@ -1,7 +1,7 @@
| tst.js:20:17:20:33 | onlySideEffects() | the $@ does not return anything, yet the return value is used. | tst.js:11:5:13:5 | functio ... )\\n } | function onlySideEffects |
| tst.js:24:13:24:29 | onlySideEffects() | the $@ does not return anything, yet the return value is used. | tst.js:11:5:13:5 | functio ... )\\n } | function onlySideEffects |
| tst.js:30:20:30:36 | onlySideEffects() | the $@ does not return anything, yet the return value is used. | tst.js:11:5:13:5 | functio ... )\\n } | function onlySideEffects |
| tst.js:53:10:53:34 | bothOnl ... fects() | the $@ does not return anything, yet the return value is used. | tst.js:11:5:13:5 | functio ... )\\n } | function bothOnlyHaveSideEffects |
| tst.js:53:10:53:34 | bothOnl ... fects() | the $@ does not return anything, yet the return value is used. | tst.js:48:2:50:5 | functio ... )\\n } | function bothOnlyHaveSideEffects |
| tst.js:76:12:76:46 | [1,2,3] ... n, 3)}) | the $@ does not return anything, yet the return value from the call to filter() is used. | tst.js:76:27:76:45 | n => {equals(n, 3)} | callback function |
| tst.js:80:12:80:50 | filter( ... 3) } ) | the $@ does not return anything, yet the return value from the call to filter() is used. | tst.js:80:28:80:48 | x => { ... x, 3) } | callback function |
| tst.js:53:10:53:34 | bothOnl ... fects() | the $@ does not return anything, yet the return value is used. | tst.js:11:5:13:5 | functio ... )\\n } | function onlySideEffects |
| tst.js:53:10:53:34 | bothOnl ... fects() | the $@ does not return anything, yet the return value is used. | tst.js:48:2:50:5 | functio ... )\\n } | function onlySideEffects2 |
| tst.js:76:12:76:46 | [1,2,3] ... n, 3)}) | the $@ does not return anything, yet the return value from the call to filter is used. | tst.js:76:27:76:45 | n => {equals(n, 3)} | callback function |
| tst.js:80:12:80:50 | filter( ... 3) } ) | the $@ does not return anything, yet the return value from the call to filter is used. | tst.js:80:28:80:48 | x => { ... x, 3) } | callback function |

3
javascript/ql/test/query-tests/Statements/UseOfReturnlessFunction/tst.js
View File

@@ -79,4 +79,7 @@
import { filter } from 'lodash'
var bar = filter([1,2,4], x => { equals(x, 3) } ) // NOT OK!
console.log(bar);
var baz = [1,2,3].filter(n => {n === 3}) // OK
console.log(baz);
})();