ESLint for Ember

ESLint allows to easily write plugins for various needs thanks to its specific architecture. This time am interested to write a plugin for EmberJS. Based on my previous experience, I realized that the scope of work seemed to be not very large, after having some preparatory work done.

Function to detect observers and computed properties (CP)

First of all we need a function that will determine the observer’s body or CP’s body. It is necessary that this function should be able to identify both the EmberJS syntax — with enabled or disabled EXTEND_PROTOTYPES (docs).

var o = require("object-path");
 
/**
 * Check if node is ember-field (like computed property or observer)
 * Supports both syntax (see EXTEND_PROTOTYPES options in the ember-docs)
 *
 * @param {ASTNode} node
 * @param {string} defaultName name like "Ember.computed" (`computed`)
 * @param {string} extendedName name like "function() {}.property" (`property`)
 * @returns {boolean}
 * @private
 */
function _isEmberFieldByType(node, defaultName, extendedName) {
  var type = o.get(node, "type");
  if (type !== "CallExpression") {
    return false;
  }
  var name = o.get(node, "callee.name");
  if (name === defaultName) {
    return true;
  }
  var calleeType = o.get(node, "callee.type");
  if (calleeType === "MemberExpression") {
    if (o.get(node, "callee.object.name") === "Ember" && (o.get(node, "callee.property.name") === defaultName || o.get(node, "callee.property.value") === defaultName)) {
      return true;
    }
    if (o.get(node, "callee.object.type") === "FunctionExpression" && (o.get(node, "callee.property.name") === extendedName || o.get(node, "callee.property.value") === extendedName)) {
      return true;
    }
  }
  return false;
}
 
/**
 * Check if node is an observer
 *
 * @param {ASTNode} node
 * @returns {boolean} true - it is, false - otherwise
 */
function isObserver(node) {
  return _isEmberFieldByType(node, "observes", "observes");
}
 
/**
 * Check if node is a computed property
 *
 * @param {ASTNode} node
 * @returns {boolean} true - it is, false - otherwise
 */
function isComputedProperty(node) {
  return _isEmberFieldByType(node, "computed", "property");
}

In terms of AST the observer differs from the CP with keywords only and syntax tree structure is the same. So, one common function and two wrappers are enough.

Function to get the list of dependent keys

We need a function to get the list of dependent keys for CP and observers. It’s result should not depend on EXTEND_PROTOTYPES value as well.

var o = require("object-path");
 
/**
 * Get list of dependent keys for computed property or observer
 * Should be called after check with <code>isEmberField</code>
 *
 * @param {ASTNode} node
 * @param {boolean} [doUnique] default false
 * @returns {string[]|null} list of dependent keys or <code>null</code> if node doesn't not have <code>arguments</code>
 * @private
 */
function getDependentKeys(node, doUnique) {
  var _doUnique = !!doUnique;
  var keys = o.has(node, "arguments") ? o.get(node, "arguments").map(function (arg) {
    return o.get(arg, "value");
  }).filter(function (value) {
    return "string" === typeof value;
  }) : null;
  return keys && _doUnique ? keys.filter(function (value, index, self) {
    return self.indexOf(value) === index;
  }) : keys;
}

This function should be used for the nodes that passed isComputedProperty or isObserver. The second argument is a flag that determines if the resulting list of dependent keys is filtered out of duplicates.

Function for expanding «braces expansion»

EmberJS allows using braces in the dependent keys (example, a.{b,c} instead of a.b,a.c). It seems we need a function to expand such expressions.

The next is taken from EmberJS sources:

var o = require("object-path");
 
/**
 * Expand strings like 'a.{b,c}' to ['a.b', 'a.c']
 * Code originally taken from https://github.com/emberjs/ember.js/blob/v2.3.0/packages/ember-metal/lib/expand_properties.js
 *
 * @param {string[]} keys
 * @param {boolean} [doUnique] default false
 * @returns {Array}
 */
function expandDependentKeys(keys, doUnique) {
  var _doUnique = !!doUnique;
  var ret = [];
  var _keys = Array.isArray(keys) ? keys : [keys];
  _keys.forEach(function (key) {
    var parts = key.split(/\{|\}/);
    var properties = [parts];
 
    for (var i = 0; i < parts.length; i++) {
      var part = parts[i];
      if (part.indexOf(",") !== -1) {
        properties = _duplicateAndReplace(properties, part.split(","), i);
      }
    }
    for (i = 0; i < properties.length; i++) {
      ret.push(properties[i].join(""));
    }
  });
  return ret && _doUnique ? ret.filter(function (value, index, self) {
    return self.indexOf(value) === index;
  }) : ret;
}
 
