Repository: ember-cli/eslint-plugin-ember
Pull request #583 (2019-11-10 19:59:47)

Update `no-observers` rule to handle decorators
Helps with #566 to prepare for Ember Octane.


Patch:
diff --git a/docs/rules/no-observers.md b/docs/rules/no-observers.md
index cb10150f..21cbc0e1 100644
--- a/docs/rules/no-observers.md
+++ b/docs/rules/no-observers.md
@@ -14,8 +14,31 @@ Observers do have some limited uses. They let you reflect state from your applic
 {{input value=text key-up="change"}}
 ```
 
+## Examples
+
+Examples of **incorrect** code for this rule:
+
+```js
+export default Controller.extend({
+  change: Ember.observer('text', function() {
+    console.log(`change detected: ${this.text}`);
+  },
+});
+```
+
+```js
+import { observes } from '@ember-decorators/object';
+class FooComponent extends Component {
+  @observes('text')
+  change() {
+    console.log(`change detected: ${this.text}`);
+  }
+}
+```
+
+Examples of **correct** code for this rule:
+
 ```javascript
-// GOOD
 export default Controller.extend({
   actions: {
     change() {
@@ -23,11 +46,4 @@ export default Controller.extend({
     },
   },
 });
-
-// BAD
-export default Controller.extend({
-  change: Ember.observer('text', function() {
-    console.log(`change detected: ${this.text}`);
-  },
-});
 ```
diff --git a/lib/rules/no-observers.js b/lib/rules/no-observers.js
index b1010187..1d9a64c7 100644
--- a/lib/rules/no-observers.js
+++ b/lib/rules/no-observers.js
@@ -6,6 +6,8 @@ const ember = require('../utils/ember');
 // General rule - Don't use observers
 //------------------------------------------------------------------------------
 
+const ERROR_MESSAGE = "Don't use observers if possible";
+
 module.exports = {
   meta: {
     docs: {
@@ -18,11 +20,11 @@ module.exports = {
     fixable: null, // or "code" or "whitespace"
   },
 
-  create(context) {
-    const message = "Don't use observers if possible";
+  ERROR_MESSAGE,
 
+  create(context) {
     const report = function(node) {
-      context.report(node, message);
+      context.report(node, ERROR_MESSAGE);
     };
 
     return {
@@ -31,6 +33,12 @@ module.exports = {
           report(node);
         }
       },
+
+      Decorator(node) {
+        if (ember.isObserverDecorator(node)) {
+          report(node);
+        }
+      },
     };
   },
 };
diff --git a/lib/utils/ember.js b/lib/utils/ember.js
index 5667302f..5ea03e57 100644
--- a/lib/utils/ember.js
+++ b/lib/utils/ember.js
@@ -55,6 +55,8 @@ module.exports = {
   isReopenClassObject,
 
   isEmberObjectImplementingUnknownProperty,
+
+  isObserverDecorator,
 };
 
 // Private
@@ -527,3 +529,13 @@ function isEmberObjectImplementingUnknownProperty(node) {
   }
   return false;
 }
+
+function isObserverDecorator(node) {
+  assert(types.isDecorator(node), 'Should only call this function on a Decorator');
+  return (
+    types.isDecorator(node) &&
+    types.isCallExpression(node.expression) &&
+    types.isIdentifier(node.expression.callee) &&
+    node.expression.callee.name === 'observes'
+  );
+}
diff --git a/lib/utils/types.js b/lib/utils/types.js
index f3ac845c..30886530 100644
--- a/lib/utils/types.js
+++ b/lib/utils/types.js
@@ -12,6 +12,7 @@ module.exports = {
   isClassPropertyWithDecorator,
   isConciseArrowFunctionWithCallExpression,
   isConditionalExpression,
+  isDecorator,
   isExpressionStatement,
   isFunctionDeclaration,
   isFunctionExpression,
@@ -164,6 +165,16 @@ function isConditionalExpression(node) {
   return node !== undefined && node.type === 'ConditionalExpression';
 }
 
+/**
+ * Check whether or not a node is a Decorator.
+ *
+ * @param {Object} node The node to check.
+ * @returns {boolean} Whether or not the node is a Decorator.
+ */
+function isDecorator(node) {
+  return node !== undefined && node.type === 'Decorator';
+}
+
 /**
  * Check whether or not a node is an ExpressionStatement.
  *


Test patch:
diff --git a/tests/lib/rules/no-observers.js b/tests/lib/rules/no-observers.js
index bb2afe1c..3a094274 100644
--- a/tests/lib/rules/no-observers.js
+++ b/tests/lib/rules/no-observers.js
@@ -5,18 +5,33 @@
 const rule = require('../../../lib/rules/no-observers');
 const RuleTester = require('eslint').RuleTester;
 
+const { ERROR_MESSAGE } = rule;
+
 // ------------------------------------------------------------------------------
 // Tests
 // ------------------------------------------------------------------------------
 
 const eslintTester = new RuleTester({
-  parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+  parser: require.resolve('babel-eslint'),
+  parserOptions: {
+    ecmaVersion: 6,
+    sourceType: 'module',
+    ecmaFeatures: {
+      legacyDecorators: true,
+    },
+  },
 });
 
 eslintTester.run('no-observers', rule, {
   valid: [
     'export default Controller.extend();',
     'export default Controller.extend({actions: {},});',
+    `
+    import { action } from '@ember/object';
+    class FooComponent extends Component {
+      @action
+      clickHandler() {}
+    }`,
   ],
   invalid: [
     {
@@ -24,7 +39,23 @@ eslintTester.run('no-observers', rule, {
       output: null,
       errors: [
         {
-          message: "Don't use observers if possible",
+          message: ERROR_MESSAGE,
+          type: 'CallExpression',
+        },
+      ],
+    },
+    {
+      code: `
+      import { observes } from '@ember-decorators/object';
+      class FooComponent extends Component {
+        @observes('baz')
+        bar() {}
+      }`,
+      output: null,
+      errors: [
+        {
+          message: ERROR_MESSAGE,
+          type: 'Decorator',
         },
       ],
     },
diff --git a/tests/lib/utils/ember-test.js b/tests/lib/utils/ember-test.js
index 497af93c..34086489 100644
--- a/tests/lib/utils/ember-test.js
+++ b/tests/lib/utils/ember-test.js
@@ -1160,3 +1160,32 @@ describe('isEmberObjectImplementingUnknownProperty', () => {
     expect(emberUtils.isEmberObjectImplementingUnknownProperty(node)).toBeFalsy();
   });
 });
+
+describe('isObserverDecorator', () => {
+  it('should be true for an observer decorator', () => {
+    const node = babelEslint.parse(`
+    import { observes } from '@ember-decorators/object';
+    class FooComponent extends Component {
+      @observes('baz')
+      bar() {}
+    }`).body[1].body.body[0].decorators[0];
+    expect(emberUtils.isObserverDecorator(node)).toBeTruthy();
+  });
+
+  it('should be false for another type of decorator', () => {
+    const node = babelEslint.parse(`
+    import { action } from '@ember/object';
+    class FooComponent extends Component {
+      @action
+      clickHandler() {}
+    }`).body[1].body.body[0].decorators[0];
+    expect(emberUtils.isObserverDecorator(node)).toBeFalsy();
+  });
+
+  it('throws when called on a non-decorator', () => {
+    const node = babelEslint.parse('const x = 123;').body[0];
+    expect(() => emberUtils.isObserverDecorator(node)).toThrow(
+      'Should only call this function on a Decorator'
+    );
+  });
+});