Merge pull request #883 from bmish/no-assignment-of-untracked-properties-used-in-tracking-contexts-macros

Handle computed property macros in `no-assignment-of-untracked-properties-used-in-tracking-contexts` rule

Author: bmish

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

@@ -3,93 +3,34 @@
 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 { getMacros } = require('../utils/computed-property-macros');
 const {
-  expandKeys,
+  findComputedPropertyDependentKeys,
   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.
+ * Gets a set of tracked properties 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
+ * @returns {Set<string>} - set 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);
+  return new Set(
+    nodeClassDeclaration.body.body
+      .filter(
+        (node) =>
+          types.isClassProperty(node) &&
+          decoratorUtils.hasDecorator(node, trackedImportName) &&
+          types.isIdentifier(node.key)
+      )
+      .map((node) => node.key.name)
+  );
 }
 
 class Stack {
@@ -137,6 +78,7 @@ module.exports = {
     let computedImportName = undefined;
     let trackedImportName = undefined;
     let setImportName = undefined;
+    let macroImportNames = new Map();
 
     // State being tracked for the current class we're inside.
     const classStack = new Stack();
@@ -147,6 +89,14 @@ module.exports = {
           computedImportName =
             computedImportName || getImportIdentifier(node, '@ember/object', 'computed');
           setImportName = setImportName || getImportIdentifier(node, '@ember/object', 'set');
+        } else if (node.source.value === '@ember/object/computed') {
+          macroImportNames = new Map(
+            getMacros().map((macro) => [
+              macro,
+              macroImportNames.get(macro) ||
+                getImportIdentifier(node, '@ember/object/computed', macro),
+            ])
+          );
         } else if (node.source.value === '@glimmer/tracking') {
           trackedImportName =
             trackedImportName || getImportIdentifier(node, '@glimmer/tracking', 'tracked');
@@ -156,12 +106,14 @@ module.exports = {
       // Native JS class:
       ClassDeclaration(node) {
         // Gather computed property dependent keys from this class.
-        const computedPropertyDependentKeys = new Set(
-          findComputedPropertyDependentKeys(node, computedImportName)
+        const computedPropertyDependentKeys = findComputedPropertyDependentKeys(
+          node,
+          computedImportName,
+          macroImportNames
         );
 
         // Gather tracked properties from this class.
-        const trackedProperties = new Set(findTrackedProperties(node, trackedImportName));
+        const trackedProperties = findTrackedProperties(node, trackedImportName);
 
         // Keep track of whether we're inside a Glimmer component.
         const isGlimmerComponent = emberUtils.isGlimmerComponent(context, node);
@@ -178,8 +130,10 @@ module.exports = {
         // Classic class:
         if (emberUtils.isAnyEmberCoreModule(context, node)) {
           // Gather computed property dependent keys from this class.
-          const computedPropertyDependentKeys = new Set(
-            findComputedPropertyDependentKeys(node, computedImportName)
+          const computedPropertyDependentKeys = findComputedPropertyDependentKeys(
+            node,
+            computedImportName,
+            macroImportNames
           );
 
           // No tracked properties in classic classes.

lib/utils/computed-property-dependent-keys.js

@@ -1,11 +1,22 @@
 'use strict';
 
 const javascriptUtils = require('./javascript');
+const { getName } = require('../utils/utils');
+const types = require('../utils/types');
+const decoratorUtils = require('../utils/decorators');
+const assert = require('assert');
+const {
+  getMacrosFromImportNames,
+  getTrackedArgumentCount,
+  macroToCanonicalName,
+} = require('../utils/computed-property-macros');
 
 module.exports = {
   collapseKeys,
   expandKeys,
   expandKey,
+  findComputedPropertyDependentKeys,
+  getComputedPropertyDependentKeys,
   computedPropertyDependencyMatchesKeyPath,
   keyExistsAsPrefixInList,
 };
@@ -161,3 +172,99 @@ function computedPropertyDependencyMatchesKeyPath(dependency, keyPath) {
 function keyExistsAsPrefixInList(keys, key) {
   return keys.some((currentKey) => computedPropertyDependencyMatchesKeyPath(currentKey, key));
 }
+
+/**
+ * Gets the set of computed property dependency keys used inside a class.
+ *
+ * @param {Node} nodeClass - Node for the class
+ * @returns {Set<string>} - set of dependent keys used inside the class
+ */
+function findComputedPropertyDependentKeys(nodeClass, computedImportName, macroImportNames) {
+  if (types.isClassDeclaration(nodeClass)) {
+    // Native JS class.
+    return new Set(
+      javascriptUtils.flatMap(nodeClass.body.body, (node) => {
+        // Check for `computed` itself.
+        const computedDecorator = decoratorUtils.findDecorator(node, computedImportName);
+        if (computedDecorator) {
+          return getComputedPropertyDependentKeys(computedDecorator.expression);
+        }
+
+        // Check for a computed macro.
+        const macroNames = getMacrosFromImportNames(computedImportName, macroImportNames);
+        const macroDecorator = decoratorUtils.findDecoratorByNameCallback(node, (decoratorName) =>
+          macroNames.has(decoratorName)
+        );
+        if (macroDecorator) {
+          return getComputedPropertyDependentKeys(
+            macroDecorator.expression,
+            getTrackedArgumentCount(
+              macroToCanonicalName(
+                decoratorUtils.getDecoratorName(macroDecorator),
+                macroImportNames
+              )
+            )
+          );
+        }
+
+        return [];
+      })
+    );
+  } else if (types.isCallExpression(nodeClass)) {
+    // Classic class.
+    return new Set(
+      javascriptUtils.flatMap(
+        nodeClass.arguments.filter(types.isObjectExpression),
+        (classObject) => {
+          return javascriptUtils.flatMap(classObject.properties, (node) => {
+            if (types.isProperty(node)) {
+              const name = getName(node.value);
+              if (!name) {
+                return [];
+              }
+
+              // Check for `computed` itself.
+              if (name === computedImportName) {
+                return getComputedPropertyDependentKeys(node.value);
+              }
+
+              // Check for a computed macro.
+              const macroNames = getMacrosFromImportNames(computedImportName, macroImportNames);
+              if (macroNames.has(name)) {
+                return getComputedPropertyDependentKeys(
+                  node.value,
+                  getTrackedArgumentCount(macroToCanonicalName(name, macroImportNames))
+                );
+              }
+            }
+            return [];
+          });
+        }
+      )
+    );
+  } else {
+    assert(false, 'Unexpected node type for a class.');
+  }
+
+  return new Set();
+}
+
+/**
+ * Gets the list of string dependent keys from a computed property.
+ *
+ * @param {Node} node - the computed property node
+ * @param {number} dependentKeyArgumentCount - number of arguments to check for dependent keys
+ * @returns {String[]} - the list of string dependent keys from this computed property
+ */
+function getComputedPropertyDependentKeys(node, dependentKeyArgumentCount = Number.MAX_VALUE) {
+  if (!node.arguments) {
+    return [];
+  }
+
+  return expandKeys(
+    node.arguments
+      .slice(0, dependentKeyArgumentCount)
+      .filter((arg) => arg.type === 'Literal' && typeof arg.value === 'string')
+      .map((node) => node.value)
+  );
+}

lib/utils/computed-property-macros.js

@@ -0,0 +1,94 @@
+const assert = require('assert');
+
+module.exports = {
+  getMacros,
+  getMacrosFromImportNames,
+  getTrackedArgumentCount,
+  macroToCanonicalName,
+};
+
+/**
+ * Example macros:
+ * - and('x', 'y', 'z') can have any number of tracked dependent key arguments
+ * - mapBy('chores', 'done', true) only has a single tracked dependent key argument
+ */
+const MACROS_TO_TRACKED_ARGUMENT_COUNT = {
+  alias: 1,
+  and: Number.MAX_VALUE,
+  bool: 1,
+  collect: Number.MAX_VALUE,
+  deprecatingAlias: 1,
+  empty: 1,
+  equal: 1,
+  filter: 1,
+  filterBy: 1,
+  gt: 1,
+  gte: 1,
+  intersect: Number.MAX_VALUE,
+  lt: 1,
+  lte: 1,
+  map: 1,
+  mapBy: 1,
+  match: 1,
+  max: 1,
+  min: 1,
+  none: 1,
+  not: 1,
+  notEmpty: 1,
+  oneWay: 1,
+  or: Number.MAX_VALUE,
+  readOnly: 1,
+  reads: 1,
+  setDiff: 2,
+  sort: 1,
+  sum: Number.MAX_VALUE,
+  union: Number.MAX_VALUE,
+  uniq: 1,
+  uniqBy: 1,
+};
+
+/**
+ * @returns {string[]} - a list of all macros.
+ */
+function getMacros() {
+  return Object.keys(MACROS_TO_TRACKED_ARGUMENT_COUNT);
+}
+
+/**
+ * Example of return value: ['and', 'readOnly', 'computed.and', 'computed.readOnly', ...]
+ *
+ * @param {string} computedImportName - name that computed is imported under
+ * @param {set<string>} macroImportNames
+ * @returns {set<string>} - a set containing all possible macro names to check for.
+ */
+function getMacrosFromImportNames(computedImportName, macroImportNames) {
+  return new Set([
+    ...macroImportNames.values(),
+    ...(computedImportName ? getMacros().map((macro) => `${computedImportName}.${macro}`) : []),
+  ]);
+}
+
+/**
+ * @param {string} macro
+ * @returns {number} - the number arguments that are tracked dependent keys for a given macro.
+ */
+function getTrackedArgumentCount(macro) {
+  assert(typeof macro === 'string', 'macro parameter should be a string');
+
+  return MACROS_TO_TRACKED_ARGUMENT_COUNT[macro];
+}
+
+/**
+ * @param {string} macro - imported macro name
+ * @param {map<string,string>} macroImportNames
+ * @returns {string} the original name of the imported macro
+ */
+function macroToCanonicalName(macro, macroImportNames) {
+  assert(typeof macro === 'string', 'macro parameter should be a string');
+
+  return macro.includes('.')
+    ? macro.slice(macro.lastIndexOf('.') + 1) // Removes the leading `computed.`
+    : [...macroImportNames.keys()].find(
+        (canonicalName) => macroImportNames.get(canonicalName) === macro
+      );
+}