function _duplicateAndReplace(properties, currentParts, index) {
  var all = [];
  properties.forEach(function (property) {
    currentParts.forEach(function (part) {
      var current = property.slice(0);
      current[index] = part;
      all.push(current);
    });
  });
  return all;
}
Function to detect CP getter and setter

EmberJS allows to write getters and setters for CP separately. We need a function to detect if some node is getter or setter.

var o = require("object-path");
 
/**
 * Try to detect computed property's body like part of
 *
 *    computed('a', 'b', {
 *      get () { ... }
 *    });
 *
 * @param {ASTNode} node
 * @param {string} method "get|set"
 * @returns {boolean}
 * @private
 */
function _isCpAccessor1(node, method) {
  var type = o.get(node, "type");
  if (type !== "FunctionExpression") {
    return false;
  }
  if (o.get(node, "parent.key.name") !== method) {
    return false;
  }
  var pParent = o.get(node, "parent.parent");
  if (!pParent) {
    return false;
  }
  if (o.get(pParent, "type") !== "ObjectExpression") {
    return false;
  }
  var ppParent = o.get(pParent, "parent"); // more parents!
  if (o.get(ppParent, "type") !== "CallExpression") {
    return false;
  }
  var callee = o.get(ppParent, "callee");
  if (o.get(callee, "type") === "Identifier" && o.get(callee, "name") === "computed") {
    return true;
  }
  if (o.get(callee, "type") === "MemberExpression") {
    var caller = n.getCaller(callee);
    return caller === "Ember.computed";
  }
  return false; // don't know how you could get here
}
 
/**
 * Try to detect computed property's body like part of
 *
 *    computed('a', 'b', function () {
 *      // ...
 *    });
 *
 * @param {ASTNode} node
 * @returns {boolean}
 * @private
 */
function _isCpGetter2(node) {
  var type = o.get(node, "type");
  if (type !== "FunctionExpression") {
    return false;
  }
  var parent = o.get(node, "parent");
  if (o.get(parent, "type") !== "CallExpression") {
    return false;
  }
  var callee = o.get(parent, "callee");
  if (o.get(callee, "type") === "Identifier" && o.get(callee, "name") === "computed") {
    return true;
  }
  if (o.get(callee, "type") === "MemberExpression") {
    var caller = n.getCaller(callee);
    return caller === "Ember.computed";
  }
  return false;
}
 
/**
 * Try to detect computed property's body like part of
 *
 *    function () {
 *      // ...
 *    }.property('a', 'b')
 *
 * @param {ASTNode} node
 * @returns {boolean}
 * @private
 */
function _isCpGetter3(node) {
  var type = o.get(node, "type");
  if (type !== "FunctionExpression") {
    return false;
  }
  return o.get(node, "parent.property.name") === "property";
}
 
/**
 * Check if node is getter for computed property
 *
 * @param {ASTNode} node
 * @returns {boolean} true - it is, false - otherwise
 */
function isCpGetter(node) {
  return _isCpAccessor1(node, "get") || _isCpGetter2(node) || _isCpGetter3(node);
}
 
/**
 * Check if node is setter for computed property
 *
 * @param {ASTNode} node
 * @returns {boolean}
 */
function isCpSetter(node) {
  return _isCpAccessor1(node, "set");
}

Getter may have 3 syntax type:

Ember.computed('a', 'b', {
  get() {/* getter */},
  set() {}
})
Ember.computed('a', 'b', function () {
  /* getter */
});
function () {
  /* getter */
}.property('a', 'b')

Setter fits only 1 syntax type — same as first getter type, but with another key-name (set instead of get). So, _isCpAccessor1 implements first type. The second argument is 'set' or 'get'. Function _isCpGetter2 implements second type. And isCpGetter3 is for third type.

Preparatory work has been completed. Let’s write some ESLint rules.

Rules

ESLint has a rule max-params that checks number of the function’s parameters.We can do similar option for observers and CP. The rule to check number of the dependent keys — max-dep-keys seems to be fine. Function to get the list of dependent keys is already implemented, so we just have to call it at the correct moment:

var ember = require("../utils/ember.js");
 
