Merge pull request #855 from bmish/no-assignment-of-computed-property…

…-dependencies

Add new rule `no-assignment-of-untracked-properties-used-in-tracking-contexts`

README.md

@@ -76,6 +76,7 @@ Rules are grouped by category to help you understand their purpose. Each rule ha
 |:---|:--------|:------------|
 |  | [computed-property-getters](./docs/rules/computed-property-getters.md) | enforce the consistent use of getters in computed properties |
 | :white_check_mark: | [no-arrow-function-computed-properties](./docs/rules/no-arrow-function-computed-properties.md) | disallow arrow functions in computed properties |
+| :wrench: | [no-assignment-of-untracked-properties-used-in-tracking-contexts](./docs/rules/no-assignment-of-untracked-properties-used-in-tracking-contexts.md) | disallow assignment of untracked properties that are used as computed property dependencies |
 | :car: | [no-computed-properties-in-native-classes](./docs/rules/no-computed-properties-in-native-classes.md) | disallow using computed properties in native classes |
 | :white_check_mark: | [no-deeply-nested-dependent-keys-with-each](./docs/rules/no-deeply-nested-dependent-keys-with-each.md) | disallow usage of deeply-nested computed property dependent keys with `@each` |
 | :white_check_mark::wrench: | [no-duplicate-dependent-keys](./docs/rules/no-duplicate-dependent-keys.md) | disallow repeating computed property dependent keys |

docs/rules/no-assignment-of-untracked-properties-used-in-tracking-contexts.md

