From ff7f8916b2b5119ac983feeae7bed9432f464529 Mon Sep 17 00:00:00 2001 From: erKam Date: Thu, 15 Oct 2026 21:39:31 +0000 Subject: [PATCH] feat(mcp): add a lint-templates agent tool It parses every component template with the project's own @angular/compiler and reports parse errors, track $index on object lists, images without alt, click handlers without a key handler, legacy structural directives and ngClass or ngStyle bindings. Refs #372. --- apps/docs/src/content/agents/mcp-server.md | 2 +- apps/docs/src/content/agents/tools.md | 24 +- .../docs/src/content/inspectors/components.md | 35 +- apps/docs/src/content/security.md | 2 +- .../skills/pangular-inspector/SKILL.md | 1 + .../fixtures/agent-metadata-derived.json | 2 + .../src/__tests__/tool-metadata.test.ts | 1 + packages/devtools/src/config.ts | 1 + packages/devtools/src/devframe.ts | 3 + .../src/rpc/__tests__/template-lint.test.ts | 352 ++++++++++++ .../devtools/src/rpc/template-lint-tool.ts | 45 ++ packages/devtools/src/rpc/template-lint.ts | 512 ++++++++++++++++++ 12 files changed, 960 insertions(+), 20 deletions(-) create mode 100644 packages/devtools/src/rpc/__tests__/template-lint.test.ts create mode 100644 packages/devtools/src/rpc/template-lint-tool.ts create mode 100644 packages/devtools/src/rpc/template-lint.ts diff --git a/apps/docs/src/content/agents/mcp-server.md b/apps/docs/src/content/agents/mcp-server.md index 9bb6336c..884cbbe4 100644 --- a/apps/docs/src/content/agents/mcp-server.md +++ b/apps/docs/src/content/agents/mcp-server.md @@ -255,7 +255,7 @@ A tool answers with a JSON object, sent as text. Most tools answer `{ "markdown" | ----------------------------------------------------------------------------------------------------------- | ------------------------------ | | Every tool whose name starts with `list-` or `inspect-` | `{ "markdown": "..." }` | | `analog-routes`, `analog-api-routes`, `analog-server-functions`, `analog-server-calls` and `analog-content` | `{ "markdown": "..." }` | -| `wait-for` | `{ "markdown": "..." }` | +| `wait-for` and `lint-templates` | `{ "markdown": "..." }` | | `build-meta` | The project and version fields | | `snapshot` | `{ "markdown": "..." }` | diff --git a/apps/docs/src/content/agents/tools.md b/apps/docs/src/content/agents/tools.md index 1c90db47..9b11d0ef 100644 --- a/apps/docs/src/content/agents/tools.md +++ b/apps/docs/src/content/agents/tools.md @@ -4,7 +4,7 @@ description: Every agent tool the devtools expose, grouped by inspector, with wh --- - Ninety-seven tools, grouped by inspector. Each one answers a question you would otherwise answer by clicking through the panel. + Ninety-eight tools, grouped by inspector. Each one answers a question you would otherwise answer by clicking through the panel. # Tools @@ -231,6 +231,28 @@ The page reads the blocks with `ng.ɵgetControlFlowBlocks`, which Angular 22.0 a Each page lists at most 50 blocks, with a note on how many more are not shown, and says when the page has more than the 500 blocks it reports. The whole answer stops at 20,000 characters. If several tabs report, pass `page` to read one. The **@for blocks** list on the [Components](../inspectors/components.md#for-blocks) page has every block. +### lint-templates + +Checks the component templates in your source, inline `template` and `templateUrl` files, with the project's own `@angular/compiler`. Reads: source. + +| Argument | Required | Value | +| ---------- | -------- | ---------------------------------------------------------------------- | +| `rule` | no | Only findings of this rule, one of the ids below. | +| `file` | no | Only findings in files whose path contains this text, such as `cart/`. | +| `severity` | no | Only findings of this severity: `error`, `warning` or `info`. | +| `limit` | no | How many findings to list, 1 to 200. Defaults to 50. | + +| Rule | Severity | Finds | +| ---------------------- | --------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `template-parse-error` | `error` | An error the template parser reports, such as an unclosed element, a broken expression or `@for` without `track`. | +| `track-by-index` | `warning` | `@for` that tracks by `$index` (or an alias of it) while its body reads fields of the item, so the items are objects. Rows that bind a form directive, such as `[formControlName]="$index"` or `[formField]`, are skipped. | +| `img-missing-alt` | `warning` | `` without `alt`, `[alt]` or `[attr.alt]`, unless it has `role="presentation"`, `role="none"` or `aria-hidden="true"`. | +| `click-without-key` | `warning` | `(click)` on a plain element such as `
` or `` with no `keydown`, `keyup` or `keypress` handler. Buttons, links, form controls and components are skipped. | +| `legacy-control-flow` | `info` | `*ngIf`, `*ngFor`, `*ngSwitchCase` or `*ngSwitchDefault` where `@if`, `@for` or `@switch` would do. Skipped before Angular 17. | +| `ngclass-ngstyle` | `info` | `ngClass` or `ngStyle` where `[class]` or `[style]` bindings would do. | + +Each finding gives its severity, rule, `file:line` and a fix. Errors come first. A template whose text has not changed since the last call is not parsed again. A backtick template built with `${}` is skipped. If the project has no `@angular/compiler` installed, the answer says so and lists nothing. + ### hydration Action Hydration of each page that was rendered on the server: how many component hosts were hydrated, skipped with `ngSkipHydration`, mismatched or created on the client, and how many `@defer` blocks are still dehydrated. It names the skipped and client-created hosts and the dehydrated blocks, at most 30 of each, and each mismatch with a line diff of the server HTML Angular expected (`-`) and the client DOM it found (`+`). Reads: page. diff --git a/apps/docs/src/content/inspectors/components.md b/apps/docs/src/content/inspectors/components.md index 4d238a79..362da995 100644 --- a/apps/docs/src/content/inspectors/components.md +++ b/apps/docs/src/content/inspectors/components.md @@ -447,23 +447,24 @@ Press Escape in the filter box to clear it. When the filter matches nothing, **C ## Agent tools -| Tool or resource | Kind | What it does | -| ------------------------------------ | -------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `pangular:get-components` | tool | Lists components and directives from source, with selector, kind, inputs, outputs, change detection, file and line. | -| `pangular:highlight` | tool | Highlights a component in the page and selects it. Takes an instance id, class name, host tag or CSS selector. Also retargets the Signals graph. Lists every instance a name matches. | -| `pangular:inspect-component` | tool | Selects one instance and returns its live detail: inputs, outputs, properties, listeners, directives and injected services. With `path`, reads one nested value instead, with `depth` and `limit`. | -| `pangular:list-components` | tool | Returns the live tree of one page as an indented outline with routed, directive host and hydration markers. Takes a `filter` with the search syntax, a `depth` and `directiveHosts`. | -| `pangular:defer-blocks` | tool | Lists the `@defer` blocks of each page, and flags blocks that failed, blocks stuck on their placeholder and blocks still dehydrated. | -| `pangular:for-blocks` | tool | Lists the `@for` blocks of each page with their item count, track kind and `@empty` block. | -| `pangular:hydration` | tool | Returns the hydration status of each page rendered on the server, and turns the outline on the page on or off. Turning the outline on or off is an action. | -| `pangular:change-detection` | tool | Starts, stops or clears a change detection recording, turns the live views on or off, and returns the slowest and most often checked components, hook, output and afterRender time, the latest cycles, or an export. | -| `pangular:diagnose-change-detection` | tool | What started the recorded cycles, components that depend on zone.js, wasted checks, inputs that break OnPush, and OnPush hints. | -| `pangular:set-component-property` | tool | Writes an input or property of one instance and runs change detection. An action. | -| `pangular:set-signal` | tool | Sets a writable signal that one instance holds as a property. An action. | -| `pangular:find-leaks` | tool | Lists retained destroyed instances, subscriptions still open after their component went away, and live effects and subscriptions per component. | -| `pangular:track-subscriptions` | tool | Starts or stops subscription tracking on a page, like **Track subscriptions**. An action. | -| `pangular:screenshot-component` | tool | Returns an image of one component's host element, drawn by the page from a copy of its DOM and computed styles, with secret fields and marked elements masked. Takes an instance id, class name, host tag or CSS selector. | -| `pangular:component-tree` | resource | The live tree per page, with the detail of the selected instance and the defer blocks. | +| Tool or resource | Kind | What it does | +| ------------------------------------ | -------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `pangular:get-components` | tool | Lists components and directives from source, with selector, kind, inputs, outputs, change detection, file and line. | +| `pangular:highlight` | tool | Highlights a component in the page and selects it. Takes an instance id, class name, host tag or CSS selector. Also retargets the Signals graph. Lists every instance a name matches. | +| `pangular:inspect-component` | tool | Selects one instance and returns its live detail: inputs, outputs, properties, listeners, directives and injected services. With `path`, reads one nested value instead, with `depth` and `limit`. | +| `pangular:list-components` | tool | Returns the live tree of one page as an indented outline with routed, directive host and hydration markers. Takes a `filter` with the search syntax, a `depth` and `directiveHosts`. | +| `pangular:defer-blocks` | tool | Lists the `@defer` blocks of each page, and flags blocks that failed, blocks stuck on their placeholder and blocks still dehydrated. | +| `pangular:for-blocks` | tool | Lists the `@for` blocks of each page with their item count, track kind and `@empty` block. | +| `pangular:lint-templates` | tool | Lints component templates in source with the project's Angular template parser: parse errors, `@for` tracking objects by `$index`, `` without `alt`, `(click)` without a key handler, `*ngIf`-style directives and `ngClass` or `ngStyle`. | +| `pangular:hydration` | tool | Returns the hydration status of each page rendered on the server, and turns the outline on the page on or off. Turning the outline on or off is an action. | +| `pangular:change-detection` | tool | Starts, stops or clears a change detection recording, turns the live views on or off, and returns the slowest and most often checked components, hook, output and afterRender time, the latest cycles, or an export. | +| `pangular:diagnose-change-detection` | tool | What started the recorded cycles, components that depend on zone.js, wasted checks, inputs that break OnPush, and OnPush hints. | +| `pangular:set-component-property` | tool | Writes an input or property of one instance and runs change detection. An action. | +| `pangular:set-signal` | tool | Sets a writable signal that one instance holds as a property. An action. | +| `pangular:find-leaks` | tool | Lists retained destroyed instances, subscriptions still open after their component went away, and live effects and subscriptions per component. | +| `pangular:track-subscriptions` | tool | Starts or stops subscription tracking on a page, like **Track subscriptions**. An action. | +| `pangular:screenshot-component` | tool | Returns an image of one component's host element, drawn by the page from a copy of its DOM and computed styles, with secret fields and marked elements masked. Takes an instance id, class name, host tag or CSS selector. | +| `pangular:component-tree` | resource | The live tree per page, with the detail of the selected instance and the defer blocks. | See [Tools](../agents/tools.md) and [Resources](../agents/resources.md). diff --git a/apps/docs/src/content/security.md b/apps/docs/src/content/security.md index 67164a7d..04386bc6 100644 --- a/apps/docs/src/content/security.md +++ b/apps/docs/src/content/security.md @@ -309,7 +309,7 @@ An app can put any text in a value, a URL, a label or an error message, includin A tool answer that names a page the server does not know lists the pages that report, with their URLs. That answer starts with the first notice too. -Answers that hold no page text carry no notice: `highlight` (instance ids, class names and tags), answers that only say there is no data yet, `lint-pipes` and the source scan tools such as `get-components` (they read your project files, not the page). The `component-tree`, `signal-graph`, `injector-tree`, `ngrx-store`, `forms` and `router` resources carry the notice in an `_untrusted` field at the top of their JSON, also when they return a truncated summary. +Answers that hold no page text carry no notice: `highlight` (instance ids, class names and tags), answers that only say there is no data yet, `lint-pipes`, `lint-templates` and the source scan tools such as `get-components` (they read your project files, not the page). The `component-tree`, `signal-graph`, `injector-tree`, `ngrx-store`, `forms` and `router` resources carry the notice in an `_untrusted` field at the top of their JSON, also when they return a truncated summary. ## Checklist diff --git a/packages/devtools/skills/pangular-inspector/SKILL.md b/packages/devtools/skills/pangular-inspector/SKILL.md index 47f97e5d..77b5853a 100644 --- a/packages/devtools/skills/pangular-inspector/SKILL.md +++ b/packages/devtools/skills/pangular-inspector/SKILL.md @@ -33,6 +33,7 @@ Pangular Inspector reads a running Angular app through Angular's debug APIs and | Why does `inject()` fail (NG0201, NullInjectorError)? | `explain-injection` with `selector` and `token` | | Which route is active, and why did navigation fail? | `inspect-route`, `explain-navigation`, `export-navigation` | | Is the route config wrong? | `lint-routes`, `list-routes` with `match` | +| Do the templates have errors or a11y problems? | `lint-templates`, with `rule` or `file` to narrow it | | Why is the form invalid or the submit dead? | `explain-form-invalid`, `explain-field`, `explain-submit` (see `pangular-forms`) | | What errors and console messages happened? | `list-errors`, `explain-error`, `list-console` (see `pangular-errors`) | | Which HTTP calls ran, and what did SSR do? | `list-http-calls`, `list-ssr-requests`, `explain-ssr-request` | diff --git a/packages/devtools/src/__tests__/fixtures/agent-metadata-derived.json b/packages/devtools/src/__tests__/fixtures/agent-metadata-derived.json index 0296dd9b..71b6260a 100644 --- a/packages/devtools/src/__tests__/fixtures/agent-metadata-derived.json +++ b/packages/devtools/src/__tests__/fixtures/agent-metadata-derived.json @@ -47,6 +47,7 @@ "lint-forms": "forms", "lint-pipes": "pipes", "lint-routes": "router", + "lint-templates": "components", "list-capacitor-calls": "capacitor", "list-capacitor-plugins": "capacitor", "list-components": "components", @@ -275,6 +276,7 @@ "lint-forms": ["forms"], "lint-pipes": ["pipes"], "lint-routes": ["router"], + "lint-templates": ["components"], "list-capacitor-calls": ["capacitor"], "list-capacitor-plugins": ["capacitor"], "list-components": ["components"], diff --git a/packages/devtools/src/__tests__/tool-metadata.test.ts b/packages/devtools/src/__tests__/tool-metadata.test.ts index e49cb9ba..eef56480 100644 --- a/packages/devtools/src/__tests__/tool-metadata.test.ts +++ b/packages/devtools/src/__tests__/tool-metadata.test.ts @@ -214,6 +214,7 @@ describe('agent tool output schemas', () => { 'pangular_analog-server-calls', 'pangular_analog-content', 'pangular_build-meta', + 'pangular_lint-templates', 'pangular_wait-for', 'pangular_snapshot', ].includes(name); diff --git a/packages/devtools/src/config.ts b/packages/devtools/src/config.ts index 49d9a7e9..5475c335 100644 --- a/packages/devtools/src/config.ts +++ b/packages/devtools/src/config.ts @@ -925,6 +925,7 @@ export const AGENT_ENTRIES: Readonly> = { // components 'get-components': { kind: 'rpc', inspectors: ['components'] }, + 'lint-templates': { kind: 'tool', inspectors: ['components'] }, 'component-tree': { kind: 'resource', inspectors: ['components'], page: true }, highlight: { kind: 'tool', inspectors: ['components'], page: true }, 'inspect-component': { kind: 'tool', inspectors: ['components'], page: true }, diff --git a/packages/devtools/src/devframe.ts b/packages/devtools/src/devframe.ts index dc23f7f1..08b33d26 100644 --- a/packages/devtools/src/devframe.ts +++ b/packages/devtools/src/devframe.ts @@ -39,6 +39,7 @@ import { sanitizeInjectorTree } from './rpc/injector-report.ts'; import { createFrameworkNames, markFramework } from './rpc/framework-tokens.ts'; import { lintPipes, lintPipesText } from './rpc/pipe-lint.ts'; import { cachedPipeLint } from './rpc/pipe-lint-cache.ts'; +import { templateLintTool } from './rpc/template-lint-tool.ts'; import { explainPipeOnPage, pipeRecordRequest, pipeRecordText } from './rpc/pipes-agent.ts'; import { createPageOwners } from './rpc/page-owners.ts'; import { trackPageSessions } from './rpc/page-sessions.ts'; @@ -2955,6 +2956,8 @@ const pangular = defineDevframe({ }, }); + agent.registerTool(templateLintTool(ctx.cwd)); + agent.registerTool({ id: 'pangular:for-blocks', title: 'List @for blocks', diff --git a/packages/devtools/src/rpc/__tests__/template-lint.test.ts b/packages/devtools/src/rpc/__tests__/template-lint.test.ts new file mode 100644 index 00000000..d60a5e40 --- /dev/null +++ b/packages/devtools/src/rpc/__tests__/template-lint.test.ts @@ -0,0 +1,352 @@ +import { mkdirSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { beforeAll, describe, expect, it } from 'vitest'; +import { fixtureDir } from './fixture-dir.ts'; +import { + lintTemplates, + projectCompiler, + templateLintText, + type TemplateCompiler, + type TemplateLintFinding, +} from '../template-lint.ts'; +import { templateLintTool } from '../template-lint-tool.ts'; + +const REPO = fileURLToPath(new URL('../../../../../', import.meta.url)); + +let compiler: TemplateCompiler; +beforeAll(async () => { + // The workspace's own @angular/compiler, loaded the way the tool loads a project's. + const found = await projectCompiler(REPO); + if (!found) throw new Error('@angular/compiler is not installed in the workspace'); + compiler = found; +}); + +function project(files: Record): string { + const dir = fixtureDir('pangular-template-lint-'); + for (const [path, content] of Object.entries(files)) { + const full = join(dir, path); + mkdirSync(join(full, '..'), { recursive: true }); + writeFileSync(full, content); + } + return dir; +} + +function component(template: string): string { + return `import { Component } from '@angular/core'; + +@Component({ + selector: 'app-view', + template: \`${template}\`, +}) +export class View {} +`; +} + +async function lint(template: string, using: TemplateCompiler = compiler) { + const dir = project({ 'src/view.ts': component(template) }); + return (await lintTemplates(dir, { compiler: using })).findings; +} + +const rules = (findings: TemplateLintFinding[]) => findings.map((f) => f.rule); + +describe('lintTemplates', () => { + describe('template-parse-error', () => { + it('reports an expression error at its line in an inline template', async () => { + const findings = await lint('\n

