Validate rules referenced by a library's rulesets at build time - #11864
Validate rules referenced by a library's rulesets at build time#11864Timothee Guerin (timotheeguerin) wants to merge 2 commits into
Conversation
commit: |
|
All changed packages have been documented.
Show changes
|
|
You can try these changes here
|
There was a problem hiding this comment.
🟡 Changes recommended
The new validation has at least one correctness gap (library presence detection) and the new tests currently mask/omit coverage for one of the new diagnostics.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds build-time validation in @typespec/library-linter to ensure that rulesets defined by the project library don’t reference missing/renamed rules or rulesets (including those owned by other loaded libraries), reducing “ruleset rot” and shifting errors left to library build time.
Changes:
- Introduces
validateRuleSets(program)to resolve and validate rule/ruleset references used by the project library’s own rulesets. - Adds new diagnostics (
unknown-rule,unknown-rule-set,invalid-rule-reference) and documents them. - Adds a new test suite covering core validation scenarios and a
.chronusfeature entry.
File summaries
| File | Description |
|---|---|
| packages/library-linter/src/validate-rulesets.ts | New validation pass that checks ruleset references against loaded linter definitions. |
| packages/library-linter/src/linter.ts | Wires the new validation into $onValidate. |
| packages/library-linter/src/lib.ts | Registers new diagnostics for missing/invalid rule and ruleset references. |
| packages/library-linter/test/validate-rulesets.test.ts | Adds tests for the new validation behavior. |
| packages/library-linter/README.md | Documents the new diagnostics and validation scope/limits. |
| .chronus/changes/library-linter-validate-rulesets-2026-8-4-22-15-0.md | Adds a feature changelog entry describing the new capability. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 5
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟢 Approval recommended
The implementation matches the stated scope/limits, adds appropriate warning diagnostics with coverage, and includes a correct .chronus feature entry for the affected package.
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 0 new
- Review effort level: Lite
A library can declare a ruleset that enables rules owned by other libraries:
Those references are plain strings, resolved lazily and only for the rulesets a consumer actually extends. Nothing in the owning library's own build ever resolves them, so when a rule is renamed or removed the ruleset silently rots — and the first person to find out is a user compiling their spec, with an error pointing at a library they don't own.
That is exactly what happened to
@azure-tools/typespec-azure-rulesets, which still referencesuse-extensible-enumandno-fixed-enum-discriminatorlong after they were dropped fromtypespec-azure-core.@typespec/library-linteralready runs on every library build (tsp compile . --import @typespec/library-linter), and everything it needs is public API:program.jsSourceFilesexposes each loaded library's$lib/$linter, andgetSourceFileLocationContextseparates the library being compiled from its dependencies. So it now checks that every rule and ruleset a library's own rulesets reference actually resolves:Two deliberate limits:
Known gap
This does not cover ruleset packages that have no TypeSpec entrypoint.
@azure-tools/typespec-azure-rulesetsand@typespec/best-practicesare pure-JS packages with nomain.tsp, sotsp compile .is never run on them and this check never fires — which means it currently catches nothing in this repo and, notably, not the azure-rulesets breakage that motivated it. Covering those needs the same validation wired intotspd doc, the only build step they run. Worth a follow-up.