From a4aadc6a2b2d537750ed9717ee02d5136115d1be Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Johan=20R=C3=B8ed?= <johanroed@gmail.com>
Date: Fri, 31 Jul 2026 16:43:21 +0200
Subject: [PATCH] fix(template-no-template-lint-directives): preserve directive
 scope on conversion
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The fixer emitted `eslint-disable` for every `template-lint-disable`, but that
directive carries three different scopes depending on placement, and only one
of them is file-wide. ember-template-lint's docs/configuration.md:

  An in-element instruction will apply to only that element
  An in-element instruction with the `-tree` suffix will apply to that element
  and all its descendants

Converting either in-element form to a file-wide `eslint-disable` widened a
one-element suppression into the rest of the file, so a single migrated
directive silently swallowed every later violation of that rule:

    <div
      {{! template-lint-disable no-positive-tabindex }}
      tabindex="1"
    ></div>
    <span tabindex="2"></span>

ember-template-lint reports the <span>; after `--fix` nothing reported at all.

ESLint has no element scope, but an `eslint-disable` / `eslint-enable` pair
delimits an arbitrary region, which reproduces every template-lint scope
exactly. Closing the region as the element's first child covers the opening tag
alone; closing it after the element covers the subtree. So:

  standing alone   -> eslint-disable (rest of file)
  in opening tag   -> eslint-disable before element, eslint-enable first child
  `-tree` suffix   -> eslint-disable before element, eslint-enable after it

`-tree` directives were previously not matched at all and migrated to nothing
without a report; they now convert.

Removing the converted directive is by line only when the comment has that line
to itself. A directive written inline — `<div {{! template-lint-disable … }}
tabindex="1">` — shares its line with the tag, and reaching back to the line
start would delete `<div ` along with the comment, so the inline form strips
just the comment and the single space it sat in.

The closing comment is inserted without a surrounding newline: `{{! }}`
compiles away, but a newline would add a whitespace text node and can change
inline layout.

Tests assert suppression scope end-to-end — migrate, then lint — against the
scopes ember-template-lint 7.9.3 produces on the same templates, rather than
only checking the rewritten comment text.

The rule doc claimed the file-wide output was correct and that `-tree` had no
ESLint equivalent; the README claimed only two scopes were expressible. Both
are corrected.
---
 README.md                                     |  12 +-
 .../template-no-template-lint-directives.md   |  51 ++++-
 .../template-no-template-lint-directives.js   | 119 ++++++++----
 .../template-no-template-lint-directives.js   | 181 +++++++++++++++++-
 4 files changed, 319 insertions(+), 44 deletions(-)

diff --git a/README.md b/README.md
index a62ffd2983..362ca95a07 100644
--- a/README.md
+++ b/README.md
@@ -206,7 +206,7 @@ or
 
 ### Replacing `template-lint-disable` comments
 
-Inline disable directives need to be rewritten to ESLint's syntax, prefixed with `ember/template-`. For now, only two scopes are supported: the next line, or the rest of the file. For example, replace:
+Inline disable directives need to be rewritten to ESLint's syntax, prefixed with `ember/template-`. ESLint's own scopes are the next line and the rest of the file, but an `eslint-disable` / `eslint-enable` pair delimits an arbitrary region, so every template-lint scope has an exact equivalent. For example, replace:
 
 ```hbs
 {{!template-lint-disable no-invalid-role}}
@@ -218,7 +218,15 @@ with:
 {{!eslint-disable-next-line ember/template-no-invalid-role}}
 ```
 
-The [`template-no-template-lint-directives`](docs/rules/template-no-template-lint-directives.md) rule (enabled by the `template-lint-migration` config) does this rewrite for you: run `eslint --fix` once and it converts every `template-lint-disable` / `template-lint-enable` comment in your templates.
+or, to cover a region rather than one line, bracket it:
+
+```hbs
+{{!eslint-disable ember/template-no-invalid-role}}
+<div role='range'></div>
+{{!eslint-enable ember/template-no-invalid-role}}
+```
+
+The [`template-no-template-lint-directives`](docs/rules/template-no-template-lint-directives.md) rule (enabled by the `template-lint-migration` config) does this rewrite for you: run `eslint --fix` once and it converts every `template-lint-disable` / `template-lint-enable` comment in your templates, including the element-scoped and `-tree` forms, preserving each directive's original scope.
 
 To disable a rule for an entire `.gjs`/`.gts` file, use a regular ESLint file-level directive in the JS region — it applies to the `<template>` contents as well:
 
