ESLint for Ember – a little more rules (1.7.0)

ESLint + Ember

I continue working on the plugin ember-cleanup for ESLint. Version 1.7.0 is now released with four new rules.

The plugin may be installed using npm (install globally if eslint is installed globally):

npm i eslint-plugin-ember-cleanup

`square-brackets` (#)

In the one of the previous versions (1.3.0) I wrote a rule one-level-each. It checks @each usage in the dependent keys. One of the cases is @each-existence in the end of the key (the rule proposes to replace it with []). Soon after and I realized that there is a list of restrictions for []. So the rule `square-brackets` appeared. It consists of 3 checks:

  • Dependent key should end with [] but not with @each (this check is moved from rule one-level-each).
  • [] should be only in the end of the dependent key.
  • Dot should be before [] (sometimes such typos happen)

The rule is pretty simple:

module.exports = function(context) {
  var m1 = "Dependent key should not end with `@each`, use `[]` instead.";
  var m2 = "`[]` should be at the end of the dependent key.";
  var m3 = "Dot should be before `[]`.";
 
  function onlyUnique(value, index, self) {
    return self.indexOf(value) === index;
  }
 
  return {
    "CallExpression": function (node) {
      if (ember.isEmberField(node)) {
        var dependentKeys = ember.getDependentKeys(node);
        dependentKeys = ember.expandDependentKeys(dependentKeys.filter(onlyUnique));
        var report1 = false, report2 = false, report3 = false;
        for (var i = 0; i < dependentKeys.length; i++) {
          var key = dependentKeys[i];
          var subKeys = key.split(".");
          if (subKeys[subKeys.length - 1] === "@each") {
            report1 = true;
            continue;
          }
          var pos = key.indexOf("[]");
          if (pos === -1) {
            continue;
          }
          if (key.length !== pos + 2) {
            report2 = true;
            continue;
          }
          if (key[pos - 1] !== ".") {
            report3 = true;
          }
        }
        if (report1) {context.report(node, m1);}
        if (report2) {context.report(node, m2);}
        if (report3) {context.report(node, m3);}
      }
    }
  };
 
};

The rule checks if each CallExpression is a CP or an observer. If it is, list of the dependent keys is taken (with braces expansion). Each key is verified for issues. Key-check is stopped, if one of them is detected. It means that if a key has a few errors (for example – a[].b), user will only first warning.

`no-empty-declaration` (#)

ember-cli generators allow to fast create “skeletons” for routes, components,controllers etc (e.g. ember g <type> instance). At the same time there are often situations, when developer generates something and leaves it “empty” (it often happens with routes). Such code doesn’t throw any errors, but it also doesn’t make any sense. Rule no-empty-declaration searches for code-blocks like Route|Component|Controller.extend({/* empty */}).

From the perspective of “complexity”, the rule is quite simple:

var message = "Empty `extend` is redundant.";
 
module.exports = function(context) {
 
  var options = context.options[0] || {};
  var allowedFor = options.allowedFor || [];
  var allowedForL = allowedFor.length;
 
  return {
 
    "CallExpression": function (node) {
      var caller = n.getCaller(node);
      if (n.getFunctionName(node) !== "extend") {
        return;
      }
      for (var i = 0; i < allowedForL; i++) {
        if (caller.indexOf(allowedFor[i]) !== -1) {
          return;
        }
      }
      var args = node.arguments;
      if (args.length) {
        if (args[0].type !== "ObjectExpression") {
          return;
        }
        if(!args[0].properties.length) {
          context.report(node, message);
        }
      }
      else {
        context.report(node, message);
      }
    }
 
  };
 
};

The rule takes a list of types that may be “empty”. For example – options {allowedFor: ["Model"]} with force rule to ignore code like Model.extend({}).

`no-define-property` (#)

Ember docs for method defineProperty states the following:

This is a low-level method used by other parts of the API. You almost never want to call this method directly. Instead you should use Ember.mixin() to define new properties.

Once again, your application won’t break, if you use this method. However Ember authors knowingly recommend not to use it. The reason is very simple – readability. The entities, which descriptions are located at the same place, are read / maintained / expanded much easier than those which properties scattered throughout the code. It’s a chance to get code duplication and complexity increasing:

// don't do this
 
var obj = Ember.Object.create({p1: [1, 2, 3]});
/*
    ... some lines of code ...
*/
Ember.defineProperty(obj, 'p2', Ember.computed.sum('p1'));
/*
    ... more code ...
*/
Ember.defineProperty(obj, 'p3', Ember.computed.bool('p2'));

It might be like this:

// a little bit better
 
var O = Ember.Object.extend({
    p1: [],
    p2: Ember.computed.sum('p1'),
    p3: Ember.computed.bool('p2')
});
 
var obj = O.create({p1: [1, 2, 3]});

Rule’s code is here:

 
var n = require("../utils/node.js");
var ember = require("../utils/ember.js");
 
module.exports = function (context) {
 
  var m = "`Ember.defineProperty` should not be used. Use `Ember.mixin()` to define new properties.";
 
  var definePropertyFromEmber = false;
 
  return {
 
    "VariableDeclarator": function (node) {
      var id = node.id;
      if (!id) {
        return;
      }
      var init = node.init;
      if (id.type === "Identifier" && id.name === "defineProperty" && init) {
        if (init.type === "MemberExpression") {
          var caller = n.getCaller(init);
          if (caller === "Ember.defineProperty" || caller === "Em.defineProperty") {
            definePropertyFromEmber = true;
          }
        }
      }
      if (id.type === "ObjectPattern" && ember.isEmber(init.name)) {
        var properties = id.properties;
        var pLength = properties.length;
        for (var i = 0; i < pLength; i++) {
          var prop = properties[i];
          if (prop.key.name === "defineProperty" && prop.value.name === "defineProperty") {
            definePropertyFromEmber = true;
            break;
          }
        }
      }
    },
 
    "CallExpression": function (node) {
      var caller = n.cleanCaller(n.getCaller(node));
      if (caller === "Ember.defineProperty" || caller === "Em.defineProperty") {
        context.report(node, m);
      }
      else {
        if (caller === "defineProperty" && definePropertyFromEmber) {
          context.report(node, m);
        }
      }
    }
 
  }
 
};

So much code? Is it so hard to check CallExpression for equality to Ember|Em.defineProperty? It isn’t a problem when defineProperty is called from Ember directly. The problem appears when defineProperty is called “as is”. Why? Because native JS Object also has such method and its calls are allowed:

const {defineProperty} = Object;
defineProperty(obj, 'p1', {/**/}); // valid
Ember.defineProperty(obj, 'p1', {/**/}); // error
 
// some other place
const {defineProperty} = Ember;
defineProperty(obj, 'p1', {/**/}); // error

A function-handler for VariableDeclarator tries to find defineProperty-declaration and determine, if it is from Ember. The Rule checks usage Ember|Em.defineProperty or just defineProperty (if it’s from Ember) inside CallExpression-handler.

`cp-macro-alias` (#)

How often do you see a code like this:

Ember.computed('a', function() {
    return this.get('a');
});
 
function () {
    return this.get('a');
}.property('a')
 
Ember.computed('a', function() {
    return this.get('a.b');
});
 
function () {
    return this.get('a.b');
}.property('a')

Well, I see this not so often, but it happens. The above code fragments correspond to Ember.computed.alias('...'). BTW, last two functions contain an error – CP returns a.b, but dependent key is just a.

What is the distinctive “feature” of this code

  • There is only one instruction inside function’s body – ReturnStatement
  • This instruction is a call of this.get with one string argument or call of Em|Ember.get with two arguments – first is a ThisExpression and second is string.

Rule’s code:

var m = "May be simplified to `computed.alias`";
 
module.exports = function (context) {
 
  return {
 
    "CallExpression": function (node) {
      if (ember.isComputedProperty(node)) {
        var cpBody = ember.getCpBody(node);
        if (cpBody && cpBody.type === "FunctionExpression") {
          var body = cpBody.body.body;
          if (body.length === 1) {
            var statement = body[0];
            if (statement.type === "ReturnStatement") {
              var ret = statement.argument;
              if (ret.type === "CallExpression") {
                var callee = n.getCaller(ret);
                var args = ret.arguments;
                if ("this.get" === callee && args.length === 1 && args[0].type === "Literal") {
                  return context.report(node, m);
                }
                if (["Em.get", "Ember.get", "get"].indexOf(callee) !== -1 && args.length === 2 && args[0].type === "ThisExpression" && args[1].type === "Literal") {
                  return context.report(node, m);
                }
              }
            }
          }
        }
      }
    }
 
  };
 
};

P.S.

To date, the plugin contains more than 20 rules, but I won’t stop working on it. There is no limit to perfection (even if it is about the code).

, ,

Add comment

Top ↑ | Main page | Back