module.exports = function(context) {
 
  var options = context.options[0] || {};
  var allowedKeysCount = options.max || 3;
  var tryExpandKeys = options.tryExpandKeys;
 
  return {
    "CallExpression": function (node) {
      if (ember.isEmberField(node)) {
        var keys = ember.getDependentKeys(node);
        if (tryExpandKeys) {
          keys = ember.expandDependentKeys(keys);
        }
        if (Array.isArray(keys) && keys.length > allowedKeysCount) {
          context.report(node, "Too many dependent keys {{keys}}. Maximum allowed is {{max}}.", {keys: keys.length, max: allowedKeysCount});
        }
      }
    }
  };
 
};

Here ../utils/ember.js — is a file where utility functions for EmberJS are stored.

A little bit detailed about what’s going on in the rule. The nodes with type CallExpression are checked if they are observers or CP. Next getDependentKeys returns the list of the dependent keys. Braces expanding may be done (according to the tryExpandKeys value). Finally, the received array size is compared to the maximum value allowed. If the array is bigger than allowed, the node is reported.

We got first rule. Let’s try something else. For example, it’s possible that some dependent key is used twice for the same observer or CP. It won’t cause any error (EmberJS will remove duplicates), but duplicating doesn’t make any sense here. And it’s not so difficult to detect such things. New rule is called no-dup-keys:

var ember = require("../utils/ember.js");
 
module.exports = function(context) {
 
  var options = context.options[0] || {};
  var tryExpandKeys = options.tryExpandKeys;
 
  function onlyUnique(value, index, self) {
    return self.indexOf(value) === index;
  }
 
  return {
    "CallExpression": function (node) {
      if (ember.isEmberField(node)) {
        var keys = ember.getDependentKeys(node);
        var uniqueKeys = keys.filter(onlyUnique);
        if (tryExpandKeys) {
          keys = ember.expandDependentKeys(keys);
          uniqueKeys = keys.filter(onlyUnique);
        }
        if (keys && uniqueKeys && keys.length !== uniqueKeys.length) {
          context.report(node, "Some dependent keys are duplicated.");
        }
      }
    }
  };
 
};

Second rule is complete. Only 10 lines of code. Node is checked by isEmberField and it’s dependent keys are checked for uniqueness (after brace have been expanded). If some of them are duplicated, the node is reported.

We have expanded braces in the previous rules. But can be «close» them back? I mean, can we detect that list of dependent keys contains items that may be grouped with braces? For example, keys a.b, a.c may be written as a.{b,c}.The rule will be called cp-brace-expansion.

var ember = require("../utils/ember.js");
 
module.exports = function (context) {
 
  function onlyUnique(value, index, self) {
    return self.indexOf(value) === index;
  }
 
  return {
 
    "CallExpression": function (node) {
      if (ember.isEmberField(node)) {
        var keys = ember.getDependentKeys(node);
        var keysMatrix = keys.map(function (key) {
          return key.split(".");
        });
        var fsLine = [];
        var lsLine = [];
        for (var i = 0; i < keysMatrix.length; i++) {
          fsLine.push(keysMatrix[i][0]);
          lsLine.push(keysMatrix[i][keysMatrix[i].length - 1]);
        }
        var fsUnique = fsLine.filter(onlyUnique);
        var lsUnique = lsLine.filter(onlyUnique);
 
        if (fsLine.length !== fsUnique.length || lsLine.length !== lsUnique.length) {
          context.report(node, "Some dependent keys may be grouped with Brace Expansion.");
        }
      }
    }
  };
 
};

Rule execution is started same as others. Node is checked to be observer or CP. Each key is splitted by .. New matrix looks like [['a', 'b', 'c'], ['a', 'd']]. Two new vectors are generated from it — one with each first element, another — with each last element (['a', 'a'] и ['c', 'd']). Both are filtered out of duplicates. Next step is to compare new arrays with the old ones. If at least one pair is not equal (['a', 'a'].length !== ['a'].length) the node is reported.

Before writing new rules, we should define one more utility-function. Its goal is to get full path of the object’s method call. Example — var a = b.c.d();. b.c.d and b.c are nodes with type MemberExpression. The feature is that node b.c.d has object-field as b.c — second MemberExpression:

{
  "expression": {
    "type": "MemberExpression",
    "object": {
      "type": "MemberExpression",
      "object": {
        "type": "Identifier",
        "name": "b"
      },
      "property": {
        "type": "Identifier",
        "name": "c"
      }
    },
    "property": {
      "type": "Identifier",
      "name": "d"
    }
  }
}
Function to get full call-path
var o = require("object-path");
 
