diff --git a/README.md b/README.md index 2addbecdfd..d1128e89a9 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 0000000000..17456e6c23 --- /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 91536da784..ba66f74202 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 8b93e7eda7..d350e052e9 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 0000000000..03a69ffec8 --- /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]); + } + }); + } + } + }; + } +}; 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 0000000000..8f749fca5f --- /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 92cd24250b..bf4c949005 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(); }); });