Rethink detecting global jquery var assignments

Author: nlfurniss

lib/rules/no-global-jquery.js

@@ -2,11 +2,9 @@
  * @fileOverview Disallow the use of global `$`.
  */
 'use strict';
-
 const { getEmberImportBinding } = require('../utils/imports');
 const {
-  isJQueryCallee,
-  isVariableAssigmentJquery,
+  isVariableAssigmentGlobalJquery,
   JQUERY_ALIASES
 } = require('../utils/jquery');
 const { collectObjectPatternBindings } = require('../utils/destructed-binding');
@@ -34,7 +32,6 @@ module.exports = {
   create(context) {
     let emberImportBinding;
     let destructuredAssignment = null;
-    let jQueryLocalName = null;
 
     function isDestructured(node) {
       return node && node.callee && node.callee.name === destructuredAssignment;
@@ -54,13 +51,13 @@ module.exports = {
       },
 
       VariableDeclarator(node) {
-        jQueryLocalName = isVariableAssigmentJquery(node);
+        if (isVariableAssigmentGlobalJquery(node)) {
+          context.report(node, MESSAGE);
+        }
       },
 
       CallExpression(node) {
-        if (jQueryLocalName && !isJQueryCallee(node, jQueryLocalName)) {
-          return;
-        } else if (!isDestructured(node) && isGlobalJquery(node)) {
+        if (!isDestructured(node) && isGlobalJquery(node)) {
           context.report(node, MESSAGE);
         }
       }

lib/rules/no-jquery-methods.js

@@ -4,7 +4,7 @@
 'use strict';
 
 const { get } = require('../utils/get');
-const { isJQueryCallee, isVariableAssigmentJquery } = require('../utils/jquery');
+const { isJQueryCallee, isVariableAssigmentGlobalJquery } = require('../utils/jquery');
 
 function getMessage(blackListName) {
   return `The use of jQuery's ${blackListName} method has been deprecated.`;
@@ -40,7 +40,7 @@ module.exports = {
       },
 
       VariableDeclarator(node) {
-        jQueryLocalName = isVariableAssigmentJquery(node);
+        jQueryLocalName = isVariableAssigmentGlobalJquery(node);
       },
 
       CallExpression(node) {

lib/utils/jquery.js

@@ -3,8 +3,8 @@ const { get } = require('../utils/get');
 const JQUERY_ALIASES = ['$', 'jQuery'];
 
 function isJQueryCallee(node, name = null) {
-  const calleePropertyNameisJquery = get(node, 'object.callee.property.name') === '$';
-  const calleeNameIsJquery = get(node, 'object.callee.name') === '$';
+  const calleePropertyNameisJquery = JQUERY_ALIASES.includes(get(node, 'object.callee.property.name'));
+  const calleeNameIsJquery = JQUERY_ALIASES.includes(get(node, 'object.callee.name'));
   const calleeNameIsLocalJquery = get(node, 'object.callee.name') === name;
   const ObjectNameisLocalJquery = get(node, 'object.name') === name;
 
@@ -14,20 +14,19 @@ function isJQueryCallee(node, name = null) {
     || ObjectNameisLocalJquery;
 }
 
-function isVariableAssigmentJquery(node) {
+function isVariableAssigmentGlobalJquery(node) {
   const varDecIDName = get(node, 'id.name');
   // Get the name of what was assigned to the variable.
-  const varAssignment = get(node, 'init.callee.property.name')
-    || get(node, 'init.property.name')
-    || get(node, 'init.name')
-    || '';
-  const varAssignmentIsJquery = JQUERY_ALIASES.includes(varAssignment);
+  const varAssignment = get(node, 'init.callee.property.name') || get(node, 'init.name');
+    // A variable's callee object's name tells us whether this is a usage of Ember.$ or global $.
+  const varCalleeObjName = get(node, 'init.callee.object.name');
+  const varAssignmentIsGlobalJquery = JQUERY_ALIASES.includes(varAssignment) && varCalleeObjName !== 'Ember';
 
-  return varAssignmentIsJquery ? varDecIDName : null;
+  return varAssignmentIsGlobalJquery ? varDecIDName : null;
 }
 
 module.exports = {
   isJQueryCallee,
-  isVariableAssigmentJquery,
+  isVariableAssigmentGlobalJquery,
   JQUERY_ALIASES
 };

tests/lib/rules/no-global-jquery.js

@@ -11,33 +11,13 @@ ruleTester.run('no-global-jquery', rule, {
   valid: [
     {
       code: `
-        import Ember from 'ember';
-        const $ = Ember.$;
-
-        export default Ember.Component({
-          func() {
-            $('.class').focus();
+        export default Component.extend({
+          focusWithJQuery() {
+            const find = Ember.$;
+            find('.class').focus();
           }
-        });`,
-      parserOptions,
-      errors: [{
-        message: MESSAGE
-      }]
-    },
-    {
-      code: `
-        import Ember from 'ember';
-        const jQuery = Ember.$;
-
-        export default Ember.Component({
-          func() {
-            jQuery('.class').focus();
-          }
-        });`,
-      parserOptions,
-      errors: [{
-        message: MESSAGE
-      }]
+        })`,
+      parserOptions
     },
     {
       code: `
@@ -90,10 +70,7 @@ ruleTester.run('no-global-jquery', rule, {
             this.el = $('.test');
           }
         });`,
-      parserOptions,
-      errors: [{
-        message: MESSAGE
-      }]
+      parserOptions
     },
     {
       code: `
@@ -108,10 +85,7 @@ ruleTester.run('no-global-jquery', rule, {
             }
           }
         });`,
-      parserOptions,
-      errors: [{
-        message: MESSAGE
-      }]
+      parserOptions
     },
     {
       code: `
@@ -124,10 +98,7 @@ ruleTester.run('no-global-jquery', rule, {
             this.el = foo('.test');
           }
         });`,
-      parserOptions,
-      errors: [{
-        message: MESSAGE
-      }]
+      parserOptions
     },
     {
       code: `
@@ -142,10 +113,7 @@ ruleTester.run('no-global-jquery', rule, {
             }
           }
         });`,
-      parserOptions,
-      errors: [{
-        message: MESSAGE
-      }]
+      parserOptions
     }
   ],
   invalid: [
@@ -200,6 +168,91 @@ ruleTester.run('no-global-jquery', rule, {
       errors: [{
         message: MESSAGE
       }]
+    },
+    {
+      code: `
+        const jayQuery = $;
+        export default Ember.Component({
+          badFun() {
+            jayQuery('.class');
+          }
+        });`,
+      parserOptions,
+      errors: [{
+        message: MESSAGE
+      }]
+    },
+    {
+      code: `
+        const jayQuery = $;
+        export default Ember.Component({
+          badFun() {
+            jayQuery.ajax();
+          }
+        });`,
+      parserOptions,
+      errors: [{
+        message: MESSAGE
+      }]
+    },
+    {
+      code: `
+        const jayQuery = jQuery;
+        export default Ember.Component({
+          badFun() {
+            jayQuery('.class');
+          }
+        });`,
+      parserOptions,
+      errors: [{
+        message: MESSAGE
+      }]
+    },
+    {
+      code: `
+        const jayQuery = jQuery;
+        export default Ember.Component({
+          badFun() {
+            jayQuery.ajax();
+          }
+        });`,
+      parserOptions,
+      errors: [{
+        message: MESSAGE
+      }]
+    },
+    {
+      code: `
+        export default Ember.Component({
+          func() {
+            let a = jQuery;
+            let b = 'notDollarSign';
+
+            a('.find-me').click();
+          }
+        });`,
+      parserOptions,
+      errors: [{
+        message: MESSAGE
+      }]
+    },
+    {
+      code: `
+        export default Component.extend({
+          doStuffWithJQuery() {
+            let find = jQuery;
+            find('.class').focus();
+          },
+
+          nonJQueryStuff() {
+            let find = document.querySelector;
+            find('.class').focus();
+          }
+        })`,
+      parserOptions,
+      errors: [{
+        message: MESSAGE
+      }]
     }
   ]
 });