diff --git a/javascript/extractor/src/com/semmle/js/extractor/AbstractDetector.java b/javascript/extractor/src/com/semmle/js/extractor/AbstractDetector.java index b8a5f1df0fcf..8b1ed9cc5a68 100644 --- a/javascript/extractor/src/com/semmle/js/extractor/AbstractDetector.java +++ b/javascript/extractor/src/com/semmle/js/extractor/AbstractDetector.java @@ -12,6 +12,7 @@ import com.semmle.js.ast.Node; import com.semmle.js.ast.ParenthesizedExpression; import com.semmle.js.ast.Program; +import com.semmle.js.ast.SequenceExpression; import com.semmle.js.ast.Statement; import com.semmle.js.ast.TryStatement; import com.semmle.js.ast.UnaryExpression; @@ -38,27 +39,10 @@ protected boolean visitStatements(List stmts) { protected boolean visitStatement(Statement stmt) { if (stmt instanceof ExpressionStatement) { - Expression e = stripParens(((ExpressionStatement) stmt).getExpression()); - - // check whether `e` is an iife; if so, recursively check its body - - // strip off unary operators to handle `!function(){...}()` - if (e instanceof UnaryExpression) e = ((UnaryExpression) e).getArgument(); - - if (e instanceof CallExpression && ((CallExpression) e).getArguments().isEmpty()) { - Expression callee = stripParens(((CallExpression) e).getCallee()); - if (callee instanceof IFunction) { - Node body = ((IFunction) callee).getBody(); - if (body instanceof BlockStatement) - return visitStatements(((BlockStatement) body).getBody()); - } - } - - if (visitExpression(e)) return true; - + return visitExpression(((ExpressionStatement) stmt).getExpression()); } else if (stmt instanceof VariableDeclaration) { for (VariableDeclarator decl : ((VariableDeclaration) stmt).getDeclarations()) { - Expression init = stripParens(decl.getInit()); + Expression init = decl.getInit(); if (visitExpression(init)) return true; } @@ -77,17 +61,13 @@ protected boolean visitStatement(Statement stmt) { return false; } - private static Expression stripParens(Expression e) { - if (e instanceof ParenthesizedExpression) - return stripParens(((ParenthesizedExpression) e).getExpression()); - return e; - } - /** * Recursively check {@code e} if it's a call or an assignment. */ protected boolean visitExpression(Expression e) { - if (e instanceof CallExpression) { + if (e instanceof ParenthesizedExpression) { + return visitExpression(((ParenthesizedExpression) e).getExpression()); + } else if (e instanceof CallExpression) { CallExpression call = (CallExpression) e; Expression callee = call.getCallee(); // recurse, to handle things like `foo()()` @@ -102,6 +82,18 @@ protected boolean visitExpression(Expression e) { if (!"=".equals(assgn.getOperator())) return false; return visitExpression(assgn.getRight()); + } else if (e instanceof UnaryExpression) { + return visitExpression(((UnaryExpression) e).getArgument()); + } else if (e instanceof IFunction) { + Node body = ((IFunction) e).getBody(); + if (body instanceof BlockStatement) { + return visitStatement((BlockStatement) body); + } + } else if (e instanceof SequenceExpression) { + SequenceExpression seq = (SequenceExpression) e; + for (Expression child : seq.getExpressions()) { + if (visitExpression(child)) return true; + } } return false; } diff --git a/javascript/ql/test/library-tests/ModuleTypes/tests.expected b/javascript/ql/test/library-tests/ModuleTypes/tests.expected index a76b0d2b89cb..103a65951db0 100644 --- a/javascript/ql/test/library-tests/ModuleTypes/tests.expected +++ b/javascript/ql/test/library-tests/ModuleTypes/tests.expected @@ -8,3 +8,4 @@ | modulePackage/tst.js:1:1:1:75 | | es2015 | | require.js:1:1:7:1 | | node | | script.js:1:1:1:35 | | non-module | +| toplevel-comma.js:1:1:5:0 | | node | diff --git a/javascript/ql/test/library-tests/ModuleTypes/toplevel-comma.js b/javascript/ql/test/library-tests/ModuleTypes/toplevel-comma.js new file mode 100644 index 000000000000..4a0a5eee01b5 --- /dev/null +++ b/javascript/ql/test/library-tests/ModuleTypes/toplevel-comma.js @@ -0,0 +1,4 @@ +(() => {}), +(() => { + const fs = require('fs'); +})();