From 4e54af3b413e1f6e1d22a65f2ccdc1ab1e2c3f44 Mon Sep 17 00:00:00 2001 From: Esben Sparre Andreasen Date: Wed, 31 Oct 2018 09:09:09 +0100 Subject: [PATCH 1/2] JS: introduce 'Util::describeExpression' --- javascript/ql/src/semmle/javascript/Util.qll | 35 +++++++++++++++++ .../Util/describeExpression.expected | 36 ++++++++++++++++++ .../library-tests/Util/describeExpression.ql | 4 ++ javascript/ql/test/library-tests/Util/tst.js | 16 +++++++- .../UselessConditional.expected | 38 +++++++++---------- 5 files changed, 109 insertions(+), 20 deletions(-) create mode 100644 javascript/ql/test/library-tests/Util/describeExpression.expected create mode 100644 javascript/ql/test/library-tests/Util/describeExpression.ql diff --git a/javascript/ql/src/semmle/javascript/Util.qll b/javascript/ql/src/semmle/javascript/Util.qll index f68afa5814bb..482b26a54fcf 100644 --- a/javascript/ql/src/semmle/javascript/Util.qll +++ b/javascript/ql/src/semmle/javascript/Util.qll @@ -1,3 +1,5 @@ +import javascript + /** * Provides general-purpose utility predicates. */ @@ -34,3 +36,36 @@ bindingset[str, maxLength, explanation] string truncate(string str, int maxLength, string explanation) { if str.length() > maxLength then result = str.prefix(maxLength) + explanation else result = str } + +/** + * Gets a string that describes `e`. + */ +string describeExpression(Expr e) { + if e instanceof InvokeExpr then + exists (string prefix, string suffix | + result = prefix + suffix | + ( if e instanceof NewExpr then + prefix = "constructor call" + else if e instanceof MethodCallExpr then + prefix = "method call" + else + prefix = "call" + ) and + ( + if exists(e.(InvokeExpr).getCalleeName()) then + suffix = " to " + e.(InvokeExpr).getCalleeName() + else + suffix = "" + ) + ) + else if e instanceof Comparison then + result = "comparison" + else if e instanceof VarAccess then + result = "use of variable '" + e.(VarAccess).getName() + "'" + else if e instanceof PropAccess and exists (e.(PropAccess).getPropertyName()) then + result = "use of property '" + e.(PropAccess).getPropertyName() + "'" + else if e instanceof LogNotExpr then + result = "negation" + else + result = "expression" +} \ No newline at end of file diff --git a/javascript/ql/test/library-tests/Util/describeExpression.expected b/javascript/ql/test/library-tests/Util/describeExpression.expected new file mode 100644 index 000000000000..ae3a43d2bbd3 --- /dev/null +++ b/javascript/ql/test/library-tests/Util/describeExpression.expected @@ -0,0 +1,36 @@ +| tst.js:1:1:15:2 | (functi ... !'';\\n}) | expression | +| tst.js:1:2:15:1 | functio ... !'';\\n} | expression | +| tst.js:2:5:2:5 | v | use of variable 'v' | +| tst.js:3:5:3:5 | o | use of variable 'o' | +| tst.js:3:5:3:7 | o.m | use of property 'm' | +| tst.js:3:5:3:9 | o.m() | method call to m | +| tst.js:3:7:3:7 | m | expression | +| tst.js:4:5:4:5 | f | use of variable 'f' | +| tst.js:4:5:4:7 | f() | call to f | +| tst.js:5:5:5:5 | x | use of variable 'x' | +| tst.js:5:5:5:10 | x == y | comparison | +| tst.js:5:10:5:10 | y | use of variable 'y' | +| tst.js:6:5:6:5 | o | use of variable 'o' | +| tst.js:6:5:6:7 | o.p | use of property 'p' | +| tst.js:6:7:6:7 | p | expression | +| tst.js:7:5:7:11 | new K() | constructor call to K | +| tst.js:7:9:7:9 | K | use of variable 'K' | +| tst.js:8:5:8:5 | a | use of variable 'a' | +| tst.js:8:5:8:9 | a + b | expression | +| tst.js:8:9:8:9 | b | use of variable 'b' | +| tst.js:9:5:9:10 | new "" | constructor call | +| tst.js:9:9:9:10 | "" | expression | +| tst.js:10:5:10:6 | "" | expression | +| tst.js:10:5:10:8 | ""() | call | +| tst.js:11:5:11:5 | o | use of variable 'o' | +| tst.js:11:5:11:8 | o[x] | expression | +| tst.js:11:7:11:7 | x | use of variable 'x' | +| tst.js:12:5:12:5 | o | use of variable 'o' | +| tst.js:12:5:12:10 | o['x'] | use of property 'x' | +| tst.js:12:7:12:9 | 'x' | expression | +| tst.js:13:5:13:5 | o | use of variable 'o' | +| tst.js:13:5:13:10 | o['x'] | use of property 'x' | +| tst.js:13:5:13:12 | o['x']() | method call to x | +| tst.js:13:7:13:9 | 'x' | expression | +| tst.js:14:5:14:7 | !'' | negation | +| tst.js:14:6:14:7 | '' | expression | diff --git a/javascript/ql/test/library-tests/Util/describeExpression.ql b/javascript/ql/test/library-tests/Util/describeExpression.ql new file mode 100644 index 000000000000..82188500a617 --- /dev/null +++ b/javascript/ql/test/library-tests/Util/describeExpression.ql @@ -0,0 +1,4 @@ +import semmle.javascript.Util + +from Expr e +select e, describeExpression(e) \ No newline at end of file diff --git a/javascript/ql/test/library-tests/Util/tst.js b/javascript/ql/test/library-tests/Util/tst.js index a9eba82d5340..a403d03e4dc2 100644 --- a/javascript/ql/test/library-tests/Util/tst.js +++ b/javascript/ql/test/library-tests/Util/tst.js @@ -1 +1,15 @@ -// used by qltest to identify the language +(function() { + v; + o.m(); + f(); + x == y; + o.p; + new K(); + a + b; + new ""; + ""(); + o[x]; + o['x']; + o['x'](); + !''; +}); diff --git a/javascript/ql/test/query-tests/Statements/UselessConditional/UselessConditional.expected b/javascript/ql/test/query-tests/Statements/UselessConditional/UselessConditional.expected index de351ec51b9d..9fdb5e0d6c64 100644 --- a/javascript/ql/test/query-tests/Statements/UselessConditional/UselessConditional.expected +++ b/javascript/ql/test/query-tests/Statements/UselessConditional/UselessConditional.expected @@ -1,22 +1,22 @@ | UselessConditional.js:5:7:5:12 | !lines | This expression always evaluates to false. | | UselessConditional.js:12:34:12:79 | (v = ne ... k] = v) | This logical 'and' expression can be replaced with a comma expression. | -| UselessConditional.js:17:9:17:9 | a | Variable 'a' always evaluates to false here. | -| UselessConditional.js:18:9:18:9 | b | Variable 'b' always evaluates to false here. | -| UselessConditional.js:21:9:21:9 | a | Variable 'a' always evaluates to false here. | -| UselessConditional.js:22:9:22:9 | b | Variable 'b' always evaluates to false here. | -| UselessConditional.js:26:6:26:6 | x | Variable 'x' always evaluates to true here. | -| UselessConditional.js:27:7:27:13 | new X() | This expression always evaluates to true. | -| UselessConditional.js:28:7:28:7 | x | Variable 'x' always evaluates to true here. | -| UselessConditional.js:29:8:29:8 | x | Variable 'x' always evaluates to true here. | -| UselessConditional.js:30:8:30:14 | new X() | This expression always evaluates to true. | -| UselessConditional.js:33:7:33:7 | x | Variable 'x' always evaluates to false here. | -| UselessConditional.js:54:9:54:13 | known | Variable 'known' always evaluates to false here. | -| UselessConditional.js:60:9:60:15 | unknown | Variable 'unknown' always evaluates to false here. | -| UselessConditional.js:65:5:65:5 | x | Variable 'x' always evaluates to true here. | -| UselessConditional.js:76:13:76:13 | x | Variable 'x' always evaluates to true here. | -| UselessConditional.js:82:13:82:13 | x | Variable 'x' always evaluates to true here. | +| UselessConditional.js:17:9:17:9 | a | Use of variable 'a' always evaluates to false here. | +| UselessConditional.js:18:9:18:9 | b | Use of variable 'b' always evaluates to false here. | +| UselessConditional.js:21:9:21:9 | a | Use of variable 'a' always evaluates to false here. | +| UselessConditional.js:22:9:22:9 | b | Use of variable 'b' always evaluates to false here. | +| UselessConditional.js:26:6:26:6 | x | Use of variable 'x' always evaluates to true here. | +| UselessConditional.js:27:7:27:13 | new X() | This constructor call to X always evaluates to true. | +| UselessConditional.js:28:7:28:7 | x | Use of variable 'x' always evaluates to true here. | +| UselessConditional.js:29:8:29:8 | x | Use of variable 'x' always evaluates to true here. | +| UselessConditional.js:30:8:30:14 | new X() | This constructor call to X always evaluates to true. | +| UselessConditional.js:33:7:33:7 | x | Use of variable 'x' always evaluates to false here. | +| UselessConditional.js:54:9:54:13 | known | Use of variable 'known' always evaluates to false here. | +| UselessConditional.js:60:9:60:15 | unknown | Use of variable 'unknown' always evaluates to false here. | +| UselessConditional.js:65:5:65:5 | x | Use of variable 'x' always evaluates to true here. | +| UselessConditional.js:76:13:76:13 | x | Use of variable 'x' always evaluates to true here. | +| UselessConditional.js:82:13:82:13 | x | Use of variable 'x' always evaluates to true here. | | UselessConditional.js:89:10:89:16 | x, true | This expression always evaluates to true. | -| UselessConditionalGood.js:58:12:58:13 | x2 | Variable 'x2' always evaluates to false here. | -| UselessConditionalGood.js:69:12:69:13 | xy | Variable 'xy' always evaluates to false here. | -| UselessConditionalGood.js:85:12:85:13 | xy | Variable 'xy' always evaluates to false here. | -| UselessConditionalGood.js:97:12:97:13 | xy | Variable 'xy' always evaluates to false here. | +| UselessConditionalGood.js:58:12:58:13 | x2 | Use of variable 'x2' always evaluates to false here. | +| UselessConditionalGood.js:69:12:69:13 | xy | Use of variable 'xy' always evaluates to false here. | +| UselessConditionalGood.js:85:12:85:13 | xy | Use of variable 'xy' always evaluates to false here. | +| UselessConditionalGood.js:97:12:97:13 | xy | Use of variable 'xy' always evaluates to false here. | From 651f32514bab45d40a1da5f0670c3ecde3c48dc2 Mon Sep 17 00:00:00 2001 From: Esben Sparre Andreasen Date: Wed, 31 Oct 2018 09:09:40 +0100 Subject: [PATCH 2/2] JS: use 'Util::describeExpression' in js/trivial-conditional --- .../ql/src/Statements/UselessConditional.ql | 5 +-- .../UselessConditional.expected | 36 +++++++++---------- 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/javascript/ql/src/Statements/UselessConditional.ql b/javascript/ql/src/Statements/UselessConditional.ql index a740e8628d8b..83a1c9a3b5b6 100644 --- a/javascript/ql/src/Statements/UselessConditional.ql +++ b/javascript/ql/src/Statements/UselessConditional.ql @@ -138,10 +138,7 @@ where isConditional(cond, op.asExpr()) and // otherwise we just report that `op` always evaluates to `cv` else ( sel = op.asExpr().stripParens() and - if sel instanceof VarAccess then - msg = "Variable '" + sel.(VarAccess).getVariable().getName() + "' always evaluates to " + cv + " here." - else - msg = "This expression always evaluates to " + cv + "." + msg = "This " + describeExpression(sel) + " always evaluates to " + cv + "." ) select sel, msg diff --git a/javascript/ql/test/query-tests/Statements/UselessConditional/UselessConditional.expected b/javascript/ql/test/query-tests/Statements/UselessConditional/UselessConditional.expected index 9fdb5e0d6c64..ae363adfdeab 100644 --- a/javascript/ql/test/query-tests/Statements/UselessConditional/UselessConditional.expected +++ b/javascript/ql/test/query-tests/Statements/UselessConditional/UselessConditional.expected @@ -1,22 +1,22 @@ -| UselessConditional.js:5:7:5:12 | !lines | This expression always evaluates to false. | +| UselessConditional.js:5:7:5:12 | !lines | This negation always evaluates to false. | | UselessConditional.js:12:34:12:79 | (v = ne ... k] = v) | This logical 'and' expression can be replaced with a comma expression. | -| UselessConditional.js:17:9:17:9 | a | Use of variable 'a' always evaluates to false here. | -| UselessConditional.js:18:9:18:9 | b | Use of variable 'b' always evaluates to false here. | -| UselessConditional.js:21:9:21:9 | a | Use of variable 'a' always evaluates to false here. | -| UselessConditional.js:22:9:22:9 | b | Use of variable 'b' always evaluates to false here. | -| UselessConditional.js:26:6:26:6 | x | Use of variable 'x' always evaluates to true here. | +| UselessConditional.js:17:9:17:9 | a | This use of variable 'a' always evaluates to false. | +| UselessConditional.js:18:9:18:9 | b | This use of variable 'b' always evaluates to false. | +| UselessConditional.js:21:9:21:9 | a | This use of variable 'a' always evaluates to false. | +| UselessConditional.js:22:9:22:9 | b | This use of variable 'b' always evaluates to false. | +| UselessConditional.js:26:6:26:6 | x | This use of variable 'x' always evaluates to true. | | UselessConditional.js:27:7:27:13 | new X() | This constructor call to X always evaluates to true. | -| UselessConditional.js:28:7:28:7 | x | Use of variable 'x' always evaluates to true here. | -| UselessConditional.js:29:8:29:8 | x | Use of variable 'x' always evaluates to true here. | +| UselessConditional.js:28:7:28:7 | x | This use of variable 'x' always evaluates to true. | +| UselessConditional.js:29:8:29:8 | x | This use of variable 'x' always evaluates to true. | | UselessConditional.js:30:8:30:14 | new X() | This constructor call to X always evaluates to true. | -| UselessConditional.js:33:7:33:7 | x | Use of variable 'x' always evaluates to false here. | -| UselessConditional.js:54:9:54:13 | known | Use of variable 'known' always evaluates to false here. | -| UselessConditional.js:60:9:60:15 | unknown | Use of variable 'unknown' always evaluates to false here. | -| UselessConditional.js:65:5:65:5 | x | Use of variable 'x' always evaluates to true here. | -| UselessConditional.js:76:13:76:13 | x | Use of variable 'x' always evaluates to true here. | -| UselessConditional.js:82:13:82:13 | x | Use of variable 'x' always evaluates to true here. | +| UselessConditional.js:33:7:33:7 | x | This use of variable 'x' always evaluates to false. | +| UselessConditional.js:54:9:54:13 | known | This use of variable 'known' always evaluates to false. | +| UselessConditional.js:60:9:60:15 | unknown | This use of variable 'unknown' always evaluates to false. | +| UselessConditional.js:65:5:65:5 | x | This use of variable 'x' always evaluates to true. | +| UselessConditional.js:76:13:76:13 | x | This use of variable 'x' always evaluates to true. | +| UselessConditional.js:82:13:82:13 | x | This use of variable 'x' always evaluates to true. | | UselessConditional.js:89:10:89:16 | x, true | This expression always evaluates to true. | -| UselessConditionalGood.js:58:12:58:13 | x2 | Use of variable 'x2' always evaluates to false here. | -| UselessConditionalGood.js:69:12:69:13 | xy | Use of variable 'xy' always evaluates to false here. | -| UselessConditionalGood.js:85:12:85:13 | xy | Use of variable 'xy' always evaluates to false here. | -| UselessConditionalGood.js:97:12:97:13 | xy | Use of variable 'xy' always evaluates to false here. | +| UselessConditionalGood.js:58:12:58:13 | x2 | This use of variable 'x2' always evaluates to false. | +| UselessConditionalGood.js:69:12:69:13 | xy | This use of variable 'xy' always evaluates to false. | +| UselessConditionalGood.js:85:12:85:13 | xy | This use of variable 'xy' always evaluates to false. | +| UselessConditionalGood.js:97:12:97:13 | xy | This use of variable 'xy' always evaluates to false. |