@@ -0,0 +1,76 @@
+# no-assignment-of-untracked-properties-used-in-tracking-contexts
+
+: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.
+
+Ember 3.13 added an assertion that fires when using assignment `this.x = 123` on an untracked property that is used in a tracking context such as a computed property.
+
+> You attempted to update "propertyX" to "valueY",
+but it is being tracked by a tracking context, such as a template, computed property, or observer.
+>
+> In order to make sure the context updates properly, you must invalidate the property when updating it.
+>
+> You can mark the property as `@tracked`, or use `@ember/object#set` to do this.
+
+## Rule Details
+
+This rule catches assignments of untracked properties that are used as computed property dependency keys.
+
+## Examples
+
+Examples of **incorrect** code for this rule:
+
+```js
+import { computed } from '@ember/object';
+import Component from '@ember/component';
+
+class MyComponent extends Component {
+  @computed('x') get myProp() {
+    return this.x;
+  }
+  myFunction() {
+    this.x = 123; // Not okay to use assignment here.
+  }
+}
+```
+
+Examples of **correct** code for this rule:
+
+```js
+import { computed, set } from '@ember/object';
+import Component from '@ember/component';
+
+class MyComponent extends Component {
+  @computed('x') get myProp() {
+    return this.x;
+  }
+  myFunction() {
+    set(this, 'x', 123); // Okay because it uses set.
+  }
+}
+```
+
+```js
+import { computed, set } from '@ember/object';
+import Component from '@ember/component';
+import { tracked } from '@glimmer/tracking';
+
+class MyComponent extends Component {
+  @tracked x;
+  @computed('x') get myProp() {
+    return this.x;
+  }
+  myFunction() {
+    this.x = 123; // Okay because `x` is a tracked property.
+  }
+}
+```
+
+## Migration
+
+The autofixer for this rule will update assignments to use `set`. Alternatively, you can begin using tracked properties.
+
+## References
+
+* [Spec](https://api.emberjs.com/ember/release/functions/@ember%2Fobject/set) for `set()`
+* [Spec](https://api.emberjs.com/ember/3.16/functions/@glimmer%2Ftracking/tracked) for `@tracked`
+* [Guide](https://guides.emberjs.com/release/upgrading/current-edition/tracked-properties/) for tracked properties

lib/index.js

@@ -14,6 +14,7 @@ module.exports = {
     'new-module-imports': require('./rules/new-module-imports'),
     'no-actions-hash': require('./rules/no-actions-hash'),
     'no-arrow-function-computed-properties': require('./rules/no-arrow-function-computed-properties'),
+    'no-assignment-of-untracked-properties-used-in-tracking-contexts': require('./rules/no-assignment-of-untracked-properties-used-in-tracking-contexts'),
     'no-attrs-in-components': require('./rules/no-attrs-in-components'),
     'no-attrs-snapshot': require('./rules/no-attrs-snapshot'),
     'no-capital-letters-in-routes': require('./rules/no-capital-letters-in-routes'),

lib/rules/no-assignment-of-untracked-properties-used-in-tracking-contexts.js

@@ -0,0 +1,276 @@
+'use strict';
+
+const emberUtils = require('../utils/ember');
+const types = require('../utils/types');
+const decoratorUtils = require('../utils/decorators');
+const javascriptUtils = require('../utils/javascript');
+const propertySetterUtils = require('../utils/property-setter');
+const assert = require('assert');
+const { getImportIdentifier } = require('../utils/import');
+const {
+  expandKeys,
+  keyExistsAsPrefixInList,
+} = require('../utils/computed-property-dependent-keys');
+
+const ERROR_MESSAGE =
+  "Use `set(this, 'propertyName', 'value')` instead of assignment for untracked properties that are used as computed property dependencies (or convert to using tracked properties).";
+
+/**
+ * Gets the list of string dependent keys from a computed property.
+ *
+ * @param {Node} node - the computed property node
+ * @returns {String[]} - the list of string dependent keys from this computed property
+ */
+function getComputedPropertyDependentKeys(node) {
+  if (!node.arguments) {
+    return [];
+  }
+
+  return expandKeys(
+    node.arguments
+      .filter((arg) => arg.type === 'Literal' && typeof arg.value === 'string')
+      .map((node) => node.value)
+  );
+}
+
+/**
+ * Gets a list of computed property dependency keys used inside a class.
+ *
+ * @param {Node} nodeClass - Node for the class
+ * @returns {String[]} - list of dependent keys used inside the class
+ */
+function findComputedPropertyDependentKeys(nodeClass, computedImportName) {
+  if (types.isClassDeclaration(nodeClass)) {
+    // Native JS class.
+    return javascriptUtils.flatMap(nodeClass.body.body, (node) => {
+      const computedDecorator = decoratorUtils.findDecorator(node, computedImportName);
+      if (computedDecorator) {
+        return getComputedPropertyDependentKeys(computedDecorator.expression);
+      } else {
+        return [];
+      }
+    });
+  } else if (types.isCallExpression(nodeClass)) {
+    // Classic class.
+    return javascriptUtils.flatMap(
+      nodeClass.arguments.filter(types.isObjectExpression),
+      (classObject) => {
+        return javascriptUtils.flatMap(classObject.properties, (node) => {
+          if (
+            types.isProperty(node) &&
+            emberUtils.isComputedProp(node.value) &&
+            node.value.arguments
+          ) {
+            return getComputedPropertyDependentKeys(node.value);
+          } else {
+            return [];
+          }
+        });
+      }
+    );
+  } else {
+    assert(false, 'Unexpected node type for a class.');
+  }
+
+  return [];
+}
+
+/**
+ * Gets a list of tracked properties used inside a class.
+ *
+ * @param {Node} nodeClass - Node for the class
+ * @returns {String[]} - list of tracked properties used inside the class
+ */
+function findTrackedProperties(nodeClassDeclaration, trackedImportName) {
+  return nodeClassDeclaration.body.body
+    .filter(
+      (node) =>
+        types.isClassProperty(node) &&
+        decoratorUtils.hasDecorator(node, trackedImportName) &&
+        types.isIdentifier(node.key)
+    )
+    .map((node) => node.key.name);
+}
+
+class Stack {
+  constructor() {
+    this.stack = new Array();
+  }
+  pop() {
+    return this.stack.pop();
+  }
+  push(item) {
+    this.stack.push(item);
+  }
+  peek() {
+    return this.stack.length > 0 ? this.stack[this.stack.length - 1] : undefined;
+  }
+  size() {
+    return this.stack.length;
+  }
+}
+
+module.exports = {
+  meta: {
+    type: 'problem',
+    docs: {
+      description:
+        'disallow assignment of untracked properties that are used as computed property dependencies',
+      category: 'Computed Properties',
+      recommended: false,
+      url:
+        'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/no-assignment-of-untracked-properties-used-in-tracking-contexts.md',
+    },
+    fixable: 'code',
+    schema: [],
+  },
+
+  ERROR_MESSAGE,
+
+  create(context) {
+    if (emberUtils.isTestFile(context.getFilename())) {
+      // This rule does not apply to test files.
+      return {};
+    }
+
+    // State being tracked for this file.
+    let computedImportName = undefined;
+    let trackedImportName = undefined;
+    let setImportName = undefined;
+
+    // State being tracked for the current class we're inside.
+    const classStack = new Stack();
+
+    return {
+      ImportDeclaration(node) {
+        if (node.source.value === '@ember/object') {
+          computedImportName =
+            computedImportName || getImportIdentifier(node, '@ember/object', 'computed');
+          setImportName = setImportName || getImportIdentifier(node, '@ember/object', 'set');
+        } else if (node.source.value === '@glimmer/tracking') {
+          trackedImportName =
+            trackedImportName || getImportIdentifier(node, '@glimmer/tracking', 'tracked');
+        }
+      },
+
+      // Native JS class:
+      ClassDeclaration(node) {
+        // Gather computed property dependent keys from this class.
+        const computedPropertyDependentKeys = new Set(
+          findComputedPropertyDependentKeys(node, computedImportName)
+        );
+
+        // Gather tracked properties from this class.
+        const trackedProperties = new Set(findTrackedProperties(node, trackedImportName));
+
+        // Keep track of whether we're inside a Glimmer component.
+        const isGlimmerComponent = emberUtils.isGlimmerComponent(context, node);
+
+        classStack.push({
+          node,
+          computedPropertyDependentKeys,
+          trackedProperties,
+          isGlimmerComponent,
+        });
+      },
+
+      CallExpression(node) {
+        // Classic class:
+        if (emberUtils.isAnyEmberCoreModule(context, node)) {
+          // Gather computed property dependent keys from this class.
+          const computedPropertyDependentKeys = new Set(
+            findComputedPropertyDependentKeys(node, computedImportName)
+          );
+
+          // No tracked properties in classic classes.
+          const trackedProperties = new Set();
+
+          // Keep track of whether we're inside a Glimmer component.
+          const isGlimmerComponent = emberUtils.isGlimmerComponent(context, node);
+
+          classStack.push({
+            node,
+            computedPropertyDependentKeys,
+            trackedProperties,
+            isGlimmerComponent,
+          });
+        }
+      },
+
+      'ClassDeclaration:exit'(node) {
+        if (classStack.size() > 0 && classStack.peek().node === node) {
+          // Leaving current (native) class.
+          classStack.pop();
+        }
+      },
+
+      'CallExpression:exit'(node) {
+        if (classStack.size() > 0 && classStack.peek().node === node) {
+          // Leaving current (classic) class.
+          classStack.pop();
+        }
+      },
+
+      AssignmentExpression(node) {
+        if (classStack.size() === 0) {
+          // Not inside a class.
+          return;
+        }
+
+        // Ensure this is an assignment with `this.x = ` or `this.x.y = `.
+        if (!propertySetterUtils.isThisSet(node)) {
+          return;
+        }
+
+        const currentClass = classStack.peek();
+
+        const sourceCode = context.getSourceCode();
+        const nodeTextLeft = sourceCode.getText(node.left);
+        const nodeTextRight = sourceCode.getText(node.right);
+        const propertyName = nodeTextLeft.replace('this.', '');
+
+        if (currentClass.isGlimmerComponent && propertyName.startsWith('args.')) {
+          // The Glimmer component args hash is automatically tracked so ignored it.
+          return;
+        }
+
+        if (
+          !currentClass.computedPropertyDependentKeys.has(propertyName) &&
+          !keyExistsAsPrefixInList(
+            [...currentClass.computedPropertyDependentKeys.keys()],
+            propertyName
+          )
+        ) {
+          // Haven't seen this property as a computed property dependent key so ignore it.
+          return;
+        }
+
+        if (currentClass.trackedProperties.has(propertyName)) {
+          // Assignment is fine with tracked properties so ignore it.
+          return;
+        }
+
+        context.report({
+          node,
+          message: ERROR_MESSAGE,
+          fix(fixer) {
+            if (setImportName) {
+              // `set` is already imported.
+              return fixer.replaceText(
+                node,
+                `${setImportName}(this, '${propertyName}', ${nodeTextRight})`
+              );
+            } else {
+              // Need to add an import statement for `set`.
+              const sourceCode = context.getSourceCode();
+              return [
+                fixer.insertTextBefore(sourceCode.ast, "import { set } from '@ember/object';\n"),
+                fixer.replaceText(node, `set(this, '${propertyName}', ${nodeTextRight})`),
+              ];
+            }
+          },
+        });
+      },
+    };
+  },
+};