Repository: ember-cli/eslint-plugin-ember
Pull request #378 (2019-01-17 04:02:32)

Add new `no-unnecessary-service-injection-argument` rule
## Disallow unnecessary argument when injecting service

### Rule name: `no-unnecessary-service-injection-argument`

It's not necessary to specify an injected service's name as an argument when the property name matches the service name.

### Rule Details

Examples of **incorrect** code for this rule:

```js
import Component from '@ember/component';
import { inject as service } from '@ember/service';
export default Component.extend({
  myServiceName: service('myServiceName')
});
```

Examples of **correct** code for this rule:

```js
export default Component.extend({
  myServiceName: service()
});
```

```js
export default Component.extend({
  myServiceName: service('my-service-name')
});
```

```js
export default Component.extend({
  otherSpecialName: service('my-service-name')
});
```

### References

* Ember [Services](https://guides.emberjs.com/release/applications/services/) guide
* Ember [inject](https://emberjs.com/api/ember/release/functions/@ember%2Fservice/inject) function spec

rwjblue: I disagree with some of the examples 😺 

```
  myServiceName: service('my-service-name')
```

^ should absolutely be allowed, it avoids needless runtime camelization <-> dasherization from happening in the resolver and is a _good thing_.

Making the rule focus on single word injections removes this concern...
bmish: @rwjblue thanks for the feedback. I have updated the rule to only disallow exact name matches, meaning dasherized names will now be allowed as an argument.

Out of curiosity, is the performance hit from camelization <-> dasherization actually meaningful? I assumed it would negligible, and that we should prefer to write the cleanest code instead of worrying about that.
bmish: @rwjblue any other concerns? I addressed your previous comment.

Patch:
diff --git a/README.md b/README.md
index 2addbecd..d1128e89 100644
--- a/README.md
+++ b/README.md
@@ -97,6 +97,7 @@ The `--fix` option on the command line automatically fixes problems reported by
 | :white_check_mark: | [no-on-calls-in-components](./docs/rules/no-on-calls-in-components.md) | Prevents usage of `on` to call lifecycle hooks in components |
 | :white_check_mark: | [no-restricted-resolver-tests](./docs/rules/no-restricted-resolver-tests.md) | Prevents the use of patterns that use the restricted resolver in tests. |
 | :wrench: | [no-unnecessary-route-path-option](./docs/rules/no-unnecessary-route-path-option.md) | Disallow unnecessary route `path` option |
+| :wrench: | [no-unnecessary-service-injection-argument](./docs/rules/no-unnecessary-service-injection-argument.md) | Disallow unnecessary argument when injecting service |
 | :wrench: | [use-ember-get-and-set](./docs/rules/use-ember-get-and-set.md) | Enforces usage of Ember.get and Ember.set |
 
 
diff --git a/docs/rules/no-unnecessary-service-injection-argument.md b/docs/rules/no-unnecessary-service-injection-argument.md
new file mode 100644
index 00000000..17456e6c
--- /dev/null
+++ b/docs/rules/no-unnecessary-service-injection-argument.md
@@ -0,0 +1,43 @@
+## Disallow unnecessary argument when injecting service
+
+### Rule name: `no-unnecessary-service-injection-argument`
+
+It's not necessary to specify an injected service's name as an argument when the property name matches the service name.
+
+### Rule Details
+
+Examples of **incorrect** code for this rule:
+
+```js
+import Component from '@ember/component';
+import { inject as service } from '@ember/service';
+
+export default Component.extend({
+  myServiceName: service('myServiceName')
+});
+```
+
+Examples of **correct** code for this rule:
+
+```js
+export default Component.extend({
+  myServiceName: service()
+});
+```
+
+```js
+export default Component.extend({
+  myServiceName: service('my-service-name')
+});
+```
+
+```js
+export default Component.extend({
+  otherSpecialName: service('my-service-name')
+});
+```
+
+### References
+
+* Ember [Services](https://guides.emberjs.com/release/applications/services/) guide
+* Ember [inject](https://emberjs.com/api/ember/release/functions/@ember%2Fservice/inject) function spec
diff --git a/lib/index.js b/lib/index.js
index 91536da7..ba66f742 100644
--- a/lib/index.js
+++ b/lib/index.js
@@ -33,6 +33,7 @@ module.exports = {
     'no-test-and-then': require('./rules/no-test-and-then'),
     'no-test-import-export': require('./rules/no-test-import-export'),
     'no-unnecessary-route-path-option': require('./rules/no-unnecessary-route-path-option'),
+    'no-unnecessary-service-injection-argument': require('./rules/no-unnecessary-service-injection-argument'),
     'order-in-components': require('./rules/order-in-components'),
     'order-in-controllers': require('./rules/order-in-controllers'),
     'order-in-models': require('./rules/order-in-models'),
diff --git a/lib/recommended-rules.js b/lib/recommended-rules.js
index 8b93e7ed..d350e052 100644
--- a/lib/recommended-rules.js
+++ b/lib/recommended-rules.js
@@ -35,6 +35,7 @@ module.exports = {
   "ember/no-test-and-then": "off",
   "ember/no-test-import-export": "off",
   "ember/no-unnecessary-route-path-option": "off",
+  "ember/no-unnecessary-service-injection-argument": "off",
   "ember/order-in-components": "off",
   "ember/order-in-controllers": "off",
   "ember/order-in-models": "off",
diff --git a/lib/rules/no-unnecessary-service-injection-argument.js b/lib/rules/no-unnecessary-service-injection-argument.js
new file mode 100644
index 00000000..03a69ffe
--- /dev/null
+++ b/lib/rules/no-unnecessary-service-injection-argument.js
@@ -0,0 +1,46 @@
+'use strict';
+
+const utils = require('../utils/utils');
+const emberUtils = require('../utils/ember');
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+const ERROR_MESSAGE = "Don't specify injected service name as an argument when it matches the property name.";
+
+module.exports = {
+  meta: {
+    docs: {
+      description: 'Disallow unnecessary argument when injecting service',
+      category: 'Best Practices',
+      recommended: false
+    },
+    fixable: 'code',
+    ERROR_MESSAGE
+  },
+
+  create(context) {
+    return {
+      Property(node) {
+        if (!emberUtils.isInjectedServiceProp(node.value) ||
+            node.value.arguments.length !== 1 ||
+            !utils.isLiteral(node.value.arguments[0])) {
+          return;
+        }
+
+        const keyName = node.key.name;
+        const firstArgValue = node.value.arguments[0].value;
+        if (keyName === firstArgValue) {
+          context.report({
+            node: node.value.arguments[0],
+            message: ERROR_MESSAGE,
+            fix(fixer) {
+              return fixer.remove(node.value.arguments[0]);
+            }
+          });
+        }
+      }
+    };
+  }
+};


Test patch:
diff --git a/tests/lib/rules/no-unnecessary-service-injection-argument.js b/tests/lib/rules/no-unnecessary-service-injection-argument.js
new file mode 100644
index 00000000..8f749fca
--- /dev/null
+++ b/tests/lib/rules/no-unnecessary-service-injection-argument.js
@@ -0,0 +1,77 @@
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const rule = require('../../../lib/rules/no-unnecessary-service-injection-argument');
+const RuleTester = require('eslint').RuleTester;
+
+const { ERROR_MESSAGE } = rule;
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+const ruleTester = new RuleTester({
+  parserOptions: {
+    ecmaVersion: 2015,
+    sourceType: 'module'
+  }
+});
+
+ruleTester.run('no-unnecessary-service-injection-argument', rule, {
+  valid: [
+    // No argument:
+    'export default Component.extend({ serviceName: service() });',
+    'export default Component.extend({ serviceName: inject() });',
+    'const controller = Controller.extend({ serviceName: service() });',
+
+    // Property name matches service name but service name uses dashes
+    // (allowed because it avoids needless runtime camelization <-> dasherization in the resolver):
+    "export default Component.extend({ specialName: service('service-name') });",
+    "export default Component.extend({ specialName: inject('service-name') });",
+    "const controller = Controller.extend({ serviceName: service('service-name') });",
+
+    // Property name does not match service name:
+    "export default Component.extend({ specialName: service('service-name') });",
+    "export default Component.extend({ specialName: inject('service-name') });",
+    "const controller = Controller.extend({ specialName: service('service-name') });",
+
+    // When usage is ignored because of additional arguments:
+    "export default Component.extend({ serviceName: service('serviceName', EXTRA_PROPERTY) });",
+    "export default Component.extend({ serviceName: inject('serviceName', EXTRA_PROPERTY) });",
+
+    // Not Ember's `service()` function:
+    "export default Component.extend({ serviceName: otherFunction('serviceName') });",
+    "export default Component.extend({ serviceName: service.otherFunction('serviceName') });",
+    "export default Component.extend({ serviceName: inject.otherFunction('serviceName') });"
+  ],
+  invalid: [
+    // `Component` examples:
+    {
+      code:
+        "export default Component.extend({ serviceName: service('serviceName') });",
+      output: 'export default Component.extend({ serviceName: service() });',
+      errors: [{ message: ERROR_MESSAGE, type: 'Literal' }]
+    },
+    {
+      code:
+        "export default Component.extend({ serviceName: inject('serviceName') });",
+      output: 'export default Component.extend({ serviceName: inject() });',
+      errors: [{ message: ERROR_MESSAGE, type: 'Literal' }]
+    },
+
+    // `Controller` examples:
+    {
+      code:
+        "const controller = Controller.extend({ serviceName: service('serviceName') });",
+      output: 'const controller = Controller.extend({ serviceName: service() });',
+      errors: [{ message: ERROR_MESSAGE, type: 'Literal' }]
+    },
+    {
+      code:
+        "const controller = Controller.extend({ serviceName: inject('serviceName') });",
+      output: 'const controller = Controller.extend({ serviceName: inject() });',
+      errors: [{ message: ERROR_MESSAGE, type: 'Literal' }]
+    }
+  ]
+});
diff --git a/tests/lib/utils/ember-test.js b/tests/lib/utils/ember-test.js
index 92cd2425..bf4c9490 100644
--- a/tests/lib/utils/ember-test.js
+++ b/tests/lib/utils/ember-test.js
@@ -254,6 +254,12 @@ describe('isInjectedServiceProp', () => {
 
     node = parse('inject()');
     expect(emberUtils.isInjectedServiceProp(node)).toBeTruthy();
+
+    node = parse('otherFunction()');
+    expect(emberUtils.isInjectedServiceProp(node)).toBeFalsy();
+
+    node = parse('service.otherFunction()');
+    expect(emberUtils.isInjectedServiceProp(node)).toBeFalsy();
   });
 });