Merge pull request #373 from asger-semmle/jsx-factory-import

Approved by xiemaisi

change-notes/1.19/analysis-javascript.md

@@ -38,6 +38,7 @@
 | Unused variable, import, function or class | Fewer results | This rule now flags import statements with multiple unused imports once. |
 | User-controlled bypass of security check | Fewer results | This rule no longer flags conditions that guard early returns. The precision of this rule has been revised to "medium". Results are no longer shown on LGTM by default. | 
 | Whitespace contradicts operator precedence | Fewer false-positive results | This rule no longer flags operators with asymmetric whitespace. |
+| Unused import | Fewer false-positive results | This rule no longer flags imports used by the `transform-react-jsx` Babel plugin. |
 
 ## Changes to QL libraries
 

javascript/ql/src/Declarations/UnusedVariable.ql

@@ -56,10 +56,17 @@ predicate isPropertyFilter(UnusedLocal v) {
 predicate isReactImportForJSX(UnusedLocal v) {
   exists (ImportSpecifier is |
     is.getLocal() = v.getADeclaration() and
-    exists (JSXNode jsx | jsx.getTopLevel() = is.getTopLevel()) |
-    v.getName() = "React" or
-    // also accept legacy `@jsx` pragmas
+    exists (JSXNode jsx | jsx.getTopLevel() = is.getTopLevel())
+    |
+    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())
   )
 }
 

javascript/ql/src/semmle/javascript/frameworks/Babel.qll

@@ -28,6 +28,62 @@ module Babel {
         result.(JSONArray).getElementStringValue(0) = pluginName
       )
     }
+
+    /**
+     * Gets a file affected by this Babel configuration.
+     */
+    Container getAContainerInScope() {
+      result = getFile().getParentContainer()
+      or
+      result = getAContainerInScope().getAChildContainer() and
+      // File-relative .babelrc search stops at any package.json or .babelrc file.
+      not result.getAChildContainer() = any(PackageJSON pkg).getFile() and
+      not result.getAChildContainer() = any(Config pkg).getFile()
+    }
+
+    /**
+     * Holds if this configuration applies to `tl`.
+     */
+    predicate appliesTo(TopLevel tl) {
+      tl.getFile() = getAContainerInScope()
+    }
+  }
+
+  /**
+   * Configuration object for a Babel plugin.
+   */
+  class Plugin extends JSONValue {
+    Config cfg;
+    string pluginName;
+
+    Plugin() {
+      this = cfg.getPluginConfig(pluginName)
+    }
+
+    /** Gets the name of the plugin being installed. */
+    string getPluginName() {
+      result = pluginName
+    }
+
+    /** Gets the enclosing Babel configuration object. */
+    Config getConfig() {
+      result = cfg
+    }
+
+    /** Gets the options value passed to the plugin, if any. */
+    JSONValue getOptions() {
+      result = this.(JSONArray).getElementValue(1)
+    }
+
+    /** Gets a named option from the option object, if present. */
+    JSONValue getOption(string name) {
+      result = getOptions().(JSONObject).getPropValue(name)
+    }
+
+    /** Holds if this plugin applies to `tl`. */
+    predicate appliesTo(TopLevel tl) {
+      cfg.appliesTo(tl)
+    }
   }
 
   /**
@@ -38,11 +94,9 @@ module Babel {
    * each path is of the form `{ "rootPathPrefix": "...", "rootPathSuffix": "..." }` and explicitly
    * specifies a mapping from a path prefix to a root.
    */