/**
 * @param {ASTNode} node
 */
function getCaller (node) {
  var o, p;
  if (obj.get(node, "type") === "MemberExpression") {
    o = obj.get(node, "object.type") === "ThisExpression" ? "this" :
      (obj.get(node, "object.type") === "MemberExpression" ? getCaller(node.object) : obj.get(node, "object.name"));
    p = obj.get(node, "property.name") || obj.get(node, "property.value");
    return p ? o + "." + p : o;
  }
  var callee = obj.get(node, "callee");
  if (!callee) {
    return "";
  }
  if (obj.get(callee, "type") === "MemberExpression") {
    o = obj.get(callee, "object.type") === "ThisExpression" ? "this" :
      (obj.get(callee, "object.type") === "MemberExpression" ? getCaller(callee.object) : obj.get(callee, "object.name"));
    p = obj.get(callee, "property.name") || obj.get(callee, "property.value");
    return p ? o + "." + p : o;
  }
  return obj.get(callee, "name");
}

The function takes 1 argument — node (MemberExpression or CallExpression). Function returns equal calls-chain. Function considers that this-call (ThisExpression) may be done and that call may contain not only dot and square brackets (a['b'].c instead of a.b.c). In case when MemberExpression’s object is another MemberExpression, getCaller is called recursively. Function doesn’t consider calls for array elements (like a.b[1].c()). Although there is no current need for this.

As mentioned, CP has getter (to get value) and setter (to set dependent values). It is logical that no set should be inside getter. The rule to check this condition is called no-set-in-getter:

var ember = require("../utils/ember.js");
 
module.exports = function(context) {
 
  var insideCpGetter = false;
 
  return {
    "FunctionExpression": function (node) {
      if (ember.isCpGetter(node)) {
        insideCpGetter = true;
      }
    },
    "FunctionExpression:exit": function (node) {
      if (ember.isCpGetter(node)) {
        insideCpGetter = false;
      }
    },
    "CallExpression": function (node) {
      if (insideCpGetter) {
        var caller = getCaller(node);
        if (["set", "Ember.set", "this.set"].indexOf(caller) !== -1) {
          context.report(node, "Ember-setter should not be used inside getter.");
        }
      }
    }
  };
 
};

The rule checks calls for set, Ember.set, this.set. If such call exists the node is reported. The limiting condition is that calls are checked inside getters only. Getter is just a function. So, the rule tracks nodes FunctionExpression and FunctionExpression:exit. Each function’s entry-point is checked to be getter. If it is, the flag insideCpGetter is set to the true. On function-exit this flag is set to the false. This avoids false positives.

It is enough «complex» rules for now. Let’s make some ordinary things. For example, ESLint has a rule no-console. EmberJS has Ember.Logger that should be used instead of console. We won’t do the same rule with another error-message. Our rule will track not only calls like console.METHOD() but also console['METHOD']. In JS terms there is no difference for both call-types. But there is a small difference in AST terms: METHOD is node.property.name if the first case and it is node.property.value in the second case. Our rule will be called no-console (as an original):

var o = require("object-path");
 
var consoleMethods = ['info', 'debug', 'warn', 'error', 'log'];
 
module.exports = function(context) {
 
  return {
 
    "MemberExpression": function(node) {
      var object = o.get(node, "object.name");
      if (object !== "console") {
        return;
      }
      var method = o.get(node, "property.name") || o.get(node, "property.value");
      if (!method) {
        return;
      }
      if (consoleMethods.indexOf(method) !== -1) {
        context.report(node, "`Ember.Logger.{{method}}` should be used.", {method: method});
      }
    }
 
  };
 
};

Function setTimeout is pretty useful for delayed code execution. Although EmberJS recommends using its own Ember.run.later and but not setTimeout directly. New rule will track setTimeout usage and warn user of doing this (no-settimeout).

var o = require("object-path");
 
module.exports = function(context) {
 
  var reportMsg = "`Ember.run.later` should be used.";
 
  return {
 
    "CallExpression": function(node) {
      var callee = o.get(node, "callee");
      if (o.get(callee, "name") === "setTimeout") {
        context.report(node, reportMsg);
        return;
      }
      var caller = getCaller(callee);
      if (["window.setTimeout", "setTimeout"].indexOf(caller) !== -1) {
        context.report(node, reportMsg);
      }
    }
 
  };
 
};

