Skip to content

Commit

Permalink
feat: add new rule no-implicit-service-injection-argument
Browse files Browse the repository at this point in the history
  • Loading branch information
bmish committed May 9, 2021
1 parent f481ce9 commit 772c0f3
Show file tree
Hide file tree
Showing 5 changed files with 229 additions and 0 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ Rules are grouped by category to help you understand their purpose. Each rule ha

| | Rule ID | Description |
|:---|:--------|:------------|
| :wrench: | [no-implicit-service-injection-argument](./docs/rules/no-implicit-service-injection-argument.md) | disallow omitting the injected service name argument |
| | [no-restricted-service-injections](./docs/rules/no-restricted-service-injections.md) | disallow injecting certain services under certain paths |
| :wrench: | [no-unnecessary-service-injection-argument](./docs/rules/no-unnecessary-service-injection-argument.md) | disallow unnecessary argument when injecting services |
| | [no-unused-services](./docs/rules/no-unused-services.md) | disallow unused service injections (see rule doc for limitations) |
Expand Down
40 changes: 40 additions & 0 deletions docs/rules/no-implicit-service-injection-argument.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# no-implicit-service-injection-argument

:wrench: The `--fix` option on the [command line](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems) can automatically fix some of the problems reported by this rule.

Disallow omitting the service name argument when injecting a service.

Some developers may prefer to be more explicit about what service is being injected instead of relying on the implicit and potentially costly lookup/normalization of the service from the property name.

## Examples

Examples of **incorrect** code for this rule:

```js
import Component from '@ember/component';
import { inject as service } from '@ember/service';

export default class Page extends Component {
@service() serviceName;
}
```

Examples of **correct** code for this rule:

```js
import Component from '@ember/component';
import { inject as service } from '@ember/service';

export default class Page extends Component {
@service('service-name') serviceName;
}
```

## Related Rules

