New rule: no-duplicate-dependent-keys

docs/rules/no-duplicate-dependent-keys.md

@@ -0,0 +1,29 @@
+# Disallow repeating dependent keys (`no-duplicate-dependent-keys`)
+
+## Rule Details
+
+This rule makes it easy to spot repeating dependent keys in computed properties.
+
+Examples of **incorrect** code for this rule:
+
+```js
+computed('foo.bar', 'foo.baz', 'foo.qux', 'foo.bar', function() {
+  //...
+})
+// or using brace expansions
+computed('foo.{bar,baz,qux}', 'foo.bar', function() {
+  //...
+})
+```
+
+Examples of **correct** code for this rule:
+
+```js
+computed('foo.bar', 'foo.baz', 'foo.qux', function() {
+  //...
+})
+// or using brace expansions
+computed('foo.{bar,baz,qux}', 'bar.foo', function() {
+  //...
+})
+```

lib/rules/no-duplicate-dependent-keys.js

@@ -0,0 +1,30 @@
+'use strict';
+
+const utils = require('../utils/utils');
+
+const MESSAGE = 'Dependent keys should not be repeated';
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+module.exports = {
+  meta: {
+    docs: {
+      description: 'Disallow repeating dependent keys',
+      category: 'General',
+      recommended: true
+    },
+    fixable: null,
+    message: MESSAGE,
+  },
+
+  create(context) {
+    return {
+      CallExpression(node) {
+        if (utils.hasDuplicateDependentKeys(node)) {
+          context.report(node, MESSAGE);
+        }
+      }
+    };
+  }
+};

lib/utils/utils.js

@@ -21,8 +21,13 @@ module.exports = {
   parseCallee,
   parseArgs,
   findUnorderedProperty,
+  parseDependentKeys,
+  unwrapBraceExpressions,
+  hasDuplicateDependentKeys,
 };
 
+const ember = require('./ember');
+
 /**
  * Find nodes of given name
  *
@@ -270,3 +275,62 @@ function findUnorderedProperty(arr) {
 
   return null;
 }
+
+/**
+ * Checks whether a computed property has duplicate dependent keys.
+ *
+ * @param  {CallExpression} callExp Given call expression
+ * @return {Boolean}        Flag whether dependent keys present.
+ */
+function hasDuplicateDependentKeys(callExp) {
+  if (!ember.isComputedProp(callExp)) return false;
+
+  const dependentKeys = parseDependentKeys(callExp);
+  const uniqueKeys = dependentKeys
+    .filter((val, index, self) => self.indexOf(val) === index);
+  return uniqueKeys.length !== dependentKeys.length;
+}
+
+/**
+ * Parses dependent keys from call expression and returns them in an array.
+ *
+ * It also unwraps the expressions, so that `model.{foo,bar}` becomes `model.foo, model.bar`.
+ *
+ * @param  {CallExpression} callExp CallExpression to examine
+ * @return {Array}          Array of unwrapped dependent keys
+ */
+function parseDependentKeys(callExp) {
+  const isMemberExpCallExp = callExp.arguments.length === 0 && callExp.callee.object;
+  const args = isMemberExpCallExp ? callExp.callee.object.arguments : callExp.arguments;
+
+  const dependentKeys = args
+    .filter(arg => isLiteral(arg))
+    .map(literal => literal.value);
+  return unwrapBraceExpressions(dependentKeys);
+}
+
+/**
+ * Unwraps brace expressions.
+ *
+ * @param  {Array} dependentKeys array of strings containing unprocessed dependent keys.
+ * @return {Array} Array of unwrapped dependent keys
+ */
+function unwrapBraceExpressions(dependentKeys) {
+  const braceExpressionRegexp = /\.{.+}/g;
+
+  const unwrappedExpressions = dependentKeys.map((key) => {
+    if (!braceExpressionRegexp.test(key)) return key;
+
+    const parts = key.split('.');
+    const prefix = parts[0];
+    const properties = parts[1]
+      .replace('{', '')
+      .replace('}', '')
+      .split(',');
+
+    return properties.map(property => `${prefix}.${property}`);
+  });
+
+  return unwrappedExpressions
+    .reduce((acc, cur) => acc.concat(cur), []);
+}

tests/lib/rules/no-duplicate-dependent-keys.js

