From ff72d1cdbb98686798435404e9bf5565458d6a19 Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Wed, 21 Oct 2026 14:26:16 +0800 Subject: [PATCH 1/8] docs: add linter PR review guidelines Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 111 ++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 84bc089e25..a6c0837382 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -34,6 +34,117 @@ This repo pins its development tools with [mise](https://mise.jdx.dev) via `mise - Use `pnpm change add` to add a change description for the touched packages - Types of changes are described in `.chronus/config.yaml` +## Linter PR Review Guidelines + +Use these guidelines when implementing or reviewing native TypeSpec linter rules, +including Swagger/LintDiff migrations. See the +[rule authoring guide](../website/src/content/docs/docs/howtos/contributing/creating-linter-rules.md) +for package selection, scaffolding, and registration. + +### Implementation and scope + +- Prefer existing rules, supported compiler/library APIs, and native linter callbacks over + custom walkers, adapter factories, or mutable dispatch registries. Keep helpers and + deduplication only where they serve a concrete requirement. +- State the native rule's applicability, exclusions, diagnostic targets, and enablement. + Availability in a package is not activation in a ruleset. Keep implementation, registration, + rulesets, tests, documentation, and change descriptions consistent. +- Evaluate simplifications against required behavior, not just the current implementation + or tests. Separate genuine policy decisions from behavior-preserving cleanup; do not + silently change scope, exceptions, or released suppression behavior. +- Keep unrelated sample upgrades and fixture synchronization in separate PRs when possible. + Explain dependencies, fixture provenance, and how copied baselines stay synchronized. + +### Tests + +- Keep the rule's canonical behavior suite in its owning package's `test/rules/` directory. + Test the shipped rule directly with package-local testers and supported public fixture APIs. + Do not recreate the implementation, filter away unexpected diagnostics, or use private + decorator/state APIs merely to construct a hypothetical case. +- Assert observable behavior: accepted and rejected inputs, diagnostic messages/counts, and + exact authored targets. Prefer built-in source markers to calculated string offsets. + For imported-file targets, submit the marked file through the tester's multi-file input. +- Cover distinct semantic boundaries rather than every Cartesian product. Use representative + verbs, statuses, visibility values, metadata placements, and declaration kinds when they + take the same branch. Remove cosmetic duplicates and repeated full ARM template positives. +- Preserve discriminating regressions: ambiguity and order independence, state isolation, + recursive boundaries, authored/inherited targets, and duplicate-report prevention where + the rule owns those behaviors. A test must fail if the behavior it claims to protect is + removed; an exempt fixture that passes for another reason does not prove an exemption. +- Respect responsibility boundaries. Do not repeat generic compiler behavior, suppression + machinery, or another rule's independent assertions in this suite. Retain integration + coverage for a concrete interaction or regression. Helper tests do not replace coverage + of rule-specific filtering or selection performed by the caller. +- Put package registration, legacy-name absence, and ruleset-default assertions in the + appropriate public-entry/ruleset suite rather than repeating them per semantic case. + Prefer behavioral assertions to spies on resolver calls; retain internal-path assertions + only when needed to prove a specific side-effect or performance regression. +- Use existing test/build/lint commands. Report focused tests, broader checks, external + integration results, and pending CI separately; do not turn a timeout or known failure + into a success claim. For substantial traversal/resolution changes, measure representative + base/head timing and memory rather than treating diagnostic counts as performance evidence. + +### Rule documentation + +- Write for TypeSpec specification authors: explain what is required, where it applies, + why it matters, and how to correct a violation. Use native TypeSpec concepts and concise, + actionable language rather than implementation terminology. +- Do not fill the public rule page with visitor mechanics, test strategy, internal helper + details, or instructions for AI implementers. Keep implementation notes in contributor + material or PR evidence, not the author-facing explanation. +- Keep legacy validator mappings and parity limitations in a dedicated migration/provenance + section, such as `LintDiff Equivalent`, rather than interleaving them with native guidance. + Discuss emitted OpenAPI only when it explains a real user-facing requirement or consequence. +- Edit the authored `packages//src/rules/.md` and regenerate public reference + documentation with the owning package's existing command; do not patch generated pages alone. + +### Documentation examples + +- Show the smallest incorrect/correct pair that exposes the rule-owned difference. Omit + repeated imports, service/provider setup, resource scaffolding, and unchanged defaults; + state assumed context when necessary. Examples need not each be standalone specifications. +- Choose the intended audience explicitly. For ARM resource scenarios, prefer standard ARM + templates and supported named customization arguments over handwritten routes/operations. + Use the appropriate standard templates for other audiences when available. If no supported + template can demonstrate the violation, explain that limitation in PR evidence. +- Ensure both sides reach the intended check: the correction must fix the violation rather + than escape through an unrelated exemption. Preserve relevant key/wire names, payload fields, + response shapes, or decorator placement while removing irrelevant boilerplate. +- Validate the actual published excerpts with the owning rule and minimal explicit public + setup, checking the intended diagnostic contrast. A larger test inspired by an example + does not validate the example's current text. Distinguish excerpt validation from standalone + compilation and continuous coverage; do not add bespoke Markdown parsers or fence-count tests. + +### Changelog entries + +- Run `pnpm change add` from the Azure repository root. Commit unreleased Azure package entries + under **`.chronus/changes/` at the repository root**, not `core/.chronus/changes/`, a package-local + folder, or a generated `CHANGELOG.md`. `core/` is the separate TypeSpec compiler submodule. +- Use the actual npm package names in `packages` and a `changeKind` from `.chronus/config.yaml`. + Describe the user-visible result: name the rule, the violation/fix, and any default ruleset + enablement or compatibility impact. Distinguish introducing a rule from registering it disabled. + For example: + + ```markdown + --- + changeKind: feature + packages: + - "@azure-tools/typespec-azure-resource-manager" + --- + + Add the `resource-missing-read` lint rule requiring a read operation for ARM resources with create or update operations. + ``` + +- Inspect existing task-owned unreleased entries first. Update the existing feature/fix entry + to describe the final shipped behavior instead of adding a new entry for each review round. + Do not add a redundant `internal` entry for test pruning, helper reuse, formatting, or + syncing with the base branch when the same package/change already has user-facing coverage. +- Preserve unrelated entries and distinct package coverage, including a rulesets package when + its registration/enablement changes. Choose `feature`, `fix`, `breaking`, or another configured + kind according to the actual effect, not merely the fact that the PR is a migration. +- Run `pnpm chronus status` to check coverage. Do not add meaningless entries just to satisfy + package detection; Markdown and test-only changes are excluded by `.chronus/config.yaml`. + ## Branch and PR Workflow - When creating worktrees or branches for new work, base them off the main Azure fork's `main` branch (Azure/typespec-azure). Depending on the user's local git remote setup, this may be called `upstream` or `origin`. From c01ae3abb1caf37f86ba662f7a1c20831335eee7 Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Wed, 21 Oct 2026 15:00:01 +0800 Subject: [PATCH 2/8] docs: generalize and shorten linter review guidance Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 123 +++++++++----------------------- 1 file changed, 32 insertions(+), 91 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index a6c0837382..d279e89ea8 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -36,114 +36,55 @@ This repo pins its development tools with [mise](https://mise.jdx.dev) via `mise ## Linter PR Review Guidelines -Use these guidelines when implementing or reviewing native TypeSpec linter rules, -including Swagger/LintDiff migrations. See the +Apply these guidelines to all linter PRs. See the [rule authoring guide](../website/src/content/docs/docs/howtos/contributing/creating-linter-rules.md) for package selection, scaffolding, and registration. ### Implementation and scope -- Prefer existing rules, supported compiler/library APIs, and native linter callbacks over - custom walkers, adapter factories, or mutable dispatch registries. Keep helpers and - deduplication only where they serve a concrete requirement. -- State the native rule's applicability, exclusions, diagnostic targets, and enablement. - Availability in a package is not activation in a ruleset. Keep implementation, registration, - rulesets, tests, documentation, and change descriptions consistent. -- Evaluate simplifications against required behavior, not just the current implementation - or tests. Separate genuine policy decisions from behavior-preserving cleanup; do not - silently change scope, exceptions, or released suppression behavior. -- Keep unrelated sample upgrades and fixture synchronization in separate PRs when possible. - Explain dependencies, fixture provenance, and how copied baselines stay synchronized. +- Prefer existing rules, supported APIs, and native linter callbacks over custom infrastructure. +- Keep applicability, exceptions, targets, and enablement consistent across code, registration, + rulesets, tests, docs, and changelogs. Distinguish registration from ruleset activation. +- Keep PRs focused. Separate behavior-preserving cleanup from intentional policy changes. ### Tests -- Keep the rule's canonical behavior suite in its owning package's `test/rules/` directory. - Test the shipped rule directly with package-local testers and supported public fixture APIs. - Do not recreate the implementation, filter away unexpected diagnostics, or use private - decorator/state APIs merely to construct a hypothetical case. -- Assert observable behavior: accepted and rejected inputs, diagnostic messages/counts, and - exact authored targets. Prefer built-in source markers to calculated string offsets. - For imported-file targets, submit the marked file through the tester's multi-file input. -- Cover distinct semantic boundaries rather than every Cartesian product. Use representative - verbs, statuses, visibility values, metadata placements, and declaration kinds when they - take the same branch. Remove cosmetic duplicates and repeated full ARM template positives. -- Preserve discriminating regressions: ambiguity and order independence, state isolation, - recursive boundaries, authored/inherited targets, and duplicate-report prevention where - the rule owns those behaviors. A test must fail if the behavior it claims to protect is - removed; an exempt fixture that passes for another reason does not prove an exemption. -- Respect responsibility boundaries. Do not repeat generic compiler behavior, suppression - machinery, or another rule's independent assertions in this suite. Retain integration - coverage for a concrete interaction or regression. Helper tests do not replace coverage - of rule-specific filtering or selection performed by the caller. -- Put package registration, legacy-name absence, and ruleset-default assertions in the - appropriate public-entry/ruleset suite rather than repeating them per semantic case. - Prefer behavioral assertions to spies on resolver calls; retain internal-path assertions - only when needed to prove a specific side-effect or performance regression. -- Use existing test/build/lint commands. Report focused tests, broader checks, external - integration results, and pending CI separately; do not turn a timeout or known failure - into a success claim. For substantial traversal/resolution changes, measure representative - base/head timing and memory rather than treating diagnostic counts as performance evidence. +- Test the shipped rule in its owning package's `test/rules/` suite using public APIs and + package-local testers. Assert accepted/rejected inputs, diagnostic messages/counts, and + authored targets; use built-in source markers instead of calculated offsets. +- Cover distinct boundaries with representative cases, not exhaustive cross-products. + Preserve regressions that fail when the protected behavior is removed. +- Avoid duplicating compiler behavior or another rule's independent assertions. Keep integration + tests for concrete interactions and registration/default checks in public-entry/ruleset suites. + Prefer behavioral assertions; do not hide unexpected diagnostics with filtering. +- Run existing checks and distinguish local results from pending or failed CI. + Measure representative timing and memory for substantial traversal/resolution changes. ### Rule documentation -- Write for TypeSpec specification authors: explain what is required, where it applies, - why it matters, and how to correct a violation. Use native TypeSpec concepts and concise, - actionable language rather than implementation terminology. -- Do not fill the public rule page with visitor mechanics, test strategy, internal helper - details, or instructions for AI implementers. Keep implementation notes in contributor - material or PR evidence, not the author-facing explanation. -- Keep legacy validator mappings and parity limitations in a dedicated migration/provenance - section, such as `LintDiff Equivalent`, rather than interleaving them with native guidance. - Discuss emitted OpenAPI only when it explains a real user-facing requirement or consequence. -- Edit the authored `packages//src/rules/.md` and regenerate public reference - documentation with the owning package's existing command; do not patch generated pages alone. +- Write for TypeSpec authors: explain the requirement, applicability, rationale, and correction. + Keep implementation details and agent instructions out of public rule pages. +- Edit `packages//src/rules/.md` and regenerate reference docs with the owning + package's command; do not patch generated pages alone. ### Documentation examples -- Show the smallest incorrect/correct pair that exposes the rule-owned difference. Omit - repeated imports, service/provider setup, resource scaffolding, and unchanged defaults; - state assumed context when necessary. Examples need not each be standalone specifications. -- Choose the intended audience explicitly. For ARM resource scenarios, prefer standard ARM - templates and supported named customization arguments over handwritten routes/operations. - Use the appropriate standard templates for other audiences when available. If no supported - template can demonstrate the violation, explain that limitation in PR evidence. -- Ensure both sides reach the intended check: the correction must fix the violation rather - than escape through an unrelated exemption. Preserve relevant key/wire names, payload fields, - response shapes, or decorator placement while removing irrelevant boilerplate. -- Validate the actual published excerpts with the owning rule and minimal explicit public - setup, checking the intended diagnostic contrast. A larger test inspired by an example - does not validate the example's current text. Distinguish excerpt validation from standalone - compilation and continuous coverage; do not add bespoke Markdown parsers or fence-count tests. +- Show a minimal incorrect/correct pair. Omit unrelated setup and state assumed context; + each excerpt need not be standalone. +- Prefer audience-appropriate standard templates and supported customization arguments. + Both examples must reach the intended check; fix the violation rather than bypassing it. +- Validate the published excerpts with the rule and explicit public setup, not just similar + larger fixtures. Do not add bespoke Markdown parsers or fence-count tests. ### Changelog entries -- Run `pnpm change add` from the Azure repository root. Commit unreleased Azure package entries - under **`.chronus/changes/` at the repository root**, not `core/.chronus/changes/`, a package-local - folder, or a generated `CHANGELOG.md`. `core/` is the separate TypeSpec compiler submodule. -- Use the actual npm package names in `packages` and a `changeKind` from `.chronus/config.yaml`. - Describe the user-visible result: name the rule, the violation/fix, and any default ruleset - enablement or compatibility impact. Distinguish introducing a rule from registering it disabled. - For example: - - ```markdown - --- - changeKind: feature - packages: - - "@azure-tools/typespec-azure-resource-manager" - --- - - Add the `resource-missing-read` lint rule requiring a read operation for ARM resources with create or update operations. - ``` - -- Inspect existing task-owned unreleased entries first. Update the existing feature/fix entry - to describe the final shipped behavior instead of adding a new entry for each review round. - Do not add a redundant `internal` entry for test pruning, helper reuse, formatting, or - syncing with the base branch when the same package/change already has user-facing coverage. -- Preserve unrelated entries and distinct package coverage, including a rulesets package when - its registration/enablement changes. Choose `feature`, `fix`, `breaking`, or another configured - kind according to the actual effect, not merely the fact that the PR is a migration. -- Run `pnpm chronus status` to check coverage. Do not add meaningless entries just to satisfy - package detection; Markdown and test-only changes are excluded by `.chronus/config.yaml`. +- Run `pnpm change add` from the repository root. Put Azure entries in root **`.chronus/changes/`**, + not `core/.chronus/changes/`, package folders, or generated `CHANGELOG.md` files. +- Use affected npm package names and a `changeKind` from `.chronus/config.yaml`. Describe the + final user-visible behavior, rule enablement, and compatibility impact. +- Update existing entries for the same unreleased change instead of adding redundant `internal` + entries for review cleanup. Preserve unrelated entries and distinct package coverage. + Check coverage with `pnpm chronus status`. ## Branch and PR Workflow From eaad429bdd3758d0089722ac757cd1aef8cb124a Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Wed, 21 Oct 2026 15:08:05 +0800 Subject: [PATCH 3/8] docs: focus changelog guidance on review updates Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index d279e89ea8..d403999c00 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -78,13 +78,8 @@ for package selection, scaffolding, and registration. ### Changelog entries -- Run `pnpm change add` from the repository root. Put Azure entries in root **`.chronus/changes/`**, - not `core/.chronus/changes/`, package folders, or generated `CHANGELOG.md` files. -- Use affected npm package names and a `changeKind` from `.chronus/config.yaml`. Describe the - final user-visible behavior, rule enablement, and compatibility impact. -- Update existing entries for the same unreleased change instead of adding redundant `internal` - entries for review cleanup. Preserve unrelated entries and distinct package coverage. - Check coverage with `pnpm chronus status`. +- For changes made in response to review comments, update the existing changelog entry + rather than adding a redundant one. ## Branch and PR Workflow From e117c12a23ce60ca98ddbb6380cca85a783d487c Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Wed, 21 Oct 2026 15:14:11 +0800 Subject: [PATCH 4/8] docs: direct changelog guidance at reviewing agents Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index d403999c00..39de4e475f 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -78,8 +78,8 @@ for package selection, scaffolding, and registration. ### Changelog entries -- For changes made in response to review comments, update the existing changelog entry - rather than adding a redundant one. +- When reviewing a PR, check for redundant changelog entries. Ensure entries describe the + final changes, not the PR's revision history or intermediate review-driven cleanup. ## Branch and PR Workflow From e8d2308a2dfbd311beb67dd0162736ab95c2766a Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Wed, 21 Oct 2026 15:28:52 +0800 Subject: [PATCH 5/8] docs: strengthen linter PR review guidance Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 39de4e475f..9391bb6538 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -43,6 +43,12 @@ for package selection, scaffolding, and registration. ### Implementation and scope - Prefer existing rules, supported APIs, and native linter callbacks over custom infrastructure. +- Evaluate simplification and consolidation against the native guideline and released + compatibility, not merely the proposed design or its tests. Justify custom resolvers and + special cases with supported inputs that need them. +- Use the narrowest semantic APIs needed. Prefer native types and structured metadata over + generated-output heuristics; handle returned resolution diagnostics explicitly rather than + treating failed resolution as compliance. - Keep applicability, exceptions, targets, and enablement consistent across code, registration, rulesets, tests, docs, and changelogs. Distinguish registration from ruleset activation. - Keep PRs focused. Separate behavior-preserving cleanup from intentional policy changes. @@ -52,11 +58,17 @@ for package selection, scaffolding, and registration. - Test the shipped rule in its owning package's `test/rules/` suite using public APIs and package-local testers. Assert accepted/rejected inputs, diagnostic messages/counts, and authored targets; use built-in source markers instead of calculated offsets. +- Extend the existing canonical behavior suite for each rule, including when its filename + differs from the public rule name. Avoid duplicate suites and per-rule tests that inspect + implementation source to enforce architecture. - Cover distinct boundaries with representative cases, not exhaustive cross-products. Preserve regressions that fail when the protected behavior is removed. - Avoid duplicating compiler behavior or another rule's independent assertions. Keep integration tests for concrete interactions and registration/default checks in public-entry/ruleset suites. Prefer behavioral assertions; do not hide unexpected diagnostics with filtering. +- Reconcile final PR acceptance claims with evidence at the appropriate test layer. Preserve + distinct rule-owned coverage during cleanup; remove framework-only cases and stale claims + rather than relocating redundant tests. - Run existing checks and distinguish local results from pending or failed CI. Measure representative timing and memory for substantial traversal/resolution changes. @@ -73,8 +85,10 @@ for package selection, scaffolding, and registration. each excerpt need not be standalone. - Prefer audience-appropriate standard templates and supported customization arguments. Both examples must reach the intended check; fix the violation rather than bypassing it. -- Validate the published excerpts with the rule and explicit public setup, not just similar - larger fixtures. Do not add bespoke Markdown parsers or fence-count tests. +- Validate the actual published excerpts with the target rule explicitly enabled and + reproducible public setup. Both must compile without unrelated errors; assert the intended + diagnostics for the incorrect example and none from that rule for the correction. + Do not add bespoke Markdown parsers or fence-count tests. ### Changelog entries From 4625ab093b60bd6cdbdf8a1c41004b5e57b85c54 Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Thu, 22 Oct 2026 10:27:44 +0800 Subject: [PATCH 6/8] Update .github/copilot-instructions.md Co-authored-by: Timothee Guerin --- .github/copilot-instructions.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 9391bb6538..a8e671edcb 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -42,7 +42,8 @@ for package selection, scaffolding, and registration. ### Implementation and scope -- Prefer existing rules, supported APIs, and native linter callbacks over custom infrastructure. +- Reuse existing rules, supported APIs and native callbacks; avoid custom dispatchers or redundant + program walks. Collect cross-node state in callbacks and report in `exit` when needed. - Evaluate simplification and consolidation against the native guideline and released compatibility, not merely the proposed design or its tests. Justify custom resolvers and special cases with supported inputs that need them. From be0087eb32b62c1932a8aea25b6368644e1820cb Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Thu, 22 Oct 2026 10:27:54 +0800 Subject: [PATCH 7/8] Update .github/copilot-instructions.md Co-authored-by: Timothee Guerin --- .github/copilot-instructions.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index a8e671edcb..570bcbdaf9 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -56,9 +56,9 @@ for package selection, scaffolding, and registration. ### Tests -- Test the shipped rule in its owning package's `test/rules/` suite using public APIs and - package-local testers. Assert accepted/rejected inputs, diagnostic messages/counts, and - authored targets; use built-in source markers instead of calculated offsets. +- Use shared package testers and `createLinterRuleTester`; no file-local `createTester` definitions. + Assert shipped behavior through valid/invalid inputs and diagnostic messages/counts/targets. + Use built-in markers, not calculated offsets. - Extend the existing canonical behavior suite for each rule, including when its filename differs from the public rule name. Avoid duplicate suites and per-rule tests that inspect implementation source to enforce architecture. From e4b3d3cc4326e408081e518d586acd58cbfb155c Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Thu, 22 Oct 2026 10:36:55 +0800 Subject: [PATCH 8/8] docs: move linter review guidance into code-review skill Make lean, rule-scoped test coverage explicit while preserving distinct regression coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 62 ---------------------------- .github/skills/code-review/SKILL.md | 64 +++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 62 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 570bcbdaf9..84bc089e25 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -34,68 +34,6 @@ This repo pins its development tools with [mise](https://mise.jdx.dev) via `mise - Use `pnpm change add` to add a change description for the touched packages - Types of changes are described in `.chronus/config.yaml` -## Linter PR Review Guidelines - -Apply these guidelines to all linter PRs. See the -[rule authoring guide](../website/src/content/docs/docs/howtos/contributing/creating-linter-rules.md) -for package selection, scaffolding, and registration. - -### Implementation and scope - -- Reuse existing rules, supported APIs and native callbacks; avoid custom dispatchers or redundant - program walks. Collect cross-node state in callbacks and report in `exit` when needed. -- Evaluate simplification and consolidation against the native guideline and released - compatibility, not merely the proposed design or its tests. Justify custom resolvers and - special cases with supported inputs that need them. -- Use the narrowest semantic APIs needed. Prefer native types and structured metadata over - generated-output heuristics; handle returned resolution diagnostics explicitly rather than - treating failed resolution as compliance. -- Keep applicability, exceptions, targets, and enablement consistent across code, registration, - rulesets, tests, docs, and changelogs. Distinguish registration from ruleset activation. -- Keep PRs focused. Separate behavior-preserving cleanup from intentional policy changes. - -### Tests - -- Use shared package testers and `createLinterRuleTester`; no file-local `createTester` definitions. - Assert shipped behavior through valid/invalid inputs and diagnostic messages/counts/targets. - Use built-in markers, not calculated offsets. -- Extend the existing canonical behavior suite for each rule, including when its filename - differs from the public rule name. Avoid duplicate suites and per-rule tests that inspect - implementation source to enforce architecture. -- Cover distinct boundaries with representative cases, not exhaustive cross-products. - Preserve regressions that fail when the protected behavior is removed. -- Avoid duplicating compiler behavior or another rule's independent assertions. Keep integration - tests for concrete interactions and registration/default checks in public-entry/ruleset suites. - Prefer behavioral assertions; do not hide unexpected diagnostics with filtering. -- Reconcile final PR acceptance claims with evidence at the appropriate test layer. Preserve - distinct rule-owned coverage during cleanup; remove framework-only cases and stale claims - rather than relocating redundant tests. -- Run existing checks and distinguish local results from pending or failed CI. - Measure representative timing and memory for substantial traversal/resolution changes. - -### Rule documentation - -- Write for TypeSpec authors: explain the requirement, applicability, rationale, and correction. - Keep implementation details and agent instructions out of public rule pages. -- Edit `packages//src/rules/.md` and regenerate reference docs with the owning - package's command; do not patch generated pages alone. - -### Documentation examples - -- Show a minimal incorrect/correct pair. Omit unrelated setup and state assumed context; - each excerpt need not be standalone. -- Prefer audience-appropriate standard templates and supported customization arguments. - Both examples must reach the intended check; fix the violation rather than bypassing it. -- Validate the actual published excerpts with the target rule explicitly enabled and - reproducible public setup. Both must compile without unrelated errors; assert the intended - diagnostics for the incorrect example and none from that rule for the correction. - Do not add bespoke Markdown parsers or fence-count tests. - -### Changelog entries - -- When reviewing a PR, check for redundant changelog entries. Ensure entries describe the - final changes, not the PR's revision history or intermediate review-driven cleanup. - ## Branch and PR Workflow - When creating worktrees or branches for new work, base them off the main Azure fork's `main` branch (Azure/typespec-azure). Depending on the user's local git remote setup, this may be called `upstream` or `origin`. diff --git a/.github/skills/code-review/SKILL.md b/.github/skills/code-review/SKILL.md index 6a5d512659..5ba772f933 100644 --- a/.github/skills/code-review/SKILL.md +++ b/.github/skills/code-review/SKILL.md @@ -133,3 +133,67 @@ files even though most generated client implementation files are ignored. cite the governing rule or the relevant `file:line`, and suggest a concrete fix in one or two sentences. Prioritize correctness, compatibility, release impact, and meaningful coverage; don't repeat another finding or post speculative suggestions. + +## Linter PR review guidelines + +Apply these guidelines to all linter PRs. See the +[rule authoring guide](../../../website/src/content/docs/docs/howtos/contributing/creating-linter-rules.md) +for package selection, scaffolding, and registration. + +### Implementation and scope + +- Reuse existing rules, supported APIs and native callbacks; avoid custom dispatchers or redundant + program walks. Collect cross-node state in callbacks and report in `exit` when needed. +- Evaluate simplification and consolidation against the native guideline and released + compatibility, not merely the proposed design or its tests. Justify custom resolvers and + special cases with supported inputs that need them. +- Use the narrowest semantic APIs needed. Prefer native types and structured metadata over + generated-output heuristics; handle returned resolution diagnostics explicitly rather than + treating failed resolution as compliance. +- Keep applicability, exceptions, targets, and enablement consistent across code, registration, + rulesets, tests, docs, and changelogs. Distinguish registration from ruleset activation. +- Keep PRs focused. Separate behavior-preserving cleanup from intentional policy changes. + +### Tests + +- Use shared package testers and `createLinterRuleTester`; no file-local `createTester` definitions. + Assert shipped behavior through valid/invalid inputs and diagnostic messages/counts/targets. + Use built-in markers, not calculated offsets. +- Extend the existing canonical behavior suite for each rule, including when its filename + differs from the public rule name. Avoid duplicate suites and per-rule tests that inspect + implementation source to enforce architecture. +- Keep tests lean and scoped to the rule's responsibilities. Use representative valid/invalid + cases and distinct boundaries, not exhaustive cross-products; avoid redundant permutations + and assertions about unrelated behavior. Preserve regressions that protect distinct + rule-owned behavior. +- Avoid duplicating compiler behavior or another rule's independent assertions. Keep integration + tests for concrete interactions and registration/default checks in public-entry/ruleset suites. + Prefer behavioral assertions; do not hide unexpected diagnostics with filtering. +- Reconcile final PR acceptance claims with evidence at the appropriate test layer. Preserve + distinct rule-owned coverage during cleanup; remove framework-only cases and stale claims + rather than relocating redundant tests. +- Run existing checks and distinguish local results from pending or failed CI. + Measure representative timing and memory for substantial traversal/resolution changes. + +### Rule documentation + +- Write for TypeSpec authors: explain the requirement, applicability, rationale, and correction. + Keep implementation details and agent instructions out of public rule pages. +- Edit `packages//src/rules/.md` and regenerate reference docs with the owning + package's command; do not patch generated pages alone. + +### Documentation examples + +- Show a minimal incorrect/correct pair. Omit unrelated setup and state assumed context; + each excerpt need not be standalone. +- Prefer audience-appropriate standard templates and supported customization arguments. + Both examples must reach the intended check; fix the violation rather than bypassing it. +- Validate the actual published excerpts with the target rule explicitly enabled and + reproducible public setup. Both must compile without unrelated errors; assert the intended + diagnostics for the incorrect example and none from that rule for the correction. + Do not add bespoke Markdown parsers or fence-count tests. + +### Changelog entries + +- When reviewing a PR, check for redundant changelog entries. Ensure entries describe the + final changes, not the PR's revision history or intermediate review-driven cleanup.