Add no-modifier-without-element-usage rule

README.md

@@ -297,6 +297,7 @@ To disable a rule for an entire `.gjs`/`.gts` file, use a regular ESLint file-le
 
 | Name                                                                                                               | Description                                                                          | 💼 | 🔧 | 💡 |
 | :----------------------------------------------------------------------------------------------------------------- | :----------------------------------------------------------------------------------- | :- | :- | :- |
+| [no-modifier-without-element-usage](docs/rules/no-modifier-without-element-usage.md)                               | disallow modifiers that never use their element                                      |    |    |    |
 | [template-builtin-component-arguments](docs/rules/template-builtin-component-arguments.md)                         | disallow setting certain attributes on builtin components                            | 📋 |    |    |
 | [template-no-action-modifiers](docs/rules/template-no-action-modifiers.md)                                         | disallow usage of {{action}} modifiers                                               |    | 🔧 |    |
 | [template-no-action-on-submit-button](docs/rules/template-no-action-on-submit-button.md)                           | disallow action attribute on submit buttons                                          | 📋 |    |    |

docs/rules/no-modifier-without-element-usage.md

@@ -0,0 +1,96 @@
+# ember/no-modifier-without-element-usage
+
+<!-- end auto-generated rule header -->
+
+Disallow modifiers that never use their element.
+
+A modifier exists to give an element behavior that only the DOM node can provide: event listeners, focus, measurement, or handing the node to a third-party library. A modifier that ignores its element has historically caused infinite render loops, and lead to confusion.
+
+## Rule Details
+
+This rule reports a modifier whose element is never referenced.
+
+Both modifier function and class styles from `ember-modifier` are checked.
+
+## Examples
+
+Examples of **incorrect** code for this rule:
+
+```js
+import { modifier } from 'ember-modifier';
+
+// The element is never used
+modifier((element, positional) => {
+  trackEvent(positional[0]);
+});
+```
+
+```js
+import { modifier } from 'ember-modifier';
+
+// No element parameter at all
+modifier(() => {
+  trackEvent('rendered');
+});
+```
+
+```js
+import Modifier from 'ember-modifier';
+
+// A class modifier that ignores its element
+export default class Track extends Modifier {
+  modify(element, [name]) {
+    trackEvent(name);
+  }
+}
+```
+
+Examples of **correct** code for this rule:
+
+```js
+import { modifier } from 'ember-modifier';
+
+modifier((element) => {
+  element.focus();
+});
+```
+
+```js
+import { modifier } from 'ember-modifier';
+
+// Passing the element along counts as usage
+modifier((element, positional) => {
+  const chart = new Chart(element, positional[0]);
+
+  return () => chart.destroy();
+});
+```
+
+```js
+import Modifier from 'ember-modifier';
+
+export default class Track extends Modifier {
+  modify(element, [name]) {
+    element.dataset.trackedAs = name;
+  }
+}
+```
+
+## Migration
+
+A modifier that does not use its element usually wants one of these instead:
+
+- derived state, so the value is computed where it is read rather than pushed on render
+- a resource, for behavior with setup and teardown that is not tied to an element
+- an event handler on the element that already triggers the behavior
+
+## Related Rules
+
+- [no-at-ember-render-modifiers](no-at-ember-render-modifiers.md)
+- [template-no-at-ember-render-modifiers](template-no-at-ember-render-modifiers.md)
+- [no-modifier-argument-destructuring](no-modifier-argument-destructuring.md)
+
+## References
+
+- [ember-modifier](https://github.com/ember-modifier/ember-modifier)
+- [Ember Autotracking](https://guides.emberjs.com/release/in-depth-topics/autotracking-in-depth/)

eslint.config.js

@@ -194,6 +194,7 @@ module.exports = [
       'n/no-unsupported-features/es-syntax': 'off',
       'no-console': 'off',
       'no-undef': 'off',
+      'no-param-reassign': 'off',
       'no-unused-expressions': 'off',
       'no-unused-labels': 'off',
       'no-unused-vars': 'off',

lib/rules/no-modifier-without-element-usage.js

@@ -0,0 +1,202 @@
+'use strict';
+
+const { getImportIdentifier } = require('../utils/import');
+
+const ERROR_MESSAGE =
+  'This modifier never uses its element. Modifiers exist to add behavior to an element, so element-free logic belongs somewhere else.';
+
+/**
+ * Unwraps a parameter to the node that binds a name, so that `...element` and
+ * `element = fallback` are treated the same as `element`.
+ */
+function unwrapParam(param) {
+  if (!param) {
+    return null;
+  }
+
+  if (param.type === 'RestElement') {
+    return unwrapParam(param.argument);
+  }
+
+  if (param.type === 'AssignmentPattern') {
+    return unwrapParam(param.left);
+  }
+
+  return param;
+}
+
+function isFunction(node) {
+  return (
+    Boolean(node) &&
+    (node.type === 'ArrowFunctionExpression' ||
+      node.type === 'FunctionExpression' ||
+      node.type === 'FunctionDeclaration')
+  );
+}
+
+/**
+ * A destructuring pattern reads the element to build its bindings, so it counts
+ * as usage without any reference to look up.
+ */
+function isElementUsed(sourceCode, fnNode) {
+  const param = unwrapParam(fnNode.params[0]);
+
+  if (!param) {
+    return false;
+  }
+
+  if (param.type !== 'Identifier') {
+    return true;
+  }
+
+  const variable = sourceCode
+    .getDeclaredVariables(fnNode)
+    .find((candidate) => candidate.defs.some((def) => def.name === param));
+
+  return Boolean(variable) && variable.references.length > 0;
+}
+
+function isThisElement(node) {
+  if (node.object.type !== 'ThisExpression') {
+    return false;
+  }
+
+  return node.computed
+    ? node.property.type === 'Literal' && node.property.value === 'element'
+    : node.property.type === 'Identifier' && node.property.name === 'element';
+}
+
+function isModifyMember(node) {
+  return (
+    (node.type === 'MethodDefinition' || node.type === 'PropertyDefinition') &&
+    !node.static &&
+    !node.computed &&
+    node.key.type === 'Identifier' &&
+    node.key.name === 'modify'
+  );
+}
+
+function getModifyFunction(modifyMember) {
+  if (modifyMember.type === 'MethodDefinition') {
+    return modifyMember.value;
+  }
+
+  return isFunction(modifyMember.value) ? modifyMember.value : null;
+}
+
+/** @type {import('eslint').Rule.RuleModule} */
+module.exports = {
+  meta: {
+    type: 'suggestion',
+    docs: {
+      description: 'disallow modifiers that never use their element',
+      category: 'Best Practices',
+      recommended: false,
+      url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/no-modifier-without-element-usage.md',
+    },
+    fixable: null,
+    schema: [],
+    messages: {
+      main: ERROR_MESSAGE,
+    },
+  },
+
+  ERROR_MESSAGE,
+
+  create(context) {
+    const { sourceCode } = context;
+
+    let functionModifierName;
+    const classModifierNames = new Set();
+    const classStack = [];
+
+    function enterClass(node) {
+      const isModifier =
+        node.superClass?.type === 'Identifier' && classModifierNames.has(node.superClass.name);
+      const modifyMember = isModifier ? node.body.body.find(isModifyMember) : undefined;
+
+      classStack.push({
+        node,
+        isModifier,
+        modifyMember,
+        modifyFn: modifyMember && getModifyFunction(modifyMember),
+        usesThisElement: false,
+      });
+    }
+
+    function exitClass() {
+      const { node, isModifier, modifyMember, modifyFn, usesThisElement } = classStack.pop();
+
+      if (!isModifier || usesThisElement) {
+        return;
+      }
+
+      // `modify` assigned from elsewhere cannot be checked here.
+      if (modifyMember && !modifyFn) {
+        return;
+      }
+
+      if (modifyFn && isElementUsed(sourceCode, modifyFn)) {
+        return;
+      }
+
+      context.report({
+        node: modifyFn?.params[0] ?? modifyMember?.key ?? node.id ?? node.superClass,
+        messageId: 'main',
+      });
+    }
+
+    return {
+      ImportDeclaration(node) {
+        if (node.source.value !== 'ember-modifier') {
+          return;
+        }
+
+        functionModifierName ??= getImportIdentifier(node, 'ember-modifier', 'modifier');
+
+        for (const name of [
+          getImportIdentifier(node, 'ember-modifier'),
+          getImportIdentifier(node, 'ember-modifier', 'ClassBasedModifier'),
+        ]) {
+          if (name) {
+            classModifierNames.add(name);
+          }
+        }
+      },
+
+      CallExpression(node) {
+        if (!functionModifierName) {
+          return;
+        }
+
+        if (node.callee.type !== 'Identifier' || node.callee.name !== functionModifierName) {
+          return;
+        }
+
+        const callback = node.arguments[0];
+
+        if (!isFunction(callback) || isElementUsed(sourceCode, callback)) {
+          return;
+        }
+
+        context.report({
+          node: callback.params[0] ?? node.callee,
+          messageId: 'main',
+        });
+      },
+
+      ClassDeclaration: enterClass,
+      ClassExpression: enterClass,
+      'ClassDeclaration:exit': exitClass,
+      'ClassExpression:exit': exitClass,
+
+      MemberExpression(node) {
+        const classInfo = classStack.at(-1);
+
+        if (classInfo?.isModifier && isThisElement(node)) {
+          classInfo.usesThisElement = true;
+        }
+      },
+    };
+  },
+};