Skip to content

Commit

Permalink
Merge pull request #827 from bmish/no-restricted-service-injections
Browse files Browse the repository at this point in the history
Add new rule `no-restricted-service-injections`
  • Loading branch information
bmish authored Jun 2, 2020
2 parents 641e313 + d462a81 commit b7a216e
Show file tree
Hide file tree
Showing 7 changed files with 376 additions and 2 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ Rules are grouped by category to help you understand their purpose. Each rule ha
| | [named-functions-in-promises](./docs/rules/named-functions-in-promises.md) | enforce usage of named functions in promises |
| :white_check_mark: | [no-incorrect-calls-with-inline-anonymous-functions](./docs/rules/no-incorrect-calls-with-inline-anonymous-functions.md) | disallow inline anonymous functions as arguments to `debounce`, `once`, and `scheduleOnce` |
| :white_check_mark: | [no-invalid-debug-function-arguments](./docs/rules/no-invalid-debug-function-arguments.md) | disallow usages of Ember's `assert()` / `warn()` / `deprecate()` functions that have the arguments passed in the wrong order. |
| | [no-restricted-service-injections](./docs/rules/no-restricted-service-injections.md) | disallow injecting certain services under certain paths |

### Routes

Expand Down
53 changes: 53 additions & 0 deletions docs/rules/no-restricted-service-injections.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# no-restricted-service-injections

In some parts of your application, you may prefer to disallow certain services from being injected. This can be useful for:

* Deprecating services one folder at a time
* Creating isolation between different parts of your application

## Rule Details

This rule disallows injecting specified services under specified paths.

## Examples

With this example configuration:

```json
[
"error",
{
"paths": ["folder1", "folder2", "folder3"],
"services": ["deprecated-service"],
"error": "Please stop using this service as it is in the process of being deprecated",
},
{
"paths": ["isolated-folder"],
"services": ["service-disallowed-for-use-in-isolated-folder"],
},
{
"services": ["service-disallowed-anywhere"],
},
]
```

This would be disallowed:

```js
// folder1/my-component.js

class MyComponent extends Component {
@service deprecatedService;
}
```

## Configuration

* object[] -- containing the following properties:
* string[] -- `services` -- list of (kebab-case) service names that should be disallowed from being injected under the specified paths
* string[] -- `paths` -- optional list of regexp file paths that injecting the specified services should be disallowed under (omit this field to match any path)
* string -- `error` -- optional custom error message to display for violations

## Related Rules