-  class RootImportConfig extends JSONArray {
-    Config cfg;
-
+  class RootImportConfig extends Plugin {
     RootImportConfig() {
-      this = cfg.getPluginConfig("babel-plugin-root-import")
+      pluginName = "babel-plugin-root-import"
     }
 
     /**
@@ -62,15 +116,16 @@ module Babel {
      */
     private JSONObject getARootPathSpec() {
       // ["babel-plugin-root-import", <spec>]
-      result = getElementValue(1) and
+      result = getOptions() and
       exists(result.getPropValue("rootPathSuffix"))
       or
       exists (JSONArray pathSpecs |
         // ["babel-plugin-root-import", [ <spec>... ] ]
-        pathSpecs = getElementValue(1)
+        pathSpecs = getOptions()
         or
         // ["babel-plugin-root-import", { "paths": [ <spec> ... ] }]
-        pathSpecs = getElementValue(1).(JSONObject).getPropValue("paths") |
+        pathSpecs = getOption("paths")
+        |
         result = pathSpecs.getElementValue(_)
       )
     }
@@ -95,20 +150,13 @@ module Babel {
     Folder getFolder() {
       result = getFile().getParentContainer()
     }
-
-    /**
-     * Holds if this configuration applies to `tl`.
-     */
-    predicate appliesTo(TopLevel tl) {
-      tl.getFile().getParentContainer+() = getFolder()
-    }
   }
 
   /**
    * An import path expression that may be transformed by `babel-plugin-root-import`.
    */
   private class BabelRootTransformedPathExpr extends PathExpr, Expr {
-    RootImportConfig cfg;
+    RootImportConfig plugin;
     string rawPath;
     string prefix;
     string mappedPrefix;
@@ -116,16 +164,16 @@ module Babel {
 
     BabelRootTransformedPathExpr() {
       this instanceof PathExpr and
-      cfg.appliesTo(getTopLevel()) and
+      plugin.appliesTo(getTopLevel()) and
       rawPath = getStringValue() and
       prefix = rawPath.regexpCapture("(.)/(.*)", 1) and
       suffix = rawPath.regexpCapture("(.)/(.*)", 2) and
-      mappedPrefix = cfg.getRoot(prefix)
+      mappedPrefix = plugin.getRoot(prefix)
     }
 
     /** Gets the configuration that applies to this path. */
-    RootImportConfig getConfig() {
-      result = cfg
+    RootImportConfig getPlugin() {
+      result = plugin
     }
 
     override string getValue() {
@@ -134,7 +182,7 @@ module Babel {
 
     override Folder getSearchRoot(int priority) {
       priority = 0 and
-      result = cfg.getFolder()
+      result = plugin.getFolder()
     }
   }
 
@@ -149,7 +197,24 @@ module Babel {
     }
 
     override Folder getARootFolder() {
-      result = pathExpr.getConfig().getFolder()
+      result = pathExpr.getPlugin().getFolder()
+    }
+  }
+
+  /**
+   * A configuration object for the `transform-react-jsx` plugin.
+   *
+   * The plugin option `{"pragma": xxx}` specifies a variable name used to instantiate
+   * JSX elements.
+   */
+  class TransformReactJsxConfig extends Plugin {
+    TransformReactJsxConfig() {
+      pluginName = "transform-react-jsx"
+    }
+
+    /** Gets the name of the variable used to create JSX elements. */
+    string getJsxFactoryVariableName() {
+      result = getOption("pragma").(JSONString).getValue()
     }
   }
 }

javascript/ql/test/query-tests/Declarations/UnusedVariable/Babelrc/.babelrc.json

@@ -0,0 +1,5 @@
+{
+  "plugins": [
+    ["transform-react-jsx", { "pragma": "h" }]
+  ]
+}

javascript/ql/test/query-tests/Declarations/UnusedVariable/Babelrc/importPragma.jsx

@@ -0,0 +1,4 @@
+import { h } from 'preact'; // OK - JSX element uses 'h' after babel compilation
+import { q } from 'preact'; // NOT OK - not used 
+
+export default (<div>Hello</div>);

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

@@ -1,6 +1,8 @@
+| Babelrc/importPragma.jsx:2:1:2:27 | import  ... react'; | Unused import q. |
 | decorated.ts:1:1:1:126 | import  ... where'; | Unused import actionHandler. |
 | decorated.ts:4:10:4:12 | fun | Unused function fun. |
 | externs.js:6:5:6:13 | iAmUnused | Unused variable iAmUnused. |
+| 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. |
 | typeoftype.ts:9:7:9:7 | y | Unused variable y. |

javascript/ql/test/query-tests/Declarations/UnusedVariable/importWithoutPragma.jsx

@@ -0,0 +1,3 @@
+import { h } from 'preact'; // NOT OK - not in scope of .babelrc file
+
+export default (<div>Hello</div>);