Repository: ember-cli/eslint-plugin-ember
Pull request #1196 (2021-05-13 22:01:44)

Check import when detecting observer usage in `order-in-*` rules
This PR fixes `isObserverProp` to take an additional parameter `importedObserverName` which checks for the name that `observer` was imported under instead of always "observer".


Patch:
diff --git a/lib/rules/order-in-components.js b/lib/rules/order-in-components.js
index ed37840d..4c4855d6 100644
--- a/lib/rules/order-in-components.js
+++ b/lib/rules/order-in-components.js
@@ -87,6 +87,7 @@ module.exports = {
 
     let importedInjectName;
     let importedEmberName;
+    let importedObserverName;
 
     const sourceCode = context.getSourceCode();
     const { scopeManager } = sourceCode;
@@ -100,6 +101,10 @@ module.exports = {
           importedInjectName =
             importedInjectName || getImportIdentifier(node, '@ember/service', 'inject');
         }
+        if (node.source.value === '@ember/object') {
+          importedObserverName =
+            importedObserverName || getImportIdentifier(node, '@ember/object', 'observer');
+        }
       },
       CallExpression(node) {
         if (!ember.isEmberComponent(context, node)) {
@@ -113,6 +118,7 @@ module.exports = {
           order,
           importedEmberName,
           importedInjectName,
+          importedObserverName,
           scopeManager
         );
       },
diff --git a/lib/rules/order-in-controllers.js b/lib/rules/order-in-controllers.js
index b7f2607e..cbe66073 100644
--- a/lib/rules/order-in-controllers.js
+++ b/lib/rules/order-in-controllers.js
@@ -59,6 +59,7 @@ module.exports = {
 
     let importedInjectName;
     let importedEmberName;
+    let importedObserverName;
 
     const sourceCode = context.getSourceCode();
     const { scopeManager } = sourceCode;
@@ -72,6 +73,10 @@ module.exports = {
           importedInjectName =
             importedInjectName || getImportIdentifier(node, '@ember/service', 'inject');
         }
+        if (node.source.value === '@ember/object') {
+          importedObserverName =
+            importedObserverName || getImportIdentifier(node, '@ember/object', 'observer');
+        }
       },
       CallExpression(node) {
         if (!ember.isEmberController(context, node)) {
@@ -85,6 +90,7 @@ module.exports = {
           order,
           importedEmberName,
           importedInjectName,
+          importedObserverName,
           scopeManager
         );
       },
diff --git a/lib/rules/order-in-models.js b/lib/rules/order-in-models.js
index 7f4b80dc..a413d7bb 100644
--- a/lib/rules/order-in-models.js
+++ b/lib/rules/order-in-models.js
@@ -54,6 +54,7 @@ module.exports = {
 
     let importedInjectName;
     let importedEmberName;
+    let importedObserverName;
 
     const sourceCode = context.getSourceCode();
     const { scopeManager } = sourceCode;
@@ -67,6 +68,10 @@ module.exports = {
           importedInjectName =
             importedInjectName || getImportIdentifier(node, '@ember/service', 'inject');
         }
+        if (node.source.value === '@ember/object') {
+          importedObserverName =
+            importedObserverName || getImportIdentifier(node, '@ember/object', 'observer');
+        }
       },
       CallExpression(node) {
         if (!ember.isDSModel(node, filePath)) {
@@ -80,6 +85,7 @@ module.exports = {
           order,
           importedEmberName,
           importedInjectName,
+          importedObserverName,
           scopeManager
         );
       },
diff --git a/lib/rules/order-in-routes.js b/lib/rules/order-in-routes.js
index afdb2fa0..86298585 100644
--- a/lib/rules/order-in-routes.js
+++ b/lib/rules/order-in-routes.js
@@ -82,6 +82,7 @@ module.exports = {
 
     let importedInjectName;
     let importedEmberName;
+    let importedObserverName;
 
     const sourceCode = context.getSourceCode();
     const { scopeManager } = sourceCode;
@@ -95,6 +96,10 @@ module.exports = {
           importedInjectName =
             importedInjectName || getImportIdentifier(node, '@ember/service', 'inject');
         }
+        if (node.source.value === '@ember/object') {
+          importedObserverName =
+            importedObserverName || getImportIdentifier(node, '@ember/object', 'observer');
+        }
       },
       CallExpression(node) {
         if (!ember.isEmberRoute(context, node)) {
@@ -107,6 +112,7 @@ module.exports = {
           order,
           importedEmberName,
           importedInjectName,
+          importedObserverName,
           scopeManager
         );
       },
diff --git a/lib/utils/ember.js b/lib/utils/ember.js
index 896e20d3..f366837b 100644
--- a/lib/utils/ember.js
+++ b/lib/utils/ember.js
@@ -361,11 +361,12 @@ function isInjectedControllerProp(node, importedEmberName) {
  * * Ember.observer()
  * @param {node} node
  * @param {string} importedEmberName name that `Ember` is imported under
+ * @param {string} importedObserverName name that `observer` is imported under
  * @returns
  */
-function isObserverProp(node, importedEmberName) {
+function isObserverProp(node, importedEmberName, importedObserverName) {
   return (
-    isPropOfType(node, 'observer') ||
+    isPropOfType(node, importedObserverName) ||
     (types.isProperty(node) &&
       types.isCallExpression(node.value) &&
       types.isMemberExpression(node.value.callee) &&
@@ -618,25 +619,25 @@ function getEmberImportAliasName(importDeclaration) {
   return importDeclaration.specifiers[0].local.name;
 }
 
-function isSingleLineFn(property, importedEmberName) {
+function isSingleLineFn(property, importedEmberName, importedObserverName) {
   return (
     (types.isMethodDefinition(property) && utils.getSize(property) === 1) ||
     (property.value &&
       types.isCallExpression(property.value) &&
       utils.getSize(property.value) === 1 &&
-      !isObserverProp(property, importedEmberName) &&
+      !isObserverProp(property, importedEmberName, importedObserverName) &&
       (isComputedProp(property.value, importedEmberName, 'computed', { includeSuffix: true }) ||
         !types.isCallWithFunctionExpression(property.value)))
   );
 }
 
-function isMultiLineFn(property, importedEmberName) {
+function isMultiLineFn(property, importedEmberName, importedObserverName) {
   return (
     (types.isMethodDefinition(property) && utils.getSize(property) > 1) ||
     (property.value &&
       types.isCallExpression(property.value) &&
       utils.getSize(property.value) > 1 &&
-      !isObserverProp(property, importedEmberName) &&
+      !isObserverProp(property, importedEmberName, importedObserverName) &&
       (isComputedProp(property.value, importedEmberName, 'computed', { includeSuffix: true }) ||
         !types.isCallWithFunctionExpression(property.value)))
   );
diff --git a/lib/utils/property-order.js b/lib/utils/property-order.js
index 22e0db2a..2d92d892 100644
--- a/lib/utils/property-order.js
+++ b/lib/utils/property-order.js
@@ -52,7 +52,14 @@ const NAMES = {
 };
 
 // eslint-disable-next-line complexity
-function determinePropertyType(node, parentType, ORDER, importedEmberName, importedInjectName) {
+function determinePropertyType(
+  node,
+  parentType,
+  ORDER,
+  importedEmberName,
+  importedInjectName,
+  importedObserverName
+) {
   if (ember.isInjectedServiceProp(node, importedEmberName, importedInjectName)) {
     return 'service';
   }
@@ -98,7 +105,7 @@ function determinePropertyType(node, parentType, ORDER, importedEmberName, impor
     }
   }
 
-  if (ember.isObserverProp(node, importedEmberName)) {
+  if (ember.isObserverProp(node, importedEmberName, importedObserverName)) {
     return 'observer';
   }
 
@@ -106,11 +113,11 @@ function determinePropertyType(node, parentType, ORDER, importedEmberName, impor
     return 'actions';
   }
 
-  if (ember.isSingleLineFn(node, importedEmberName)) {
+  if (ember.isSingleLineFn(node, importedEmberName, importedObserverName)) {
     return 'single-line-function';
   }
 
-  if (ember.isMultiLineFn(node, importedEmberName)) {
+  if (ember.isMultiLineFn(node, importedEmberName, importedObserverName)) {
     return 'multi-line-function';
   }
 
@@ -196,6 +203,7 @@ function reportUnorderedProperties(
   ORDER,
   importedEmberName,
   importedInjectName,
+  importedObserverName,
   scopeManager
 ) {
   let maxOrder = -1;
@@ -212,7 +220,8 @@ function reportUnorderedProperties(
       parentType,
       ORDER,
       importedEmberName,
-      importedInjectName
+      importedInjectName,
+      importedObserverName
     );
     const order = getOrder(ORDER, type);
 


Test patch:
diff --git a/tests/lib/rules/order-in-components.js b/tests/lib/rules/order-in-components.js
index 53aef16d..2212f7a9 100644
--- a/tests/lib/rules/order-in-components.js
+++ b/tests/lib/rules/order-in-components.js
@@ -96,7 +96,8 @@ eslintTester.run('order-in-components', rule, {
 
         ghi: alias("def")
       });`,
-    `export default Component.extend({
+    `import {observer} from '@ember/object';
+    export default Component.extend({
         levelOfHappiness: computed("attitude", "health", () => {
         }),
 
@@ -108,7 +109,8 @@ eslintTester.run('order-in-components', rule, {
 
         actions: {}
       });`,
-    `export default Component.extend({
+    `import {observer} from '@ember/object';
+    export default Component.extend({
         abc: observer("aaaa", () => {
         }),
 
@@ -123,6 +125,7 @@ eslintTester.run('order-in-components', rule, {
       });`,
     `
       import {inject as service} from '@ember/service';
+      import {observer} from '@ember/object';
       export default Component.extend({
         igh: service(),
 
@@ -629,12 +632,14 @@ eslintTester.run('order-in-components', rule, {
       ],
     },
     {
-      code: `export default Component.extend({
+      code: `import {observer} from '@ember/object';
+      export default Component.extend({
         levelOfHappiness: observer("attitude", "health", () => {
         }),
         vehicle: alias("car")
       });`,
-      output: `export default Component.extend({
+      output: `import {observer} from '@ember/object';
+      export default Component.extend({
         vehicle: alias("car"),
               levelOfHappiness: observer("attitude", "health", () => {
         }),
@@ -642,19 +647,21 @@ eslintTester.run('order-in-components', rule, {
       errors: [
         {
           message:
-            'The "vehicle" single-line function should be above the "levelOfHappiness" observer on line 2',
-          line: 4,
+            'The "vehicle" single-line function should be above the "levelOfHappiness" observer on line 3',
+          line: 5,
         },
       ],
     },
     {
-      code: `export default Component.extend({
+      code: `import {observer} from '@ember/object';
+      export default Component.extend({
         levelOfHappiness: observer("attitude", "health", () => {
         }),
         aaa: computed("attitude", "health", () => {
         })
       });`,
-      output: `export default Component.extend({
+      output: `import {observer} from '@ember/object';
+      export default Component.extend({
         aaa: computed("attitude", "health", () => {
         }),
               levelOfHappiness: observer("attitude", "health", () => {
@@ -663,19 +670,21 @@ eslintTester.run('order-in-components', rule, {
       errors: [
         {
           message:
-            'The "aaa" multi-line function should be above the "levelOfHappiness" observer on line 2',
-          line: 4,
+            'The "aaa" multi-line function should be above the "levelOfHappiness" observer on line 3',
+          line: 5,
         },
       ],
     },
     {
-      code: `export default Component.extend({
+      code: `import {observer} from '@ember/object';
+      export default Component.extend({
         init() {
         },
         levelOfHappiness: observer("attitude", "health", () => {
         })
       });`,
-      output: `export default Component.extend({
+      output: `import {observer} from '@ember/object';
+      export default Component.extend({
         levelOfHappiness: observer("attitude", "health", () => {
         }),
               init() {
@@ -684,8 +693,8 @@ eslintTester.run('order-in-components', rule, {
       errors: [
         {
           message:
-            'The "levelOfHappiness" observer should be above the "init" lifecycle hook on line 2',
-          line: 4,
+            'The "levelOfHappiness" observer should be above the "init" lifecycle hook on line 3',
+          line: 5,
         },
       ],
     },
diff --git a/tests/lib/rules/order-in-controllers.js b/tests/lib/rules/order-in-controllers.js
index 115fb2f9..9fcd9f9e 100644
--- a/tests/lib/rules/order-in-controllers.js
+++ b/tests/lib/rules/order-in-controllers.js
@@ -41,7 +41,8 @@ eslintTester.run('order-in-controllers', rule, {
         _customAction2: function() {},
         tSomeTask: task(function* () {})
       });`,
-    `export default Controller.extend({
+    `import {observer} from '@ember/object';
+    export default Controller.extend({
         queryParams: [],
         customProp: "test",
         comp: computed("test", function() {}),
@@ -277,13 +278,15 @@ eslintTester.run('order-in-controllers', rule, {
       ],
     },
     {
-      code: `export default Controller.extend({
+      code: `import {observer} from '@ember/object';
+      export default Controller.extend({
         test: "asd",
         obs: observer("asd", function() {}),
         comp: computed("asd", function() {}),
         actions: {}
       });`,
-      output: `export default Controller.extend({
+      output: `import {observer} from '@ember/object';
+      export default Controller.extend({
         test: "asd",
         comp: computed("asd", function() {}),
         obs: observer("asd", function() {}),
@@ -291,8 +294,8 @@ eslintTester.run('order-in-controllers', rule, {
       });`,
       errors: [
         {
-          message: 'The "comp" single-line function should be above the "obs" observer on line 3',
-          line: 4,
+          message: 'The "comp" single-line function should be above the "obs" observer on line 4',
+          line: 5,
         },
       ],
     },
diff --git a/tests/lib/utils/ember-test.js b/tests/lib/utils/ember-test.js
index d0cd6583..64aa4690 100644
--- a/tests/lib/utils/ember-test.js
+++ b/tests/lib/utils/ember-test.js
@@ -1221,12 +1221,14 @@ describe('isObserverProp', () => {
   describe('classic classes', () => {
     it("should check if it's an observer prop using destructured import", () => {
       const context = new FauxContext(`
+        import {observer} from '@ember/object';
         export default Controller.extend({
           someObserver: observer(),
         });
       `);
-      const node = context.ast.body[0].declaration.arguments[0].properties[0];
-      expect(emberUtils.isObserverProp(node)).toBeTruthy();
+      const importName = context.ast.body[0].specifiers[0].local.name;
+      const node = context.ast.body[1].declaration.arguments[0].properties[0];
+      expect(emberUtils.isObserverProp(node, undefined, importName)).toBeTruthy();
     });
 
     it("should check if it's an observer prop with full import", () => {
@@ -1241,6 +1243,16 @@ describe('isObserverProp', () => {
       expect(emberUtils.isObserverProp(node, importName)).toBeTruthy();
     });
 
+    it("should check that it's not an observer prop without import", () => {
+      const context = new FauxContext(`
+        export default Controller.extend({
+          someObserver: observer(),
+        });
+      `);
+      const node = context.ast.body[0].declaration.arguments[0].properties[0];
+      expect(emberUtils.isObserverProp(node)).toBeFalsy();
+    });
+
     it("should check that it's not an observer prop without full import", () => {
       const context = new FauxContext(`
         export default Controller.extend({
@@ -1253,36 +1265,42 @@ describe('isObserverProp', () => {
 
     it("should check if it's an observer prop with multi-line observer", () => {
       const context = new FauxContext(`
+        import {observer} from '@ember/object';
         export default Component.extend({
           levelOfHappiness: observer("attitude", "health", () => {
           }),
           vehicle: alias("car")
         });
       `);
-      const node = context.ast.body[0].declaration.arguments[0].properties[0];
-      expect(emberUtils.isObserverProp(node)).toBeTruthy();
+      const importName = context.ast.body[0].specifiers[0].local.name;
+      const node = context.ast.body[1].declaration.arguments[0].properties[0];
+      expect(emberUtils.isObserverProp(node, undefined, importName)).toBeTruthy();
     });
   });
 
   describe('native classes', () => {
     it("should check if it's an observer prop using decorator", () => {
       const context = new FauxContext(`
+        import {observer} from '@ember/object';
         class MyController extends Controller {
           @observer someObserver;
         }
       `);
-      const node = context.ast.body[0].body.body[0];
-      expect(emberUtils.isObserverProp(node)).toBeTruthy();
+      const importName = context.ast.body[0].specifiers[0].local.name;
+      const node = context.ast.body[1].body.body[0];
+      expect(emberUtils.isObserverProp(node, undefined, importName)).toBeTruthy();
     });
 
     it("should check if it's an observer prop using decorator with arg", () => {
       const context = new FauxContext(`
+        import {observer} from '@ember/object';
         class MyController extends Controller {
           @observer("someArg") someObserver() {};
         }
       `);
-      const node = context.ast.body[0].body.body[0];
-      expect(emberUtils.isObserverProp(node)).toBeTruthy();
+      const importName = context.ast.body[0].specifiers[0].local.name;
+      const node = context.ast.body[1].body.body[0];
+      expect(emberUtils.isObserverProp(node, undefined, importName)).toBeTruthy();
     });
 
     it("should check that it's not an observer prop", () => {
diff --git a/tests/lib/utils/property-order-test.js b/tests/lib/utils/property-order-test.js
index f53b6850..bc3fb6b7 100644
--- a/tests/lib/utils/property-order-test.js
+++ b/tests/lib/utils/property-order-test.js
@@ -55,10 +55,10 @@ describe('determinePropertyType', () => {
           currentUser: service(),
         });`
       );
-      const importName = context.ast.body[0].specifiers[0].local.name;
+      const importInjectName = context.ast.body[0].specifiers[0].local.name;
       const node = context.ast.body[1].declaration.arguments[0].properties[0];
       expect(
-        propertyOrder.determinePropertyType(node, 'controller', [], undefined, importName)
+        propertyOrder.determinePropertyType(node, 'controller', [], undefined, importInjectName)
       ).toStrictEqual('service');
     });
 
@@ -166,12 +166,23 @@ describe('determinePropertyType', () => {
 
     it('should determine observer-type props', () => {
       const context = new FauxContext(
-        `export default Controller.extend({
+        `import {observer} from '@ember/object';
+        export default Controller.extend({
           someObvs: observer(),
         });`
       );
-      const node = context.ast.body[0].declaration.arguments[0].properties[0];
-      expect(propertyOrder.determinePropertyType(node, 'controller')).toStrictEqual('observer');
+      const importObserverName = context.ast.body[0].specifiers[0].local.name;
+      const node = context.ast.body[1].declaration.arguments[0].properties[0];
+      expect(
+        propertyOrder.determinePropertyType(
+          node,
+          'controller',
+          [],
+          undefined,
+          undefined,
+          importObserverName
+        )
+      ).toStrictEqual('observer');
     });
 
     it('should determine actions', () => {
@@ -298,10 +309,10 @@ describe('determinePropertyType', () => {
           @service currentUser;
         }`
       );
-      const importName = context.ast.body[0].specifiers[0].local.name;
+      const importInjectName = context.ast.body[0].specifiers[0].local.name;
       const node = context.ast.body[1].body.body[0];
       expect(
-        propertyOrder.determinePropertyType(node, 'controller', [], undefined, importName)
+        propertyOrder.determinePropertyType(node, 'controller', [], undefined, importInjectName)
       ).toStrictEqual('service');
     });
 
@@ -357,12 +368,23 @@ describe('determinePropertyType', () => {
 
     it('should determine observer-type props', () => {
       const context = new FauxContext(
-        `class MyController extends Controller {
+        `import {observer} from '@ember/object';
+        class MyController extends Controller {
           @observer someObvs;
         }`
       );
-      const node = context.ast.body[0].body.body[0];
-      expect(propertyOrder.determinePropertyType(node, 'controller')).toStrictEqual('observer');
+      const importObserverName = context.ast.body[0].specifiers[0].local.name;
+      const node = context.ast.body[1].body.body[0];
+      expect(
+        propertyOrder.determinePropertyType(
+          node,
+          'controller',
+          [],
+          undefined,
+          undefined,
+          importObserverName
+        )
+      ).toStrictEqual('observer');
     });
 
     it('should determine single-line functions', () => {
@@ -417,7 +439,7 @@ describe('reportUnorderedProperties', () => {
         '',
         jest.fn()
       );
-      const importName = context.ast.body[0].specifiers[0].local.name;
+      const importInjectName = context.ast.body[0].specifiers[0].local.name;
       const node = context.ast.body[1].declaration;
 
       propertyOrder.reportUnorderedProperties(
@@ -426,7 +448,8 @@ describe('reportUnorderedProperties', () => {
         'controller',
         order,
         undefined,
-        importName
+        importInjectName,
+        undefined
       );
       expect(context.report).not.toHaveBeenCalled();
     });
@@ -443,7 +466,7 @@ describe('reportUnorderedProperties', () => {
         '',
         jest.fn()
       );
-      const importName = context.ast.body[0].specifiers[0].local.name;
+      const importInjectName = context.ast.body[0].specifiers[0].local.name;
       const node = context.ast.body[1].declaration;
 
       propertyOrder.reportUnorderedProperties(
@@ -452,7 +475,7 @@ describe('reportUnorderedProperties', () => {
         'controller',
         order,
         undefined,
-        importName
+        importInjectName
       );
       expect(context.report).toHaveBeenCalled(); // eslint-disable-line jest/prefer-called-with
     });
@@ -471,7 +494,7 @@ describe('reportUnorderedProperties', () => {
         '',
         jest.fn()
       );
-      const importName = context.ast.body[0].specifiers[0].local.name;
+      const importInjectName = context.ast.body[0].specifiers[0].local.name;
       const node = context.ast.body[1].declaration;
 
       propertyOrder.reportUnorderedProperties(
@@ -480,7 +503,7 @@ describe('reportUnorderedProperties', () => {
         'controller',
         order,
         undefined,
-        importName
+        importInjectName
       );
       expect(context.report).not.toHaveBeenCalled();
     });
@@ -497,7 +520,7 @@ describe('reportUnorderedProperties', () => {
         '',
         jest.fn()
       );
-      const importName = context.ast.body[0].specifiers[0].local.name;
+      const importInjectName = context.ast.body[0].specifiers[0].local.name;
       const node = context.ast.body[1].declaration;
 
       propertyOrder.reportUnorderedProperties(
@@ -506,7 +529,7 @@ describe('reportUnorderedProperties', () => {
         'controller',
         order,
         undefined,
-        importName
+        importInjectName
       );
       expect(context.report).toHaveBeenCalled(); // eslint-disable-line jest/prefer-called-with
     });