Fix no-global-jquery returning false positive for var assignment #85

Author: nlfurniss

lib/rules/no-global-jquery.js

@@ -4,18 +4,22 @@
 'use strict';
 
 const { getEmberImportBinding } = require('../utils/imports');
+const {
+  isJQueryCallee,
+  isVariableAssigmentJquery,
+  JQUERY_ALIASES
+} = require('../utils/jquery');
 const { collectObjectPatternBindings } = require('../utils/destructed-binding');
 
 const MESSAGE = 'Do not use global `$` or `jQuery`.';
-const ALIASES = ['$', 'jQuery'];
 
 /**
  * Determines if this expression matches a global jQuery invocation, either `$` or `jQuery`.
  * @param {ASTNode} node The identifier node.
  * @returns {Boolean} Returns true if the expression matches, otherwise false.
  */
 function isGlobalJquery(node) {
-  return node.callee && ALIASES.includes(node.callee.name);
+  return node.callee && JQUERY_ALIASES.includes(node.callee.name);
 }
 
 module.exports = {
@@ -30,6 +34,7 @@ module.exports = {
   create(context) {
     let emberImportBinding;
     let destructuredAssignment = null;
+    let jQueryLocalName = null;
 
     function isDestructured(node) {
       return node && node.callee && node.callee.name === destructuredAssignment;
@@ -48,8 +53,14 @@ module.exports = {
         }
       },
 
+      VariableDeclarator(node) {
+        jQueryLocalName = isVariableAssigmentJquery(node);
+      },
+
       CallExpression(node) {
-        if (!isDestructured(node) && isGlobalJquery(node)) {
+        if (jQueryLocalName && !isJQueryCallee(node, jQueryLocalName)) {
+          return;
+        } else if (!isDestructured(node) && isGlobalJquery(node)) {
           context.report(node, MESSAGE);
         }
       }

lib/rules/no-jquery-methods.js

@@ -4,24 +4,12 @@
 'use strict';
 
 const { get } = require('../utils/get');
-const JQUERY_ALIASES = ['$', 'jQuery'];
+const { isJQueryCallee, isVariableAssigmentJquery } = require('../utils/jquery');
 
 function getMessage(blackListName) {
   return `The use of jQuery's ${blackListName} method has been deprecated.`;
 }
 
-function isJQueryCaller(node, name = null) {
-  const calleePropertyNameisJquery = get(node, 'object.callee.property.name') === '$';
-  const calleeNameIsJquery = get(node, 'object.callee.name') === '$';
-  const calleeNameIsLocalJquery = get(node, 'object.callee.name') === name;
-  const ObjectNameisLocalJquery = get(node, 'object.name') === name;
-
-  return calleePropertyNameisJquery
-    || calleeNameIsJquery
-    || calleeNameIsLocalJquery
-    || ObjectNameisLocalJquery;
-}
-
 function getLocalImportName(node, sourceName) {
   if (get(node, 'source.value').includes(sourceName)) {
     return get(node, 'specifiers').map((specifier) => specifier.local.name);
@@ -52,17 +40,7 @@ module.exports = {
       },
 
       VariableDeclarator(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);
-
-        if (varAssignmentIsJquery) {
-          jQueryLocalName = varDecIDName;
-        }
+        jQueryLocalName = isVariableAssigmentJquery(node);
       },
 
       CallExpression(node) {
@@ -82,7 +60,7 @@ module.exports = {
         let blackListName = BLACKLIST.includes(propertyName) ? propertyName : false;
         let isThisExpression = get(node, 'object.type').includes('ThisExpression');
 
-        if (!isThisExpression && blackListName && isJQueryCaller(node, jQueryLocalName)) {
+        if (!isThisExpression && blackListName && isJQueryCallee(node, jQueryLocalName)) {
           context.report({ node: node.property, message: getMessage(blackListName) });
         }
       }

lib/utils/jquery.js

@@ -0,0 +1,33 @@
+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 calleeNameIsLocalJquery = get(node, 'object.callee.name') === name;
+  const ObjectNameisLocalJquery = get(node, 'object.name') === name;
+
+  return calleePropertyNameisJquery
+    || calleeNameIsJquery
+    || calleeNameIsLocalJquery
+    || ObjectNameisLocalJquery;
+}
+
+function isVariableAssigmentJquery(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);
+
+  return varAssignmentIsJquery ? varDecIDName : null;
+}
+
+module.exports = {
+  isJQueryCallee,
+  isVariableAssigmentJquery,
+  JQUERY_ALIASES
+};

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

@@ -9,6 +9,36 @@ const parserOptions = {
 
 ruleTester.run('no-global-jquery', rule, {
   valid: [
+    {
+      code: `
+        import Ember from 'ember';
+        const $ = Ember.$;
+
+        export default Ember.Component({
+          func() {
+            $('.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
+      }]
+    },
     {
       code: `
         export default Ember.Component({