Repository: ember-cli/eslint-plugin-ember
Pull request #1731 (2023-01-03 19:10:23)

Handle new service import style in several rules
Previously, only `import { inject as service } from '@ember/service'` was handled. This adds support for `import { service } from '@ember/service'`.

This was added in Ember 4.1:
https://blog.emberjs.com/ember-4-1-released
https://github.com/emberjs/ember.js/pull/19776
https://api.emberjs.com/ember/4.9/functions/@ember%2Fservice/service


wagenet: @bmish Done!

Patch:
diff --git a/lib/rules/no-deprecated-router-transition-methods.js b/lib/rules/no-deprecated-router-transition-methods.js
index 1b691c77..60b5c1eb 100644
--- a/lib/rules/no-deprecated-router-transition-methods.js
+++ b/lib/rules/no-deprecated-router-transition-methods.js
@@ -12,6 +12,7 @@ function getBaseFixSteps(fixer, context, currentClass) {
   let serviceInjectImportName = currentClass.serviceInjectImportName;
   let routerServicePropertyName = currentClass.routerServicePropertyName;
 
+  // For now, we insert the legacy form. If we can detect the Ember version we can insert the new version instead.
   if (!serviceInjectImportName) {
     fixSteps.push(
       fixer.insertTextBefore(
@@ -130,7 +131,9 @@ module.exports = {
       ImportDeclaration(node) {
         if (node.source.value === '@ember/service') {
           serviceInjectImportName =
-            serviceInjectImportName || getImportIdentifier(node, '@ember/service', 'inject');
+            serviceInjectImportName ||
+            getImportIdentifier(node, '@ember/service', 'inject') ||
+            getImportIdentifier(node, '@ember/service', 'service');
         }
       },
 
diff --git a/lib/rules/no-implicit-service-injection-argument.js b/lib/rules/no-implicit-service-injection-argument.js
index 075c1568..f7659e46 100644
--- a/lib/rules/no-implicit-service-injection-argument.js
+++ b/lib/rules/no-implicit-service-injection-argument.js
@@ -79,7 +79,9 @@ module.exports = {
         }
         if (node.source.value === '@ember/service') {
           importedInjectName =
-            importedInjectName || getImportIdentifier(node, '@ember/service', 'inject');
+            importedInjectName ||
+            getImportIdentifier(node, '@ember/service', 'inject') ||
+            getImportIdentifier(node, '@ember/service', 'service');
         }
       },
       Property(node) {
diff --git a/lib/rules/no-private-routing-service.js b/lib/rules/no-private-routing-service.js
index 19d624d7..80d0a292 100644
--- a/lib/rules/no-private-routing-service.js
+++ b/lib/rules/no-private-routing-service.js
@@ -62,10 +62,10 @@ module.exports = {
     let importedEmberName;
 
     // Handle either ClassProperty (ESLint v7) or PropertyDefinition (ESLint v8).
-    function visitClassPropertyOrPropertyDefinition(node) {
+    function visitClassPropertyOrPropertyDefinition(node, importedInjectName) {
       if (
         !node.decorators ||
-        !decoratorUtils.isClassPropertyOrPropertyDefinitionWithDecorator(node, 'service')
+        !decoratorUtils.isClassPropertyOrPropertyDefinitionWithDecorator(node, importedInjectName)
       ) {
         return;
       }
@@ -92,7 +92,9 @@ module.exports = {
         }
         if (node.source.value === '@ember/service') {
           importedInjectName =
-            importedInjectName || getImportIdentifier(node, '@ember/service', 'inject');
+            importedInjectName ||
+            getImportIdentifier(node, '@ember/service', 'inject') ||
+            getImportIdentifier(node, '@ember/service', 'service');
         }
       },
       Property(node) {
@@ -105,8 +107,12 @@ module.exports = {
         }
       },
 
-      ClassProperty: visitClassPropertyOrPropertyDefinition,
-      PropertyDefinition: visitClassPropertyOrPropertyDefinition,
+      ClassProperty(node) {
+        visitClassPropertyOrPropertyDefinition(node, importedInjectName);
+      },
+      PropertyDefinition(node) {
+        visitClassPropertyOrPropertyDefinition(node, importedInjectName);
+      },
 
       Literal(node) {
         if (
diff --git a/lib/rules/no-restricted-service-injections.js b/lib/rules/no-restricted-service-injections.js
index 91203770..9504e663 100644
--- a/lib/rules/no-restricted-service-injections.js
+++ b/lib/rules/no-restricted-service-injections.js
@@ -123,7 +123,9 @@ module.exports = {
         }
         if (node.source.value === '@ember/service') {
           importedInjectName =
-            importedInjectName || getImportIdentifier(node, '@ember/service', 'inject');
+            importedInjectName ||
+            getImportIdentifier(node, '@ember/service', 'inject') ||
+            getImportIdentifier(node, '@ember/service', 'service');
         }
       },
       // Handles:
diff --git a/lib/rules/no-unnecessary-service-injection-argument.js b/lib/rules/no-unnecessary-service-injection-argument.js
index 99410a76..340ae796 100644
--- a/lib/rules/no-unnecessary-service-injection-argument.js
+++ b/lib/rules/no-unnecessary-service-injection-argument.js
@@ -64,7 +64,9 @@ module.exports = {
         }
         if (node.source.value === '@ember/service') {
           importedInjectName =
-            importedInjectName || getImportIdentifier(node, '@ember/service', 'inject');
+            importedInjectName ||
+            getImportIdentifier(node, '@ember/service', 'inject') ||
+            getImportIdentifier(node, '@ember/service', 'service');
         }
       },
       Property(node) {
diff --git a/lib/rules/no-unused-services.js b/lib/rules/no-unused-services.js
index 8a0dc826..3d577a1c 100644
--- a/lib/rules/no-unused-services.js
+++ b/lib/rules/no-unused-services.js
@@ -139,7 +139,9 @@ module.exports = {
         }
         if (node.source.value === '@ember/service') {
           importedInjectName =
-            importedInjectName || getImportIdentifier(node, '@ember/service', 'inject');
+            importedInjectName ||
+            getImportIdentifier(node, '@ember/service', 'inject') ||
+            getImportIdentifier(node, '@ember/service', 'service');
         }
         if (node.source.value === '@ember-decorators/object') {
           importedObservesName =
diff --git a/lib/rules/require-computed-property-dependencies.js b/lib/rules/require-computed-property-dependencies.js
index 55cb28b3..cd58b03e 100644
--- a/lib/rules/require-computed-property-dependencies.js
+++ b/lib/rules/require-computed-property-dependencies.js
@@ -449,7 +449,9 @@ module.exports = {
         }
         if (node.source.value === '@ember/service') {
           importedInjectName =
-            importedInjectName || getImportIdentifier(node, '@ember/service', 'inject');
+            importedInjectName ||
+            getImportIdentifier(node, '@ember/service', 'inject') ||
+            getImportIdentifier(node, '@ember/service', 'service');
         }
       },
 


Test patch:
diff --git a/tests/lib/rules/no-deprecated-router-transition-methods.js b/tests/lib/rules/no-deprecated-router-transition-methods.js
index 2ac605f7..1752e966 100644
--- a/tests/lib/rules/no-deprecated-router-transition-methods.js
+++ b/tests/lib/rules/no-deprecated-router-transition-methods.js
@@ -680,6 +680,43 @@ import Controller from '@ember/controller';
         },
       ],
     },
+    // Injecting with `service` export
+    {
+      filename: 'routes/index.js',
+      code: `
+      import Route from '@ember/routing/route';
+      import { service } from '@ember/service';
+
+      export default class SettingsRoute extends Route {
+        @service session;
+
+        beforeModel() {
+          if (!this.session.isAuthenticated) {
+            this.transitionTo('login');
+          }
+        }
+      }`,
+      output: `
+      import Route from '@ember/routing/route';
+      import { service } from '@ember/service';
+
+      export default class SettingsRoute extends Route {
+        @service('router') router;
+@service session;
+
+        beforeModel() {
+          if (!this.session.isAuthenticated) {
+            this.router.transitionTo('login');
+          }
+        }
+      }`,
+      errors: [
+        {
+          messageId: 'main',
+          type: 'MemberExpression',
+        },
+      ],
+    },
     // Test multiple classes in a single file
     {
       filename: 'routes/index.js',
diff --git a/tests/lib/rules/no-implicit-service-injection-argument.js b/tests/lib/rules/no-implicit-service-injection-argument.js
index 8acc3f1c..28a146eb 100644
--- a/tests/lib/rules/no-implicit-service-injection-argument.js
+++ b/tests/lib/rules/no-implicit-service-injection-argument.js
@@ -10,6 +10,7 @@ const { ERROR_MESSAGE } = rule;
 const EMBER_IMPORT = "import Ember from 'ember';";
 const INJECT_IMPORT = "import {inject} from '@ember/service';";
 const SERVICE_IMPORT = "import {inject as service} from '@ember/service';";
+const NEW_SERVICE_IMPORT = "import {service} from '@ember/service';";
 
 //------------------------------------------------------------------------------
 // Tests
@@ -36,6 +37,7 @@ ruleTester.run('no-implicit-service-injection-argument', rule, {
 
     // With argument (native class)
     `${SERVICE_IMPORT} class Test { @service('service-name') serviceName }`,
+    `${NEW_SERVICE_IMPORT} class Test { @service('service-name') serviceName }`,
 
     // Not Ember's `service()` function (classic class):
     'export default Component.extend({ serviceName: otherFunction() });',
@@ -69,13 +71,18 @@ ruleTester.run('no-implicit-service-injection-argument', rule, {
       output: `${SERVICE_IMPORT} export default Component.extend({ 'serviceName': service('service-name') });`,
       errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }],
     },
-
     // Decorator:
     {
       code: `${SERVICE_IMPORT} class Test { @service() serviceName }`,
       output: `${SERVICE_IMPORT} class Test { @service('service-name') serviceName }`,
       errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }],
     },
+    // Decorator with new import
+    {
+      code: `${NEW_SERVICE_IMPORT} class Test { @service() serviceName }`,
+      output: `${NEW_SERVICE_IMPORT} class Test { @service('service-name') serviceName }`,
+      errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }],
+    },
     {
       // Decorator with no parenthesis
       code: `${SERVICE_IMPORT} class Test { @service serviceName }`,
diff --git a/tests/lib/rules/no-private-routing-service.js b/tests/lib/rules/no-private-routing-service.js
index 1e668e20..e95581fe 100644
--- a/tests/lib/rules/no-private-routing-service.js
+++ b/tests/lib/rules/no-private-routing-service.js
@@ -13,6 +13,7 @@ const {
 
 const EMBER_IMPORT = "import Ember from 'ember';";
 const SERVICE_IMPORT = "import {inject as service} from '@ember/service';";
+const NEW_SERVICE_IMPORT = "import {service} from '@ember/service';";
 
 //------------------------------------------------------------------------------
 // Tests
@@ -47,6 +48,7 @@ ruleTester.run('no-private-routing-service', rule, {
     `${SERVICE_IMPORT} export default class MyComponent extends Component { @service('router') routing; }`,
     `${SERVICE_IMPORT} export default class MyComponent extends Component { @service routing; }`,
     `${SERVICE_IMPORT} export default class MyComponent extends Component { @service('routing') routing; }`,
+    `${NEW_SERVICE_IMPORT} export default class MyComponent extends Component { @service('routing') routing; }`,
     `
     export default class MyComponent extends Component {
       @computed('-routing', 'lastName')
@@ -95,6 +97,28 @@ ruleTester.run('no-private-routing-service', rule, {
         },
       ],
     },
+    // Octane, new import
+    {
+      code: `${NEW_SERVICE_IMPORT} export default class MyComponent extends Component { @service('-routing') routing; }`,
+      output: null,
+      errors: [
+        {
+          message: PRIVATE_ROUTING_SERVICE_ERROR_MESSAGE,
+          // type could be ClassProperty (ESLint v7) or PropertyDefinition (ESLint v8)
+        },
+      ],
+    },
+    // Octane, new import, renamed
+    {
+      code: "import {service as aliasedService} from '@ember/service'; export default class MyComponent extends Component { @aliasedService('-routing') routing; }",
+      output: null,
+      errors: [
+        {
+          message: PRIVATE_ROUTING_SERVICE_ERROR_MESSAGE,
+          // type could be ClassProperty (ESLint v7) or PropertyDefinition (ESLint v8)
+        },
+      ],
+    },
 
     // _routerMicrolib (`catchRouterMicrolib` option on)
     {
diff --git a/tests/lib/rules/no-restricted-service-injections.js b/tests/lib/rules/no-restricted-service-injections.js
index a5de9585..d3eea967 100644
--- a/tests/lib/rules/no-restricted-service-injections.js
+++ b/tests/lib/rules/no-restricted-service-injections.js
@@ -5,6 +5,7 @@ const { DEFAULT_ERROR_MESSAGE } = rule;
 
 const EMBER_IMPORT = "import Ember from 'ember';";
 const SERVICE_IMPORT = "import {inject as service} from '@ember/service';";
+const NEW_SERVICE_IMPORT = "import {service} from '@ember/service';";
 
 const ruleTester = new RuleTester({
   parser: require.resolve('@babel/eslint-parser'),
@@ -46,6 +47,12 @@ ruleTester.run('no-restricted-service-injections', rule, {
       code: `${SERVICE_IMPORT} class MyComponent extends Component { @service('myService') randomName }`,
       options: [{ paths: ['app/components'], services: ['abc'] }],
     },
+    {
+      // Service name doesn't match (with decorator and new import)
+      filename: 'app/components/path.js',
+      code: `${NEW_SERVICE_IMPORT} class MyComponent extends Component { @service('myService') randomName }`,
+      options: [{ paths: ['app/components'], services: ['abc'] }],
+    },
     {
       // Service scope doesn't match:
       filename: 'app/components/path.js',
@@ -80,6 +87,14 @@ ruleTester.run('no-restricted-service-injections', rule, {
       options: [{ paths: ['app/components'], services: ['my-service'] }],
       errors: [{ message: DEFAULT_ERROR_MESSAGE, type: 'Property' }],
     },
+    {
+      // Without service name argument and new import:
+      filename: 'app/components/path.js',
+      code: `${NEW_SERVICE_IMPORT} Component.extend({ myService: service() })`,
+      output: null,
+      options: [{ paths: ['app/components'], services: ['my-service'] }],
+      errors: [{ message: DEFAULT_ERROR_MESSAGE, type: 'Property' }],
+    },
     {
       // Without service name argument (property name as string literal):
       filename: 'app/components/path.js',
diff --git a/tests/lib/rules/no-unnecessary-service-injection-argument.js b/tests/lib/rules/no-unnecessary-service-injection-argument.js
index 55d04913..79822b4c 100644
--- a/tests/lib/rules/no-unnecessary-service-injection-argument.js
+++ b/tests/lib/rules/no-unnecessary-service-injection-argument.js
@@ -10,6 +10,7 @@ const { ERROR_MESSAGE } = rule;
 const EMBER_IMPORT = "import Ember from 'ember';";
 const SERVICE_IMPORT = "import {inject} from '@ember/service';";
 const RENAMED_SERVICE_IMPORT = "import {inject as service} from '@ember/service';";
+const NEW_SERVICE_IMPORT = "import {service} from '@ember/service';";
 
 //------------------------------------------------------------------------------
 // Tests
@@ -50,6 +51,7 @@ ruleTester.run('no-unnecessary-service-injection-argument', rule, {
     `${RENAMED_SERVICE_IMPORT} const controller = Controller.extend({ serviceName: service('service-name') });`,
     `${RENAMED_SERVICE_IMPORT} class Test { @service("service-name") serviceName }`,
     `${RENAMED_SERVICE_IMPORT} class Test { @service("service-name") 'serviceName' }`,
+    `${NEW_SERVICE_IMPORT} class Test { @service("service-name") 'serviceName' }`,
 
     // Property name does not match service name:
     `${EMBER_IMPORT} const controller = Controller.extend({ specialName: Ember.inject.service('service-name') });`,
@@ -105,5 +107,11 @@ ruleTester.run('no-unnecessary-service-injection-argument', rule, {
       output: `${RENAMED_SERVICE_IMPORT} class Test { @service() serviceName }`,
       errors: [{ message: ERROR_MESSAGE, type: 'Literal' }],
     },
+    // Decorator, with new import
+    {
+      code: `${NEW_SERVICE_IMPORT} class Test { @service("serviceName") serviceName }`,
+      output: `${NEW_SERVICE_IMPORT} class Test { @service() serviceName }`,
+      errors: [{ message: ERROR_MESSAGE, type: 'Literal' }],
+    },
   ],
 });
diff --git a/tests/lib/rules/no-unused-services.js b/tests/lib/rules/no-unused-services.js
index 6e35988d..3335f138 100644
--- a/tests/lib/rules/no-unused-services.js
+++ b/tests/lib/rules/no-unused-services.js
@@ -19,6 +19,7 @@ const ruleTester = new RuleTester({
 
 const SERVICE_NAME = 'fooName';
 const SERVICE_IMPORT = "import {inject as service} from '@ember/service';";
+const NEW_SERVICE_IMPORT = "import {service} from '@ember/service';";
 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, observer as ob} from '@ember/object';";
@@ -131,18 +132,18 @@ function generateObserverUseCasesFor(propertyName, renamed = false) {
  * Generate an array of valid test cases
  * @returns {Array}
  */
-function generateValid() {
+function generateValid(importString = SERVICE_IMPORT) {
   const valid = [];
 
   const useCases = generateUseCasesFor(SERVICE_NAME);
   for (const use of useCases) {
     valid.push(
-      `${SERVICE_IMPORT} class MyClass { @service('foo') ${SERVICE_NAME}; fooFunc() {${use}} }`,
-      `${SERVICE_IMPORT} class MyClass { @service() ${SERVICE_NAME}; fooFunc() {${use}} }`,
-      `${SERVICE_IMPORT} class MyClass { @service() '${SERVICE_NAME}'; fooFunc() {${use}} }`,
-      `${SERVICE_IMPORT} Component.extend({ ${SERVICE_NAME}: service('foo'), fooFunc() {${use}} });`,
-      `${SERVICE_IMPORT} Component.extend({ ${SERVICE_NAME}: service(), fooFunc() {${use}} });`,
-      `${SERVICE_IMPORT} Component.extend({ '${SERVICE_NAME}': service(), fooFunc() {${use}} });`
+      `${importString} class MyClass { @service('foo') ${SERVICE_NAME}; fooFunc() {${use}} }`,
+      `${importString} class MyClass { @service() ${SERVICE_NAME}; fooFunc() {${use}} }`,
+      `${importString} class MyClass { @service() '${SERVICE_NAME}'; fooFunc() {${use}} }`,
+      `${importString} Component.extend({ ${SERVICE_NAME}: service('foo'), fooFunc() {${use}} });`,
+      `${importString} Component.extend({ ${SERVICE_NAME}: service(), fooFunc() {${use}} });`,
+      `${importString} Component.extend({ '${SERVICE_NAME}': service(), fooFunc() {${use}} });`
     );
   }
 
@@ -154,8 +155,8 @@ function generateValid() {
     const imports = idx === 0 ? EO_IMPORTS : RENAMED_EO_IMPORTS;
     for (const use of useCases) {
       valid.push(
-        `${SERVICE_IMPORT}${imports} class MyClass { @service() ${SERVICE_NAME}; fooFunc() {${use}} }`,
-        `${SERVICE_IMPORT}${imports} Component.extend({ ${SERVICE_NAME}: service(), fooFunc() {${use}} });`
+        `${importString}${imports} class MyClass { @service() ${SERVICE_NAME}; fooFunc() {${use}} }`,
+        `${importString}${imports} Component.extend({ ${SERVICE_NAME}: service(), fooFunc() {${use}} });`
       );
     }
   }
@@ -173,6 +174,7 @@ const emberObjectUses2 = generateEmberObjectUseCasesFor('unrelatedProp').join(''
 ruleTester.run('no-unused-services', rule, {
   valid: [
     ...generateValid(),
+    ...generateValid(NEW_SERVICE_IMPORT),
     ...generateMacroUseCasesFor(SERVICE_NAME),
     ...generateMacroUseCasesFor(SERVICE_NAME, true),
     ...generateComputedUseCasesFor(SERVICE_NAME),
@@ -203,6 +205,23 @@ ruleTester.run('no-unused-services', rule, {
         },
       ],
     },
+    // With new import
+    {
+      code: `${NEW_SERVICE_IMPORT} class MyClass { @service('foo') ${SERVICE_NAME}; fooFunc() {${nonUses}} }`,
+      output: null,
+      errors: [
+        {
+          messageId: 'main',
+          suggestions: [
+            {
+              messageId: 'removeServiceInjection',
+              output: `${NEW_SERVICE_IMPORT} class MyClass {  fooFunc() {${nonUses}} }`,
+            },
+          ],
+          // type could be ClassProperty (ESLint v7) or PropertyDefinition (ESLint v8)
+        },
+      ],
+    },
     {
       code: `${SERVICE_IMPORT} class MyClass { @service() ${SERVICE_NAME}; fooFunc() {${nonUses}} }`,
       output: null,
diff --git a/tests/lib/rules/require-computed-property-dependencies.js b/tests/lib/rules/require-computed-property-dependencies.js
index 6711a84c..59c652f4 100644
--- a/tests/lib/rules/require-computed-property-dependencies.js
+++ b/tests/lib/rules/require-computed-property-dependencies.js
@@ -93,6 +93,23 @@ ruleTester.run('require-computed-property-dependencies', rule, {
         'serviceNameInStringLiteral': service() // Property name as string literal.
       });
     `,
+    // Should ignore injected service names via `service` method:
+    `
+      import Ember from 'ember';
+      import Component from '@ember/component';
+      import { service } from '@ember/service';
+      Component.extend({
+        intl: service(),
+        myProperty: Ember.computed('name', function() {
+          console.log(this.intl);
+          return this.name + this.intl.t('some.translation.key');
+          console.log(this.otherService);
+          console.log(this.serviceNameInStringLiteral);
+        }),
+        otherService: service(), // Service injection coming after computed property.
+        'serviceNameInStringLiteral': service() // Property name as string literal.
+      });
+    `,
     // Should ignore the left side of an assignment.
     "import Ember from 'ember'; Ember.computed('right', function() { this.left = this.right; })",
     // Should ignore the left side of an assignment with nested path.