@@ -0,0 +1,70 @@
+'use strict';
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const rule = require('../../../lib/rules/no-duplicate-dependent-keys');
+const RuleTester = require('eslint').RuleTester;
+
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+const ruleTester = new RuleTester();
+const parserOptions = { ecmaVersion: 6, sourceType: 'module' };
+ruleTester.run('no-duplicate-dependent-keys', rule, {
+  valid: [
+    {
+      code: `
+      {
+        foo: computed('model.foo', 'model.bar', 'model.baz', function() {})
+      }
+      `,
+      parserOptions,
+    },
+    {
+      code: `
+      {
+        foo: computed('model.{foo,bar}', 'model.qux', function() {})
+      }
+      `,
+      parserOptions,
+    },
+    {
+      code: `
+      {
+        foo: Ember.computed('model.{foo,bar}', 'model.qux', function() {
+        }).volatile()
+      }
+      `,
+      parserOptions,
+    }
+  ],
+
+  invalid: [
+    {
+      code: `
+      {
+        foo: computed('model.foo', 'model.bar', 'model.baz', 'model.foo', function() {})
+      }
+      `,
+      parserOptions,
+      errors: [{
+        message: rule.meta.message,
+      }]
+    },
+    {
+      code: `
+      {
+        foo: computed('model.{foo,bar}', 'model.bar', function() {})
+      }
+      `,
+      parserOptions,
+      errors: [{
+        message: rule.meta.message,
+      }]
+    }
+  ]
+});

tests/lib/utils/utils-test.js

@@ -186,3 +186,86 @@ describe('parseArgs', () => {
     expect(parsedArgs).toEqual(['asd', 'qwe', 'zxc']);
   });
 });
+
+describe('parseDependentKeys', () => {
+  it('should parse dependent keys from callexpression', () => {
+    const node = parse("computed('model.{foo,bar}', 'model.bar')");
+    expect(utils.parseDependentKeys(node)).toEqual([
+      'model.foo', 'model.bar', 'model.bar',
+    ]);
+  });
+
+  it('should work when no dependent keys present', () => {
+    const node = parse('computed(function() {})');
+    expect(utils.parseDependentKeys(node)).toEqual([]);
+  });
+
+  it('should handle dependent keys and function arguments', () => {
+    const node = parse("computed('model.{foo,bar}', 'model.bar', function() {})");
+    expect(utils.parseDependentKeys(node)).toEqual([
+      'model.foo', 'model.bar', 'model.bar',
+    ]);
+  });
+
+  it('should handle dependent keys and function arguments in MemberExpression', () => {
+    const node = parse(`
+      computed('model.{foo,bar}', 'model.bar', function() {
+      }).volatile();
+    `);
+    expect(utils.parseDependentKeys(node)).toEqual([
+      'model.foo', 'model.bar', 'model.bar',
+    ]);
+  });
+});
+
+describe('unwrapBraceExpressions', () => {
+  it('should unwrap simple dependent keys', () => {
+    expect(utils.unwrapBraceExpressions([
+      'model.foo', 'model.bar'
+    ])).toEqual(['model.foo', 'model.bar']);
+  });
+
+  it('should unwrap dependent keys with braces', () => {
+    expect(utils.unwrapBraceExpressions([
+      'model.{foo,bar}', 'model.bar'
+    ])).toEqual(['model.foo', 'model.bar', 'model.bar']);
+  });
+
+  it('should unwrap more complex dependent keys', () => {
+    expect(utils.unwrapBraceExpressions([
+      'model.{foo,bar}', 'model.bar', 'data.{foo,baz,qux}'
+    ])).toEqual([
+      'model.foo', 'model.bar', 'model.bar', 'data.foo', 'data.baz', 'data.qux',
+    ]);
+  });
+});
+
+describe('hasDuplicateDependentKeys', () => {
+  it('reports duplicate dependent keys in computed calls', () => {
+    let node = parse("computed('model.{foo,bar}', 'model.bar')");
+    expect(utils.hasDuplicateDependentKeys(node)).toBeTruthy();
+    node = parse("Ember.computed('model.{foo,bar}', 'model.bar')");
+    expect(utils.hasDuplicateDependentKeys(node)).toBeTruthy();
+  });
+
+  it('ignores not repeated dependentKeys', () => {
+    let node = parse("computed('model.{foo,bar}', 'model.qux')");
+    expect(utils.hasDuplicateDependentKeys(node)).not.toBeTruthy();
+    node = parse("Ember.computed('model.{foo,bar}', 'model.qux')");
+    expect(utils.hasDuplicateDependentKeys(node)).not.toBeTruthy();
+    node = parse("computed('model.{foo,bar}', 'model.qux').volatile()");
+    expect(utils.hasDuplicateDependentKeys(node)).not.toBeTruthy();
+  });
+
+  it('ignores non-computed calls', () => {
+    const node = parse("foo('model.{foo,bar}', 'model.bar')");
+    expect(utils.hasDuplicateDependentKeys(node)).not.toBeTruthy();
+  });
+
+  it('reports duplicate dependent keys in computed calls with MemberExp', () => {
+    let node = parse("Ember.computed('model.{foo,bar}', 'model.bar').volatile()");
+    expect(utils.hasDuplicateDependentKeys(node)).toBeTruthy();
+    node = parse("computed('model.{foo,bar}', 'model.bar').volatile()");
+    expect(utils.hasDuplicateDependentKeys(node)).toBeTruthy();
+  });
+});