Skip to content

Commit

Permalink
Merge pull request #1194 from bmish/no-restricted-property-modifications
Browse files Browse the repository at this point in the history
Add new rule `no-restricted-property-modifications`
  • Loading branch information
bmish authored May 30, 2021
2 parents 1f69e6a + f04a47f commit 353dd37
Show file tree
Hide file tree
Showing 4 changed files with 449 additions and 0 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ Rules are grouped by category to help you understand their purpose. Each rule ha
| [no-html-safe](./docs/rules/no-html-safe.md) | disallow the use of `htmlSafe` | | |
| [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. | :white_check_mark: | |
| [no-restricted-property-modifications](./docs/rules/no-restricted-property-modifications.md) | disallow modifying the specified properties | | :wrench: |
| [require-fetch-import](./docs/rules/require-fetch-import.md) | enforce explicit import for `fetch()` | | |

### Routes
Expand Down
60 changes: 60 additions & 0 deletions docs/rules/no-restricted-property-modifications.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
# no-restricted-property-modifications

:wrench: The `--fix` option on the [command line](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems) can automatically fix some of the problems reported by this rule.

There are some properties, especially globally-injected ones, that you may want to treat as read-only, and ensure that no one modifies them.

## Rule Details

This rule prevents modifying the specified properties.

It also disallows using computed property macros like `alias` and `reads` that enable the specified properties to be indirectly modified.

## Examples

All examples assume a configuration of `properties: ['currentUser']`.

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

```js
import Component from '@ember/component';
import { alias, reads } from '@ember/object/computed';

export default class MyComponent extends Component {
@alias('currentUser') aliasForCurrentUser1; // Not allowed
@reads('currentUser') aliasForCurrentUser2; // Not allowed

@alias('currentUser.somePermission1') somePermission1; // Not allowed
@reads('currentUser.somePermission2') somePermission2; // Not allowed

myFunction() {
this.set('currentUser', {}); // Not allowed
this.set('currentUser.somePermission', true); // Not allowed

this.currentUser = {}; // Not allowed
this.currentUser.somePermission = true; // Not allowed
}
}
```

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

```js
import Component from '@ember/component';
import { readOnly } from '@ember/object/computed';

export default class MyComponent extends Component {
@readOnly('currentUser.somePermission') somePermission; // Allowed

myFunction() {
console.log(this.currentUser.somePermission); // Allowed
}
}
```

## Configuration

* object -- containing the following properties:
* `String[]` -- `properties` -- array of names of properties that should not be modified (modifying child/nested/sub-properties of these is also not allowed)

Not yet implemented: There is currently no way to configure whether sub-properties are restricted from modification. To make this configurable, the `properties` array option could be updated to also accept objects of the form `{ name: 'myPropertyName', includeSubProperties: false }` where `includeSubProperties` defaults to `true`.
180 changes: 180 additions & 0 deletions lib/rules/no-restricted-property-modifications.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
'use strict';

const { isStringLiteral } = require('../utils/types');
const { getImportIdentifier } = require('../utils/import');
const { isThisSet: isThisSetNative } = require('../utils/property-setter');
const { nodeToDependentKey } = require('../utils/property-getter');

module.exports = {
meta: {
type: 'problem',
docs: {
description: 'disallow modifying the specified properties',
category: 'Miscellaneous',
url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/no-restricted-property-modifications.md',
},
fixable: 'code',
schema: {
type: 'array',
minItems: 1,
maxItems: 1,
items: [
{
type: 'object',
properties: {
properties: {
type: 'array',
items: {
type: 'string',
},
minItems: 1,
uniqueItems: true,
},
},
required: ['properties'],
additionalProperties: false,
},
],
},
messages: {
doNotUseAssignment: 'Do not use assignment on properties that should not be modified.',
doNotUseSet: 'Do not call `set` on properties that should not be modified.',
useReadOnlyMacro:
'Use the `readOnly` computed property macro for properties that should not be modified.',
},
},
create(context) {
let importedComputedName;
let importedReadsName;
let importedAliasName;

const readOnlyProperties = context.options[0].properties;

return {
ImportDeclaration(node) {
if (node.source.value === '@ember/object') {
importedComputedName =
importedComputedName || getImportIdentifier(node, '@ember/object', 'computed');
} else if (node.source.value === '@ember/object/computed') {
importedReadsName =
importedReadsName || getImportIdentifier(node, '@ember/object/computed', 'reads');
importedAliasName =
importedAliasName || getImportIdentifier(node, '@ember/object/computed', 'alias');
}
},

AssignmentExpression(node) {
if (!isThisSetNative(node)) {
return;
}

const dependentKey = nodeToDependentKey(node.left, context);
if (
readOnlyProperties.includes(dependentKey) ||
readOnlyProperties.some((property) => dependentKey.startsWith(`${property}.`))
) {
context.report({
node,
messageId: 'doNotUseAssignment',
});
}
},

CallExpression(node) {
if (
isReadOnlyPropertyUsingAliasOrReads(
node,
readOnlyProperties,
importedComputedName,
importedAliasName,
importedReadsName
)
) {
context.report({
node,
messageId: 'useReadOnlyMacro',
fix(fixer) {
const argumentText0 = context.getSourceCode().getText(node.arguments[0]);
return node.callee.type === 'MemberExpression'
? fixer.replaceText(node, `${importedComputedName}.readOnly(${argumentText0})`)
: fixer.replaceText(node, `readOnly(${argumentText0})`);
},
});
} else if (isThisSetReadOnlyProperty(node, readOnlyProperties)) {
context.report({ node, messageId: 'doNotUseSet' });
}
},
};
},
};

function isReadOnlyPropertyUsingAliasOrReads(
node,
readOnlyProperties,
importedComputedName,
importedAliasName,
importedReadsName
) {
// Looks for: reads('readOnlyProperty') or alias('readOnlyProperty')
return (
(isAliasComputedProperty(node, importedComputedName, importedAliasName) ||
isReadsComputedProperty(node, importedComputedName, importedReadsName)) &&
node.arguments.length === 1 &&
isStringLiteral(node.arguments[0]) &&
isReadOnlyProperty(node.arguments[0].value, readOnlyProperties)
);
}

function isThisSet(node) {
// Looks for: this.set('readOnlyProperty...', ...);
return (
node.callee.type === 'MemberExpression' &&
node.callee.object.type === 'ThisExpression' &&
node.callee.property.type === 'Identifier' &&
node.callee.property.name === 'set' &&
node.arguments.length === 2 &&
isStringLiteral(node.arguments[0])
);
}

function isThisSetReadOnlyProperty(node, readOnlyProperties) {
return isThisSet(node) && isReadOnlyProperty(node.arguments[0].value, readOnlyProperties);
}

function isAliasComputedProperty(node, importedComputedName, importedAliasName) {
return (
isIdentifierCall(node, importedAliasName) ||
isMemberExpressionCall(node, importedComputedName, 'alias')
);
}

function isReadsComputedProperty(node, importedComputedName, importedReadsName) {
return (
isIdentifierCall(node, importedReadsName) ||
isMemberExpressionCall(node, importedComputedName, 'reads')
);
}

function isIdentifierCall(node, name) {
return (
node.type === 'CallExpression' && node.callee.type === 'Identifier' && node.callee.name === name
);
}

function isMemberExpressionCall(node, object, name) {
return (
node.type === 'CallExpression' &&
node.callee.type === 'MemberExpression' &&
node.callee.object.type === 'Identifier' &&
node.callee.object.name === object &&
node.callee.property.type === 'Identifier' &&
node.callee.property.name === name
);
}

function isReadOnlyProperty(property, readOnlyProperties) {
return (
readOnlyProperties.includes(property) ||
readOnlyProperties.some((propertyCurrent) => property.startsWith(`${propertyCurrent}.`))
);
}
Loading

0 comments on commit 353dd37

Please sign in to comment.