* [no-unnecessary-service-injection-argument](https://github.com/ember-cli/eslint-plugin-ember/blob/master/docs/rules/no-unnecessary-service-injection-argument.md) is the opposite of this rule

## 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
4 changes: 4 additions & 0 deletions docs/rules/no-unnecessary-service-injection-argument.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ export default Component.extend({
});
```

## Related Rules

* [no-implicit-service-injection-argument](https://github.com/ember-cli/eslint-plugin-ember/blob/master/docs/rules/no-implicit-service-injection-argument.md) is the opposite of this rule

## References

* Ember [Services](https://guides.emberjs.com/release/applications/services/) guide
Expand Down
106 changes: 106 additions & 0 deletions lib/rules/no-implicit-service-injection-argument.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
'use strict';

const types = require('../utils/types');
const emberUtils = require('../utils/ember');
const { getImportIdentifier } = require('../utils/import');

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

const ERROR_MESSAGE = "Don't omit the argument for the injected service name.";

module.exports = {
meta: {
type: 'suggestion',
docs: {
description: 'disallow omitting the injected service name argument',
category: 'Services',
recommended: false,
url:
'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/no-implicit-service-injection-argument.md',
},
fixable: 'code',
schema: [],
},

ERROR_MESSAGE,

create(context) {
let importedInjectName;
let importedEmberName;

return {
ImportDeclaration(node) {
if (node.source.value === 'ember') {
importedEmberName = importedEmberName || getImportIdentifier(node, 'ember');
}
if (node.source.value === '@ember/service') {
importedInjectName =
importedInjectName || getImportIdentifier(node, '@ember/service', 'inject');
}
},
Property(node) {
// Classic classes.

if (
!emberUtils.isInjectedServiceProp(node, importedEmberName, importedInjectName) ||
node.value.arguments.length > 0
) {
// Already has the service name argument.
return;
}

if (node.value.arguments.length === 0) {
context.report({
node: node.value,
message: ERROR_MESSAGE,
fix(fixer) {
const sourceCode = context.getSourceCode();
return fixer.insertTextAfter(
sourceCode.getTokenAfter(node.value.callee),
`'${node.key.name}'`
);
},
});
}
},

ClassProperty(node) {
// Native classes.

if (
!emberUtils.isInjectedServiceProp(node, importedEmberName, importedInjectName) ||
node.decorators.length !== 1
) {
return;
}

if (
types.isCallExpression(node.decorators[0].expression) &&
node.decorators[0].expression.arguments.length > 0
) {
// Already has the service name argument.
return;
}

context.report({
node: node.decorators[0].expression,
message: ERROR_MESSAGE,
fix(fixer) {
const sourceCode = context.getSourceCode();
const serviceName = node.key.name;
return node.decorators[0].expression.type === 'CallExpression'
? // Add after parenthesis.
fixer.insertTextAfter(
sourceCode.getTokenAfter(node.decorators[0].expression.callee),
`'${serviceName}'`
)
: // No parenthesis yet so we need to add them.
fixer.insertTextAfter(node.decorators[0].expression, `('${serviceName}')`);
},
});
},
};
},
};
78 changes: 78 additions & 0 deletions tests/lib/rules/no-implicit-service-injection-argument.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const rule = require('../../../lib/rules/no-implicit-service-injection-argument');
const RuleTester = require('eslint').RuleTester;

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';";

//------------------------------------------------------------------------------
// Tests
//------------------------------------------------------------------------------

const ruleTester = new RuleTester({
parserOptions: {
ecmaVersion: 6,
sourceType: 'module',
ecmaFeatures: { legacyDecorators: true },
},
parser: require.resolve('@babel/eslint-parser'),
});

ruleTester.run('no-implicit-service-injection-argument', rule, {
valid: [
// With argument (classic class):
`${EMBER_IMPORT} export default Component.extend({ serviceName: Ember.inject.service('serviceName') });`,
`${INJECT_IMPORT} export default Component.extend({ serviceName: inject('serviceName') });`,
`${SERVICE_IMPORT} export default Component.extend({ serviceName: service('serviceName') });`,
`${SERVICE_IMPORT} export default Component.extend({ serviceName: service('service-name') });`,
`${SERVICE_IMPORT} export default Component.extend({ serviceName: service('random') });`,
`${SERVICE_IMPORT} export default Component.extend({ serviceName: service(\`service-name\`) });`,

// With argument (native class)
`${SERVICE_IMPORT} class Test { @service('service-name') serviceName }`,

// Not Ember's `service()` function (classic class):
'export default Component.extend({ serviceName: otherFunction() });',
`${SERVICE_IMPORT} export default Component.extend({ serviceName: service.foo() });`,

// Not Ember's `service()` function (native class):
`${SERVICE_IMPORT} class Test { @otherDecorator() name }`,
`${SERVICE_IMPORT} class Test { @service.foo() name }`,
`${SERVICE_IMPORT} class Test { @foo.service() name }`,

// Spread syntax
'export default Component.extend({ ...foo });',
],
invalid: [
// Classic class
{
code: `${SERVICE_IMPORT} export default Component.extend({ serviceName: service() });`,
output: `${SERVICE_IMPORT} export default Component.extend({ serviceName: service('serviceName') });`,
errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }],
},
{
code: `${INJECT_IMPORT} export default Component.extend({ serviceName: inject() });`,
output: `${INJECT_IMPORT} export default Component.extend({ serviceName: inject('serviceName') });`,
errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }],
},

// Decorator:
{
code: `${SERVICE_IMPORT} class Test { @service() serviceName }`,
output: `${SERVICE_IMPORT} class Test { @service('serviceName') serviceName }`,
errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }],
},
{
// Decorator with no parenthesis
code: `${SERVICE_IMPORT} class Test { @service serviceName }`,
output: `${SERVICE_IMPORT} class Test { @service('serviceName') serviceName }`,
errors: [{ message: ERROR_MESSAGE, type: 'Identifier' }],
},
],
});

0 comments on commit 772c0f3

Please sign in to comment.