JS: Handle sequence expressions in module detection

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<Statement> 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;
   }

javascript/ql/test/library-tests/ModuleTypes/tests.expected

@@ -8,3 +8,4 @@
 | modulePackage/tst.js:1:1:1:75 | <toplevel> | es2015 |
 | require.js:1:1:7:1 | <toplevel> | node |
 | script.js:1:1:1:35 | <toplevel> | non-module |
+| toplevel-comma.js:1:1:5:0 | <toplevel> | node |

javascript/ql/test/library-tests/ModuleTypes/toplevel-comma.js

@@ -0,0 +1,4 @@
+(() => {}),
+(() => {
+    const fs = require('fs');
+})();