Merge pull request #1194 from bmish/no-restricted-property-modifications

bmish · web-flow · commit 353dd371ca1c · 2021-05-30T14:25:44.000-04:00
Add new rule `no-restricted-property-modifications`
diff --git a/README.md b/README.md
@@ -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
diff --git a/docs/rules/no-restricted-property-modifications.md b/docs/rules/no-restricted-property-modifications.md
@@ -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`.
diff --git a/lib/rules/no-restricted-property-modifications.js b/lib/rules/no-restricted-property-modifications.js
@@ -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}.`))
+  );
+}
diff --git a/tests/lib/rules/no-restricted-property-modifications.js b/tests/lib/rules/no-restricted-property-modifications.js
