Improve detection of UMD modules.

We previously required the `define` to appear directly as an expression statement, but there are common patterns where this is not the case.

Author: Max Schaefer

javascript/ql/lib/semmle/javascript/AMD.qll

@@ -5,6 +5,7 @@
 
 import javascript
 private import semmle.javascript.internal.CachedStages
+private import Expressions.ExprHasNoEffect
 
 /**
  * An AMD `define` call.
@@ -26,7 +27,7 @@ private import semmle.javascript.internal.CachedStages
  */
 class AmdModuleDefinition extends CallExpr {
   AmdModuleDefinition() {
-    getParent() instanceof ExprStmt and
+    inVoidContext(this) and
     getCallee().(GlobalVarAccess).getName() = "define" and
     exists(int n | n = getNumArgument() |
       n = 1

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

@@ -12,6 +12,8 @@ amoModule_exports
 | tst5.js:1:1:6:3 | <toplevel> | foo | tst5.js:3:14:3:18 | a.foo |
 | tst.js:1:1:6:3 | <toplevel> | bar | tst.js:4:14:4:18 | b.bar |
 | tst.js:1:1:6:3 | <toplevel> | foo | tst.js:3:14:3:18 | a.foo |
+| umd2.js:1:1:12:4 | <toplevel> | bar | umd2.js:9:14:9:18 | a.foo |
+| umd2.js:1:1:12:4 | <toplevel> | foo | umd2.js:10:14:10:18 | b.bar |
 | umd.js:1:1:14:4 | <toplevel> | bar | umd.js:11:14:11:18 | a.foo |
 | umd.js:1:1:14:4 | <toplevel> | foo | umd.js:12:14:12:18 | b.bar |
 amdModule
@@ -25,6 +27,7 @@ amdModule
 | tst4.js:1:1:11:3 | <toplevel> | tst4.js:1:1:11:2 | define( ...   };\\n}) |
 | tst5.js:1:1:6:3 | <toplevel> | tst5.js:1:1:6:2 | define( ...   };\\n}) |
 | tst.js:1:1:6:3 | <toplevel> | tst.js:1:1:6:2 | define( ...   };\\n}) |
+| umd2.js:1:1:12:4 | <toplevel> | umd2.js:5:13:5:47 | define( ... actory) |
 | umd.js:1:1:14:4 | <toplevel> | umd.js:4:9:4:43 | define( ... actory) |
 getDependencyParameter
 | tst2.js:1:1:3:2 | define( ...  42;\\n}) | exports | tst2.js:1:30:1:36 | exports |
@@ -35,6 +38,8 @@ getDependencyParameter
 | tst5.js:1:1:6:2 | define( ...   };\\n}) | ./dir/b | tst5.js:1:40:1:44 | {bar} |
 | tst.js:1:1:6:2 | define( ...   };\\n}) | ./a | tst.js:1:37:1:37 | a |
 | tst.js:1:1:6:2 | define( ...   };\\n}) | ./dir/b | tst.js:1:40:1:40 | b |
+| umd2.js:5:13:5:47 | define( ... actory) | ./a | umd2.js:7:19:7:19 | a |
+| umd2.js:5:13:5:47 | define( ... actory) | ./dir/b | umd2.js:7:22:7:22 | b |
 | umd.js:4:9:4:43 | define( ... actory) | ./a | umd.js:9:19:9:19 | a |
 | umd.js:4:9:4:43 | define( ... actory) | ./dir/b | umd.js:9:22:9:22 | b |
 amdModuleDefinition
@@ -48,6 +53,8 @@ amdModuleDefinition
 | tst4.js:1:1:11:2 | define( ...   };\\n}) | tst4.js:6:11:11:1 | functio ...    };\\n} |
 | tst5.js:1:1:6:2 | define( ...   };\\n}) | tst5.js:1:28:6:1 | functio ...    };\\n} |
 | tst.js:1:1:6:2 | define( ...   };\\n}) | tst.js:1:28:6:1 | functio ...    };\\n} |
+| umd2.js:5:13:5:47 | define( ... actory) | umd2.js:1:22:1:28 | factory |
+| umd2.js:5:13:5:47 | define( ... actory) | umd2.js:7:9:12:1 | functio ...    };\\n} |
 | umd.js:4:9:4:43 | define( ... actory) | umd.js:1:18:1:24 | factory |
 | umd.js:4:9:4:43 | define( ... actory) | umd.js:9:9:14:1 | functio ...    };\\n} |
 amdModuleDependencies
@@ -61,6 +68,8 @@ amdModuleDependencies
 | tst5.js:1:1:6:2 | define( ...   };\\n}) | tst5.js:1:16:1:24 | './dir/b' |
 | tst.js:1:1:6:2 | define( ...   };\\n}) | tst.js:1:9:1:13 | './a' |
 | tst.js:1:1:6:2 | define( ...   };\\n}) | tst.js:1:16:1:24 | './dir/b' |
+| umd2.js:5:13:5:47 | define( ... actory) | umd2.js:5:21:5:25 | './a' |
+| umd2.js:5:13:5:47 | define( ... actory) | umd2.js:5:28:5:36 | './dir/b' |
 | umd.js:4:9:4:43 | define( ... actory) | umd.js:4:17:4:21 | './a' |
 | umd.js:4:9:4:43 | define( ... actory) | umd.js:4:24:4:32 | './dir/b' |
 amdModuleExportedSymbol
