Repository: ember-cli/eslint-plugin-ember
Pull request #742 (2020-03-28 16:17:11)

Detect invalid position of `@each` or `[]` in `no-invalid-dependent-keys` rule
CC: @TheMBTH


Patch:
diff --git a/README.md b/README.md
index eb84feb0..3b08e9a0 100644
--- a/README.md
+++ b/README.md
@@ -80,7 +80,7 @@ Rules are grouped by category to help you understand their purpose. Each rule ha
 | :white_check_mark: | [no-deeply-nested-dependent-keys-with-each](./docs/rules/no-deeply-nested-dependent-keys-with-each.md) | disallow usage of deeply-nested computed property dependent keys with `@each` |
 | :white_check_mark: | [no-duplicate-dependent-keys](./docs/rules/no-duplicate-dependent-keys.md) | disallow repeating computed property dependent keys |
 | :wrench: | [no-incorrect-computed-macros](./docs/rules/no-incorrect-computed-macros.md) | disallow incorrect usage of computed property macros |
-|  | [no-invalid-dependent-keys](./docs/rules/no-invalid-dependent-keys.md) | disallow invalid dependent keys in computed properties |
+| :wrench: | [no-invalid-dependent-keys](./docs/rules/no-invalid-dependent-keys.md) | disallow invalid dependent keys in computed properties |
 | :white_check_mark: | [no-side-effects](./docs/rules/no-side-effects.md) | disallow unexpected side effects in computed properties |
 | :white_check_mark: | [no-volatile-computed-properties](./docs/rules/no-volatile-computed-properties.md) | disallow volatile computed properties |
 | :wrench: | [require-computed-macros](./docs/rules/require-computed-macros.md) | require using computed property macros when possible |
diff --git a/docs/rules/no-invalid-dependent-keys.md b/docs/rules/no-invalid-dependent-keys.md
index c478836a..94372f5c 100644
--- a/docs/rules/no-invalid-dependent-keys.md
+++ b/docs/rules/no-invalid-dependent-keys.md
@@ -1,6 +1,8 @@
 # no-invalid-dependent-keys
 
-Dependent keys used for computed properties have to be valid
+:wrench: The `--fix` option on the [command line](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems) can automatically fix some of the problems reported by this rule.
+
+Dependent keys used for computed properties have to be valid.
 
 ## Rule Details
 
@@ -10,11 +12,11 @@ Currently implemented:
 
 - Unbalanced open and closed braces. These can be hard to track for complex computed properties
 and are usually unchecked since the expressions are passed as Strings.
+- Invalid position of `@each` or `[]`
 
 Not yet implemented:
 
 - Check for invalid characters
-- Check for invalid position of @each or []
 
 ## Examples
 
@@ -22,19 +24,57 @@ Examples of **incorrect** code for this rule:
 
 ```js
 export default Component.extend({
+  // Unbalanced braces:
   fullName: computed('user.{firstName,lastName', {
     // Code
   })
 });
 ```
 
-Examples of **correct** code for this rule:
+```js
+export default Component.extend({
+  // Terminal `@each`:
+  items: computed('arr.@each', {
+    // Code
+  })
+});
+```
 
 ```js
+export default Component.extend({
+  // `[]` in the middle:
+  items: computed('arr.[].id', {
+    // Code
+  })
+});
+```
+
+Examples of **correct** code for this rule:
 
+```js
 export default Component.extend({
   fullName: computed('user.{firstName,lastName}', {
     // Code
   })
 });
 ```
