Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new no-unnecessary-service-injection-argument rule #378

Merged
merged 1 commit into from Mar 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Expand Up @@ -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 |


Expand Down
43 changes: 43 additions & 0 deletions 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
1 change: 1 addition & 0 deletions lib/index.js
Expand Up @@ -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'),
Expand Down
1 change: 1 addition & 0 deletions lib/recommended-rules.js
Expand Up @@ -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",
Expand Down
46 changes: 46 additions & 0 deletions 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]);
}
});
}
}
};
}
};
77 changes: 77 additions & 0 deletions 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' }]
}
]
bmish marked this conversation as resolved.
Show resolved Hide resolved
});
6 changes: 6 additions & 0 deletions tests/lib/utils/ember-test.js
Expand Up @@ -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();
});
});

Expand Down