@@ -77,6 +86,8 @@ amdModuleExportedSymbol
 | tst5.js:1:1:6:3 | <toplevel> | foo |
 | tst.js:1:1:6:3 | <toplevel> | bar |
 | tst.js:1:1:6:3 | <toplevel> | foo |
+| umd2.js:1:1:12:4 | <toplevel> | bar |
+| umd2.js:1:1:12:4 | <toplevel> | foo |
 | umd.js:1:1:14:4 | <toplevel> | bar |
 | umd.js:1:1:14:4 | <toplevel> | foo |
 amdModuleExpr
@@ -88,6 +99,12 @@ amdModuleExpr
 | tst4.js:1:1:11:2 | define( ...   };\\n}) | tst4.js:7:12:10:5 | {\\n      ... r\\n    } | tst4.js:7:12:10:5 | {\\n      ... r\\n    } |
 | tst5.js:1:1:6:2 | define( ...   };\\n}) | tst5.js:2:12:5:5 | {\\n      ... r\\n    } | tst5.js:2:12:5:5 | {\\n      ... r\\n    } |
 | tst.js:1:1:6:2 | define( ...   };\\n}) | tst.js:2:12:5:5 | {\\n      ... r\\n    } | tst.js:2:12:5:5 | {\\n      ... r\\n    } |
+| umd2.js:5:13:5:47 | define( ... actory) | umd2.js:1:22:1:28 | factory | umd2.js:1:22:1:28 | factory |
+| umd2.js:5:13:5:47 | define( ... actory) | umd2.js:1:22:1:28 | factory | umd2.js:7:9:12:1 | functio ...    };\\n} |
+| umd2.js:5:13:5:47 | define( ... actory) | umd2.js:1:22:1:28 | factory | umd2.js:8:12:11:5 | {\\n      ... r\\n    } |
+| umd2.js:5:13:5:47 | define( ... actory) | umd2.js:8:12:11:5 | {\\n      ... r\\n    } | umd2.js:1:22:1:28 | factory |
+| umd2.js:5:13:5:47 | define( ... actory) | umd2.js:8:12:11:5 | {\\n      ... r\\n    } | umd2.js:7:9:12:1 | functio ...    };\\n} |
+| umd2.js:5:13:5:47 | define( ... actory) | umd2.js:8:12:11:5 | {\\n      ... r\\n    } | umd2.js:8:12:11:5 | {\\n      ... r\\n    } |
 | umd.js:4:9:4:43 | define( ... actory) | umd.js:1:18:1:24 | factory | umd.js:1:18:1:24 | factory |
 | umd.js:4:9:4:43 | define( ... actory) | umd.js:1:18:1:24 | factory | umd.js:9:9:14:1 | functio ...    };\\n} |
 | umd.js:4:9:4:43 | define( ... actory) | umd.js:1:18:1:24 | factory | umd.js:10:12:13:5 | {\\n      ... r\\n    } |
@@ -103,5 +120,9 @@ amdModuleImportedModule
 | tst5.js:1:1:6:3 | <toplevel> | tst5.js:1:16:1:24 | './dir/b' | dir/b.js:1:1:3:3 | <toplevel> |
 | tst.js:1:1:6:3 | <toplevel> | tst.js:1:9:1:13 | './a' | a.js:1:1:3:3 | <toplevel> |
 | tst.js:1:1:6:3 | <toplevel> | tst.js:1:16:1:24 | './dir/b' | dir/b.js:1:1:3:3 | <toplevel> |
+| umd2.js:1:1:12:4 | <toplevel> | umd2.js:3:34:3:47 | require('./a') | a.js:1:1:3:3 | <toplevel> |
+| umd2.js:1:1:12:4 | <toplevel> | umd2.js:3:50:3:67 | require('./dir/b') | dir/b.js:1:1:3:3 | <toplevel> |
+| umd2.js:1:1:12:4 | <toplevel> | umd2.js:5:21:5:25 | './a' | a.js:1:1:3:3 | <toplevel> |
+| umd2.js:1:1:12:4 | <toplevel> | umd2.js:5:28:5:36 | './dir/b' | dir/b.js:1:1:3:3 | <toplevel> |
 | umd.js:1:1:14:4 | <toplevel> | umd.js:4:17:4:21 | './a' | a.js:1:1:3:3 | <toplevel> |
 | umd.js:1:1:14:4 | <toplevel> | umd.js:4:24:4:32 | './dir/b' | dir/b.js:1:1:3:3 | <toplevel> |

javascript/ql/test/library-tests/AMD/umd2.js

@@ -0,0 +1,12 @@
+; (function (global, factory) {
+    typeof exports === 'object' && typeof module !== 'undefined' ?
+        module.exports = factory(require('./a'), require('./dir/b')) :
+        typeof define === 'function' && define.amd ?
+            define(['./a', './dir/b'], factory) :
+            global.mymodule = factory(global.a, global.dir.b)
+}(this, function (a, b) {
+    return {
+        bar: a.foo,
+        foo: b.bar
+    };
+}));