New Events Not Delegated rule

front/src/js/directives/offendersDirectives.js

@@ -109,7 +109,7 @@
         } else if (context.length > 3) {
             html += ' and ' + (context.length - 2) + ' more...';
         }
-        return html + '}';
+        return html + ')';
     }
 
     function isJQuery(node) {
@@ -670,10 +670,8 @@
 
             if (node.data.callDetails.context && node.data.callDetails.context.length === 0) {
                 html += '<h4>Called on 0 jQuery element</h4><p class="advice">Useless function call, as the jQuery object is empty.</p>';
-            } else if (node.data.type === 'jQuery - bind' && node.data.callDetails.context.length > 3) {
-                html += '<p class="advice">The .bind() method attaches the event listener to each jQuery element one by one. Using the .on() method with event delegation is preferable if available (from v1.7).</p>';
-            } else if (node.data.type === 'jQuery - on' && node.data.callDetails.context.length > 3) {
-                html += '<p class="advice">The .on() method used this way attaches the event listener to each jQuery element one by one. Using the event delegation version of the method is preferable if available (from v1.7).</p>';
+            } else if (node.eventNotDelegated) {
+                html += '<p class="advice">This binding should use Event Delegation instead of binding each element one by one.</p>';
             }
 
             if (node.data.resultsNumber === 0) {

front/src/views/rule.html

@@ -83,6 +83,21 @@
                         Function <b>{{offender.functionName}}</b> used {{offender.count}} times
                     </div>
 
+                    <div ng-if="policyName === 'jQueryNotDelegatedEvents'">
+                        function <b>{{offender.functionName}}</b> used on {{offender.contextLength}} DOM elements without event delegation
+                        <div class="offenderButton" ng-if="offender.backtrace.length == 0">no backtrace</div>
+                        <div class="offenderButton opens" ng-if="offender.backtrace.length > 0">
+                            backtrace
+                            <div class="backtrace">
+                                <div ng-repeat="obj in offender.backtrace track by $index">
+                                    <span ng-if="obj.functionName">{{obj.functionName}}()</span>
+                                    <url-link url="obj.file" max-length="60"></url-link>
+                                    line {{obj.line}}
+                                </div>
+                            </div>
+                        </div>
+                    </div>
+
                     <div ng-if="policyName === 'documentWriteCalls'">
                         <b>{{offender.writeFn}}</b>
                         <span ng-if="offender.from">

lib/metadata/policies.js

@@ -439,6 +439,15 @@ var policies = {
         "isAbnormalThreshold": 0,
         "hasOffenders": true
     },
+    "jQueryNotDelegatedEvents": {
+        "tool": "jsExecutionTransformer",
+        "label": "Events not delegated",
+        "message": "<p>This is the number of events that are bound with the .bind() or the .on() function without using <a href=\"https://learn.jquery.com/events/event-delegation/\" target=\"_blank\">event delegation</a>.</p><p>This means jQuery binds each element contained in the object one by one. This is bad for performance.</p>",
+        "isOkThreshold": 1,
+        "isBadThreshold": 100,
+        "isAbnormalThreshold": 180,
+        "hasOffenders": true
+    },
     "cssParsingErrors": {
         "tool": "phantomas",
         "label": "CSS syntax error",

lib/metadata/scoreProfileGeneric.json

@@ -38,9 +38,10 @@
         "jQuery": {
             "label": "jQuery",
             "policies": {
-                "jQueryVersion": 10,
+                "jQueryVersion": 1,
                 "jQueryVersionsLoaded": 1,
-                "jQueryFunctionsUsed": 1
+                "jQueryFunctionsUsed": 1,
+                "jQueryNotDelegatedEvents": 1
             }
         },
         "cssComplexity": {

lib/tools/jsExecutionTransformer.js

@@ -13,16 +13,24 @@ var jsExecutionTransformer = function() {
         var metrics = {
             DOMaccesses: 0,
             DOMaccessesOnScroll: 0,
-            queriesWithoutResults: 0,
-            jQueryCalls: 0,
-            jQueryCallsOnEmptyObject: 0,
-            jQueryNotDelegatedEvent: 0
+            queriesWithoutResults: 0
         };
 
         var offenders = {
 
         };
 
+        var hasjQuery = (data.toolsResults.phantomas.metrics.jQueryVersionsLoaded > 0);
+        if (hasjQuery) {
+            metrics.jQueryCalls = 0;
+            metrics.jQueryFunctionsUsed = 0;
+            metrics.jQueryCallsOnEmptyObject = 0;
+            metrics.jQueryNotDelegatedEvents = 0;
+
+            offenders.jQueryFunctionsUsed = [];
+            offenders.jQueryNotDelegatedEvents = [];
+        }
+
         try {
 
             debug('Starting JS execution transformation');
@@ -31,11 +39,17 @@ var jsExecutionTransformer = function() {
             if (javascriptExecutionTree.children) {
                 javascriptExecutionTree.children.forEach(function(node) {
                     
-                    var contextLenght = (node.data.callDetails && node.data.callDetails.context) ? node.data.callDetails.context.length : null;
-
-                    if ((node.data.type === 'jQuery - bind' || node.data.type === 'jQuery - on') && contextLenght > 3) {
-                        metrics.jQueryNotDelegatedEvent += contextLenght - 1;
+                    var contextLength = (node.data.callDetails && node.data.callDetails.context) ? node.data.callDetails.context.length : null;
+
+                    if (isABindWithoutEventDelegation(node, contextLength)) {
+                        metrics.jQueryNotDelegatedEvents += contextLength;
+                        offenders.jQueryNotDelegatedEvents.push({
+                            functionName: node.data.type.substring(9),
+                            contextLength: contextLength,
+                            backtrace: offendersHelpers.backtraceToArray(node.data.backtrace)
+                        });
                         node.warning = true;
+                        node.eventNotDelegated = true;
                     }
 
                     if (node.data.resultsNumber === 0) {
@@ -43,7 +57,7 @@ var jsExecutionTransformer = function() {
                         node.warning = true;
                     }
 
-                    if (contextLenght === 0) {
+                    if (contextLength === 0) {
                         metrics.jQueryCallsOnEmptyObject ++;
                         node.warning = true;
                     }
@@ -87,9 +101,7 @@ var jsExecutionTransformer = function() {
                 });
 
                 // Count the number of different jQuery functions called
-                if (data.toolsResults.phantomas.metrics.jQueryVersionsLoaded) {
-                    metrics.jQueryFunctionsUsed = 0;
-                    offenders.jQueryFunctionsUsed = [];
+                if (hasjQuery) {
 
                     jQueryFunctionsCollection.sort().forEach(function(fnName, cnt) {
                         if (fnName === 'jQuery - find') {
@@ -181,6 +193,30 @@ var jsExecutionTransformer = function() {
 
         return count;
     }
+
+    function isPureString(str) {
+        return typeof str === 'string' && str[0] !== '{' && str !== '(function)' && str !== '[Object]' && str !== '[Array]' && str !== 'true' && str !== 'false' && str !== 'undefined' && str !== 'unknown' && str !== 'null';
+    }
+
+    function isABindWithoutEventDelegation(node, contextLength) {
+        // Count only on larger bindings
+        if (contextLength <= 3) {
+            return false;
+        }
+
+        if (node.data.type === 'jQuery - on' && node.data.callDetails.arguments[1] && !isPureString(node.data.callDetails.arguments[1])) {
+            return true;
+        }
+
+        if (node.data.type.indexOf('jQuery - ') === 0 && node.children && node.children.length === 1) {
+            var child = node.children[0];
+            if (child.data.type === 'jQuery - on' && child.data.callDetails.arguments[1] && !isPureString(child.data.callDetails.arguments[1])) {
+                return true;
+            }
+        }
+
+        return false;
+    }
 };
 
 module.exports = new jsExecutionTransformer();

test/www/jquery-page.html

@@ -20,6 +20,7 @@
 
         // Let's start the spaghetti code!!!
 
+        $('li').bind('mouseover', function() {});
         $('ul').find('li');
         $('li', $('#foo'));
         var foo = document.getElementById('foo');