Improve behaviour of the "on" lint

README.md

@@ -132,7 +132,7 @@ Users may use the shareable [eslint-config-angular](https://github.com/dustinspe
         "angular/no-private-call": 2,
         "angular/no-service-method": 2,
         "angular/no-services": [2, ["$http", "$resource", "Restangular"]],
-        "angular/on-watch": 2,
+        "angular/on": 2,
         "angular/rest-service": 0,
         "angular/service-name": 2,
         "angular/timeout-service": 2,
@@ -191,7 +191,7 @@ Users may use the shareable [eslint-config-angular](https://github.com/dustinspe
 | no-private-call           | All scope's properties/methods starting with $$ are used internally by AngularJS. You should not use them directly. Exception can be allowed with this option: {allow:['$$watchers']} |
 | no-service-method         | You should prefer the factory() method instead of service() [Y040](https://github.com/johnpapa/angular-styleguide#style-y040)|
 | no-services               | Some services should be used only in a specific AngularJS service (Ajax-based service for example), in order to follow the separation of concerns paradigm. The second parameter specifies the services. The third parameter can be a list of angular objects (controller, factory, etc.). Or second parameter can be an object, where keys are angular object names and value is a list of services (like {controller: ['$http'], factory: ['$q']}) |
-| on-watch                  | Watch and On methods on the scope object should be assigned to a variable, in order to be deleted in a $destroy event handler |
+| on                        | "On" methods on the scope object should be assigned to a variable, in order to be deleted in a $destroy event handler |
 | rest-service              | Check the service used to send request to your REST API. This rule can have one parameter, with one of the following values: $http, $resource or Restangular ('rest-service': [0, '$http']). |
 | service-name              | All your services should have a name starting with the parameter you can define in your config object. The second parameter can be a Regexp wrapped in quotes. You can not prefix your services by "$" (reserved keyword for AngularJS services) ("service-name":  [2, "ng"]) [Y125](https://github.com/johnpapa/angular-styleguide#style-y125) |
 | timeout-service           | Instead of the default setTimeout function, you should use the AngularJS wrapper service $timeout [Y181](https://github.com/johnpapa/angular-styleguide#style-y181) |

index.js

@@ -38,7 +38,7 @@ rulesConfiguration.addRule('no-jquery-angularelement', 2);
 rulesConfiguration.addRule('no-private-call', 2);
 rulesConfiguration.addRule('no-services', [2, ['$http', '$resource', 'Restangular', '$q']]);
 rulesConfiguration.addRule('no-service-method', 2);
-rulesConfiguration.addRule('on-watch', 2);
+rulesConfiguration.addRule('on', 2);
 rulesConfiguration.addRule('rest-service', 0);
 rulesConfiguration.addRule('service-name', 0);
 rulesConfiguration.addRule('timeout-service', 2);

rules/on-watch.js → rules/on.js

@@ -9,10 +9,9 @@ module.exports = function(context) {
 
     /**
      * Return true if the given node is a call expression calling a function
-     * named '$on' or '$watch' on an object named '$scope', '$rootScope' or
-     * 'scope'.
+     * named '$on' on an object named '$scope' or 'scope'.
      */
-    function isScopeOnOrWatch(node, scopes) {
+    function isScopeOn(node) {
         if (node.type !== 'CallExpression') {
             return false;
         }
@@ -37,32 +36,35 @@ module.exports = function(context) {
         var objectName = parentObject.name;
         var functionName = accessedFunction.name;
 
-        return scopes.indexOf(objectName) >= 0 && (functionName === '$on' ||
-                                            functionName === '$watch');
+        return ['$scope', 'scope'].indexOf(objectName) >= 0 && functionName === '$on';
     }
 
     /**
-     * Return true if the given node is a call expression that has a first
+     * Return true if the given node or its parent is a call expression that has a first
      * argument of the string '$destroy'.
      */
     function isFirstArgDestroy(node) {
-        var args = node.arguments;
+        var args = (node.arguments || []).length > 0 ? node.arguments : node.parent.arguments;
 
-        return (args.length >= 1 &&
+        return args &&
+                (args.length >= 1 &&
                 args[0].type === 'Literal' &&
                 args[0].value === '$destroy');
     }
 
     return {
 
         CallExpression: function(node) {
-            if (isScopeOnOrWatch(node, ['$rootScope']) && !isFirstArgDestroy(node)) {
-                if (node.parent.type !== 'VariableDeclarator' &&
-                    node.parent.type !== 'AssignmentExpression' &&
-                    !(isScopeOnOrWatch(node.parent, ['$rootScope', '$scope', 'scope']) &&
-                     isFirstArgDestroy(node.parent))) {
-                    report(node, node.callee.property.name);
-                }
+            if (!isScopeOn(node)) {
+                return;
+            }
+
+            if (isFirstArgDestroy(node)) {
+                return;
+            }
+
+            if (node.parent.type !== 'VariableDeclarator' && node.parent.type !== 'AssignmentExpression') {
+                report(node, node.callee.property.name);
             }
         }
     };

test/on.js

@@ -0,0 +1,32 @@
+'use strict';
+
+// ------------------------------------------------------------------------------
+// Requirements
+// ------------------------------------------------------------------------------
+
+var rule = require('../rules/on');
+var RuleTester = require('eslint').RuleTester;
+
+// ------------------------------------------------------------------------------
+// Tests
+// ------------------------------------------------------------------------------
+
+var eslintTester = new RuleTester();
+
+eslintTester.run('on', rule, {
+    valid: [
+        'var variable = scope.$on()',
+        'var variable = $scope.$on()',
+        '$scope.$on("$destroy")',
+        '$scope.$on("$destroy", $scope.$on())',
+        '$scope.$on("$destroy", $rootScope.$on())',
+
+        // false positive check
+        '$on()'
+
+    ],
+    invalid: [
+        {code: '$scope.$on()', errors: [{message: 'The "$on" call should be assigned to a variable, in order to be destroyed during the $destroy event'}]},
+        {code: 'scope.$on()', errors: [{message: 'The "$on" call should be assigned to a variable, in order to be destroyed during the $destroy event'}]}
+    ]
+});