This rule is used for calls like window.setTimeout(), window['setTImeout']() and setTimeout(). If one of them exists, user will be warned to use Ember.run.later.

JS has a good operator typeof, but EmberJS has better method Ember.typeOf, which gives more accurate result. This relates situations when typeof returns object (for null, {}, new Date(), new RegExp() etc). New rule will track typeof usage with the list of the predefined types:

var o = require("object-path");
 
module.exports = function(context) {
 
  var options = context.options[0];
  var disallowed = options.disallowed;
 
  return {
 
    "UnaryExpression": function(node) {
      if (node.operator === "typeof") {
        var expression = node.parent;
        var needed = o.get(expression, "left.operator") === "typeof" ? o.get(expression, "right") : o.get(expression, "left");
        if (o.get(needed, "type") === "Literal") {
          var type = o.get(needed, "value");
          if (disallowed.indexOf(type) !== -1) {
            context.report(node, "`Ember.typeOf` can give more accurate result.");
          }
        }
      }
    }
 
  };
 
};

typeof — it’s UnaryExpression. We specify a list of types for which he rule will report node in the typeofcomparison. It’s done for situations when typeof and Ember.typeOf return same value for some types (like strings). But for objects results are going to be different. So, if the rule is configured to «fire» only typeof with object, we’ll get next:

typeof a === 'object'; // error
'object' === typeof a; // error
typeof a === 'function'; // valid
typeof a === 'string'; // valid

Ember.typeOf is good enough. Although there is a separated method for arrays and arrays-like variables — Ember.isArray. New rule will track Ember.typeOf usage for array detections and will recommend using Ember.isArray. And new rule should track Array.isArray usage. There is no sense to detect typeof usage, because for array it returns 'object' (not useful in this case). New rule is called no-is-array:

var o = require("object-path");
 
module.exports = function(context) {
 
  var m = "`Ember.isArray` is better to detect arrays and array-like variables.";
 
  return {
    "CallExpression": function (node) {
      var callee = o.get(node, "callee");
      var caller = getCaller(node);
      if (caller === "Array.isArray") {
        context.report(node, m);
      }
      else {
        if (["Ember.typeOf", "typeOf"].indexOf(caller) !== -1) {
          var parent = o.get(node, "parent");
          if (parent) {
            var type = o.get(parent, "left.type") === "Literal" ? o.get(parent, "left.value") : o.get(parent, "right.value");
            if (type === "array") {
              context.report(node, m);
            }
          }
        }
      }
    }
 
  }
 
};
Conclusion

All rules described are available in the github(eslint-plugin-ember-cleanup) and npm (current package version is 1.2.1):

npm i -g eslint-plugin-ember-cleanup

List of rules with short description:

  • max-dep-keys Checks number of dependent keys for observers and computed properties
  • no-console Proposes using Ember.Logger instead of console
  • no-dup-keys Checks for duplicated dependent keys for observers and computed properties
  • cp-brace-expansion Checks dependent keys for possibility to do brace expansion
  • no-typeof Proposes using Ember.typeOf instead of built-in typeof for some types check
  • no-is-array Checks for array detection and propose to use Ember.isArray
  • no-set-in-getter Disallows Ember.set, this.set inside computed properties getters
  • no-settimeout Proposes using Ember.run.later instead of setTimeout
  • destructuring Looks for usage Ember.* many times and propose to replace it with const {} = Ember;
  • no-throw Proposes using Ember.assert instead of throwing errors

ESLint config example:

{
  "plugins": [
    "ember-cleanup"
  ],
  "rules": {
    "ember-cleanup/destructuring": 1,
    "ember-cleanup/max-dep-keys": [2, {"max": 5, "tryExpandKeys": true}],
    "ember-cleanup/no-console": 1,
    "ember-cleanup/no-dup-keys": [2, {"tryExpandKeys": true}],
    "ember-cleanup/no-settimeout": 2,
    "ember-cleanup/no-throw": 1,
    "ember-cleanup/cp-brace-expansion": 2,
    "ember-cleanup/no-typeof": [2, {"disallowed": ["object"]}],
    "ember-cleanup/no-is-array": 2,
    "ember-cleanup/no-set-in-getter": 2
  }
}

, ,

Оставить комментарий

Top ↑ | Main page | Back