Repository: ember-cli/eslint-plugin-ember
Pull request #1072 (2021-01-19 00:24:21)

Improve jquery detection in `jquery-ember-run` rule
* Uses the ReferenceTracker class like our other jQuery rules to better detect all global/imported jQuery calls
* Add import checking
* Reduce false positives and false negatives

Similar to https://github.com/ember-cli/eslint-plugin-ember/pull/1063.

CC: @BarryThePenguin


Patch:
diff --git a/lib/rules/jquery-ember-run.js b/lib/rules/jquery-ember-run.js
index a5035dde..81b5e7c3 100644
--- a/lib/rules/jquery-ember-run.js
+++ b/lib/rules/jquery-ember-run.js
@@ -2,24 +2,14 @@
 
 const utils = require('../utils/utils');
 const types = require('../utils/types');
-const ember = require('../utils/ember');
+const { getImportIdentifier } = require('../utils/import');
+const { ReferenceTracker } = require('eslint-utils');
+const { globalMap, esmMap } = require('../utils/jquery');
 
 //------------------------------------------------------------------------------
 // General rule - Don’t use jQuery without Ember Run Loop
 //------------------------------------------------------------------------------
 
-const isJqueryUsed = function (node) {
-  return (
-    types.isMemberExpression(node) &&
-    types.isCallExpression(node.object) &&
-    ember.isModule(node.object, '$')
-  );
-};
-
-const isRunUsed = function (node) {
-  return ember.isModule(node, 'run');
-};
-
 const ERROR_MESSAGE = "Don't use jQuery without Ember Run Loop";
 
 module.exports = {
@@ -43,28 +33,86 @@ module.exports = {
       context.report(node, ERROR_MESSAGE);
     };
 
-    return {
-      CallExpression(node) {
-        const callee = node.callee;
-        const fnNodes = utils.findNodes(node.arguments, 'ArrowFunctionExpression');
+    let importedEmberName;
+    let importedBindName;
+
+    function checkJqueryCall(node) {
+      if (
+        // Check to see if this jquery call looks like: $(...).on(() => { ... }));
+        node.parent.type === 'MemberExpression' &&
+        node.parent.object === node &&
+        node.parent.property.type === 'Identifier' &&
+        node.parent.property.name === 'on' &&
+        node.parent.parent.type === 'CallExpression'
+      ) {
+        const onCall = node.parent.parent;
+        const fnNodes = utils.findNodes(onCall.arguments, 'ArrowFunctionExpression');
+        fnNodes.forEach((fnNode) => {
+          const fnBody = fnNode.body.body;
+          const fnExpressions = utils.findNodes(fnBody, 'ExpressionStatement');
+          fnExpressions.forEach((fnExpression) => {
+            const expression = fnExpression.expression;
 
-        if (isJqueryUsed(callee) && fnNodes.length > 0) {
-          fnNodes.forEach((fnNode) => {
-            const fnBody = fnNode.body.body;
-            const fnExpressions = utils.findNodes(fnBody, 'ExpressionStatement');
+            // Check for imported call to: bind()
+            const isBindCall =
+              types.isCallExpression(expression) &&
+              expression.callee.type === 'Identifier' &&
+              expression.callee.name === importedBindName;
 
-            fnExpressions.forEach((fnExpression) => {
-              const expression = fnExpression.expression;
+            // Check for old-style: Ember.run.bind()
+            const isEmberBindCall =
+              types.isCallExpression(expression) &&
+              expression.callee.type === 'MemberExpression' &&
+              expression.callee.property.type === 'Identifier' &&
+              expression.callee.property.name === 'bind' &&
+              expression.callee.object.type === 'MemberExpression' &&
+              expression.callee.object.property.type === 'Identifier' &&
+              expression.callee.object.property.name === 'run' &&
+              expression.callee.object.object.type === 'Identifier' &&
+              expression.callee.object.object.name === importedEmberName;
 
-              if (
-                types.isCallExpression(expression) &&
-                types.isMemberExpression(expression.callee) &&
-                !isRunUsed(expression)
-              ) {
-                report(expression.callee);
-              }
-            });
+            if (!isBindCall && !isEmberBindCall) {
+              report(expression.callee);
+            }
           });
+        });
+      }
+    }
+
+    return {
+      ImportDeclaration(node) {
+        if (node.source.value === 'ember') {
+          importedEmberName = importedEmberName || getImportIdentifier(node, 'ember');
+        }
+        if (node.source.value === '@ember/runloop') {
+          importedBindName =
+            importedBindName || getImportIdentifier(node, '@ember/runloop', 'bind');
+        }
+      },
+
+      'Program:exit'() {
+        const scope = context.getScope();
+        const tracker = new ReferenceTracker(scope);
+
+        /**
+         * Global references
+         *
+         * eg; $(body) and $.post()
+         */
+        for (const { node } of tracker.iterateGlobalReferences(globalMap)) {
+          checkJqueryCall(node);
+        }
+
+        /**
+         * ESM references
+         *   import $ from 'jquery'
+         *   import { $ as jq } from 'ember'
+         *
+         * eg;
+         *   $(body) and jq.post()
+         */
+        for (const { node } of tracker.iterateEsmReferences(esmMap)) {
+          checkJqueryCall(node);
         }
       },
     };
diff --git a/lib/rules/no-global-jquery.js b/lib/rules/no-global-jquery.js
index c3cac9cb..1710af2f 100644
--- a/lib/rules/no-global-jquery.js
+++ b/lib/rules/no-global-jquery.js
@@ -1,23 +1,10 @@
 'use strict';
 
 const { ReferenceTracker } = require('eslint-utils');
-const jqueryMethods = require('../utils/jquery-methods');
+const { globalMap } = require('../utils/jquery');
 
 const ERROR_MESSAGE = 'Do not use global `$` or `jQuery`';
 
-const jqueryMap = {
-  [ReferenceTracker.CALL]: true,
-};
-
-for (const method of jqueryMethods) {
-  jqueryMap[method] = { [ReferenceTracker.CALL]: true };
-}
-
-const traceMap = {
-  $: jqueryMap,
-  jQuery: jqueryMap,
-};
-
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
@@ -43,7 +30,7 @@ module.exports = {
       Program() {
         const tracker = new ReferenceTracker(context.getScope());
 
-        for (const { node } of tracker.iterateGlobalReferences(traceMap)) {
+        for (const { node } of tracker.iterateGlobalReferences(globalMap)) {
           context.report(node, ERROR_MESSAGE);
         }
       },
diff --git a/lib/rules/no-jquery.js b/lib/rules/no-jquery.js
index aaeba802..282d5671 100644
--- a/lib/rules/no-jquery.js
+++ b/lib/rules/no-jquery.js
@@ -2,7 +2,7 @@
 
 const { ReferenceTracker } = require('eslint-utils');
 const { isCallExpression } = require('../utils/types');
-const jqueryMethods = require('../utils/jquery-methods');
+const { globalMap, esmMap } = require('../utils/jquery');
 
 //------------------------------------------------------------------------------
 // General rule -  Disallow usage of jQuery
@@ -10,33 +10,6 @@ const jqueryMethods = require('../utils/jquery-methods');
 
 const ERROR_MESSAGE = 'Do not use jQuery';
 
-const jqueryMap = {
-  [ReferenceTracker.CALL]: true,
-};
-
-for (const method of jqueryMethods) {
-  jqueryMap[method] = { [ReferenceTracker.CALL]: true };
-}
-
-const globalMap = {
-  $: jqueryMap,
-  jQuery: jqueryMap,
-};
-
-const esmMap = {
-  jquery: {
-    [ReferenceTracker.ESM]: true,
-    default: jqueryMap,
-  },
-  ember: {
-    [ReferenceTracker.ESM]: true,
-    default: {
-      $: jqueryMap,
-    },
-    $: jqueryMap,
-  },
-};
-
 module.exports = {
   meta: {
     type: 'suggestion',
diff --git a/lib/utils/jquery.js b/lib/utils/jquery.js
new file mode 100644
index 00000000..f8da316a
--- /dev/null
+++ b/lib/utils/jquery.js
@@ -0,0 +1,51 @@
+const { ReferenceTracker } = require('eslint-utils');
+const jqueryMethods = require('../utils/jquery-methods');
+
+const jqueryMap = {
+  [ReferenceTracker.CALL]: true,
+};
+
+for (const method of jqueryMethods) {
+  jqueryMap[method] = { [ReferenceTracker.CALL]: true };
+}
+
+/**
+ * Global references
+ *
+ * eg; $(body) and $.post()
+ *
+ * For use with ReferenceTracker: tracker.iterateGlobalReferences();
+ */
+const globalMap = {
+  $: jqueryMap,
+  jQuery: jqueryMap,
+};
+
+/**
+ * ESM references
+ *   import $ from 'jquery'
+ *   import { $ as jq } from 'ember'
+ *
+ * eg;
+ *   $(body) and jq.post()
+ *
+ * For use with ReferenceTracker: tracker.iterateEsmReferences();
+ */
+const esmMap = {
+  jquery: {
+    [ReferenceTracker.ESM]: true,
+    default: jqueryMap,
+  },
+  ember: {
+    [ReferenceTracker.ESM]: true,
+    default: {
+      $: jqueryMap,
+    },
+    $: jqueryMap,
+  },
+};
+
+module.exports = {
+  globalMap,
+  esmMap,
+};


Test patch:
diff --git a/tests/lib/rules/jquery-ember-run.js b/tests/lib/rules/jquery-ember-run.js
index 23ee48a1..d07d28ff 100644
--- a/tests/lib/rules/jquery-ember-run.js
+++ b/tests/lib/rules/jquery-ember-run.js
@@ -26,8 +26,12 @@ eslintTester.run('jquery-ember-run', rule, {
      $("#item").on("click", () => { Ember.run.bind(this, this.handle); });`,
 
     // Global jQuery
-    `import { bind } from "@ember/runloop";
-     $("#item").on("click", () => { bind(this, this.handle); });`,
+    {
+      code: `
+      import { bind } from "@ember/runloop";
+      $("#item").on("click", () => { bind(this, this.handle); });`,
+      globals: { $: true },
+    },
 
     // Imported jQuery
     `import { bind } from "@ember/runloop";
@@ -35,11 +39,16 @@ eslintTester.run('jquery-ember-run', rule, {
      $("#item").on("click", () => { bind(this, this.handle); });`,
 
     // No callback
-    '$("#item");',
-    '$("#item").on("click");',
+    'import $ from "jquery"; $("#item");',
+    'import $ from "jquery"; $("#item").on("click");',
+    'import $ from "jquery"; $("#item").on("click", function() {});',
+    'import $ from "jquery"; $("#item").on("click", () => {});',
 
     // Callback but not jQuery
     'notJquery("#item").on("click", () => {this.handle();});',
+
+    // Not `on`
+    'import $ from "jquery"; $("#item").notOn("click", () => {this.handle();});',
   ],
   invalid: [
     {
@@ -58,6 +67,7 @@ eslintTester.run('jquery-ember-run', rule, {
     {
       // Global jQuery
       code: '$("#item").on("click", () => {this.handle();});',
+      globals: { $: true },
       output: null,
       errors: [{ message: ERROR_MESSAGE, type: 'MemberExpression' }],
     },
@@ -67,5 +77,31 @@ eslintTester.run('jquery-ember-run', rule, {
       output: null,
       errors: [{ message: ERROR_MESSAGE, type: 'MemberExpression' }],
     },
+    {
+      // With function call that isn't `bind`
+      code: 'import $ from "jquery"; $("#item").on("click", () => { notBind(); });',
+      output: null,
+      errors: [{ message: ERROR_MESSAGE, type: 'Identifier' }],
+    },
+    {
+      // With function call that isn't `Ember`
+      code: 'import $ from "jquery"; $("#item").on("click", () => { notEmber.run.bind(); });',
+      output: null,
+      errors: [{ message: ERROR_MESSAGE, type: 'MemberExpression' }],
+    },
+    {
+      // With function call that isn't `run`
+      code:
+        'import Ember from "ember"; import $ from "jquery"; $("#item").on("click", () => { Ember.notRun.bind(); });',
+      output: null,
+      errors: [{ message: ERROR_MESSAGE, type: 'MemberExpression' }],
+    },
+    {
+      // With function call that isn't `bind`
+      code:
+        'import Ember from "ember"; import $ from "jquery"; $("#item").on("click", () => { Ember.run.notBind(); });',
+      output: null,
+      errors: [{ message: ERROR_MESSAGE, type: 'MemberExpression' }],
+    },
   ],
 });