Repository: ember-cli/eslint-plugin-ember
Pull request #1160 (2021-04-20 19:36:06)

Account for observer dependent keys in `no-unused-services` rule
Fixes https://github.com/ember-cli/eslint-plugin-ember/issues/1157

This PR accounts for observer dependent keys and fixes some logic within the computed prop logic.


Patch:
diff --git a/lib/rules/no-unused-services.js b/lib/rules/no-unused-services.js
index 8f7343d8..f1ff0f6c 100644
--- a/lib/rules/no-unused-services.js
+++ b/lib/rules/no-unused-services.js
@@ -52,6 +52,7 @@ module.exports = {
     let importedEmberName;
     let importedGetName;
     let importedGetPropertiesName;
+    let importedObserverName;
     const macros = getMacros();
     const importedMacros = {};
 
@@ -113,6 +114,8 @@ module.exports = {
           importedGetPropertiesName =
             importedGetPropertiesName ||
             getImportIdentifier(node, '@ember/object', 'getProperties');
+          importedObserverName =
+            importedObserverName || getImportIdentifier(node, '@ember/object', 'observer');
         } else if (node.source.value === '@ember/object/computed') {
           for (const spec of node.specifiers) {
             const name = spec.imported.name;
@@ -145,24 +148,39 @@ module.exports = {
               includeMacro: true,
             })
           ) {
-            if (node.callee.property) {
-              // Ember.computed.or(), computed.or()
-              const macroName = node.callee.property.name;
-              if (macros.includes(macroName)) {
-                for (
-                  let idx = 0;
-                  idx < MACROS_TO_TRACKED_ARGUMENT_COUNT[macroName] && idx < node.arguments.length;
-                  idx++
-                ) {
-                  const elem = node.arguments[idx];
+            // Ember.computed(), Ember.computed.or(), computed.or()
+            if (types.isMemberExpression(node.callee)) {
+              if (
+                types.isIdentifier(node.callee.object) &&
+                node.callee.object.name === importedEmberName
+              ) {
+                // Ember.computed()
+                for (const elem of node.arguments) {
                   if (types.isStringLiteral(elem)) {
                     const name = splitValue(elem.value);
                     currentClass.uses.add(name);
                   }
                 }
+              } else if (types.isIdentifier(node.callee.property)) {
+                // Ember.computed.or(), computed.or()
+                const macroName = node.callee.property.name;
+                if (macros.includes(macroName)) {
+                  for (
+                    let idx = 0;
+                    idx < MACROS_TO_TRACKED_ARGUMENT_COUNT[macroName] &&
+                    idx < node.arguments.length;
+                    idx++
+                  ) {
+                    const elem = node.arguments[idx];
+                    if (types.isStringLiteral(elem)) {
+                      const name = splitValue(elem.value);
+                      currentClass.uses.add(name);
+                    }
+                  }
+                }
               }
             } else {
-              // Ember.computed(), computed()
+              // computed()
               for (const elem of node.arguments) {
                 if (types.isStringLiteral(elem)) {
                   const name = splitValue(elem.value);
@@ -222,6 +240,28 @@ module.exports = {
                   currentClass.uses.add(name);
                 }
               }
+            } else if (calleeName === importedObserverName) {
+              // observer('foo', ...)
+              for (const elem of node.arguments) {
+                if (types.isStringLiteral(elem)) {
+                  const name = splitValue(elem.value);
+                  currentClass.uses.add(name);
+                }
+              }
+            }
+          } else if (
+            types.isMemberExpression(node.callee) &&
+            types.isIdentifier(node.callee.object) &&
+            node.callee.object.name === importedEmberName &&
+            types.isIdentifier(node.callee.property) &&
+            node.callee.property.name === 'observer'
+          ) {
+            // Ember.observer('foo', ...)
+            for (const elem of node.arguments) {
+              if (types.isStringLiteral(elem)) {
+                const name = splitValue(elem.value);
+                currentClass.uses.add(name);
+              }
             }
           }
         }
diff --git a/lib/rules/require-tagless-components.js b/lib/rules/require-tagless-components.js
index 23dd6e2f..cdbdcb6b 100644
--- a/lib/rules/require-tagless-components.js
+++ b/lib/rules/require-tagless-components.js
@@ -8,7 +8,6 @@ const {
   isObjectExpression,
   isStringLiteral,
 } = require('../utils/types');
-const { getNodeOrNodeFromVariable } = require('../utils/utils');
 
 const ERROR_MESSAGE_REQUIRE_TAGLESS_COMPONENTS =
   "Please switch to a tagless component by setting `tagName: ''` or converting to a Glimmer component";
@@ -102,9 +101,6 @@ module.exports = {
   },
 
   create(context) {
-    const sourceCode = context.getSourceCode();
-    const { scopeManager } = sourceCode;
-
     return {
       // Handle classic components
       'CallExpression > MemberExpression[property.name="extend"]'(node) {
@@ -120,18 +116,16 @@ module.exports = {
         }
 
         for (const arg of callExpression.arguments) {
-          const resultingNode = getNodeOrNodeFromVariable(arg, scopeManager);
-
           // Ignore anything other than an object literal, since Mixins can be in here too
-          if (!isObjectExpression(resultingNode)) {
+          if (!isObjectExpression(arg)) {
             continue;
           }
 
           let tagNameNode;
 
-          if ((tagNameNode = getNonEmptyTagNameInObjectExpression(resultingNode))) {
+          if ((tagNameNode = getNonEmptyTagNameInObjectExpression(arg))) {
             context.report(tagNameNode, ERROR_MESSAGE_REQUIRE_TAGLESS_COMPONENTS);
-          } else if (hasNoTagNameInObjectExpression(resultingNode)) {
+          } else if (hasNoTagNameInObjectExpression(arg)) {
             context.report(callExpression, ERROR_MESSAGE_REQUIRE_TAGLESS_COMPONENTS);
           }
         }


Test patch:
diff --git a/tests/lib/rules/no-unused-services.js b/tests/lib/rules/no-unused-services.js
index ba709d0a..fff0271a 100644
--- a/tests/lib/rules/no-unused-services.js
+++ b/tests/lib/rules/no-unused-services.js
@@ -18,11 +18,13 @@ const ruleTester = new RuleTester({
 });
 
 const SERVICE_NAME = 'fooName';
-const EO_IMPORTS = "import {computed, get, getProperties} from '@ember/object';";
+const EO_IMPORTS = "import {computed, get, getProperties, observer} from '@ember/object';";
 const RENAMED_EO_IMPORTS =
-  "import {computed as cp, get as g, getProperties as gp} from '@ember/object';";
+  "import {computed as cp, get as g, getProperties as gp, observer as ob} from '@ember/object';";
 const ALIAS_IMPORT = "import {alias} from '@ember/object/computed';";
 const RENAMED_ALIAS_IMPORT = "import {alias as al} from '@ember/object/computed';";
+const EMBER_IMPORT = "import Ember from 'ember';";
+const RENAMED_EMBER_IMPORT = "import Em from 'ember';";
 
 /**
  * Generate an array of usecases using the given property name
@@ -75,9 +77,7 @@ function generateMacroUseCasesFor(propertyName, renamed = false) {
   const aliasName = renamed ? 'al' : 'alias';
   const aliasImport = renamed ? RENAMED_ALIAS_IMPORT : ALIAS_IMPORT;
   return [
-    `${aliasImport} class MyClass { @service('foo') ${propertyName}; @${aliasName}('${propertyName}.prop') someAlias; }`,
     `${aliasImport} class MyClass { @service() ${propertyName}; @${aliasName}('${propertyName}.prop') someAlias; }`,
-    `${aliasImport} Component.extend({ ${SERVICE_NAME}: service('foo'), someAlias: ${aliasName}('${propertyName}.prop') });`,
     `${aliasImport} Component.extend({ ${SERVICE_NAME}: service(), someAlias: ${aliasName}('${propertyName}.prop') });`,
   ];
 }
@@ -91,13 +91,33 @@ function generateMacroUseCasesFor(propertyName, renamed = false) {
 function generateComputedUseCasesFor(propertyName, renamed = false) {
   const computedName = renamed ? 'cp' : 'computed';
   const computedImport = renamed ? RENAMED_EO_IMPORTS : EO_IMPORTS;
+  const emberName = renamed ? 'Em' : 'Ember';
+  const emberImport = renamed ? RENAMED_EMBER_IMPORT : EMBER_IMPORT;
   return [
-    `${computedImport} class MyClass { @service('foo') ${propertyName}; @${computedName}('${propertyName}.prop') get someComputed() {} }`,
     `${computedImport} class MyClass { @service() ${propertyName}; @${computedName}('${propertyName}.prop') get someComputed() {} }`,
-    `${computedImport} Component.extend({ ${SERVICE_NAME}: service('foo'), someComputed: ${computedName}('${propertyName}.prop', ()=>{}) });`,
     `${computedImport} Component.extend({ ${SERVICE_NAME}: service(), someComputed: ${computedName}('${propertyName}.prop', ()=>{}) });`,
-    `${computedImport} Component.extend({ ${SERVICE_NAME}: service('foo'), someComputed: ${computedName}.alias('${propertyName}.prop', ()=>{}) });`,
     `${computedImport} Component.extend({ ${SERVICE_NAME}: service(), someComputed: ${computedName}.alias('${propertyName}.prop', ()=>{}) });`,
+    `${emberImport} Component.extend({ ${SERVICE_NAME}: service(), someComputed: ${emberName}.computed('${propertyName}.prop', ()=>{}) });`,
+    `${emberImport} Component.extend({ ${SERVICE_NAME}: service(), someComputed: ${emberName}.computed.alias('${propertyName}.prop', ()=>{}) });`,
+  ];
+}
+
+/**
+ * Generate an array of usecases with an observer using the given property name
+ * @param {String} propertyName The given property name to access
+ * @param {Boolean} renamed Whether or not the imports are renamed
+ * @returns {Array}
+ */
+function generateObserverUseCasesFor(propertyName, renamed = false) {
+  const observerName = renamed ? 'ob' : 'observer';
+  const observerImport = renamed ? RENAMED_EO_IMPORTS : EO_IMPORTS;
+  const emberName = renamed ? 'Em' : 'Ember';
+  const emberImport = renamed ? RENAMED_EMBER_IMPORT : EMBER_IMPORT;
+  return [
+    `${observerImport} Component.extend({ ${SERVICE_NAME}: service(), someObserved: ${observerName}('${propertyName}.prop', ()=>{}) });`,
+    `${observerImport} Component.extend({ ${SERVICE_NAME}: service(), someObserved: on('init', ${observerName}('${propertyName}.prop', ()=>{})) });`,
+    `${emberImport} Component.extend({ ${SERVICE_NAME}: service(), someObserved: ${emberName}.observer('${propertyName}.prop', ()=>{}) });`,
+    `${emberImport} Component.extend({ ${SERVICE_NAME}: service(), someObserved: on('init', ${emberName}.observer('${propertyName}.prop', ()=>{})) });`,
   ];
 }
 
@@ -126,9 +146,7 @@ function generateValid() {
     const imports = idx === 0 ? EO_IMPORTS : RENAMED_EO_IMPORTS;
     for (const use of useCases) {
       valid.push(
-        `${imports} class MyClass { @service('foo') ${SERVICE_NAME}; fooFunc() {${use}} }`,
         `${imports} class MyClass { @service() ${SERVICE_NAME}; fooFunc() {${use}} }`,
-        `${imports} Component.extend({ ${SERVICE_NAME}: service('foo'), fooFunc() {${use}} });`,
         `${imports} Component.extend({ ${SERVICE_NAME}: service(), fooFunc() {${use}} });`
       );
     }
@@ -142,8 +160,13 @@ function generateValid() {
     ...generateComputedUseCasesFor(SERVICE_NAME),
     ...generateComputedUseCasesFor(SERVICE_NAME, true),
   ];
+  const observerUseCases = [
+    ...generateObserverUseCasesFor(SERVICE_NAME),
+    ...generateObserverUseCasesFor(SERVICE_NAME, true),
+  ];
   valid.push(...macroUseCases);
   valid.push(...computedUseCases);
+  valid.push(...observerUseCases);
 
   return valid;
 }
diff --git a/tests/lib/rules/require-tagless-components.js b/tests/lib/rules/require-tagless-components.js
index 14153657..e9c9496b 100644
--- a/tests/lib/rules/require-tagless-components.js
+++ b/tests/lib/rules/require-tagless-components.js
@@ -92,25 +92,6 @@ ruleTester.run('require-tagless-components', rule, {
       output: null,
       errors: [{ message: ERROR_MESSAGE, line: 4, type: 'Property' }],
     },
-    {
-      // `tagName` but not the last object argument
-      code: `
-        import Component from '@ember/component';
-        export default Component.extend({ tagName: 'span' }, SomeMixin);
-      `,
-      output: null,
-      errors: [{ message: ERROR_MESSAGE, line: 3, type: 'Property' }],
-    },
-    {
-      // `tagName` but inside a variable
-      code: `
-        import Component from '@ember/component';
-        const body = { tagName: 'span' };
-        export default Component.extend(body);
-      `,
-      output: null,
-      errors: [{ message: ERROR_MESSAGE, line: 3, type: 'Property' }],
-    },
     {
       code: `
         import Component from '@ember/component';