Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linter all rulesets automatically created #3462

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions packages/compiler/src/core/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
50 changes: 36 additions & 14 deletions packages/compiler/src/core/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
DiagnosticMessages,
LibraryInstance,
LinterDefinition,
LinterResolvedDefinition,
LinterRule,
LinterRuleContext,
LinterRuleDiagnosticReport,
Expand All @@ -22,6 +23,34 @@ export interface Linter {
lint(): readonly Diagnostic[];
}

/**
* Resolve the linter definition
*/
export function resolveLinterDefinition(
libName: string,
linter: LinterDefinition
): LinterResolvedDefinition {
const rules: LinterRule<string, any>[] = 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<LibraryInstance | undefined>
Expand All @@ -37,11 +66,6 @@ export function createLinter(
lint,
};

function getLinterDefinition(library: LibraryInstance): LinterDefinition | undefined {
// eslint-disable-next-line deprecation/deprecation
return library?.linter ?? library?.definition?.linter;
}

async function extendRuleSet(ruleSet: LinterRuleSet): Promise<readonly Diagnostic[]> {
tracer.trace("extend-rule-set.start", JSON.stringify(ruleSet, null, 2));
const diagnostics = createDiagnosticCollector();
Expand All @@ -50,7 +74,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);
Expand Down Expand Up @@ -146,19 +170,17 @@ 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 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<string, any> = { ...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);
}
}
}
Expand Down
7 changes: 4 additions & 3 deletions packages/compiler/src/core/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -580,12 +580,13 @@ export async function compile(

const libDefinition: TypeSpecLibrary<any> | 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,
linter: linterDef && resolveLinterDefinition(libraryNameOrPath, linterDef),
};
}

Expand Down
10 changes: 9 additions & 1 deletion packages/compiler/src/core/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2047,7 +2047,7 @@ export interface LibraryInstance {
entrypoint: JsSourceFileNode | undefined;
metadata: LibraryMetadata;
definition?: TypeSpecLibrary<any>;
linter: LinterDefinition;
linter: LinterResolvedDefinition;
}

export type LibraryMetadata = FileLibraryMetadata | ModuleLibraryMetadata;
Expand Down Expand Up @@ -2437,6 +2437,14 @@ export interface LinterDefinition {
ruleSets?: Record<string, LinterRuleSet>;
}

export interface LinterResolvedDefinition {
readonly rules: LinterRule<string, DiagnosticMessages>[];
readonly ruleSets: {
all: LinterRuleSet;
[name: string]: LinterRuleSet;
};
}

export interface LinterRuleDefinition<N extends string, DM extends DiagnosticMessages> {
/** Rule name (without the library name) */
name: N;
Expand Down
46 changes: 42 additions & 4 deletions packages/compiler/test/core/linter.test.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -45,13 +45,13 @@ 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,
linter: resolveLinterDefinition("@typespec/test-linter", linterDef),
};

await host.compile("main.tsp");
Expand Down Expand Up @@ -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();
Expand Down
7 changes: 0 additions & 7 deletions packages/http/src/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},
},
});
7 changes: 4 additions & 3 deletions packages/tspd/src/ref-doc/extractor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
isTemplateDeclaration,
joinPaths,
JSONSchemaType,
LinterDefinition,
LinterResolvedDefinition,
LinterRuleDefinition,
LinterRuleSet,
Model,
Expand All @@ -29,6 +29,7 @@ import {
NoTarget,
Operation,
Program,
resolveLinterDefinition,
resolvePath,
Scalar,
SyntaxKind,
Expand Down Expand Up @@ -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));
}
}

Expand Down Expand Up @@ -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)),
Expand Down
2 changes: 1 addition & 1 deletion packages/tspd/src/ref-doc/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export type EmitterRefDoc = {

export type LinterRefDoc = {
/** List of rulesets provided. */
readonly ruleSets?: LinterRuleSetRefDoc[];
readonly ruleSets: LinterRuleSetRefDoc[];
readonly rules: LinterRuleRefDoc[];
};

Expand Down
Loading