diff --git a/docs/rules/template-no-template-lint-directives.md b/docs/rules/template-no-template-lint-directives.md
index 13caf9737d..0edba74561 100644
--- a/docs/rules/template-no-template-lint-directives.md
+++ b/docs/rules/template-no-template-lint-directives.md
@@ -15,9 +15,30 @@ The fixer:
 - replaces `template-lint-disable` / `template-lint-enable` with `eslint-disable` / `eslint-enable`;
 - prefixes each rule name with `ember/template-` (the namespace the rules are published under in this plugin);
 - joins multiple rule names with `,` (ESLint's directive syntax) instead of whitespace (template-lint's syntax);
-- when the directive appears inside an element's opening tag (between attributes), lifts it to its own line just before the element. ESLint scopes line-based directives from the line they appear on, and the violation typically lives on the element's start line, so leaving the directive inside the attribute list would put it after the violation it's meant to cover.
+- converts an element-scoped directive into an `eslint-disable` / `eslint-enable` pair bracketing that element (see below).
 
-The `-tree` suffix on a directive (e.g. `template-lint-disable-tree`) does **not** match this rule. ESLint has no equivalent of template-lint's subtree-scoped directives, so they need manual handling rather than a mechanical conversion.
+### Preserving scope
+
+`template-lint-disable` means different things depending on where it sits, and a
+conversion that ignores that either hides violations or floods a migration with
+noise. ESLint has no element scope, but an `eslint-disable` / `eslint-enable`
+pair delimits an arbitrary region, which reproduces every template-lint scope
+exactly:
+
+| `template-lint-disable` placement  | scope                                           | conversion                                                              |
+| ---------------------------------- | ----------------------------------------------- | ----------------------------------------------------------------------- |
+| standing alone                     | comment → end of template                       | `eslint-disable` (rest of file)                                         |
+| paired with `template-lint-enable` | between the two                                 | `eslint-disable` … `eslint-enable`                                      |
+| inside an element's opening tag    | that element's opening tag, not its descendants | `eslint-disable` before the element, `eslint-enable` as its first child |
+| with the `-tree` suffix            | that element and its descendants                | `eslint-disable` before the element, `eslint-enable` after it           |
+
+The closing comment is inserted without a surrounding newline. `{{! }}` comments
+compile away and leave no trace in the DOM, but a newline would add a whitespace
+text node, which can change inline layout.
+
+To suppress a rule across a whole file, use a file-level ESLint directive — in
+`.gjs`/`.gts` a `/* eslint-disable ember/template-… */` in the JS region also
+covers the `<template>` contents.
 
 ## Examples
 
@@ -38,7 +59,15 @@ Examples of **incorrect** code for this rule:
   class='example'
   {{! template-lint-disable no-invalid-interactive }}
   {{on 'click' this.click}}
-></div>
+><span>hi</span></div>
+```
+
+```hbs
+<div
+  class='example'
+  {{! template-lint-disable-tree no-invalid-interactive }}
+  {{on 'click' this.click}}
+><span>hi</span></div>
 ```
 
 Examples of **correct** code for this rule (i.e. what the autofix produces):
@@ -53,9 +82,23 @@ Examples of **correct** code for this rule (i.e. what the autofix produces):
 {{foo bar=baz}}
 ```
 
+The element-scoped form closes its region as the element's first child, leaving
+descendants linted:
+
+```hbs
+{{! eslint-disable ember/template-no-invalid-interactive }}
+<div
+  class='example'
+  {{on 'click' this.click}}
+>{{! eslint-enable ember/template-no-invalid-interactive }}<span>hi</span></div>
+```
+
+`-tree` closes it after the element, covering the subtree:
+
 ```hbs
 {{! eslint-disable ember/template-no-invalid-interactive }}
-<div class='example' {{on 'click' this.click}}></div>
+<div class='example' {{on 'click' this.click}}><span>hi</span></div>
+{{! eslint-enable ember/template-no-invalid-interactive }}
 ```
 
 ## When Not To Use It
