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.

Sign up for GitHub

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

Jump to bottom

Handle new service import style in several rules #1731

Merged
merged 2 commits into from Jan 3, 2023
Merged

Handle new service import style in several rules #1731

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
049cf07
Add tests for new service import
wagenet Jan 3, 2023
3341a79
Properly handle new service import
wagenet Jan 3, 2023
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
5 changes: 4 additions & 1 deletion lib/rules/no-deprecated-router-transition-methods.js
View file
Open in desktop
Expand Up @@ -12,6 +12,7 @@ function getBaseFixSteps(fixer, context, currentClass) {
let serviceInjectImportName = currentClass.serviceInjectImportName;
let routerServicePropertyName = currentClass.routerServicePropertyName;

// For now, we insert the legacy form. If we can detect the Ember version we can insert the new version instead.
if (!serviceInjectImportName) {
fixSteps.push(
fixer.insertTextBefore(
Expand Down Expand Up @@ -130,7 +131,9 @@ module.exports = {
ImportDeclaration(node) {
if (node.source.value === '@ember/service') {
serviceInjectImportName =
serviceInjectImportName || getImportIdentifier(node, '@ember/service', 'inject');
serviceInjectImportName ||
getImportIdentifier(node, '@ember/service', 'inject') ||
getImportIdentifier(node, '@ember/service', 'service');
}
},

Expand Down
4 changes: 3 additions & 1 deletion lib/rules/no-implicit-service-injection-argument.js
View file
Open in desktop
Expand Up @@ -79,7 +79,9 @@ module.exports = {
}
if (node.source.value === '@ember/service') {
importedInjectName =
importedInjectName || getImportIdentifier(node, '@ember/service', 'inject');
importedInjectName ||
getImportIdentifier(node, '@ember/service', 'inject') ||
getImportIdentifier(node, '@ember/service', 'service');
}
},
Property(node) {
Expand Down
16 changes: 11 additions & 5 deletions lib/rules/no-private-routing-service.js
View file
Open in desktop
Expand Up @@ -62,10 +62,10 @@ module.exports = {
let importedEmberName;

// Handle either ClassProperty (ESLint v7) or PropertyDefinition (ESLint v8).
function visitClassPropertyOrPropertyDefinition(node) {
function visitClassPropertyOrPropertyDefinition(node, importedInjectName) {
if (
!node.decorators ||
!decoratorUtils.isClassPropertyOrPropertyDefinitionWithDecorator(node, 'service')
!decoratorUtils.isClassPropertyOrPropertyDefinitionWithDecorator(node, importedInjectName)
) {
return;
}
Expand All @@ -92,7 +92,9 @@ module.exports = {
}
if (node.source.value === '@ember/service') {
importedInjectName =
importedInjectName || getImportIdentifier(node, '@ember/service', 'inject');
importedInjectName ||
getImportIdentifier(node, '@ember/service', 'inject') ||
getImportIdentifier(node, '@ember/service', 'service');
}
},
Property(node) {
Expand All @@ -105,8 +107,12 @@ module.exports = {
}
},

ClassProperty: visitClassPropertyOrPropertyDefinition,
PropertyDefinition: visitClassPropertyOrPropertyDefinition,
ClassProperty(node) {
visitClassPropertyOrPropertyDefinition(node, importedInjectName);
},
PropertyDefinition(node) {
visitClassPropertyOrPropertyDefinition(node, importedInjectName);
},

Literal(node) {
if (
Expand Down
4 changes: 3 additions & 1 deletion lib/rules/no-restricted-service-injections.js
View file
Open in desktop
Expand Up @@ -123,7 +123,9 @@ module.exports = {
}
if (node.source.value === '@ember/service') {
importedInjectName =
importedInjectName || getImportIdentifier(node, '@ember/service', 'inject');
importedInjectName ||
getImportIdentifier(node, '@ember/service', 'inject') ||
getImportIdentifier(node, '@ember/service', 'service');
}
},
// Handles:
Expand Down
4 changes: 3 additions & 1 deletion lib/rules/no-unnecessary-service-injection-argument.js
View file
Open in desktop
Expand Up @@ -64,7 +64,9 @@ module.exports = {
}
if (node.source.value === '@ember/service') {
importedInjectName =
importedInjectName || getImportIdentifier(node, '@ember/service', 'inject');
importedInjectName ||
getImportIdentifier(node, '@ember/service', 'inject') ||
getImportIdentifier(node, '@ember/service', 'service');
}
},
Property(node) {
Expand Down
4 changes: 3 additions & 1 deletion lib/rules/no-unused-services.js
View file
Open in desktop
Expand Up @@ -139,7 +139,9 @@ module.exports = {
}
if (node.source.value === '@ember/service') {
importedInjectName =
importedInjectName || getImportIdentifier(node, '@ember/service', 'inject');
importedInjectName ||
getImportIdentifier(node, '@ember/service', 'inject') ||
getImportIdentifier(node, '@ember/service', 'service');
}
if (node.source.value === '@ember-decorators/object') {
importedObservesName =
Expand Down
4 changes: 3 additions & 1 deletion lib/rules/require-computed-property-dependencies.js
View file
Open in desktop
Expand Up @@ -449,7 +449,9 @@ module.exports = {
}
if (node.source.value === '@ember/service') {
importedInjectName =
importedInjectName || getImportIdentifier(node, '@ember/service', 'inject');
importedInjectName ||
getImportIdentifier(node, '@ember/service', 'inject') ||
getImportIdentifier(node, '@ember/service', 'service');
}
},

Expand Down
37 changes: 37 additions & 0 deletions tests/lib/rules/no-deprecated-router-transition-methods.js
View file
Open in desktop
Expand Up @@ -680,6 +680,43 @@ import Controller from '@ember/controller';
},
],
},
// Injecting with `service` export
{
filename: 'routes/index.js',
code: `
import Route from '@ember/routing/route';
import { service } from '@ember/service';

export default class SettingsRoute extends Route {
@service session;

beforeModel() {
if (!this.session.isAuthenticated) {
this.transitionTo('login');
}
}
}`,
output: `
import Route from '@ember/routing/route';
import { service } from '@ember/service';

export default class SettingsRoute extends Route {
@service('router') router;
@service session;

beforeModel() {
if (!this.session.isAuthenticated) {
this.router.transitionTo('login');
}
}
}`,
errors: [
{
messageId: 'main',
type: 'MemberExpression',
},
],
},
// Test multiple classes in a single file
{
filename: 'routes/index.js',
Expand Down
9 changes: 8 additions & 1 deletion tests/lib/rules/no-implicit-service-injection-argument.js
View file
Open in desktop
Expand Up @@ -10,6 +10,7 @@ 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';";
const NEW_SERVICE_IMPORT = "import {service} from '@ember/service';";

//------------------------------------------------------------------------------
// Tests
Expand All @@ -36,6 +37,7 @@ ruleTester.run('no-implicit-service-injection-argument', rule, {

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

// Not Ember's `service()` function (classic class):
'export default Component.extend({ serviceName: otherFunction() });',
Expand Down Expand Up @@ -69,13 +71,18 @@ ruleTester.run('no-implicit-service-injection-argument', rule, {
output: `${SERVICE_IMPORT} export default Component.extend({ 'serviceName': service('service-name') });`,
errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }],
},

// Decorator:
{
code: `${SERVICE_IMPORT} class Test { @service() serviceName }`,
output: `${SERVICE_IMPORT} class Test { @service('service-name') serviceName }`,
errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }],
},
// Decorator with new import
{
code: `${NEW_SERVICE_IMPORT} class Test { @service() serviceName }`,
output: `${NEW_SERVICE_IMPORT} class Test { @service('service-name') serviceName }`,
errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }],
},
{
// Decorator with no parenthesis
code: `${SERVICE_IMPORT} class Test { @service serviceName }`,
Expand Down
24 changes: 24 additions & 0 deletions tests/lib/rules/no-private-routing-service.js
View file
Open in desktop
Expand Up @@ -13,6 +13,7 @@ const {

const EMBER_IMPORT = "import Ember from 'ember';";
const SERVICE_IMPORT = "import {inject as service} from '@ember/service';";
const NEW_SERVICE_IMPORT = "import {service} from '@ember/service';";

//------------------------------------------------------------------------------
// Tests
Expand Down Expand Up @@ -47,6 +48,7 @@ ruleTester.run('no-private-routing-service', rule, {
`${SERVICE_IMPORT} export default class MyComponent extends Component { @service('router') routing; }`,
`${SERVICE_IMPORT} export default class MyComponent extends Component { @service routing; }`,
`${SERVICE_IMPORT} export default class MyComponent extends Component { @service('routing') routing; }`,
`${NEW_SERVICE_IMPORT} export default class MyComponent extends Component { @service('routing') routing; }`,
`
export default class MyComponent extends Component {
@computed('-routing', 'lastName')
Expand Down Expand Up @@ -95,6 +97,28 @@ ruleTester.run('no-private-routing-service', rule, {
},
],
},
// Octane, new import
{
code: `${NEW_SERVICE_IMPORT} export default class MyComponent extends Component { @service('-routing') routing; }`,
output: null,
errors: [
{
message: PRIVATE_ROUTING_SERVICE_ERROR_MESSAGE,
// type could be ClassProperty (ESLint v7) or PropertyDefinition (ESLint v8)
},
],
},
// Octane, new import, renamed
{
code: "import {service as aliasedService} from '@ember/service'; export default class MyComponent extends Component { @aliasedService('-routing') routing; }",
output: null,
errors: [
{
message: PRIVATE_ROUTING_SERVICE_ERROR_MESSAGE,
// type could be ClassProperty (ESLint v7) or PropertyDefinition (ESLint v8)
},
],
},

// _routerMicrolib (`catchRouterMicrolib` option on)
{
Expand Down
15 changes: 15 additions & 0 deletions tests/lib/rules/no-restricted-service-injections.js
View file
Open in desktop
Expand Up @@ -5,6 +5,7 @@ const { DEFAULT_ERROR_MESSAGE } = rule;

const EMBER_IMPORT = "import Ember from 'ember';";
const SERVICE_IMPORT = "import {inject as service} from '@ember/service';";
const NEW_SERVICE_IMPORT = "import {service} from '@ember/service';";

const ruleTester = new RuleTester({
parser: require.resolve('@babel/eslint-parser'),
Expand Down Expand Up @@ -46,6 +47,12 @@ ruleTester.run('no-restricted-service-injections', rule, {
code: `${SERVICE_IMPORT} class MyComponent extends Component { @service('myService') randomName }`,
options: [{ paths: ['app/components'], services: ['abc'] }],
},
{
// Service name doesn't match (with decorator and new import)
filename: 'app/components/path.js',
code: `${NEW_SERVICE_IMPORT} class MyComponent extends Component { @service('myService') randomName }`,
options: [{ paths: ['app/components'], services: ['abc'] }],
},
{
// Service scope doesn't match:
filename: 'app/components/path.js',
Expand Down Expand Up @@ -80,6 +87,14 @@ ruleTester.run('no-restricted-service-injections', rule, {
options: [{ paths: ['app/components'], services: ['my-service'] }],
errors: [{ message: DEFAULT_ERROR_MESSAGE, type: 'Property' }],
},
{
// Without service name argument and new import:
filename: 'app/components/path.js',
code: `${NEW_SERVICE_IMPORT} Component.extend({ myService: service() })`,
output: null,
options: [{ paths: ['app/components'], services: ['my-service'] }],
errors: [{ message: DEFAULT_ERROR_MESSAGE, type: 'Property' }],
},
{
// Without service name argument (property name as string literal):
filename: 'app/components/path.js',
Expand Down
8 changes: 8 additions & 0 deletions tests/lib/rules/no-unnecessary-service-injection-argument.js
View file
Open in desktop
Expand Up @@ -10,6 +10,7 @@ const { ERROR_MESSAGE } = rule;
const EMBER_IMPORT = "import Ember from 'ember';";
const SERVICE_IMPORT = "import {inject} from '@ember/service';";
const RENAMED_SERVICE_IMPORT = "import {inject as service} from '@ember/service';";
const NEW_SERVICE_IMPORT = "import {service} from '@ember/service';";

//------------------------------------------------------------------------------
// Tests
Expand Down Expand Up @@ -50,6 +51,7 @@ ruleTester.run('no-unnecessary-service-injection-argument', rule, {
`${RENAMED_SERVICE_IMPORT} const controller = Controller.extend({ serviceName: service('service-name') });`,
`${RENAMED_SERVICE_IMPORT} class Test { @service("service-name") serviceName }`,
`${RENAMED_SERVICE_IMPORT} class Test { @service("service-name") 'serviceName' }`,
`${NEW_SERVICE_IMPORT} class Test { @service("service-name") 'serviceName' }`,

// Property name does not match service name:
`${EMBER_IMPORT} const controller = Controller.extend({ specialName: Ember.inject.service('service-name') });`,
Expand Down Expand Up @@ -105,5 +107,11 @@ ruleTester.run('no-unnecessary-service-injection-argument', rule, {
output: `${RENAMED_SERVICE_IMPORT} class Test { @service() serviceName }`,
errors: [{ message: ERROR_MESSAGE, type: 'Literal' }],
},
// Decorator, with new import
{
code: `${NEW_SERVICE_IMPORT} class Test { @service("serviceName") serviceName }`,
output: `${NEW_SERVICE_IMPORT} class Test { @service() serviceName }`,
errors: [{ message: ERROR_MESSAGE, type: 'Literal' }],
},
],
});
37 changes: 28 additions & 9 deletions tests/lib/rules/no-unused-services.js
View file
Open in desktop
Expand Up @@ -19,6 +19,7 @@ const ruleTester = new RuleTester({

const SERVICE_NAME = 'fooName';
const SERVICE_IMPORT = "import {inject as service} from '@ember/service';";
const NEW_SERVICE_IMPORT = "import {service} from '@ember/service';";
const EO_IMPORTS = "import {computed, get, getProperties, observer} from '@ember/object';";
const RENAMED_EO_IMPORTS =
"import {computed as cp, get as g, getProperties as gp, observer as ob} from '@ember/object';";
Expand Down Expand Up @@ -131,18 +132,18 @@ function generateObserverUseCasesFor(propertyName, renamed = false) {
* Generate an array of valid test cases
* @returns {Array}
*/
function generateValid() {
function generateValid(importString = SERVICE_IMPORT) {
const valid = [];

const useCases = generateUseCasesFor(SERVICE_NAME);
for (const use of useCases) {
valid.push(
`${SERVICE_IMPORT} class MyClass { @service('foo') ${SERVICE_NAME}; fooFunc() {${use}} }`,
`${SERVICE_IMPORT} class MyClass { @service() ${SERVICE_NAME}; fooFunc() {${use}} }`,
`${SERVICE_IMPORT} class MyClass { @service() '${SERVICE_NAME}'; fooFunc() {${use}} }`,
`${SERVICE_IMPORT} Component.extend({ ${SERVICE_NAME}: service('foo'), fooFunc() {${use}} });`,
`${SERVICE_IMPORT} Component.extend({ ${SERVICE_NAME}: service(), fooFunc() {${use}} });`,
`${SERVICE_IMPORT} Component.extend({ '${SERVICE_NAME}': service(), fooFunc() {${use}} });`
`${importString} class MyClass { @service('foo') ${SERVICE_NAME}; fooFunc() {${use}} }`,
`${importString} class MyClass { @service() ${SERVICE_NAME}; fooFunc() {${use}} }`,
`${importString} class MyClass { @service() '${SERVICE_NAME}'; fooFunc() {${use}} }`,
`${importString} Component.extend({ ${SERVICE_NAME}: service('foo'), fooFunc() {${use}} });`,
`${importString} Component.extend({ ${SERVICE_NAME}: service(), fooFunc() {${use}} });`,
`${importString} Component.extend({ '${SERVICE_NAME}': service(), fooFunc() {${use}} });`
);
}

Expand All @@ -154,8 +155,8 @@ function generateValid() {
const imports = idx === 0 ? EO_IMPORTS : RENAMED_EO_IMPORTS;
for (const use of useCases) {
valid.push(
`${SERVICE_IMPORT}${imports} class MyClass { @service() ${SERVICE_NAME}; fooFunc() {${use}} }`,
`${SERVICE_IMPORT}${imports} Component.extend({ ${SERVICE_NAME}: service(), fooFunc() {${use}} });`
`${importString}${imports} class MyClass { @service() ${SERVICE_NAME}; fooFunc() {${use}} }`,
`${importString}${imports} Component.extend({ ${SERVICE_NAME}: service(), fooFunc() {${use}} });`
);
}
}
Expand All @@ -173,6 +174,7 @@ const emberObjectUses2 = generateEmberObjectUseCasesFor('unrelatedProp').join(''
ruleTester.run('no-unused-services', rule, {
valid: [
...generateValid(),
...generateValid(NEW_SERVICE_IMPORT),
...generateMacroUseCasesFor(SERVICE_NAME),
...generateMacroUseCasesFor(SERVICE_NAME, true),
...generateComputedUseCasesFor(SERVICE_NAME),
Expand Down Expand Up @@ -203,6 +205,23 @@ ruleTester.run('no-unused-services', rule, {
},
],
},
// With new import
{
code: `${NEW_SERVICE_IMPORT} class MyClass { @service('foo') ${SERVICE_NAME}; fooFunc() {${nonUses}} }`,
output: null,
errors: [
{
messageId: 'main',
suggestions: [
{
messageId: 'removeServiceInjection',
output: `${NEW_SERVICE_IMPORT} class MyClass { fooFunc() {${nonUses}} }`,
},
],
// type could be ClassProperty (ESLint v7) or PropertyDefinition (ESLint v8)
},
],
},
{
code: `${SERVICE_IMPORT} class MyClass { @service() ${SERVICE_NAME}; fooFunc() {${nonUses}} }`,
output: null,
Expand Down