Skip to content

Commit

Permalink
Add new rule no-implicit-injections (#1714)
Browse files Browse the repository at this point in the history
Co-authored-by: Bryan Mishkin <[email protected]>
Fixes #1178
  • Loading branch information
rtablada authored Dec 30, 2022
1 parent 78cbc5a commit 6cc7860
Show file tree
Hide file tree
Showing 8 changed files with 1,187 additions and 4 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ module.exports = {
| [new-module-imports](docs/rules/new-module-imports.md) | enforce using "New Module Imports" from Ember RFC #176 || | |
| [no-array-prototype-extensions](docs/rules/no-array-prototype-extensions.md) | disallow usage of Ember's `Array` prototype extensions | | 🔧 | |
| [no-function-prototype-extensions](docs/rules/no-function-prototype-extensions.md) | disallow usage of Ember's `function` prototype extensions || | |
| [no-implicit-injections](docs/rules/no-implicit-injections.md) | enforce usage of implicit service injections | | 🔧 | |
| [no-mixins](docs/rules/no-mixins.md) | disallow the usage of mixins || | |
| [no-new-mixins](docs/rules/no-new-mixins.md) | disallow the creation of new mixins || | |
| [no-observers](docs/rules/no-observers.md) | disallow usage of observers || | |
Expand Down
116 changes: 116 additions & 0 deletions docs/rules/no-implicit-injections.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
# ember/no-implicit-injections

🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix).

<!-- end auto-generated rule header -->

Ember 3.26 introduced a deprecation for relying on implicit service injections or allowing addons to implicitly inject services into all classes of certain types. Support for this is dropped in Ember 4.0.

In many applications, `this.store` from Ember Data is often used without injecting the `store` service in Controllers or Routes. Other addons may also have included implicit service injections via initializers and the `application.inject` API.

To resolve this deprecation, a service should be explicitly declared and injected using the [service injection decorator](https://api.emberjs.com/ember/3.28/functions/@ember%2Fservice/inject).

## Rule Details

This rule checks for a configured list of previously auto injected services and warns if they are used in classes without explicit injected service properties.

## Examples

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

```js
// routes/index.js
import Route from '@ember/routing/route';

export default class IndexRoute extends Route {
model() {
return this.store.findAll('rental');
}
}

```

```js
// controllers/index.js
import Controller from '@ember/controller';
import { action } from '@ember/object';

export default class IndexController extends Controller {
@action
loadUsers() {
return this.store.findAll('user');
}
}
```

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

```js
// routes/index.js
import Route from '@ember/routing/route';
import { inject as service } from '@ember/service';

export default class IndexRoute extends Route {
@service('store') store;

model() {
return this.store.findAll('rental');
}
}
```

```js
// controller/index.js
import Route from '@ember/routing/route';
import { action } from '@ember/object';
import { inject as service } from '@ember/service';

export default class IndexController extends Controller {
@service('store') store;

@action
loadUsers() {
return this.store.findAll('user');
}
}
```

## Migration

The autofixer for this rule will update classes and add injections for the configured services.

## Configuration

This lint rule will search for instances of `store` used in routes or controllers by default. If you have other services that you would like to check for uses of, the configuration can be overridden.

- object -- containing the following properties:
- array -- `denyList` -- Array of configuration objects configuring the lint rule to check for use of implicit injected services
- string -- `service` -- The (kebab-case) service name that should be checked for implicit injections and error if found
- array -- `propertyName` -- The property name where the service would be injected in classes. This defaults to the camel case version of the `service` config above.
- array -- `moduleNames` -- Array of string listing the types of classes (`Controller`, `Route`, `Component`, etc) to check for implicit injections. If an array is declared, only those class types will be checked for implicit injection. (Defaults to checking all Ember Module class files/types)

Example config:

```js
module.exports = {
rules: {
'ember/no-implicit-injections': ['error', {
denyList: [
// Ember Responsive Used to Auto Inject the media service in Components/Controllers
{ service: 'media', moduleNames: ['Component', 'Controller'] },
// Ember CLI Flash Used to Auto Inject the flashMessages service in all modules
{ service: 'flash-messages' },
// Check for uses of the store in Routes or Controllers
{ service: 'store', moduleNames: ['Route', 'Controller'] },
// Check for the feature service injected as "featureChecker"
{ service: 'feature', propertyName: 'featureChecker' },
]
}]
}
}
```

## References

- [Deprecation](https://deprecations.emberjs.com/v3.x/#toc_implicit-injections)
- [Ember Data Store Service](https://api.emberjs.com/ember-data/release/classes/Store)
242 changes: 242 additions & 0 deletions lib/rules/no-implicit-injections.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,242 @@
'use strict';

const assert = require('assert');
const emberUtils = require('../utils/ember');
const { getImportIdentifier } = require('../utils/import');
const Stack = require('../utils/stack');
const types = require('../utils/types');
const camelCase = require('lodash.camelcase');

const defaultServiceConfig = { service: 'store', moduleNames: ['Route', 'Controller'] };
const MODULE_TYPES = [
'Component',
'GlimmerComponent',
'Controller',
'Mixin',
'Route',
'Service',
'ArrayProxy',
'ObjectProxy',
'EmberObject',
'Helper',
];

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

function fixService(fixer, currentClass, serviceInjectImportName, failedConfig) {
const serviceInjectPath = failedConfig.service;

return currentClass.node.type === 'CallExpression'
? fixer.insertTextBefore(
currentClass.node.arguments[0].properties[0],
`${failedConfig.propertyName}: ${serviceInjectImportName}('${serviceInjectPath}'),\n`
)
: fixer.insertTextBefore(
currentClass.node.body.body[0],
`@${serviceInjectImportName}('${serviceInjectPath}') ${failedConfig.propertyName};\n`
);
}

function normalizeConfiguration(denyList) {
return denyList.map((config) => ({
service: config.service,
propertyName: config.propertyName ?? camelCase(config.service),
moduleNames: config.moduleNames ?? MODULE_TYPES,
}));
}

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
meta: {
type: 'suggestion',
docs: {
description: 'enforce usage of implicit service injections',
category: 'Deprecations',
recommended: false,
url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/no-implicit-injections.md',
},
fixable: 'code',
schema: [
{
type: 'object',
required: ['denyList'],
properties: {
denyList: {
minItems: 1,
type: 'array',
items: {
type: 'object',
default: [defaultServiceConfig],
required: ['service'],
properties: {
service: {
type: 'string',
minLength: 1,
},
propertyName: {
type: 'string',
minLength: 1,
},
moduleNames: {
type: 'array',
items: {
enum: MODULE_TYPES,
},
},
},
additionalProperties: false,
},
},
},
additionalProperties: false,
},
],
messages: {
main: 'Do not rely on implicit service injections for the "{{serviceName}}" service. Implicit service injections were deprecated in Ember 3.26 and will not work in 4.0.',
},
},

create(context) {
const options = context.options[0] || {
denyList: [defaultServiceConfig],
};
const denyList = options.denyList || [defaultServiceConfig];

for (const config of denyList) {
assert(
emberUtils.convertServiceNameToKebabCase(config.service) === config.service,
'Service name should be passed in kebab-case (all lower case)'
);

assert(
!config.service.includes('/') || config.propertyName,
'Nested services must declare a property name'
);
}

// State being tracked for this file.
let serviceInjectImportName = undefined;
const normalizedConfiguration = normalizeConfiguration(denyList);

// State being tracked for the current class we're inside.
const classStack = new Stack();

// Array of posible types that could declare existing properties on native or legacy modules
const propertyDefintionTypes = new Set([
'Property',
'ClassProperty',
'PropertyDefinition',
'MethodDefinition',
]);

function onModuleFound(node) {
// Get the name of services for the current module type
let configToCheckFor = normalizedConfiguration.filter((serviceConfig) => {
return (
serviceConfig.moduleNames === undefined ||
serviceConfig.moduleNames.some((moduleName) =>
emberUtils.isEmberCoreModule(context, node, moduleName)
)
);
});

const modulePropertyDeclarations =
node.type === 'CallExpression' ? node.arguments[0].properties : node.body.body;

// Get Services that don't have properties/service injections declared
configToCheckFor = modulePropertyDeclarations.reduce((accum, n) => {
if (propertyDefintionTypes.has(n.type)) {
return accum.filter((config) => !(config.propertyName === n.key.name));
}
return accum;
}, configToCheckFor);

classStack.push({
node,
isEmberModule: true,
configToCheckFor,
});
}

function onClassEnter(node) {
if (emberUtils.isAnyEmberCoreModule(context, node)) {
onModuleFound(node);
} else {
classStack.push({
node,
isEmberModule: false,
});
}
}

function onClassExit(node) {
// Leaving current (native) class.
if (classStack.size() > 0 && classStack.peek().node === node) {
classStack.pop();
}
}

return {
ImportDeclaration(node) {
if (node.source.value === '@ember/service') {
serviceInjectImportName =
serviceInjectImportName || getImportIdentifier(node, '@ember/service', 'inject');
}
},

ClassDeclaration: onClassEnter,
ClassExpression: onClassEnter,
CallExpression(node) {
if (emberUtils.isAnyEmberCoreModule(context, node)) {
onModuleFound(node);
}
},

'ClassDeclaration:exit': onClassExit,
'ClassExpression:exit': onClassExit,
'CallExpression:exit': onClassExit,

MemberExpression(node) {
const currentClass = classStack.peek();

if (!currentClass || !currentClass.isEmberModule) {
return;
}

if (types.isThisExpression(node.object) && types.isIdentifier(node.property)) {
const failedConfig = currentClass.configToCheckFor.find(
(s) => s.propertyName === node.property.name
);

if (failedConfig) {
context.report({
node,
messageId: 'main',
data: {
serviceName: failedConfig.service,
},
fix(fixer) {
const sourceCode = context.getSourceCode();

// service inject is already declared
if (serviceInjectImportName) {
return fixService(fixer, currentClass, serviceInjectImportName, failedConfig);
}

return [
fixer.insertTextBefore(
sourceCode.ast,
"import { inject as service } from '@ember/service';\n"
),
fixService(fixer, currentClass, 'service', failedConfig),
];
},
});
}
}
},
};
},
};
4 changes: 2 additions & 2 deletions lib/utils/ember.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ function isEmberCoreModule(context, node, moduleName) {
if (types.isCallExpression(node)) {
// "classic" class pattern
return isClassicEmberCoreModule(node, moduleName, context.getFilename());
} else if (types.isClassDeclaration(node)) {
} else if (types.isClassDeclaration(node) || node.type === 'ClassExpression') {
// native classes
if (
// class Foo extends Component
Expand Down Expand Up @@ -251,7 +251,7 @@ function isEmberCoreModule(context, node, moduleName) {
} else {
assert(
false,
'Function should only be called on a `CallExpression` (classic class) or `ClassDeclaration` (native class)'
'Function should only be called on a `CallExpression` (classic class) or `ClassDeclaration`/`ClassExpression` (native class)'
);
}
return false;
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
"ember-template-imports": "^3.1.1",
"eslint-utils": "^3.0.0",
"estraverse": "^5.2.0",
"lodash.camelcase": "^4.1.1",
"lodash.kebabcase": "^4.1.1",
"magic-string": "^0.27.0",
"requireindex": "^1.2.0",
Expand Down
Loading

0 comments on commit 6cc7860

Please sign in to comment.