From c76b72010cf565176cfa36295a9bdc44dce8cc16 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 28 May 2024 09:15:32 -0700 Subject: [PATCH 1/7] Linter `all` rulesets automatically created --- packages/compiler/src/core/linter.ts | 39 ++++++++++++++------ packages/compiler/src/core/types.ts | 8 +++++ packages/compiler/test/core/linter.test.ts | 42 ++++++++++++++++++++-- packages/http/src/linter.ts | 7 ---- 4 files changed, 77 insertions(+), 19 deletions(-) diff --git a/packages/compiler/src/core/linter.ts b/packages/compiler/src/core/linter.ts index cbb6a7f8cc..30f727da50 100644 --- a/packages/compiler/src/core/linter.ts +++ b/packages/compiler/src/core/linter.ts @@ -7,7 +7,7 @@ import { Diagnostic, DiagnosticMessages, LibraryInstance, - LinterDefinition, + LinterResolvedDefinition, LinterRule, LinterRuleContext, LinterRuleDiagnosticReport, @@ -37,9 +37,30 @@ export function createLinter( lint, }; - function getLinterDefinition(library: LibraryInstance): LinterDefinition | undefined { + function getLinterDefinition(library: LibraryInstance): LinterResolvedDefinition | undefined { // eslint-disable-next-line deprecation/deprecation - return library?.linter ?? library?.definition?.linter; + const linter = library?.linter ?? library?.definition?.linter; + + const name = library.metadata.name; + const rules: LinterRule[] = linter.rules.map((rule) => { + return { ...rule, id: `${name}/${rule.name}` }; + }); + if (linter.ruleSets && "all" in linter.ruleSets) { + return { + rules, + ruleSets: linter.ruleSets as any, + }; + } else { + return { + rules, + ruleSets: { + all: { + enable: Object.fromEntries(rules.map((x) => [x.id, true])) as any, + }, + ...linter.ruleSets, + }, + }; + } } async function extendRuleSet(ruleSet: LinterRuleSet): Promise { @@ -148,17 +169,15 @@ export function createLinter( const library = await loadLibrary(name); const linter = library && getLinterDefinition(library); if (linter?.rules) { - for (const ruleDef of linter.rules) { - const ruleId = `${name}/${ruleDef.name}`; + for (const rule of linter.rules) { tracer.trace( "register-library.rule", - `Registering rule "${ruleId}" for library "${name}".` + `Registering rule "${rule.id}" for library "${name}".` ); - const rule: LinterRule = { ...ruleDef, id: ruleId }; - if (ruleMap.has(ruleId)) { - compilerAssert(false, `Unexpected duplicate linter rule: "${ruleId}"`); + if (ruleMap.has(rule.id)) { + compilerAssert(false, `Unexpected duplicate linter rule: "${rule.id}"`); } else { - ruleMap.set(ruleId, rule); + ruleMap.set(rule.id, rule); } } } diff --git a/packages/compiler/src/core/types.ts b/packages/compiler/src/core/types.ts index acce9553f9..152b38ce66 100644 --- a/packages/compiler/src/core/types.ts +++ b/packages/compiler/src/core/types.ts @@ -2437,6 +2437,14 @@ export interface LinterDefinition { ruleSets?: Record; } +export interface LinterResolvedDefinition { + readonly rules: LinterRule[]; + readonly ruleSets: { + all: LinterRuleSet; + [name: string]: LinterRuleSet; + }; +} + export interface LinterRuleDefinition { /** Rule name (without the library name) */ name: N; diff --git a/packages/compiler/test/core/linter.test.ts b/packages/compiler/test/core/linter.test.ts index 4f13b5fe31..b8d34dd68a 100644 --- a/packages/compiler/test/core/linter.test.ts +++ b/packages/compiler/test/core/linter.test.ts @@ -45,10 +45,10 @@ describe("compiler: linter", () => { const library: LibraryInstance = { entrypoint: undefined, - metadata: { type: "module", name: "@typespec/test" }, + metadata: { type: "module", name: "@typespec/test-linter" }, module: { type: "module", path: "", mainFile: "", manifest: { name: "", version: "" } }, definition: createTypeSpecLibrary({ - name: "@typespec/test", + name: "@typespec/test-linter", diagnostics: {}, }), linter: linterDef, @@ -212,6 +212,44 @@ describe("compiler: linter", () => { }); }); + describe("when enabling a ruleset", () => { + it("/all ruleset is automatically provided and include all rules", async () => { + const linter = await createTestLinter(`model Foo {}`, { + rules: [noModelFoo], + }); + expectDiagnosticEmpty( + await linter.extendRuleSet({ + extends: ["@typespec/test-linter/all"], + }) + ); + expectDiagnostics(linter.lint(), { + severity: "warning", + code: "@typespec/test-linter/no-model-foo", + message: `Cannot call model 'Foo'`, + }); + }); + it("extending specific ruleset enable the rules inside", async () => { + const linter = await createTestLinter(`model Foo {}`, { + rules: [noModelFoo], + ruleSets: { + custom: { + enable: { "@typespec/test-linter/no-model-foo": true }, + }, + }, + }); + expectDiagnosticEmpty( + await linter.extendRuleSet({ + extends: ["@typespec/test-linter/custom"], + }) + ); + expectDiagnostics(linter.lint(), { + severity: "warning", + code: "@typespec/test-linter/no-model-foo", + message: `Cannot call model 'Foo'`, + }); + }); + }); + describe("(integration) loading in program", () => { async function diagnoseReal(code: string) { const host = await createTestHost(); diff --git a/packages/http/src/linter.ts b/packages/http/src/linter.ts index 19f5dae93e..c63f26562f 100644 --- a/packages/http/src/linter.ts +++ b/packages/http/src/linter.ts @@ -3,11 +3,4 @@ import { opReferenceContainerRouteRule } from "./rules/op-reference-container-ro export const $linter = defineLinter({ rules: [opReferenceContainerRouteRule], - ruleSets: { - all: { - enable: { - [`@typespec/http/${opReferenceContainerRouteRule.name}`]: true, - }, - }, - }, }); From 699b5de7fda8c734b9b0084f4979d0002bf0afe6 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 28 May 2024 09:19:31 -0700 Subject: [PATCH 2/7] Create linter-auto-all-ruleset-2024-4-28-16-18-46.md --- .../changes/linter-auto-all-ruleset-2024-4-28-16-18-46.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .chronus/changes/linter-auto-all-ruleset-2024-4-28-16-18-46.md diff --git a/.chronus/changes/linter-auto-all-ruleset-2024-4-28-16-18-46.md b/.chronus/changes/linter-auto-all-ruleset-2024-4-28-16-18-46.md new file mode 100644 index 0000000000..5448e32ca5 --- /dev/null +++ b/.chronus/changes/linter-auto-all-ruleset-2024-4-28-16-18-46.md @@ -0,0 +1,8 @@ +--- +# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking +changeKind: feature +packages: + - "@typespec/compiler" +--- + +Linter `all` rulesets is automatically created if not explicitly provided From bb0217da5d3f707a22df149adc992e8cf38cb519 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 28 May 2024 09:20:16 -0700 Subject: [PATCH 3/7] Create linter-auto-all-ruleset-2024-4-28-16-18-47.md --- .../changes/linter-auto-all-ruleset-2024-4-28-16-18-47.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .chronus/changes/linter-auto-all-ruleset-2024-4-28-16-18-47.md diff --git a/.chronus/changes/linter-auto-all-ruleset-2024-4-28-16-18-47.md b/.chronus/changes/linter-auto-all-ruleset-2024-4-28-16-18-47.md new file mode 100644 index 0000000000..ceea70622e --- /dev/null +++ b/.chronus/changes/linter-auto-all-ruleset-2024-4-28-16-18-47.md @@ -0,0 +1,8 @@ +--- +# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking +changeKind: feature +packages: + - "@typespec/http" +--- + +Use new compiler automatic `all` ruleset instead of explicitly provided one From dbe2fef2b7cb24d57e36dc80b549d44d6c7f888d Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 28 May 2024 09:46:14 -0700 Subject: [PATCH 4/7] fix --- packages/compiler/src/core/linter.ts | 50 ++++++++++++---------- packages/compiler/src/core/program.ts | 6 ++- packages/compiler/src/core/types.ts | 2 +- packages/compiler/test/core/linter.test.ts | 4 +- 4 files changed, 35 insertions(+), 27 deletions(-) diff --git a/packages/compiler/src/core/linter.ts b/packages/compiler/src/core/linter.ts index 30f727da50..448a007888 100644 --- a/packages/compiler/src/core/linter.ts +++ b/packages/compiler/src/core/linter.ts @@ -7,6 +7,7 @@ import { Diagnostic, DiagnosticMessages, LibraryInstance, + LinterDefinition, LinterResolvedDefinition, LinterRule, LinterRuleContext, @@ -22,6 +23,32 @@ export interface Linter { lint(): readonly Diagnostic[]; } +/** Resolve the linter definition */ +export function resolveLinterDefinition( + libName: string, + linter: LinterDefinition +): LinterResolvedDefinition { + const rules: LinterRule[] = linter.rules.map((rule) => { + return { ...rule, id: `${libName}/${rule.name}` }; + }); + if (linter.ruleSets && "all" in linter.ruleSets) { + return { + rules, + ruleSets: linter.ruleSets as any, + }; + } else { + return { + rules, + ruleSets: { + all: { + enable: Object.fromEntries(rules.map((x) => [x.id, true])) as any, + }, + ...linter.ruleSets, + }, + }; + } +} + export function createLinter( program: Program, loadLibrary: (name: string) => Promise @@ -39,28 +66,7 @@ export function createLinter( function getLinterDefinition(library: LibraryInstance): LinterResolvedDefinition | undefined { // eslint-disable-next-line deprecation/deprecation - const linter = library?.linter ?? library?.definition?.linter; - - const name = library.metadata.name; - const rules: LinterRule[] = linter.rules.map((rule) => { - return { ...rule, id: `${name}/${rule.name}` }; - }); - if (linter.ruleSets && "all" in linter.ruleSets) { - return { - rules, - ruleSets: linter.ruleSets as any, - }; - } else { - return { - rules, - ruleSets: { - all: { - enable: Object.fromEntries(rules.map((x) => [x.id, true])) as any, - }, - ...linter.ruleSets, - }, - }; - } + return library?.linter ?? library?.definition?.linter; } async function extendRuleSet(ruleSet: LinterRuleSet): Promise { diff --git a/packages/compiler/src/core/program.ts b/packages/compiler/src/core/program.ts index 19ddea2e8a..048da4fc09 100644 --- a/packages/compiler/src/core/program.ts +++ b/packages/compiler/src/core/program.ts @@ -21,7 +21,7 @@ import { } from "./entrypoint-resolution.js"; import { ExternalError } from "./external-error.js"; import { getLibraryUrlsLoaded } from "./library.js"; -import { createLinter } from "./linter.js"; +import { createLinter, resolveLinterDefinition } from "./linter.js"; import { createLogger } from "./logger/index.js"; import { createTracer } from "./logger/tracer.js"; import { createDiagnostic } from "./messages.js"; @@ -585,7 +585,9 @@ export async function compile( ...resolution, metadata, definition: libDefinition, - linter: entrypoint?.esmExports.$linter, + linter: + entrypoint?.esmExports.$linter && + resolveLinterDefinition(libraryNameOrPath, entrypoint.esmExports.$linter), }; } diff --git a/packages/compiler/src/core/types.ts b/packages/compiler/src/core/types.ts index 152b38ce66..8c2eeab612 100644 --- a/packages/compiler/src/core/types.ts +++ b/packages/compiler/src/core/types.ts @@ -2047,7 +2047,7 @@ export interface LibraryInstance { entrypoint: JsSourceFileNode | undefined; metadata: LibraryMetadata; definition?: TypeSpecLibrary; - linter: LinterDefinition; + linter: LinterResolvedDefinition; } export type LibraryMetadata = FileLibraryMetadata | ModuleLibraryMetadata; diff --git a/packages/compiler/test/core/linter.test.ts b/packages/compiler/test/core/linter.test.ts index b8d34dd68a..66392c6c29 100644 --- a/packages/compiler/test/core/linter.test.ts +++ b/packages/compiler/test/core/linter.test.ts @@ -1,7 +1,7 @@ import { describe, it } from "vitest"; import { createLinterRule, createTypeSpecLibrary } from "../../src/core/library.js"; -import { Linter, createLinter } from "../../src/core/linter.js"; +import { Linter, createLinter, resolveLinterDefinition } from "../../src/core/linter.js"; import type { LibraryInstance, LinterDefinition } from "../../src/index.js"; import { createTestHost, @@ -51,7 +51,7 @@ describe("compiler: linter", () => { name: "@typespec/test-linter", diagnostics: {}, }), - linter: linterDef, + linter: resolveLinterDefinition("@typespec/test-linter", linterDef), }; await host.compile("main.tsp"); From 6066eb256da487a7a293ab8fdb752c383accfb56 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 28 May 2024 10:09:35 -0700 Subject: [PATCH 5/7] . --- packages/compiler/src/core/linter.ts | 9 ++------- packages/compiler/src/core/program.ts | 7 +++---- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/packages/compiler/src/core/linter.ts b/packages/compiler/src/core/linter.ts index 448a007888..970c1b71a3 100644 --- a/packages/compiler/src/core/linter.ts +++ b/packages/compiler/src/core/linter.ts @@ -64,11 +64,6 @@ export function createLinter( lint, }; - function getLinterDefinition(library: LibraryInstance): LinterResolvedDefinition | undefined { - // eslint-disable-next-line deprecation/deprecation - return library?.linter ?? library?.definition?.linter; - } - async function extendRuleSet(ruleSet: LinterRuleSet): Promise { tracer.trace("extend-rule-set.start", JSON.stringify(ruleSet, null, 2)); const diagnostics = createDiagnosticCollector(); @@ -77,7 +72,7 @@ export function createLinter( const ref = diagnostics.pipe(parseRuleReference(extendingRuleSetName)); if (ref) { const library = await resolveLibrary(ref.libraryName); - const libLinterDefinition = library && getLinterDefinition(library); + const libLinterDefinition = library?.linter; const extendingRuleSet = libLinterDefinition?.ruleSets?.[ref.name]; if (extendingRuleSet) { await extendRuleSet(extendingRuleSet); @@ -173,7 +168,7 @@ export function createLinter( tracer.trace("register-library", name); const library = await loadLibrary(name); - const linter = library && getLinterDefinition(library); + const linter = library?.linter; if (linter?.rules) { for (const rule of linter.rules) { tracer.trace( diff --git a/packages/compiler/src/core/program.ts b/packages/compiler/src/core/program.ts index 048da4fc09..769a259bda 100644 --- a/packages/compiler/src/core/program.ts +++ b/packages/compiler/src/core/program.ts @@ -580,14 +580,13 @@ export async function compile( const libDefinition: TypeSpecLibrary | undefined = entrypoint?.esmExports.$lib; const metadata = computeLibraryMetadata(module, libDefinition); - + // eslint-disable-next-line deprecation/deprecation + const linterDef = entrypoint?.esmExports.$linter ?? libDefinition?.linter; return { ...resolution, metadata, definition: libDefinition, - linter: - entrypoint?.esmExports.$linter && - resolveLinterDefinition(libraryNameOrPath, entrypoint.esmExports.$linter), + linter: linterDef && resolveLinterDefinition(libraryNameOrPath, linterDef), }; } From 5c3f10e45f1757e132f0b735ac45085fb13b7121 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 28 May 2024 10:42:58 -0700 Subject: [PATCH 6/7] fix --- packages/compiler/src/core/index.ts | 1 + packages/compiler/src/core/linter.ts | 5 ++++- packages/tspd/src/ref-doc/extractor.ts | 7 ++++--- packages/tspd/src/ref-doc/types.ts | 2 +- 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/compiler/src/core/index.ts b/packages/compiler/src/core/index.ts index c022d1fe18..1166028cef 100644 --- a/packages/compiler/src/core/index.ts +++ b/packages/compiler/src/core/index.ts @@ -40,6 +40,7 @@ export { setCadlNamespace, setTypeSpecNamespace, } from "./library.js"; +export { resolveLinterDefinition } from "./linter.js"; export * from "./module-resolver.js"; export { NodeHost } from "./node-host.js"; export { Numeric, isNumeric } from "./numeric.js"; diff --git a/packages/compiler/src/core/linter.ts b/packages/compiler/src/core/linter.ts index 970c1b71a3..f0f500f717 100644 --- a/packages/compiler/src/core/linter.ts +++ b/packages/compiler/src/core/linter.ts @@ -23,7 +23,10 @@ export interface Linter { lint(): readonly Diagnostic[]; } -/** Resolve the linter definition */ +/** + * Resolve the linter definition + * @internal + */ export function resolveLinterDefinition( libName: string, linter: LinterDefinition diff --git a/packages/tspd/src/ref-doc/extractor.ts b/packages/tspd/src/ref-doc/extractor.ts index 9e13411167..6a79dc94d2 100644 --- a/packages/tspd/src/ref-doc/extractor.ts +++ b/packages/tspd/src/ref-doc/extractor.ts @@ -16,7 +16,7 @@ import { isTemplateDeclaration, joinPaths, JSONSchemaType, - LinterDefinition, + LinterResolvedDefinition, LinterRuleDefinition, LinterRuleSet, Model, @@ -29,6 +29,7 @@ import { NoTarget, Operation, Program, + resolveLinterDefinition, resolvePath, Scalar, SyntaxKind, @@ -109,7 +110,7 @@ export async function extractLibraryRefDocs( // eslint-disable-next-line deprecation/deprecation const linter = entrypoint.$linter ?? lib?.linter; if (lib && linter) { - refDoc.linter = extractLinterRefDoc(lib.name, linter); + refDoc.linter = extractLinterRefDoc(lib.name, resolveLinterDefinition(lib.name, linter)); } } @@ -622,7 +623,7 @@ function extractEmitterOptionsRefDoc( }); } -function extractLinterRefDoc(libName: string, linter: LinterDefinition): LinterRefDoc { +function extractLinterRefDoc(libName: string, linter: LinterResolvedDefinition): LinterRefDoc { return { ruleSets: linter.ruleSets && extractLinterRuleSetsRefDoc(libName, linter.ruleSets), rules: linter.rules.map((rule) => extractLinterRuleRefDoc(libName, rule)), diff --git a/packages/tspd/src/ref-doc/types.ts b/packages/tspd/src/ref-doc/types.ts index cb5cfec0fc..0cfdac62ff 100644 --- a/packages/tspd/src/ref-doc/types.ts +++ b/packages/tspd/src/ref-doc/types.ts @@ -51,7 +51,7 @@ export type EmitterRefDoc = { export type LinterRefDoc = { /** List of rulesets provided. */ - readonly ruleSets?: LinterRuleSetRefDoc[]; + readonly ruleSets: LinterRuleSetRefDoc[]; readonly rules: LinterRuleRefDoc[]; }; From 4d8ee5fe30d46f596aab1aac4092eba527832584 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 28 May 2024 11:35:56 -0700 Subject: [PATCH 7/7] . --- packages/compiler/src/core/linter.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/compiler/src/core/linter.ts b/packages/compiler/src/core/linter.ts index f0f500f717..95e0374d00 100644 --- a/packages/compiler/src/core/linter.ts +++ b/packages/compiler/src/core/linter.ts @@ -25,7 +25,6 @@ export interface Linter { /** * Resolve the linter definition - * @internal */ export function resolveLinterDefinition( libName: string,