ok

\n

{{ total( }}

\n'); + expect(findings).toHaveLength(1); + // The template starts on line 5 of view.ts; the error is two lines in. + expect(findings[0]).toMatchObject({ + rule: 'template-parse-error', + severity: 'error', + file: join('src', 'view.ts'), + line: 7, + }); + expect(findings[0]!.message).not.toMatch(/@\d+:\d+$/); + }); + + it('reports @for without track', async () => { + const findings = await lint('@for (item of items) {

{{ item }}

}'); + expect(findings).toEqual([ + expect.objectContaining({ + rule: 'template-parse-error', + message: expect.stringContaining('track'), + }), + ]); + }); + + it('reports an unclosed element in a templateUrl file at the html line', async () => { + const dir = project({ + 'src/card.ts': `@Component({ selector: 'app-card', templateUrl: './card.html' }) +export class Card {} +`, + 'src/card.html': '
\n

Title

\n
\n\n', + }); + const { findings, templates } = await lintTemplates(dir, { compiler }); + expect(templates).toBe(1); + expect(findings).toEqual([ + expect.objectContaining({ + rule: 'template-parse-error', + file: join('src', 'card.html'), + line: 3, + }), + ]); + }); + + it('reads escaped quotes in a quoted inline template without an error', async () => { + const dir = project({ + 'src/view.ts': `@Component({ selector: 'app-v', template: '

{{ a }}

' }) +export class V {} +`, + }); + const result = await lintTemplates(dir, { compiler }); + expect(result.templates).toBe(1); + expect(result.findings).toEqual([]); + }); + + it('skips a backtick template built with ${}', async () => { + const dir = project({ + 'src/view.ts': + "const body = '

';\n@Component({ selector: 'app-v', template: `${body}{{ a( }}` })\nexport class V {}\n", + }); + expect(await lintTemplates(dir, { compiler })).toMatchObject({ templates: 0, findings: [] }); + }); + }); + + describe('legacy-control-flow', () => { + it('reports *ngIf, *ngFor and *ngSwitchCase', async () => { + const findings = await lint(` +

a
+
  • {{ x }}
    • +

    a

    `); + const legacy = findings.filter((f) => f.rule === 'legacy-control-flow'); + expect(legacy.map((f) => [f.line, f.severity])).toEqual([ + [6, 'info'], + [7, 'info'], + [8, 'info'], + ]); + expect(legacy[1]!.message).toContain('`*ngFor`'); + expect(legacy[1]!.message).toContain('`@for`'); + expect(legacy[2]!.message).toContain('`@switch`'); + }); + + it('leaves @if blocks and explicit alone', async () => { + const findings = await lint(` + @if (open) {

    a

    } +

    b

    `); + expect(rules(findings)).not.toContain('legacy-control-flow'); + }); + + it('is skipped for an Angular without control flow blocks', async () => { + const old = { ...compiler, VERSION: { major: '16' } } as TemplateCompiler; + expect(rules(await lint('
    a
    ', old))).toEqual([]); + }); + }); + + describe('track-by-index', () => { + it('reports tracking by $index when the body reads fields of the item', async () => { + const findings = await lint( + '@for (user of users; track $index) {
  • {{ user.name }}
    • }', + ); + expect(findings).toEqual([ + expect.objectContaining({ rule: 'track-by-index', severity: 'warning', line: 5 }), + ]); + expect(findings[0]!.fix).toContain('track user.id'); + }); + + it('reports tracking by an alias of $index', async () => { + const findings = await lint( + "@for (user of users; let i = $index; track i) {
  • {{ user?.['name'] }}
    • }", + ); + expect(rules(findings)).toEqual(['track-by-index']); + }); + + it('leaves form array rows alone, which are found by position', async () => { + expect( + await lint( + '@for (phone of phones.controls; track $index) { }', + ), + ).toEqual([]); + expect( + await lint('@for (tag of form.tags; track $index) { }'), + ).toEqual([]); + }); + + it('leaves a stable key and a list of plain values alone', async () => { + expect( + await lint('@for (user of users; track user.id) {
  • {{ user.name }}
    • }'), + ).toEqual([]); + expect(await lint('@for (tag of tags; track $index) {
  • {{ tag }}
    • }')).toEqual([]); + expect( + await lint('@for (tag of tags; track $index) {
  • {{ this.tag.length }}
    • }'), + ).toEqual([]); + }); + }); + + describe('ngclass-ngstyle', () => { + it('reports [ngClass] and a static ngStyle', async () => { + const findings = await lint(` +

    a

    +

    b

    `); + expect(findings.map((f) => [f.rule, f.severity, f.line])).toEqual([ + ['ngclass-ngstyle', 'info', 6], + ['ngclass-ngstyle', 'info', 7], + ]); + expect(findings[0]!.fix).toContain('[class]'); + expect(findings[1]!.fix).toContain('[style]'); + }); + + it('leaves class and style bindings alone', async () => { + expect(await lint('

    a

    ')).toEqual([]); + }); + }); + + describe('img-missing-alt', () => { + it('reports an without alt', async () => { + const findings = await lint(''); + expect(findings.map((f) => [f.rule, f.severity])).toEqual([ + ['img-missing-alt', 'warning'], + ['img-missing-alt', 'warning'], + ]); + }); + + it('accepts empty, bound and attr alt, and hidden images', async () => { + expect( + await lint(` + + + + + `), + ).toEqual([]); + }); + }); + + describe('click-without-key', () => { + it('reports (click) on a
    without a key handler', async () => { + const findings = await lint('
    Open
    '); + expect(findings).toEqual([ + expect.objectContaining({ rule: 'click-without-key', severity: 'warning', line: 5 }), + ]); + expect(findings[0]!.message).toContain('`
    `'); + }); + + it('leaves buttons, key handlers, components and hidden elements alone', async () => { + expect( + await lint(` + + Go +
    Open
    + + `), + ).toEqual([]); + }); + }); + + it('reads nodes inside blocks and structural templates', async () => { + const findings = await lint(` + @if (open) { + @for (u of users; track u.id) { +
    + } + }`); + expect(rules(findings).sort()).toEqual(['img-missing-alt', 'legacy-control-flow']); + }); + + it('says so when the project has no @angular/compiler', async () => { + const dir = project({ 'src/view.ts': component('

    a

    ') }); + const result = await lintTemplates(dir); + expect(result).toEqual({ parsed: false, templates: 0, findings: [] }); + expect(templateLintText(result)).toContain('no `@angular/compiler`'); + }); + + it('does not parse an unchanged template again, and parses an edited one', async () => { + let parses = 0; + const counting = { + ...compiler, + parseTemplate: (...args: Parameters) => { + parses++; + return compiler.parseTemplate(...args); + }, + } as TemplateCompiler; + const dir = project({ + 'src/a.ts': component(''), + 'src/b.ts': component('

    b

    '), + }); + expect(rules((await lintTemplates(dir, { compiler: counting })).findings)).toEqual([ + 'img-missing-alt', + ]); + expect(parses).toBe(2); + await lintTemplates(dir, { compiler: counting }); + expect(parses).toBe(2); + writeFileSync(join(dir, 'src', 'a.ts'), component('A')); + expect((await lintTemplates(dir, { compiler: counting })).findings).toEqual([]); + expect(parses).toBe(3); + }); +}); + +describe('templateLintText', () => { + async function many() { + const dir = project({ + 'src/cart/cart.ts': component('
    a
    '), + 'src/home/home.ts': component('

    {{ a( }}

    '), + 'src/home/list.ts': component( + '@for (u of users; track $index) {
    {{ u.name }}
    }', + ), + }); + return lintTemplates(dir, { compiler }); + } + + it('lists findings errors first with rule, severity, file:line and a fix', async () => { + const text = templateLintText(await many()); + expect(text).toMatch(/^5 findings \(1 error, 3 warning, 1 info\) in 3 templates\./); + const items = text.split('\n').filter((line) => line.startsWith('- ')); + expect(items[0]).toMatch(/^- \*\*error\*\* template-parse-error at `src.home.home\.ts:5`: /); + expect(items.at(-1)).toMatch(/^- \*\*info\*\* legacy-control-flow at `src.cart.cart\.ts:5`/); + for (const item of items) expect(item).toContain(' Fix: '); + }); + + it('filters by rule, file and severity', async () => { + const result = await many(); + expect(templateLintText(result, { rule: 'img-missing-alt' })).toMatch(/^1 finding /); + const inHome = templateLintText(result, { file: 'home/' }); + expect(inHome).toMatch(/^3 findings/); + expect(inHome).not.toContain('cart.ts'); + expect(templateLintText(result, { severity: 'info' })).toContain('legacy-control-flow'); + expect(templateLintText(result, { rule: 'ngclass-ngstyle' })).toBe( + 'No template problems found by the lint rules in 3 templates with these filters.', + ); + }); + + it('refuses an unknown rule or severity', async () => { + const result = await many(); + expect(templateLintText(result, { rule: 'nope' })).toContain('Unknown rule "nope"'); + expect(templateLintText(result, { severity: 'fatal' })).toContain('Unknown severity'); + }); + + it('cuts the list at limit and says how many are left', async () => { + const text = templateLintText(await many(), { limit: 2 }); + expect(text.split('\n').filter((line) => line.startsWith('- '))).toHaveLength(2); + expect(text).toContain('3 more not shown.'); + }); + + it('says when nothing was found and when there are no templates', () => { + expect(templateLintText({ parsed: true, templates: 2, findings: [] })).toBe( + 'No template problems found by the lint rules in 2 templates.', + ); + expect(templateLintText({ parsed: true, templates: 0, findings: [] })).toBe( + 'No component templates found in source.', + ); + }); +}); + +describe('lint-templates tool', () => { + it('answers markdown for the arguments it gets', async () => { + const dir = project({ 'src/view.ts': component('') }); + const tool = templateLintTool(dir, { compiler }); + expect(tool.id).toBe('pangular:lint-templates'); + expect(tool.description.length).toBeLessThanOrEqual(600); + const answer = await tool.handler({ rule: 'img-missing-alt' }); + expect(answer.markdown).toContain('img-missing-alt at `src'); + }); +}); diff --git a/packages/devtools/src/rpc/template-lint-tool.ts b/packages/devtools/src/rpc/template-lint-tool.ts new file mode 100644 index 00000000..d6823c74 --- /dev/null +++ b/packages/devtools/src/rpc/template-lint-tool.ts @@ -0,0 +1,45 @@ +import { MARKDOWN_OUTPUT_SCHEMA, type MarkdownAnswer } from './agent-output.ts'; +import { + TEMPLATE_LINT_RULES, + lintTemplates, + templateLintText, + type TemplateCompiler, + type TemplateLintArgs, +} from './template-lint.ts'; + +/** The `lint-templates` agent tool: reads source only, so the stdio server keeps it. */ +export function templateLintTool(cwd: string, options: { compiler?: TemplateCompiler } = {}) { + return { + id: 'pangular:lint-templates', + title: 'Lint templates', + outputSchema: MARKDOWN_OUTPUT_SCHEMA, + description: + "Checks component templates in source (inline `template` and `templateUrl` files) with the project's own Angular template parser: parse errors (including `@for` without `track`), `@for` tracking objects by `$index`, `` without `alt`, `(click)` on a non-interactive element without a key handler, `*ngIf`/`*ngFor`/`*ngSwitchCase` instead of `@if`/`@for`/`@switch`, and `[ngClass]`/`[ngStyle]`. Each finding has rule, severity, file:line and a fix. Filter by `rule`, `file` or `severity`.", + inputSchema: { + type: 'object', + properties: { + rule: { + type: 'string', + enum: Object.keys(TEMPLATE_LINT_RULES), + description: 'Only findings of this rule.', + }, + file: { + type: 'string', + description: 'Only findings in files whose path contains this text, such as `cart/`.', + }, + severity: { + type: 'string', + enum: ['error', 'warning', 'info'], + description: 'Only findings of this severity.', + }, + limit: { + type: 'integer', + description: 'How many findings to list (1-200, default 50).', + }, + }, + }, + handler: async (args: TemplateLintArgs = {}): Promise => ({ + markdown: templateLintText(await lintTemplates(cwd, options), args ?? {}), + }), + }; +} diff --git a/packages/devtools/src/rpc/template-lint.ts b/packages/devtools/src/rpc/template-lint.ts new file mode 100644 index 00000000..364c78b7 --- /dev/null +++ b/packages/devtools/src/rpc/template-lint.ts @@ -0,0 +1,512 @@ +// Lints the project's component templates (inline `template:` and +// `templateUrl` files) with the project's own `@angular/compiler`, so a +// finding is read from the same AST Angular compiles, not guessed with a +// regex. The compiler is loaded from the project, never bundled: when the +// project has none, the tool says so and reports nothing. + +import { readFileSync } from 'node:fs'; +import { createRequire } from 'node:module'; +import { join, relative } from 'node:path'; +import { pathToFileURL } from 'node:url'; +import { templatesIn, type TemplateSource } from './get-pipes.ts'; +import { sourceRoots, walkFiles } from './source-scan.ts'; +import type { LintSeverity } from './pipe-lint.ts'; +import { redactText } from '../text-redact.ts'; + +export const TEMPLATE_LINT_RULES = { + 'template-parse-error': 'error', + 'track-by-index': 'warning', + 'img-missing-alt': 'warning', + 'click-without-key': 'warning', + 'legacy-control-flow': 'info', + 'ngclass-ngstyle': 'info', +} as const satisfies Record; + +export type TemplateLintRule = keyof typeof TEMPLATE_LINT_RULES; + +export interface TemplateLintFinding { + rule: TemplateLintRule; + severity: LintSeverity; + /** Path relative to the project: the `.ts` file for an inline template, the `.html` file otherwise. */ + file: string; + line: number; + message: string; + fix: string; +} + +export interface TemplateLintResult { + /** `false` when the project has no `@angular/compiler` to parse with. */ + parsed: boolean; + templates: number; + findings: TemplateLintFinding[]; +} + +type Ctor = abstract new (...args: never[]) => object; + +interface ParseErrorLike { + msg: string; + level: number; + span: { start: { offset: number } }; +} + +/** The parts of `@angular/compiler` the lint reads, checked against 22.1. */ +export interface TemplateCompiler { + VERSION?: { major: string }; + parseTemplate( + template: string, + templateUrl: string, + options?: Record, + ): { errors: ParseErrorLike[] | null; nodes: unknown[] }; + ParseErrorLevel: { ERROR: number }; + TmplAstElement: Ctor; + TmplAstTemplate: Ctor; + TmplAstForLoopBlock?: Ctor; + PropertyRead: Ctor; + SafePropertyRead?: Ctor; + KeyedRead?: Ctor; + SafeKeyedRead?: Ctor; + ImplicitReceiver: Ctor; + ThisReceiver?: Ctor; +} + +interface Span { + start: { offset: number }; +} +interface Attribute { + name: string; + value: unknown; + sourceSpan: Span; +} +interface OutputNode { + name: string; +} +interface ElementNode { + name: string; + attributes: Attribute[]; + inputs: Attribute[]; + outputs: OutputNode[]; + startSourceSpan: Span; +} +interface TemplateNode { + templateAttrs: Attribute[]; +} +interface ForLoopNode { + item: { name: string }; + trackBy?: { ast?: unknown }; + trackKeywordSpan?: Span; + contextVariables?: { name: string; value: string }[]; + children: unknown[]; + sourceSpan: Span; +} +interface ReadNode { + name?: string; + receiver: unknown; +} + +const compilers = new Map>(); + +/** The project's own `@angular/compiler`, or `undefined` when it has none. A + * miss is not remembered, so installing it works without a restart. */ +export function projectCompiler(cwd: string): Promise { + const known = compilers.get(cwd); + if (known) return known; + const loading = (async () => { + try { + const path = createRequire(join(cwd, 'package.json')).resolve('@angular/compiler'); + const mod = (await import(pathToFileURL(path).href)) as Partial; + return typeof mod.parseTemplate === 'function' ? (mod as TemplateCompiler) : undefined; + } catch { + return undefined; + } + })(); + compilers.set(cwd, loading); + void loading.then((found) => { + if (!found) compilers.delete(cwd); + }); + return loading; +} + +/** Findings per template, keyed by compiler, then project, then the file, + * offset and text of the template. A template whose text is unchanged is not + * parsed again; each scan keeps only the templates it saw. */ +const cache = new WeakMap>>(); + +export async function lintTemplates( + cwd: string, + options: { compiler?: TemplateCompiler } = {}, +): Promise { + const compiler = options.compiler ?? (await projectCompiler(cwd)); + if (!compiler) return { parsed: false, templates: 0, findings: [] }; + + const byProject = cache.get(compiler) ?? new Map>(); + cache.set(compiler, byProject); + const previous = byProject.get(cwd) ?? new Map(); + const next = new Map(); + const major = Number(compiler.VERSION?.major); + const hasControlFlow = !Number.isFinite(major) || major >= 17; + + let templates = 0; + const findings: TemplateLintFinding[] = []; + for (const root of sourceRoots(cwd)) { + walkFiles(root, (full, entry) => { + if (!entry.endsWith('.ts') || entry.endsWith('.spec.ts')) return; + let content: string; + try { + content = readFileSync(full, 'utf-8'); + } catch { + return; + } + if (!content.includes('@Component')) return; + const relPath = relative(cwd, full); + for (const template of templatesIn(content, relPath, full, cwd)) { + const text = templateText(template, content, relPath); + if (text === undefined) continue; + templates++; + const key = `${template.file}\0${template.baseOffset}\0${text}`; + const own = next.get(key) ?? previous.get(key) ?? lintTemplate(compiler, template, text); + next.set(key, own); + for (const finding of own) { + if (finding.rule === 'legacy-control-flow' && !hasControlFlow) continue; + findings.push(finding); + } + } + }); + } + byProject.set(cwd, next); + return { parsed: true, templates, findings }; +} + +/** The template as Angular sees it, with the same length so parser offsets + * map back to the file. Escapes in a quoted inline template become a space + * and the character; a backtick template with `${}` is skipped, as it is + * built at run time. */ +function templateText( + template: TemplateSource, + content: string, + relPath: string, +): string | undefined { + if (template.file !== relPath) return template.text; + const quote = content[template.baseOffset - 1]; + if (quote === '`') return template.text.includes('${') ? undefined : template.text; + return template.text.replace(/\\(.)/g, (_, ch: string) => ` ${ch === 'n' ? ' ' : ch}`); +} + +function lintTemplate( + compiler: TemplateCompiler, + template: TemplateSource, + text: string, +): TemplateLintFinding[] { + const at = (offset: number) => template.lineAt(template.baseOffset + offset); + const findings: TemplateLintFinding[] = []; + const add = (rule: TemplateLintRule, offset: number, message: string, fix: string) => + findings.push({ + rule, + severity: TEMPLATE_LINT_RULES[rule], + file: template.file, + line: at(offset), + message, + fix, + }); + + let parsed: ReturnType; + try { + parsed = compiler.parseTemplate(text, template.file, { preserveWhitespaces: false }); + } catch (error) { + add( + 'template-parse-error', + 0, + `The template parser stopped: ${redactText(error instanceof Error ? error.message : String(error))}`, + 'Fix the syntax near the reported spot; `ng build` reports the same error.', + ); + return findings; + } + + const seen = new Set(); + for (const error of parsed.errors ?? []) { + if (error.level !== compiler.ParseErrorLevel.ERROR) continue; + const offset = error.span.start.offset; + if (seen.has(offset)) continue; + seen.add(offset); + add( + 'template-parse-error', + offset, + redactText(error.msg.replace(/\s+in \S+@\d+:\d+$/, '')), + 'Fix the syntax here; `ng build` fails on the same error.', + ); + } + + walk(parsed.nodes, (node) => { + if (node instanceof compiler.TmplAstElement) elementFindings(node as ElementNode, add); + else if (node instanceof compiler.TmplAstTemplate) legacyFindings(node as TemplateNode, add); + else if (compiler.TmplAstForLoopBlock && node instanceof compiler.TmplAstForLoopBlock) { + trackFindings(compiler, node as ForLoopNode, add); + } + }); + return findings; +} + +type Add = (rule: TemplateLintRule, offset: number, message: string, fix: string) => void; + +/** Keys that hold source spans, i18n metadata or raw text, never child nodes. */ +function skipKey(key: string): boolean { + return /span$/i.test(key) || ['i18n', 'source', 'location', 'errors', 'file'].includes(key); +} + +/** Visits every template node and expression under `value` once. */ +function walk(value: unknown, visit: (node: object) => void, seen = new Set()) { + if (Array.isArray(value)) { + for (const item of value) walk(item, visit, seen); + return; + } + if (!value || typeof value !== 'object' || seen.has(value)) return; + seen.add(value); + visit(value); + for (const [key, child] of Object.entries(value)) { + if (child && typeof child === 'object' && !skipKey(key)) walk(child, visit, seen); + } +} + +const LEGACY: Record = { + ngIf: '@if', + ngFor: '@for', + ngForOf: '@for', + ngSwitchCase: '@switch', + ngSwitchDefault: '@switch', +}; + +function legacyFindings(node: TemplateNode, add: Add) { + // `templateAttrs` holds only the `*` microsyntax, so `` + // is left alone. + const attr = (node.templateAttrs ?? []).find((a) => a.name in LEGACY); + if (!attr) return; + const block = LEGACY[attr.name]; + const directive = attr.name === 'ngForOf' ? 'ngFor' : attr.name; + add( + 'legacy-control-flow', + attr.sourceSpan.start.offset, + `\`*${directive}\` is the structural directive form; Angular 17 added the built-in \`${block}\` block, which needs no import and type-checks better.`, + `Rewrite it as \`${block}\`, by hand or with \`ng generate @angular/core:control-flow\`.`, + ); +} + +const INTERACTIVE = new Set([ + 'a', + 'area', + 'audio', + 'button', + 'details', + 'embed', + 'iframe', + 'input', + 'label', + 'object', + 'option', + 'select', + 'summary', + 'textarea', + 'video', +]); + +function staticValue(element: ElementNode, name: string): string | undefined { + const attr = element.attributes.find((a) => a.name.toLowerCase() === name); + return attr && typeof attr.value === 'string' ? attr.value.trim().toLowerCase() : undefined; +} + +/** Bound by `[name]`, `[attr.name]` or `bind-name`. */ +function isBound(element: ElementNode, name: string): boolean { + return element.inputs.some((input) => input.name.toLowerCase() === name); +} + +/** Hidden from assistive tech, so neither alt text nor key handling matter. */ +function isPresentational(element: ElementNode): boolean { + const role = staticValue(element, 'role'); + return ( + role === 'presentation' || + role === 'none' || + staticValue(element, 'aria-hidden') === 'true' || + isBound(element, 'role') || + isBound(element, 'aria-hidden') + ); +} + +function elementFindings(element: ElementNode, add: Add) { + const name = element.name.toLowerCase(); + const offset = element.startSourceSpan.start.offset; + + for (const directive of ['ngClass', 'ngStyle']) { + const used = + element.inputs.find((input) => input.name === directive) ?? + element.attributes.find((attr) => attr.name === directive); + if (!used) continue; + const binding = directive === 'ngClass' ? 'class' : 'style'; + add( + 'ngclass-ngstyle', + used.sourceSpan.start.offset, + `\`${directive}\` needs an import and a directive per element; \`[${binding}]\` and \`[${binding}.x]\` bindings take the same object, array or string without one.`, + `Bind \`[${binding}]\` (or \`[${binding}.name]\`) instead and drop the ${directive === 'ngClass' ? 'NgClass' : 'NgStyle'} import.`, + ); + } + + if (name === 'img' && !element.attributes.some((a) => a.name.toLowerCase() === 'alt')) { + if (!isBound(element, 'alt') && !isPresentational(element)) { + add( + 'img-missing-alt', + offset, + '`` has no `alt`, so screen readers announce the file name or nothing.', + 'Add `alt` text that says what the image shows, or `alt=""` when it is decorative.', + ); + } + } + + // Custom elements (`app-card`) and namespaced ones (`:svg:g`) are skipped: + // a component can handle keys itself. + if (INTERACTIVE.has(name) || name.includes('-') || name.includes(':')) return; + if (!element.outputs.some((output) => output.name === 'click')) return; + if (element.outputs.some((output) => /^key(down|up|press)\b/.test(output.name))) return; + if (isPresentational(element) || staticValue(element, 'contenteditable') !== undefined) return; + add( + 'click-without-key', + offset, + `\`<${name}>\` has a \`(click)\` handler but no keyboard handler, so keyboard users cannot trigger it.`, + 'Use a `