Merge pull request #86 from nlfurniss/fix-global-jquery

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

Author: nlfurniss

lib/rules/no-global-jquery.js

@@ -2,20 +2,23 @@
  * @fileOverview Disallow the use of global `$`.
  */
 'use strict';
-
+const { get } = require('../utils/get');
 const { getEmberImportBinding } = require('../utils/imports');
+const {
+  isVariableAssigmentGlobalJquery,
+  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 +33,7 @@ module.exports = {
   create(context) {
     let emberImportBinding;
     let destructuredAssignment = null;
+    let emberJqueryVariables = [];
 
     function isDestructured(node) {
       return node && node.callee && node.callee.name === destructuredAssignment;
@@ -48,8 +52,18 @@ module.exports = {
         }
       },
 
+      VariableDeclarator(node) {
+        if (isVariableAssigmentGlobalJquery(node)) {
+          context.report(node, MESSAGE);
+        } else {
+          const varName = get(node, 'id.name');
+          emberJqueryVariables.push(varName);
+        }
+      },
+
       CallExpression(node) {
-        if (!isDestructured(node) && isGlobalJquery(node)) {
+        const calleeName = get(node, 'callee.name');
+        if (!isDestructured(node) && isGlobalJquery(node) && !emberJqueryVariables.includes(calleeName)) {
           context.report(node, MESSAGE);
         }
       }

lib/rules/no-jquery-methods.js

@@ -3,25 +3,13 @@
  */
 'use strict';
 
-const { get } = require('../utils/get');
-const JQUERY_ALIASES = ['$', 'jQuery'];
+const { get, getParent } = require('../utils/get');
+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);
@@ -40,35 +28,35 @@ module.exports = {
   },
   create(context) {
     const BLACKLIST = context.options[0] || [];
-    let jQueryLocalName = null;
+    let funcScopeJqueryVariables = [];
+    let moduleScopeJqueryVariables = [];
 
     return {
 
       ImportDeclaration(node) {
         let localName = getLocalImportName(node, 'jquery');
         if (localName.length > 0) {
-          jQueryLocalName = localName[0];
+          moduleScopeJqueryVariables.push(localName[0]);
         }
       },
 
       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 (isVariableAssigmentJquery(node)) {
+          const varName = get(node, 'id.name');
+          const isFuncScope = !!getParent(node, (parent) => parent.type === 'FunctionDeclaration'); // Determine if variable is in function scope or global scope.
 
-        if (varAssignmentIsJquery) {
-          jQueryLocalName = varDecIDName;
+          if (isFuncScope) {
+            funcScopeJqueryVariables.push(varName);
+          } else {
+            moduleScopeJqueryVariables.push(varName);
+          }
         }
       },
 
       CallExpression(node) {
         const calleeName = get(node, 'callee.property.name');
         const parentCalleeName = get(node, 'callee.object.property.name');
-        if (BLACKLIST.includes(calleeName) && parentCalleeName === '$') {
+        if (BLACKLIST.includes(calleeName) && (parentCalleeName === '$' || isJQueryCallee(node, moduleScopeJqueryVariables, funcScopeJqueryVariables))) {
           context.report({ node: node.callee.property, message: getMessage(`${parentCalleeName}.${calleeName}()`) });
         }
       },
@@ -82,9 +70,14 @@ 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, moduleScopeJqueryVariables, funcScopeJqueryVariables)) {
           context.report({ node: node.property, message: getMessage(blackListName) });
         }
+      },
+
+      FunctionDeclaration() {
+        // Reset the function scope variables for each function declaration.
+        funcScopeJqueryVariables.length = 0;
       }
     };
   }

lib/utils/jquery.js

@@ -0,0 +1,82 @@
+const { get } = require('../utils/get');
+
+const JQUERY_ALIASES = ['$', 'jQuery'];
+
+/**
+ * Determines whether a given node's callee is jQuery.
+ * @param {ASTNode} node
+ * @param {...Array} localJqueryVars Array(s) of local variables that have jQuery assigned to them.
+ * @return {Boolean}
+ */
+function isJQueryCallee(node, ...localJqueryVars) {
+  const allJqueryAliases = localJqueryVars.reduce((accumulator, newArray) => {
+    return accumulator.concat(newArray);
+  }, JQUERY_ALIASES);
+
+  const calleePropertyNameisJquery = allJqueryAliases.includes(get(node, 'object.callee.property.name'));
+  const calleeNameIsJquery = allJqueryAliases.includes(get(node, 'object.callee.name'));
+  const objectNameisJquery = allJqueryAliases.includes(get(node, 'object.name'));
+
+  return calleePropertyNameisJquery
+    || calleeNameIsJquery
+    || objectNameisJquery;
+}
+
+/**
+ * Helper function that returns the name of what was assigned to the variable.
+ * @param {ASTNode} node
+ * @return {String}
+ */
+function getVarAssignmentName(node) {
+  // Get the name of what was assigned to the variable.
+  return get(node, 'init.callee.property.name') || get(node, 'init.name') || get(node, 'init.property.name');
+}
+
+/**
+ * Determines whether a variable assignment's callee is Ember/this.
+ * @param {ASTNode} node
+ * @return {Boolean}
+ */
+function isVarAssignmentCalleeEmber(node) {
+  // A variable's callee object's name tells us whether this is a usage of Ember.$ or global $.
+  const isVarCalleeObjNameEmber = get(node, 'init.callee.object.name') === 'Ember' || get(node, 'init.object.name') === 'Ember';
+  // Whether or not the callee oject is `this.$`.
+  const isVarCalleeObjThis = get(node, 'init.callee.object.type') === 'ThisExpression';
+
+  return isVarCalleeObjNameEmber || isVarCalleeObjThis;
+}
+
+/**
+ * Determines whether a variable's assignment is global jQuery.
+ * @param {ASTNode} node
+ * @return {Boolean}
+ */
+function isVariableAssigmentGlobalJquery(node) {
+  return isVariableAssigmentJquery(node) && !isVarAssignmentCalleeEmber(node);
+}
+
+/**
+ * Determines whether a variable's assignment is Ember jQuery.
+ * @param {ASTNode} node
+ * @return {Boolean}
+ */
+function isVariableAssigmentEmberJquery(node) {
+  return isVariableAssigmentJquery(node) && isVarAssignmentCalleeEmber(node);
+}
+
+/**
+ * Determines whether a variable's assignment is Ember jQuery.
+ * @param {ASTNode} node
+ * @return {Boolean}
+ */
+function isVariableAssigmentJquery(node) {
+  return JQUERY_ALIASES.includes(getVarAssignmentName(node));
+}
+
+module.exports = {
+  isJQueryCallee,
+  isVariableAssigmentGlobalJquery,
+  isVariableAssigmentEmberJquery,
+  isVariableAssigmentJquery,
+  JQUERY_ALIASES
+};