+
+```js
+export default Component.extend({
+  items: computed('arr.[]', {
+    // Code
+  })
+});
+```
+
+```js
+export default Component.extend({
+  items: computed('arr.@each.id', {
+    // Code
+  })
+});
+```
+
+## References
+
+- [Guide](https://guides.emberjs.com/release/object-model/computed-properties/) for computed properties
diff --git a/lib/rules/no-invalid-dependent-keys.js b/lib/rules/no-invalid-dependent-keys.js
index b63f68d6..88f9188f 100644
--- a/lib/rules/no-invalid-dependent-keys.js
+++ b/lib/rules/no-invalid-dependent-keys.js
@@ -7,7 +7,24 @@ const ember = require('../utils/ember');
 // General rule -  Dependent keys used for computed properties have to be valid.
 //------------------------------------------------------------------------------
 
-const ERROR_MESSAGE = 'Invalid dependent keys: unbalanced braces';
+const ERROR_MESSAGE_UNBALANCED_BRACES = 'Found unbalanced braces in dependent key';
+const ERROR_MESSAGE_TERMINAL_AT_EACH = 'Found terminal `@each`, use `[]` instead';
+const ERROR_MESSAGE_MIDDLE_BRACKETS = '`[]` should only be used at the end of the dependent key';
+
+function hasUnbalancedBraces(str) {
+  const foundBraces = str.match(/[{}]/g) || [];
+  const openBraces = foundBraces.filter(c => c === '{').length;
+  const closeBraces = foundBraces.filter(c => c === '}').length;
+  return openBraces !== closeBraces;
+}
+
+function hasTerminalAtEach(str) {
+  return str.endsWith('.@each');
+}
+
+function hasMiddleBrackets(str) {
+  return str.includes('[].');
+}
 
 module.exports = {
   meta: {
@@ -19,11 +36,13 @@ module.exports = {
       url:
         'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/no-invalid-dependent-keys.md',
     },
-    fixable: null,
+    fixable: 'code',
     schema: [],
   },
 
-  ERROR_MESSAGE,
+  ERROR_MESSAGE_UNBALANCED_BRACES,
+  ERROR_MESSAGE_TERMINAL_AT_EACH,
+  ERROR_MESSAGE_MIDDLE_BRACKETS,
 
   create(context) {
     return {
@@ -32,23 +51,41 @@ module.exports = {
           return;
         }
 
-        const argMap = node.arguments.filter(
+        const stringArgs = node.arguments.filter(
           arg => types.isLiteral(arg) && typeof arg.value === 'string'
         );
 
-        const problemNodes = argMap.filter(node => {
-          const stringValue = node.value;
-          const foundBraces = stringValue.match(/[{}]/g) || [];
-          const openBraces = foundBraces.filter(c => c === '{').length;
-          const closeBraces = foundBraces.filter(c => c === '}').length;
-          return openBraces !== closeBraces;
-        });
+        const sourceCode = context.getSourceCode();
+
+        stringArgs.forEach(node => {
+          if (hasTerminalAtEach(node.value)) {
+            context.report({
+              node,
+              message: ERROR_MESSAGE_TERMINAL_AT_EACH,
+              fix(fixer) {
+                const nodeText = sourceCode.getText(node);
+                return fixer.replaceText(node, nodeText.replace('.@each', '.[]'));
+              },
+            });
+          }
+
+          if (hasUnbalancedBraces(node.value)) {
+            context.report({
+              node,
+              message: ERROR_MESSAGE_UNBALANCED_BRACES,
+            });
+          }
 
-        problemNodes.forEach(problemNode => {
-          context.report({
-            node: problemNode,
-            message: ERROR_MESSAGE,
-          });
+          if (hasMiddleBrackets(node.value)) {
+            context.report({
+              node,
+              message: ERROR_MESSAGE_MIDDLE_BRACKETS,
+              fix(fixer) {
+                const nodeText = sourceCode.getText(node);
+                return fixer.replaceText(node, nodeText.replace('[].', '@each.'));
+              },
+            });
+          }
         });
       },
     };


Test patch:
diff --git a/tests/lib/rules/no-invalid-dependent-keys.js b/tests/lib/rules/no-invalid-dependent-keys.js
index 5d97730b..b4f36942 100644
--- a/tests/lib/rules/no-invalid-dependent-keys.js
+++ b/tests/lib/rules/no-invalid-dependent-keys.js
@@ -1,7 +1,11 @@
 const rule = require('../../../lib/rules/no-invalid-dependent-keys');
 const RuleTester = require('eslint').RuleTester;
 
-const { ERROR_MESSAGE } = rule;
+const {
+  ERROR_MESSAGE_UNBALANCED_BRACES,
+  ERROR_MESSAGE_TERMINAL_AT_EACH,
+  ERROR_MESSAGE_MIDDLE_BRACKETS,
+} = rule;
 
 // ------------------------------------------------------------------------------
 // Tests
@@ -18,26 +22,37 @@ const eslintTester = new RuleTester({
 
 eslintTester.run('no-invalid-dependent-keys', rule, {
   valid: [
+    // Normal usage
     '{ test: computed("a", "b", function() {}) }',
     '{ test: computed(function() {}) }',
     '{ test: computed() }',
     '{ test: computed(SOME_VAR, function() {}) }',
     '{ test: computed(...SOME_ARRAY, function() {}) }',
     '{ test: computed("a.test", "b.test", function() {}) }',
+    '{ test: computed("a.[]", function() {}) }',
+    '{ test: computed("a.@each.id", function() {}) }',
+
+    // Braces
     '{ test: computed("a.{test,test2}", "b", function() {}) }',
     '{ test: computed("a.{test,test2}", "c", "b", function() {}) }',
     '{ test: computed("model.a.{test,test2}", "model.b.{test3,test4}", function() {}) }',
     '{ test: computed("foo.bar.{name,place}", "foo.qux.[]", "foo.baz.{thing,@each.stuff}", function() {}) }',
     '{ test: computed("foo.bar.{name,place,baz,stuff,test.@each.content}","foo.bar.fields.@each.{name,value}","foo.bar.custom.@each.{question,choice}", function() {})}',
     '{ test: computed("foo1.bar1.{name1,place1,baz1,stuff1,test1.@each.content1,fields1.@each.{name1,value1},custom1.@each.{question1,choice1}}", function() {})}',
+
+    // Macros
     '{ test: computed.or("model.{projects,files.@each.{isSaving,test}}", "foo.bar.place") }',
     '{ test: computed.or("foo.bar.name", "foo.bar.place") }',
+    '{ test: computed.or("foo.bar.{name,place}") }',
     '{ test: computed.and("foo.bar.name", "foo.bar.place") }',
     '{ test: Ember.computed.filterBy("a", "b", false) }',
+
+    // Unrelated functions
     'myFunction("{ key: string }, saving,test}", "b}", false)',
     'myFunction("A string containing curly braces {}}")',
   ],
   invalid: [
+    // Unbalanced braces
     {
       code: '{ test: computed("foo.{name,place", "foo.name,place}", function() {}) }',
       output: null,
@@ -45,7 +60,7 @@ eslintTester.run('no-invalid-dependent-keys', rule, {
         {
           ruleId: 'no-invalid-dependent-keys',
           severity: 1,
-          message: ERROR_MESSAGE,
+          message: ERROR_MESSAGE_UNBALANCED_BRACES,
           line: 1,
           column: 18,
           type: 'Literal',
@@ -55,7 +70,7 @@ eslintTester.run('no-invalid-dependent-keys', rule, {
         {
           ruleId: 'no-invalid-dependent-keys',
           severity: 1,
-          message: ERROR_MESSAGE,
+          message: ERROR_MESSAGE_UNBALANCED_BRACES,
           line: 1,
           column: 37,
           type: 'Literal',
@@ -71,7 +86,7 @@ eslintTester.run('no-invalid-dependent-keys', rule, {
         {
           ruleId: 'no-invalid-dependent-keys',
           severity: 1,
-          message: ERROR_MESSAGE,
+          message: ERROR_MESSAGE_UNBALANCED_BRACES,
           line: 1,
           column: 18,
           type: 'Literal',
@@ -88,7 +103,7 @@ eslintTester.run('no-invalid-dependent-keys', rule, {
         {
           ruleId: 'no-invalid-dependent-keys',
           severity: 1,
-          message: ERROR_MESSAGE,
+          message: ERROR_MESSAGE_UNBALANCED_BRACES,
           line: 1,
           column: 19,
           type: 'Literal',
@@ -105,7 +120,7 @@ eslintTester.run('no-invalid-dependent-keys', rule, {
         {
           ruleId: 'no-invalid-dependent-keys',
           severity: 1,
-          message: ERROR_MESSAGE,
+          message: ERROR_MESSAGE_UNBALANCED_BRACES,
           line: 1,
           column: 19,
           type: 'Literal',
@@ -114,10 +129,36 @@ eslintTester.run('no-invalid-dependent-keys', rule, {
         },
       ],
     },
+
+    // @each
+    {
+      code: 'computed("foo.@each", function() {})',
+      output: 'computed("foo.[]", function() {})',
+      errors: [
+        {
+          message: ERROR_MESSAGE_TERMINAL_AT_EACH,
+          type: 'Literal',
+        },
+      ],
+    },
+
+    // []
+    {
+      code: 'computed("foo.[].bar", function() {})',
+      output: 'computed("foo.@each.bar", function() {})',
+      errors: [
+        {
+          message: ERROR_MESSAGE_MIDDLE_BRACKETS,
+          type: 'Literal',
+        },
+      ],
+    },
+
+    // Decorator
     {
       code: "class Test { @computed('foo.{name,place') get someProp() { return true; } }",
       output: null,
-      errors: [{ message: ERROR_MESSAGE, type: 'Literal' }],
+      errors: [{ message: ERROR_MESSAGE_UNBALANCED_BRACES, type: 'Literal' }],
     },
   ],
 });