Merge branch 'master' into range-analysis

javascript/ql/src/Declarations/UnusedVariable.ql

@@ -49,24 +49,29 @@ predicate isPropertyFilter(UnusedLocal v) {
   )
 }
 
+predicate hasJsxInScope(UnusedLocal v) {
+  any(JSXNode n).getParent+() = v.getScope().getScopeElement()
+}
+
 /**
- * Holds if `v` is an import of React, and there is a JSX element that implicitly
- * references it.
- */
-predicate isReactImportForJSX(UnusedLocal v) {
-  exists (ImportSpecifier is |
-    is.getLocal() = v.getADeclaration() and
-    exists (JSXNode jsx | jsx.getTopLevel() = is.getTopLevel())
-    |
+ * Holds if `v` is a "React" variable that is implicitly used by a JSX element.
+*/
+predicate isReactForJSX(UnusedLocal v) {
+  hasJsxInScope(v) and
+  (
     v.getName() = "React"
     or
-    // legacy `@jsx` pragmas
-    exists (JSXPragma p | p.getTopLevel() = is.getTopLevel() | p.getDOMName() = v.getName())
-    or
-    // JSX pragma from a .babelrc file
-    exists (Babel::TransformReactJsxConfig plugin |
-      plugin.appliesTo(is.getTopLevel()) and
-      plugin.getJsxFactoryVariableName() = v.getName())
+    exists(TopLevel tl |
+      tl = v.getADeclaration().getTopLevel() |
+      // legacy `@jsx` pragmas
+      exists(JSXPragma p | p.getTopLevel() = tl | p.getDOMName() = v.getName())
+      or
+      // JSX pragma from a .babelrc file
+      exists(Babel::TransformReactJsxConfig plugin |
+        plugin.appliesTo(tl) and
+        plugin.getJsxFactoryVariableName() = v.getName()
+      )
+    )
   )
 }
 
@@ -150,7 +155,7 @@ predicate whitelisted(UnusedLocal v) {
   // exclude variables used to filter out unwanted properties
   isPropertyFilter(v) or
   // exclude imports of React that are implicitly referenced by JSX
-  isReactImportForJSX(v) or
+  isReactForJSX(v) or
   // exclude names that are used as types
   exists (VarDecl vd |
     v = vd.getVariable() |

javascript/ql/src/semmle/javascript/dataflow/TypeInference.qll

@@ -254,22 +254,18 @@ class AnalyzedFunction extends DataFlow::AnalyzedValueNode {
    * Gets a return value for a call to this function.
    */
   AbstractValue getAReturnValue() {
-    if astNode.isGenerator() or astNode.isAsync() then
-      result = TAbstractOtherObject()
-    else (
-      // explicit return value
-      result = astNode.getAReturnedExpr().analyze().getALocalValue()
+    // explicit return value
+    result = astNode.getAReturnedExpr().analyze().getALocalValue()
+    or
+    // implicit return value
+    (
+      // either because execution of the function may terminate normally
+      mayReturnImplicitly()
       or
-      // implicit return value
-      (
-        // either because execution of the function may terminate normally
-        mayReturnImplicitly()
-        or
-        // or because there is a bare `return;` statement
-        exists (ReturnStmt ret | ret = astNode.getAReturnStmt() | not exists(ret.getExpr()))
-      ) and
-      result = TAbstractUndefined()
-    )
+      // or because there is a bare `return;` statement
+      exists (ReturnStmt ret | ret = astNode.getAReturnStmt() | not exists(ret.getExpr()))
+    ) and
+    result = TAbstractUndefined()
   }
 
   /**
@@ -288,5 +284,26 @@ class AnalyzedFunction extends DataFlow::AnalyzedValueNode {
       not final instanceof ThrowStmt
     )
   }
-
+}
+
+/**
+ * Flow analysis for generator functions.
+ */
+private class AnalyzedGeneratorFunction extends AnalyzedFunction {
+  AnalyzedGeneratorFunction() { astNode.isGenerator() }
+
+  override AbstractValue getAReturnValue() {
+    result = TAbstractOtherObject()
+  }
+}
+
+/**
+ * Flow analysis for async functions.
+ */
+private class AnalyzedAsyncFunction extends AnalyzedFunction {
+  AnalyzedAsyncFunction() { astNode.isAsync() }
+
+  override AbstractValue getAReturnValue() {
+    result = TAbstractOtherObject()
+  }
 }

javascript/ql/src/semmle/javascript/dataflow/internal/AbstractPropertiesImpl.qll

@@ -39,16 +39,7 @@ AbstractValue getAnInitialPropertyValue(DefiniteAbstractValue baseVal, string pr
   )
   or
   // class members
-  exists (ClassDefinition c, DataFlow::AnalyzedNode init, MemberDefinition m |
-    m = c.getMember(propertyName) and
-    not m instanceof AccessorMethodDefinition and
-    init = m.getInit().analyze() and
-    result = init.getALocalValue() |
-    if m.isStatic() then
-      baseVal = TAbstractClass(c)
-    else
-      baseVal = AbstractInstance::of(c)
-  )
+  result = getAnInitialMemberValue(getMember(baseVal, propertyName))
   or
   // object properties
   exists (ValueProperty p |
@@ -63,6 +54,30 @@ AbstractValue getAnInitialPropertyValue(DefiniteAbstractValue baseVal, string pr
   result = TAbstractInstance(baseVal)
 }
 
+/**
+ * Gets a class member definition that we abstractly represent as a property of `baseVal`
+ * with the given `name`.
+ */
+private MemberDefinition getMember(DefiniteAbstractValue baseVal, string name) {
+  exists (ClassDefinition c | result = c.getMember(name) |
+    if result.isStatic() then
+      baseVal = TAbstractClass(c)
+    else
+      baseVal = AbstractInstance::of(c)
+  )
+}
+
+/**
+ * Gets an abstract representation of the initial value of member definition `m`.
+ *
+ * For (non-accessor) methods, this is the abstract function corresponding to the
+ * method. For fields, it is an abstract representation of their initial value(s).
+ */
+private AbstractValue getAnInitialMemberValue(MemberDefinition m) {
+  not m instanceof AccessorMethodDefinition and
+  result = m.getInit().analyze().getALocalValue()
+}
+
 /**
  * Holds if `baseVal` is an abstract value whose properties we track for the purposes
  * of `getALocalValue`.

javascript/ql/test/query-tests/Declarations/UnusedVariable/UnusedVariable.expected

@@ -7,6 +7,7 @@
 | importWithoutPragma.jsx:1:1:1:27 | import  ... react'; | Unused import h. |
 | multi-imports.js:1:1:1:29 | import  ... om 'x'; | Unused imports a, b, d. |
 | multi-imports.js:2:1:2:42 | import  ... om 'x'; | Unused imports alphabetically, ordered. |
+| require-react-in-other-scope.js:2:9:2:13 | React | Unused variable React. |
 | typeoftype.ts:9:7:9:7 | y | Unused variable y. |
 | unusedShadowed.ts:1:1:1:26 | import  ... where'; | Unused import T. |
 | unusedShadowed.ts:2:1:2:31 | import  ... where'; | Unused import object. |

javascript/ql/test/query-tests/Declarations/UnusedVariable/react-jsx.js

@@ -0,0 +1,2 @@
+var React = x; // OK
+(<e/>);

javascript/ql/test/query-tests/Declarations/UnusedVariable/require-react-1.js

@@ -0,0 +1,2 @@
+var React = require("probably-react"); // OK
+(<e/>);

javascript/ql/test/query-tests/Declarations/UnusedVariable/require-react-2.js

@@ -0,0 +1,2 @@
+var { React } = { React: require("probably-react") }; // OK
+(<e/>);

javascript/ql/test/query-tests/Declarations/UnusedVariable/require-react-3.js

@@ -0,0 +1,2 @@
+var { React } = require("probably-react"); // OK
+(<e/>);

javascript/ql/test/query-tests/Declarations/UnusedVariable/require-react-in-other-scope.js

@@ -0,0 +1,6 @@
+(function() {
+    var React  = require("probably-react"); // NOT OK
+})
+(function() {
+    (<e/>);
+})