diff --git a/lib/rules/template-no-template-lint-directives.js b/lib/rules/template-no-template-lint-directives.js
index 041e6d9cea..93975aca15 100644
--- a/lib/rules/template-no-template-lint-directives.js
+++ b/lib/rules/template-no-template-lint-directives.js
@@ -1,7 +1,7 @@
 'use strict';
 
 const DIRECTIVE_COMMENT =
-  /^(?<open>{{!(?:--)?)\s*template-lint-(?<action>disable|enable)(?<rules>\s+[^]*?)?\s*(?:--)?}}$/;
+  /^(?<open>{{!(?:--)?)\s*template-lint-(?<action>disable|enable)(?<tree>-tree)?(?<rules>\s+[^]*?)?\s*(?:--)?}}$/;
 
 /** @type {import('eslint').Rule.RuleModule} */
 module.exports = {
@@ -17,18 +17,16 @@ module.exports = {
     fixable: 'code',
     schema: [],
     messages: {
-      convert: 'Use `eslint-{{action}}` instead of `template-lint-{{action}}`.',
+      convert: 'Use `{{replacement}}` instead of `{{directive}}`.',
     },
   },
 
   create(context) {
     const sourceCode = context.sourceCode;
     // Glimmer parks comments that appear inside an element's opening tag
-    // (between attributes) on `element.comments`, not in the children. We
-    // collect them so the Program:exit pass can lift a converted directive
-    // to before its enclosing element — ESLint scopes line-based directives
-    // from where they appear, and the violation typically lives on the
-    // element's start line, not on the in-attribute line.
+    // (between attributes) on `element.comments`, not in the children. Those
+    // are the element-scoped directives, and converting them faithfully needs
+    // the element they belong to.
     const elementByCommentStart = new Map();
 
     return {
@@ -42,26 +40,26 @@ module.exports = {
 
       'Program:exit'() {
         for (const comment of sourceCode.getAllComments()) {
-          const raw = sourceCode.text.slice(...comment.range);
-          const converted = convertComment(raw);
-          if (!converted) {
+          const directive = parseDirective(sourceCode.text.slice(...comment.range));
+          if (!directive) {
             continue;
           }
           const enclosingElement = elementByCommentStart.get(comment.range[0]);
+          const scoped = directive.action === 'disable' && enclosingElement;
           context.report({
             node: comment,
             messageId: 'convert',
-            data: { action: converted.action },
+            data: {
+              directive: `template-lint-${directive.action}${directive.tree ? '-tree' : ''}`,
+              replacement: scoped ? 'eslint-disable`/`eslint-enable' : `eslint-${directive.action}`,
+            },
             fix: (fixer) =>
-              enclosingElement
-                ? liftBeforeElement(
-                    fixer,
-                    comment,
-                    converted.newComment,
-                    enclosingElement,
-                    sourceCode
-                  )
-                : fixer.replaceTextRange(comment.range, converted.newComment),
+              scoped
+                ? wrapElementInRegion(fixer, comment, directive, enclosingElement, sourceCode)
+                : fixer.replaceTextRange(
+                    comment.range,
+                    buildComment(directive, `eslint-${directive.action}`)
+                  ),
           });
         }
       },
@@ -84,12 +82,12 @@ function unquote(name) {
   return name;
 }
 
-function convertComment(rawComment) {
+function parseDirective(rawComment) {
   const match = rawComment.match(DIRECTIVE_COMMENT);
   if (!match) {
     return null;
   }
-  const { open, action, rules: rulesPart } = match.groups;
+  const { open, action, tree, rules: rulesPart } = match.groups;
   // ESLint directives use comma-separated rule names; template-lint uses
   // whitespace. Prefix each with `ember/template-` to land in the namespace
   // where the ports live.
@@ -99,25 +97,76 @@ function convertComment(rawComment) {
     .filter(Boolean)
     .map((r) => `ember/template-${unquote(r)}`)
     .join(', ');
-  const body = rules ? `eslint-${action} ${rules}` : `eslint-${action}`;
-  // Emit symmetric markers regardless of what the source did.
-  const close = open.length === 5 ? '--}}' : '}}';
   return {
     action,
-    newComment: `${open} ${body} ${close}`,
+    tree: Boolean(tree),
+    rules,
+    open,
+    // Emit symmetric markers regardless of what the source did.
+    close: open.length === 5 ? '--}}' : '}}',
   };
 }
 
-// Strip the in-attribute comment line entirely (leading indent through
-// trailing newline) and re-emit the converted directive on its own line at
-// the element's indent, just above the element.
-function liftBeforeElement(fixer, comment, newComment, element, sourceCode) {
+// Indentation to give an inserted comment: the element's own, but only when the
+// element opens its line. Nested directives are converted over several fix
+// passes, and by a later pass the element can already be preceded on its line
+// by comments an earlier pass inserted — indenting to its column then would
+// push the markup out by the width of those comments.
+function indentOf(element, text) {
+  const lineStart = element.range[0] - element.loc.start.column;
+  const prefix = text.slice(lineStart, element.range[0]);
+  return /^\s*$/.test(prefix) ? prefix : '';
+}
+
+function buildComment(directive, eslintDirective) {
+  const body = directive.rules ? `${eslintDirective} ${directive.rules}` : eslintDirective;
+  return `${directive.open} ${body} ${directive.close}`;
+}
+
+// A `template-lint-disable` sitting inside an element's opening tag is scoped
+// to that element: without `-tree` it covers the opening tag only, with `-tree`
+// it covers the element's whole subtree. ESLint has no element scope, but an
+// `eslint-disable` / `eslint-enable` pair delimits an arbitrary region, so
+// bracketing the element reproduces either scope exactly. Closing the region
+// right after the opening tag (as the element's first child) leaves
+// descendants reporting, which is the non-`-tree` behaviour.
+function wrapElementInRegion(fixer, comment, directive, element, sourceCode) {
   const text = sourceCode.text;
+  // Strip the directive, leaving the opening tag as the author wrote it. When
+  // the comment has the line to itself, take the whole line — leading indent
+  // through trailing newline — so no blank line is left behind. When it shares
+  // the line with markup, take the comment plus the single space it sat in;
+  // reaching back to the line start there would swallow the tag itself.
   const lineStart = comment.range[0] - comment.loc.start.column;
-  const lineEnd = text[comment.range[1]] === '\n' ? comment.range[1] + 1 : comment.range[1];
-  const indent = ' '.repeat(element.loc.start.column);
-  return [
-    fixer.removeRange([lineStart, lineEnd]),
-    fixer.insertTextBeforeRange(element.range, `${newComment}\n${indent}`),
+  const ownsLine = /^\s*$/.test(text.slice(lineStart, comment.range[0]));
+  let removeFrom = ownsLine ? lineStart : comment.range[0];
+  let removeTo = comment.range[1];
+  if (ownsLine) {
+    if (text[removeTo] === '\n') {
+      removeTo += 1;
+    }
+  } else if (text[removeTo] === ' ') {
+    removeTo += 1;
+  } else if (text[removeFrom - 1] === ' ') {
+    removeFrom -= 1;
+  }
+  const indent = indentOf(element, text);
+
+  const fixes = [
+    fixer.removeRange([removeFrom, removeTo]),
+    fixer.insertTextBeforeRange(
+      element.range,
+      `${buildComment(directive, 'eslint-disable')}\n${indent}`
+    ),
   ];
+
+  const enableComment = buildComment(directive, 'eslint-enable');
+  const firstChild = element.children?.[0];
+  if (!directive.tree && firstChild) {
+    fixes.push(fixer.insertTextBeforeRange(firstChild.range, enableComment));
+  } else {
+    // `-tree`, or an element with no children where the two scopes coincide.
+    fixes.push(fixer.insertTextAfterRange(element.range, `\n${indent}${enableComment}`));
+  }
+  return fixes;
 }
diff --git a/tests/lib/rules/template-no-template-lint-directives.js b/tests/lib/rules/template-no-template-lint-directives.js
index 51b3626beb..7de2209894 100644
--- a/tests/lib/rules/template-no-template-lint-directives.js
+++ b/tests/lib/rules/template-no-template-lint-directives.js
@@ -18,6 +18,8 @@ ruleTester.run('template-no-template-lint-directives', rule, {
     '/* template-lint-disable no-log */ const x = 1;',
   ],
   invalid: [
+    // A directive standing on its own applies from the comment to the end of
+    // the template, which is what `eslint-disable` already means.
     {
       code: '<template>{{! template-lint-disable no-log }}{{log "x"}}</template>',
       output: '<template>{{! eslint-disable ember/template-no-log }}{{log "x"}}</template>',
@@ -39,6 +41,11 @@ ruleTester.run('template-no-template-lint-directives', rule, {
       output: '<template>{{! eslint-disable ember/template-no-log }}{{log "x"}}</template>',
       errors: [{ messageId: 'convert' }],
     },
+    {
+      code: '<template>{{! template-lint-disable }}{{log "x"}}</template>',
+      output: '<template>{{! eslint-disable }}{{log "x"}}</template>',
+      errors: [{ messageId: 'convert' }],
+    },
     {
       code: '<template>{{! template-lint-enable no-log }}{{log "x"}}</template>',
       output: '<template>{{! eslint-enable ember/template-no-log }}{{log "x"}}</template>',
@@ -75,9 +82,9 @@ ruleTester.run('template-no-template-lint-directives', rule, {
       output: '<template>{{! eslint-disable ember/template-"no-log\' }}{{log "x"}}</template>',
       errors: [{ messageId: 'convert' }],
     },
-    // Directive inside an element's opening tag is lifted to before the
-    // element so the eslint-disable scope covers the line where the
-    // violation is reported.
+    // Inside an opening tag the directive is scoped to the element, so it
+    // becomes a disable/enable pair bracketing that element. With no children,
+    // the `-tree` and non-`-tree` scopes coincide.
     {
       code: `<template>
   <div
@@ -92,8 +99,176 @@ ruleTester.run('template-no-template-lint-directives', rule, {
     class="x"
     {{on "click" this.click}}
   ></div>
+  {{! eslint-enable ember/template-no-invalid-interactive }}
+</template>`,
+      errors: [{ messageId: 'convert' }],
+    },
+    // A directive sharing its line with markup is stripped on its own, not by
+    // the line: reaching back to the line start would delete the opening tag
+    // with it.
+    {
+      code: '<template><div {{! template-lint-disable no-invalid-interactive }} {{on "click" this.click}}></div></template>',
+      output: `<template>{{! eslint-disable ember/template-no-invalid-interactive }}
+<div {{on "click" this.click}}></div>
+{{! eslint-enable ember/template-no-invalid-interactive }}</template>`,
+      errors: [{ messageId: 'convert' }],
+    },
+    {
+      code: `<template>
+  <div class="x" {{! template-lint-disable no-invalid-interactive }}
+    {{on "click" this.click}}
+  ></div>
+</template>`,
+      output: `<template>
+  {{! eslint-disable ember/template-no-invalid-interactive }}
+  <div class="x"
+    {{on "click" this.click}}
+  ></div>
+  {{! eslint-enable ember/template-no-invalid-interactive }}
+</template>`,
+      errors: [{ messageId: 'convert' }],
+    },
+    // With children, the non-`-tree` scope covers the opening tag only, so the
+    // region closes as the element's first child and descendants keep linting.
+    {
+      code: `<template>
+  <div
+    {{! template-lint-disable no-invalid-interactive }}
+    {{on "click" this.click}}
+  ><span>hi</span></div>
+</template>`,
+      output: `<template>
+  {{! eslint-disable ember/template-no-invalid-interactive }}
+  <div
+    {{on "click" this.click}}
+  >{{! eslint-enable ember/template-no-invalid-interactive }}<span>hi</span></div>
+</template>`,
+      errors: [{ messageId: 'convert' }],
+    },
+    // `-tree` covers the subtree, so the region closes after the element.
+    {
+      code: `<template>
+  <div
+    {{! template-lint-disable-tree no-invalid-interactive }}
+    {{on "click" this.click}}
+  ><span>hi</span></div>
+</template>`,
+      output: `<template>
+  {{! eslint-disable ember/template-no-invalid-interactive }}
+  <div
+    {{on "click" this.click}}
+  ><span>hi</span></div>
+  {{! eslint-enable ember/template-no-invalid-interactive }}
 </template>`,
       errors: [{ messageId: 'convert' }],
     },
   ],
 });
+
+// The rewritten comment text is only half the contract. What has to hold is
+// that the converted directive suppresses exactly the violations the original
+// did — no more (which hides real problems) and no fewer (which floods a
+// migration with noise). These cases mirror ember-template-lint 7.9.3 run on
+// the same templates.
+describe('suppression scope after conversion', () => {
+  const { Linter } = require('eslint');
+  const plugin = require('../../../lib/index');
+  const hbsParser = require('ember-eslint-parser/hbs');
+
+  const baseConfig = {
+    files: ['**/*.hbs'],
+    languageOptions: { parser: hbsParser },
+    plugins: { ember: plugin },
+  };
+
+  function migrate(source) {
+    return new Linter({ configType: 'flat' }).verifyAndFix(
+      source,
+      { ...baseConfig, rules: { 'ember/template-no-template-lint-directives': 'error' } },
+      'a.hbs'
+    ).output;
+  }
+
+  // Each element carries a distinct tabindex, so a violation is identified by
+  // that value rather than by a line or column the fixer may have shifted.
+  function stillReported(source) {
+    const lines = source.split('\n');
+    return new Linter({ configType: 'flat' })
+      .verify(
+        source,
+        { ...baseConfig, rules: { 'ember/template-no-positive-tabindex': 'error' } },
+        'a.hbs'
+      )
+      .map((message) => lines[message.line - 1].match(/tabindex="(\d+)"/)[1]);
+  }
+
+  const ELEMENT = `<div
+  {{! template-lint-DIRECTIVE no-positive-tabindex }}
+  tabindex="1"
+>
+  <span tabindex="2"></span>
+</div>
+<a tabindex="3"></a>`;
+
+  it('scopes an in-tag directive to the opening tag, as template-lint does', () => {
+    const migrated = migrate(ELEMENT.replace('DIRECTIVE', 'disable'));
+
+    // template-lint suppresses the div's own tabindex="1" only. Its descendant
+    // and the following sibling keep reporting — verified against
+    // ember-template-lint 7.9.3 on this same template.
+    expect(stillReported(migrated)).toEqual(['2', '3']);
+  });
+
+  it('scopes a -tree directive to the subtree, as template-lint does', () => {
+    const migrated = migrate(ELEMENT.replace('DIRECTIVE', 'disable-tree'));
+
+    // Element and descendants suppressed; only the following sibling reports.
+    expect(stillReported(migrated)).toEqual(['3']);
+  });
+
+  it('keeps a standalone directive covering the rest of the template', () => {
+    const migrated = migrate(`{{! template-lint-disable no-positive-tabindex }}
+<div tabindex="1"></div>
+<a tabindex="3"></a>`);
+
+    expect(stillReported(migrated)).toEqual([]);
+  });
+
+  it('keeps the element intact when the directive shares its line with markup', () => {
+    const migrated = migrate(`<div {{! template-lint-disable no-positive-tabindex }} tabindex="1">
+  <span tabindex="2"></span>
+</div>
+<a tabindex="3"></a>`);
+
+    // The opening tag has to survive the strip — taking the comment's whole
+    // line here would delete `<div ` along with it.
+    expect(migrated).toContain('<div tabindex="1">');
+    expect(stillReported(migrated)).toEqual(['2', '3']);
+  });
+
+  it('converts nested element-scoped directives, each keeping its own scope', () => {
+    const migrated = migrate(`<div
+  {{! template-lint-disable no-positive-tabindex }}
+  tabindex="1"
+>
+  <span tabindex="2"></span>
+</div>`);
+
+    // Nested directives are converted across several fix passes; the result
+    // must still converge and must not swallow the descendant's violation.
+    expect(migrated).not.toContain('template-lint-');
+    expect(stillReported(migrated)).toEqual(['2']);
+  });
+
+  it('closes the region without adding whitespace to the element body', () => {
+    const migrated = migrate(`<div
+  {{! template-lint-disable no-positive-tabindex }}
+  tabindex="1"
+><span>hi</span></div>`);
+
+    // `{{! }}` comments compile away, but a newline would introduce a
+    // whitespace text node and can change inline rendering, so the enable
+    // comment is inserted without one.
+    expect(migrated).toContain('>{{! eslint-enable ember/template-no-positive-tabindex }}<span>');
+  });
+});