JS: simplify isReactImportForJSX to isReactForJSX

Author: Esben Sparre Andreasen

javascript/ql/src/Declarations/UnusedVariable.ql

@@ -49,17 +49,20 @@ 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(JSXNode jsx, TopLevel tl |
-    jsx.getTopLevel() = tl and
-    v.getADeclaration().getTopLevel() = tl and
-    (
-      v.getName() = "React"
-      or
+ * 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
+    exists(TopLevel tl |
+      tl = v.getADeclaration().getTopLevel() |
       // legacy `@jsx` pragmas
       exists(JSXPragma p | p.getTopLevel() = tl | p.getDOMName() = v.getName())
       or
@@ -69,12 +72,6 @@ predicate isReactImportForJSX(UnusedLocal v) {
         plugin.getJsxFactoryVariableName() = v.getName()
       )
     )
-    |
-    v.getADeclaration() = any(ImportDeclaration imp).getASpecifier().getLocal()
-    or
-    exists(VariableDeclarator vd | vd.getBindingPattern() = v.getADeclaration() |
-      vd.getInit() instanceof Require
-    )
   )
 }
 
@@ -158,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/test/query-tests/Declarations/UnusedVariable/UnusedVariable.expected

@@ -7,7 +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-3.js:1:7:1:11 | React | Unused variable React. |
+| 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-3.js

@@ -1,2 +1,2 @@
-var { React } = require("probably-react"); // OK, but not yet supported
+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/>);
+})