* The [no-restricted-imports](https://eslint.org/docs/rules/no-restricted-imports) or [import/no-restricted-paths](https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-restricted-paths.md) rules are the JavaScript import statement equivalent of this rule.
1 change: 1 addition & 0 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ module.exports = {
'no-proxies': require('./rules/no-proxies'),
'no-replace-test-comments': require('./rules/no-replace-test-comments'),
'no-restricted-resolver-tests': require('./rules/no-restricted-resolver-tests'),
'no-restricted-service-injections': require('./rules/no-restricted-service-injections'),
'no-side-effects': require('./rules/no-side-effects'),
'no-test-and-then': require('./rules/no-test-and-then'),
'no-test-import-export': require('./rules/no-test-import-export'),
Expand Down
149 changes: 149 additions & 0 deletions lib/rules/no-restricted-service-injections.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
'use strict';

const kebabCase = require('lodash.kebabcase');
const assert = require('assert');
const emberUtils = require('../utils/ember');

const DEFAULT_ERROR_MESSAGE = 'Injecting this service is not allowed from this file.';

module.exports = {
meta: {
type: 'suggestion',
docs: {
description: 'disallow injecting certain services under certain paths',
category: 'Miscellaneous',
url:
'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/no-restricted-service-injections.md',
},
schema: {
type: 'array',
minItems: 1,
items: [
{
type: 'object',
required: ['services'],
properties: {
paths: {
type: 'array',
minItems: 1,
items: {
type: 'string',
},
},
services: {
type: 'array',
minItems: 1,
items: {
type: 'string',
},
},
error: {
type: 'string',
},
},
additionalProperties: false,
},
],
},
},

DEFAULT_ERROR_MESSAGE,

create(context) {
// Validate options.
context.options.forEach((option) =>
option.services.forEach((service) =>
assert(
service.toLowerCase() === service,
'Service name should be passed in kebab-case (all lower case)'
)
)
);

// Find matching blacklist entries for this file path.
const blacklists = context.options.filter(
(option) => !option.paths || option.paths.some((path) => context.getFilename().match(path))
);

if (blacklists.length === 0) {
return {};
}

function checkForViolationAndReport(node, serviceName) {
const serviceNameKebabCase = serviceName.split('/').map(kebabCase).join('/'); // splitting is used to avoid converting folder/ to folder-

blacklists.forEach((blacklist) => {
// Blacklist services are always passed in in kebab-case, so we can do a kebab-case comparison.
if (blacklist.services.includes(serviceNameKebabCase)) {
context.report({
node,
message: blacklist.error || DEFAULT_ERROR_MESSAGE,
});
}
});
}

return {
// Handles:
// * myService: service()
// * propertyName: service('myService')
Property(node) {
if (!emberUtils.isInjectedServiceProp(node)) {
return;
}

const callExpression = node.value;

// Get the service name either from the string argument or from the property name.
if (callExpression.arguments && callExpression.arguments.length >= 1) {
if (
callExpression.arguments[0].type === 'Literal' &&
typeof callExpression.arguments[0].value === 'string'
) {
// The service name is the string argument.
checkForViolationAndReport(node, callExpression.arguments[0].value);
} else {
// Ignore this case since the argument is not a string.
}
} else {
// The service name is the property name.
checkForViolationAndReport(node, node.key.name);
}
},

// Handles:
// * @service myService
// * @service() myService
// * @service('myService') propertyName
ClassProperty(node) {
if (!emberUtils.isInjectedServiceProp(node)) {
return;
}

// Find the service decorator.
const serviceDecorator =
node.decorators &&
node.decorators.find(
(decorator) =>
(decorator.expression.type === 'Identifier' &&
decorator.expression.name === 'service') ||
(decorator.expression.type === 'CallExpression' &&
decorator.expression.callee.type === 'Identifier' &&
decorator.expression.callee.name === 'service')
);

// Get the service name either from the string argument or from the property name.
const serviceName =
serviceDecorator.expression.type === 'CallExpression' &&
serviceDecorator.expression.arguments &&
serviceDecorator.expression.arguments.length === 1 &&
serviceDecorator.expression.arguments[0].type === 'Literal' &&
typeof serviceDecorator.expression.arguments[0].value === 'string'
? serviceDecorator.expression.arguments[0].value
: node.key.name;

checkForViolationAndReport(node, serviceName);
},
};
},
};
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
"dependencies": {
"@ember-data/rfc395-data": "^0.0.4",
"ember-rfc176-data": "^0.3.13",
"lodash.kebabcase": "^4.1.1",
"snake-case": "^3.0.3"
},
"changelog": {
Expand Down
169 changes: 169 additions & 0 deletions tests/lib/rules/no-restricted-service-injections.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
const RuleTester = require('eslint').RuleTester;
const rule = require('../../../lib/rules/no-restricted-service-injections');

const { DEFAULT_ERROR_MESSAGE } = rule;

const ruleTester = new RuleTester({
parser: require.resolve('babel-eslint'),
parserOptions: {
ecmaVersion: 2015,
sourceType: 'module',
},
});

ruleTester.run('no-restricted-service-injections', rule, {
valid: [
{
// Service name doesn't match (with property name):
code: 'Component.extend({ myService: service() })',
options: [{ paths: ['app/components'], services: ['abc'] }],
filename: 'app/components/path.js',
},
{
// Service name doesn't match (with string argument):
code: "Component.extend({ randomName: service('myService') })",
options: [{ paths: ['app/components'], services: ['abc'] }],
filename: 'app/components/path.js',
},
{
// Service name doesn't match (with decorator)
code: "class MyComponent extends Component { @service('myService') randomName }",
options: [{ paths: ['app/components'], services: ['abc'] }],
filename: 'app/components/path.js',
},
{
// Service scope doesn't match:
code: "Component.extend({ randomName: service('scope/myService') })",
options: [{ paths: ['app/components'], services: ['my-service'] }],
filename: 'app/components/path.js',
},
{
// File path doesn't match:
code: 'Component.extend({ myService: service() })',
options: [{ paths: ['other/path'], services: ['my-service'] }],
filename: 'app/components/path.js',
},
{
// Not the service decorator:
code: 'Component.extend({ myService: otherDecorator() })',
options: [{ paths: ['app/components'], services: ['my-service'] }],
filename: 'app/components/path.js',
},
{
// Ignores injection due to dynamic variable usage:
code: 'Component.extend({ myService: service(SOME_VARIABLE) })',
options: [{ paths: ['app/components'], services: ['my-service'] }],
filename: 'app/components/path.js',
},
],
invalid: [
{
// Without service name argument:
code: 'Component.extend({ myService: service() })',
options: [{ paths: ['app/components'], services: ['my-service'] }],
output: null,
filename: 'app/components/path.js',
errors: [{ message: DEFAULT_ERROR_MESSAGE, type: 'Property' }],
},
{
// With camelized service name argument:
code: "Component.extend({ randomName: service('myService') })",
options: [{ paths: ['app/components'], services: ['my-service'] }],
output: null,
filename: 'app/components/path.js',
errors: [{ message: DEFAULT_ERROR_MESSAGE, type: 'Property' }],
},
{
// With dasherized service name argument:
code: "Component.extend({ randomName: service('my-service') })",
options: [{ paths: ['app/components'], services: ['my-service'] }],
output: null,
filename: 'app/components/path.js',
errors: [{ message: DEFAULT_ERROR_MESSAGE, type: 'Property' }],
},
{
// With nested, camelized service name:
code: "Component.extend({ randomName: service('scope/myService') })",
options: [{ paths: ['app/components'], services: ['scope/my-service'] }],
output: null,
filename: 'app/components/path.js',
errors: [{ message: DEFAULT_ERROR_MESSAGE, type: 'Property' }],
},
{
// With nested, dasherized service name:
code: "Component.extend({ randomName: service('scope/my-service') })",
options: [{ paths: ['app/components'], services: ['scope/my-service'] }],
output: null,
filename: 'app/components/path.js',
errors: [{ message: DEFAULT_ERROR_MESSAGE, type: 'Property' }],
},
{
// With decorator with camelized service name argument:
code: "class MyComponent extends Component { @service('myService') randomName }",
options: [{ paths: ['app/components'], services: ['my-service'] }],
output: null,
filename: 'app/components/path.js',
errors: [{ message: DEFAULT_ERROR_MESSAGE, type: 'ClassProperty' }],
},
{
// With decorator with dasherized service name argument:
code: "class MyComponent extends Component { @service('my-service') randomName }",
options: [{ paths: ['app/components'], services: ['my-service'] }],
output: null,
filename: 'app/components/path.js',
errors: [{ message: DEFAULT_ERROR_MESSAGE, type: 'ClassProperty' }],
},
{
// With decorator without service name argument (without parentheses):
code: 'class MyComponent extends Component { @service myService }',
options: [{ paths: ['app/components'], services: ['my-service'] }],
output: null,
filename: 'app/components/path.js',
errors: [{ message: DEFAULT_ERROR_MESSAGE, type: 'ClassProperty' }],
},
{
// With decorator without service name argument (with parentheses):
code: 'class MyComponent extends Component { @service() myService }',
options: [{ paths: ['app/components'], services: ['my-service'] }],
output: null,
filename: 'app/components/path.js',
errors: [{ message: DEFAULT_ERROR_MESSAGE, type: 'ClassProperty' }],
},
{
// With custom error message:
code: 'Component.extend({ myService: service() })',
options: [
{
paths: ['app/components'],
services: ['my-service'],
error: 'my-service is deprecated, please do not use it.',
},
],
output: null,
filename: 'app/components/path.js',
errors: [{ message: 'my-service is deprecated, please do not use it.', type: 'Property' }],
},
{
// With multiple violations:
code: 'Component.extend({ myService: service() })',
options: [
{ paths: ['app/components'], services: ['my-service'], error: 'Error 1' },
{ paths: ['app/components'], services: ['my-service'], error: 'Error 2' },
],
output: null,
filename: 'app/components/path.js',
errors: [
{ message: 'Error 1', type: 'Property' },
{ message: 'Error 2', type: 'Property' },
],
},
{
// Without specifying any paths (should match any path):
code: 'Component.extend({ myService: service() })',
options: [{ services: ['my-service'] }],
output: null,
filename: 'app/components/path.js',
errors: [{ message: DEFAULT_ERROR_MESSAGE, type: 'Property' }],
},
],
});
Loading

0 comments on commit b7a216e

Please sign in to comment.