Enforce deferring controller logic

Question

We are trying to apply the guidelines listed in John Papa's AngularJS Style Guide.

One of the rules which we started to follow is Defer Controller Logic:

Defer logic in a controller by delegating to services and factories.

Why?: Logic may be reused by multiple controllers when placed within a service and exposed via a function.

Why?: Logic in a service can more easily be isolated in a unit test, while the calling logic in the controller can be easily mocked.

Why?: Removes dependencies and hides implementation details from the controller.

This is something we've violated in the past by putting data retrieval logic into controllers instead of isolating it in a service.

Now I'd like to make the rule as strict as possible. Ideally, I would like angular to throw an error if one of the configured services is passed as a dependency to a controller. Is it something that can be solved on the angular level, or I should try solving it separately - for example, statically with a custom ESlint rule?

Would appreciate any insights or hints.

---

In particular, the following controller violates the rule, because it uses $http service directly:

function OrderController($http, $q, config, userInfo) {
    var vm = this;
    vm.checkCredit = checkCredit;
    vm.isCreditOk;

    function checkCredit() {
        var settings = {};

        return $http.get(settings)
            .then(function(data) {
               vm.isCreditOk = vm.total <= maxRemainingAmount;
            })
            .catch(function(error) {

            });
    };
}

^{Also, let me know if I'm getting overly concerned/crazy about the code quality :)}

Answer 1

UPD (june 2015): eslint-plugin-angular has built-in ng_no_services rule that enforces the deferring controller logic to services or factories.

---

Old answer.

Here is the ESLint custom rule that restricts a function, which name ends with Controller or Ctrl, to have any of the configured arguments passed in:

module.exports = function (context) {

    "use strict";

    var restrictedParams = context.options[0] || [];

    var check = function (node) {
        var name = node.id && node.id.name;

        if (/(Controller|Ctrl)$/.test(name) && node.params) {
            var params = node.params.map(
                function (param) {
                    return param.name;
                }
            );

            restrictedParams.filter(function (n) {
                if (params.indexOf(n) != -1) {
                    context.report(node, "This controller function uses a restricted dependency {{param}}.", {
                        param: n
                    });
                }
            });

        }
    };

    return {
        "FunctionDeclaration": check,
        "FunctionExpression": check
    }
};

Example configuration (eslint.json):

{
  "env": {
    "browser": true,
    "node": true,
    "jasmine": true
  },
  "globals": {
    "angular": true,
    "browser": true,
    "element": true,
    "by": false,
    "inject": false
  },
  "plugins": [
    "angularjs"
  ],
  "rules": {
    "ctrl-logic": [2, ["$http"]]
  }
}

Now, imagine I have the following controller defined (violating the rule):

angular
    .module("app")
    .controller("AppController", AppController);


AppController.$inject = ["$scope", "ConnectionService", "ConfigService", "StatusService", "$http"];

function AppController($scope, ConnectionService, ConfigService, StatusService, $http) {
    ...
}

If I run eslint check it would report the error:

$ grunt lint
Running "eslint:target" (eslint) task

app/app-controller.js
  8:0  error  This controller function uses a restricted dependency $http  ctrl-logic

✖ 1 problem

---

I can imagine there are ways to define the dependencies in a way that this particular rule would not spot the violation, but, if you follow the advices suggested in the style guide, it should work for you (works for me).