diff --git a/README.md b/README.md index 4ae9a5091d..274237439e 100644 --- a/README.md +++ b/README.md @@ -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) | diff --git a/docs/rules/no-implicit-service-injection-argument.md b/docs/rules/no-implicit-service-injection-argument.md new file mode 100644 index 0000000000..cd4714da86 --- /dev/null +++ b/docs/rules/no-implicit-service-injection-argument.md @@ -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 diff --git a/docs/rules/no-unnecessary-service-injection-argument.md b/docs/rules/no-unnecessary-service-injection-argument.md index 959c1870fa..a898b945f7 100644 --- a/docs/rules/no-unnecessary-service-injection-argument.md +++ b/docs/rules/no-unnecessary-service-injection-argument.md @@ -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 diff --git a/lib/rules/no-implicit-service-injection-argument.js b/lib/rules/no-implicit-service-injection-argument.js new file mode 100644 index 0000000000..baee9f74a5 --- /dev/null +++ b/lib/rules/no-implicit-service-injection-argument.js @@ -0,0 +1,105 @@ +'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}')`); + }, + }); + }, + }; + }, +}; diff --git a/tests/lib/rules/no-implicit-service-injection-argument.js b/tests/lib/rules/no-implicit-service-injection-argument.js new file mode 100644 index 0000000000..8b841a0ad5 --- /dev/null +++ b/tests/lib/rules/no-implicit-service-injection-argument.js @@ -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' }], + }, + ], +});