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 decorator support to no-unnecessary-service-injection-argument rule #781

Merged
merged 1 commit into from
Apr 14, 2020
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions docs/rules/no-unnecessary-service-injection-argument.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,3 @@ export default Component.extend({

* Ember [Services](https://guides.emberjs.com/release/applications/services/) guide
* Ember [inject](https://emberjs.com/api/ember/release/functions/@ember%2Fservice/inject) function spec

## Help Wanted

| Issue | Link |
| :-- | :-- |
| :x: Missing native JavaScript class support | [#560](https://github.com/ember-cli/eslint-plugin-ember/issues/560) |
34 changes: 30 additions & 4 deletions lib/rules/no-unnecessary-service-injection-argument.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,45 @@ module.exports = {
if (
!emberUtils.isInjectedServiceProp(node) ||
node.value.arguments.length !== 1 ||
!types.isLiteral(node.value.arguments[0])
!types.isStringLiteral(node.value.arguments[0])
) {
return;
}

const keyName = node.key.name;
const firstArgValue = node.value.arguments[0].value;
const firstArg = node.value.arguments[0];
const firstArgValue = firstArg.value;
if (keyName === firstArgValue) {
context.report({
node: node.value.arguments[0],
node: firstArg,
message: ERROR_MESSAGE,
fix(fixer) {
return fixer.remove(node.value.arguments[0]);
return fixer.remove(firstArg);
},
});
}
},

ClassProperty(node) {
if (
!emberUtils.isInjectedServiceProp(node) ||
node.decorators.length !== 1 ||
!types.isCallExpression(node.decorators[0].expression) ||
node.decorators[0].expression.arguments.length !== 1 ||
!types.isStringLiteral(node.decorators[0].expression.arguments[0])
) {
return;
}

const keyName = node.key.name;
const firstArg = node.decorators[0].expression.arguments[0];
const firstArgValue = firstArg.value;
if (keyName === firstArgValue) {
context.report({
node: firstArg,
message: ERROR_MESSAGE,
fix(fixer) {
return fixer.remove(firstArg);
},
});
}
Expand Down
68 changes: 62 additions & 6 deletions tests/lib/rules/no-unnecessary-service-injection-argument.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,31 +24,74 @@ ruleTester.run('no-unnecessary-service-injection-argument', rule, {
'export default Component.extend({ serviceName: service() });',
'export default Component.extend({ serviceName: inject() });',
'const controller = Controller.extend({ serviceName: service() });',
{
code: 'class Test { @service serviceName }',
parser: require.resolve('babel-eslint'),
parserOptions: {
ecmaVersion: 6,
sourceType: 'module',
ecmaFeatures: { legacyDecorators: true },
},
},
{
code: 'class Test { @service() serviceName }',
parser: require.resolve('babel-eslint'),
parserOptions: {
ecmaVersion: 6,
sourceType: 'module',
ecmaFeatures: { legacyDecorators: true },
},
},

// 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') });",
{
code: 'class Test { @service("service-name") serviceName }',
parser: require.resolve('babel-eslint'),
parserOptions: {
ecmaVersion: 6,
sourceType: 'module',
ecmaFeatures: { legacyDecorators: true },
},
},

// Property name does not match service name:
"const controller = Controller.extend({ specialName: service('service-name') });",
{
code: 'class Test { @service("specialName") serviceName }',
parser: require.resolve('babel-eslint'),
parserOptions: {
ecmaVersion: 6,
sourceType: 'module',
ecmaFeatures: { legacyDecorators: true },
},
},

// 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) });",

// When usage is ignored because of template literal:
'export default Component.extend({ serviceName: service(`serviceName`) });',
{
code: 'class Test { @service(`specialName`) serviceName }',
parser: require.resolve('babel-eslint'),
parserOptions: {
ecmaVersion: 6,
sourceType: 'module',
ecmaFeatures: { legacyDecorators: true },
},
},

// 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') });",

// Decorator:
{
// TODO: this should be an invalid test case.
// Still missing native class and decorator support: https://github.com/ember-cli/eslint-plugin-ember/issues/560
code: "class Test { @service('serviceName') serviceName }",
errors: [{ message: ERROR_MESSAGE, type: 'Literal' }],
code: 'class Test { @otherDecorator("name") name }',
parser: require.resolve('babel-eslint'),
parserOptions: {
ecmaVersion: 6,
Expand Down Expand Up @@ -81,5 +124,18 @@ ruleTester.run('no-unnecessary-service-injection-argument', rule, {
output: 'const controller = Controller.extend({ serviceName: inject() });',
errors: [{ message: ERROR_MESSAGE, type: 'Literal' }],
},

// Decorator:
{
code: 'class Test { @service("serviceName") serviceName }',
output: 'class Test { @service() serviceName }',
errors: [{ message: ERROR_MESSAGE, type: 'Literal' }],
parser: require.resolve('babel-eslint'),
parserOptions: {
ecmaVersion: 6,
sourceType: 'module',
ecmaFeatures: { legacyDecorators: true },
},
},
],
});