Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand All @@ -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:

Expand Down
51 changes: 47 additions & 4 deletions docs/rules/template-no-template-lint-directives.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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):
Expand All @@ -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
Expand Down
119 changes: 84 additions & 35 deletions lib/rules/template-no-template-lint-directives.js
Original file line number Diff line number Diff line change
@@ -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 = {
Expand All @@ -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 {
Expand All @@ -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}`)
),
});
}
},
Expand All @@ -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.
Expand All @@ -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;
}
Loading
Loading