Skip to content

Commit

Permalink
Detach linter from $lib and add state declaration to $lib (#2773)
Browse files Browse the repository at this point in the history
fix #2301 

## Deteach the linter from `$lib`

Having the linter rules defined in `$lib` caused this circular reference
where the rules would call to some accessor that needed the $lib
diagnostics.

With this we keep $lib as manifest and helper functions only.

## Add state key declaration
```ts
export const internalLib = createTypeSpecLibrary({
 state: {
    authentication: { description: "State for the @auth decorator" },
    header: { description: "State for the @Header decorator" },
    ...
});

// use with StateKeys.authentication
```
  • Loading branch information
timotheeguerin authored Dec 23, 2023
1 parent 204083f commit da99aa9
Show file tree
Hide file tree
Showing 25 changed files with 259 additions and 142 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"changes": [
{
"packageName": "@typespec/compiler",
"comment": "Library declaration: Deprecated linter property on `$lib` in favor of a new `$linter` variable that can be exported. This was done to prevent circular reference caused by referencing linter rules in $lib.",
"type": "none"
},
{
"packageName": "@typespec/compiler",
"comment": "Library declaration: State symbols can now be declared in the library declaration(Prefereably internal declaration added above) to have a central place to define state symbols used in your libraries.",
"type": "none"
}
],
"packageName": "@typespec/compiler"
}
10 changes: 10 additions & 0 deletions common/changes/@typespec/http/internal-lib_2023-12-22-16-12.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@typespec/http",
"comment": "Migrate to new Internal/Public library definition",
"type": "none"
}
],
"packageName": "@typespec/http"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@typespec/playground",
"comment": "Handle new `$linter` exported variable",
"type": "none"
}
],
"packageName": "@typespec/playground"
}
2 changes: 1 addition & 1 deletion docs/extending-typespec/basics.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export const $lib = createTypeSpecLibrary({
} as const);

// Optional but convenient, those are meant to be used locally in your library.
export const { reportDiagnostic, createDiagnostic, createStateSymbol } = $lib;
export const { reportDiagnostic, createDiagnostic } = $lib;
```

Diagnostics are used for linters and decorators which are covered in subsequent topics.
Expand Down
17 changes: 14 additions & 3 deletions docs/extending-typespec/create-decorators.md
Original file line number Diff line number Diff line change
Expand Up @@ -172,12 +172,12 @@ Decorators can be used to register some metadata. For this you can use the `cont

❌ Do not save the data in a global variable.

```ts
```ts file=decorators.ts
import type { DecoratorContext, Type } from "@typespec/compiler";
import type { createStateSymbol } from "./lib.js";
import type { StateKeys } from "./lib.js";

// Create a unique key
const key = createStateSymbol("customName");
const key = StateKeys.customName;
export function $customName(context: DecoratorContext, target: Type, name: string) {
// Keep a mapping between the target and a value.
context.program.stateMap(key).set(target, name);
Expand All @@ -187,6 +187,17 @@ export function $customName(context: DecoratorContext, target: Type, name: strin
}
```

```ts file=lib.ts
export const $lib = createTypeSpecLibrary({
// ...
state: {
customName: { description: "State for the @customName decorator" },
},
});

export const StateKeys = $lib.stateKeys;
```

### Reporting diagnostic on decorator or arguments

Decorator context provide the `decoratorTarget` and `getArgumentTarget` helpers
Expand Down
7 changes: 6 additions & 1 deletion docs/extending-typespec/diagnostics.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ TypeSpec compiler report errors and warnings in the spec using the diagnostic AP
### Declare the diagnostics you are reporting

```ts
import { createTypeSpecLibrary } from "@typespec/compiler";

// in lib.js
export const { reportDiagnostic, createDiagnostic, createStateSymbol } = createTypeSpecLibrary({
export const $lib = createTypeSpecLibrary({
name: "@typespec/my-lib",
diagnostics: {
// Basic diagnostic with a fixed message
Expand Down Expand Up @@ -53,6 +55,9 @@ export const { reportDiagnostic, createDiagnostic, createStateSymbol } = createT
},
},
} as const);

// Rexport the helper functions to be able to just call them directly.
export const { reportDiagnostic, createDiagnostic };
```

This will represent 3 different diagnostics with full name of
Expand Down
35 changes: 25 additions & 10 deletions docs/extending-typespec/emitters-basics.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,26 @@ To pass your emitter custom options, the options must be registered with the com

The following example extends the hello world emitter to be configured with a name:

```typescript
import { JSONSchemaType, createTypeSpecLibrary } from "@typespec/compiler";
import Path from "path";
```ts file=src/internal-lib.ts
export const $lib = createTypeSpecLibrary({
name: "MyEmitter",
diagnostics: {
// Add diagnostics here.
},
state: {
// Add state keys here for decorators.
},
});
```

```ts file=src/lib.ts
import {
JSONSchemaType,
createTypeSpecLibrary,
EmitContext,
resolvePath,
} from "@typespec/compiler";
import { internalLib } from "./lib.js";

export interface EmitterOptions {
"target-name": string;
Expand All @@ -69,15 +86,14 @@ const EmitterOptionsSchema: JSONSchemaType<EmitterOptions> = {
};

export const $lib = createTypeSpecLibrary({
name: "MyEmitter",
diagnostics: {},
internal: internalLib,
emitter: {
options: EmitterOptionsSchema,
},
});

export async function $onEmit(context: EmitContext<EmitterOptions>) {
const outputDir = Path.join(context.emitterOutputDir, "hello.txt");
const outputDir = resolvePath(context.emitterOutputDir, "hello.txt");
const name = context.options.targetName;
await context.program.host.writeFile(outputDir, `hello ${name}!`);
}
Expand Down Expand Up @@ -149,15 +165,14 @@ The following example will emit models with the `@emitThis` decorator and also a
[See creating decorator documentation for more details](./create-decorators.md)

```typescript
import { DecoratorContext, Model, createStateSymbol } from "@typespec/compiler";
import { DecoratorContext, Model } from "@typespec/compiler";
import { StateKeys } from "./lib.js";

// Decorator Setup Code

const emitThisKey = createStateSymbol("emitThis");

// @emitThis decorator
export function $emitThis(context: DecoratorContext, target: Model) {
context.program.stateSet(emitThisKey).add(target);
context.program.stateSet(StateKeys.emitThis).add(target);
}

export async function $onEmit(context: EmitContext) {
Expand Down
45 changes: 23 additions & 22 deletions docs/extending-typespec/linters.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,32 +92,33 @@ context.reportDiagnostic({

<!-- cspell:disable-next-line -->

When defining your `$lib` with `createTypeSpecLibrary`([See](./basics.md#4-create-libts)) an additional entry for `linter` can be provided
Export a `$linter` variable from your library entrypoint:

```ts
```ts title="index.ts"
export { $linter } from "./linter.js";
```

```ts title="linter.ts"
import { defineLinter } from "@typespec/compiler";
// Import the rule defined previously
import { requiredDocRule } from "./rules/required-doc.rule.js";

export const $lib = createTypeSpecLibrary({
name: "@typespec/my-linter",
diagnostics: {},
linter: {
// Include all the rules your linter is defining here.
rules: [requiredDocRule],

// Optionally a linter can provide a set of rulesets
ruleSets: {
recommended: {
// (optional) A ruleset takes a map of rules to explicitly enable
enable: { [`@typespec/my-linter/${requiredDocRule.name}`]: true },

// (optional) A rule set can extend another rule set
extends: ["@typespec/best-practices/recommended"],

// (optional) A rule set can disable a rule enabled in a ruleset it extended.
disable: {
"`@typespec/best-practices/no-a": "This doesn't apply in this ruleset.",
},
export const $linter = defineLinter({
// Include all the rules your linter is defining here.
rules: [requiredDocRule],

// Optionally a linter can provide a set of rulesets
ruleSets: {
recommended: {
// (optional) A ruleset takes a map of rules to explicitly enable
enable: { [`@typespec/my-linter/${requiredDocRule.name}`]: true },

// (optional) A rule set can extend another rule set
extends: ["@typespec/best-practices/recommended"],

// (optional) A rule set can disable a rule enabled in a ruleset it extended.
disable: {
"`@typespec/best-practices/no-a": "This doesn't apply in this ruleset.",
},
},
},
Expand Down
1 change: 1 addition & 0 deletions packages/compiler/src/core/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export {
createCadlLibrary,
createLinterRule as createRule,
createTypeSpecLibrary,
defineLinter,
paramMessage,
// eslint-disable-next-line deprecation/deprecation
setCadlNamespace,
Expand Down
25 changes: 24 additions & 1 deletion packages/compiler/src/core/library.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import {
CallableMessage,
DiagnosticMessages,
JSONSchemaValidator,
LinterDefinition,
LinterRuleDefinition,
StateDef,
TypeSpecLibrary,
TypeSpecLibraryDef,
} from "./types.js";
Expand All @@ -28,6 +30,18 @@ export function getLibraryUrlsLoaded(): Set<string> {
/** @deprecated use createTypeSpecLibrary */
export const createCadlLibrary = createTypeSpecLibrary;

function createStateKeys<T extends string>(
libName: string,
state: Record<T, StateDef> | undefined
): Record<T, symbol> {
const result: Record<string, symbol> = {};

for (const key of Object.keys(state ?? {})) {
result[key] = Symbol.for(`${libName}/${key}`);
}
return result as Record<T, symbol>;
}

/**
* Create a new TypeSpec library definition.
* @param lib Library definition.
Expand All @@ -49,10 +63,12 @@ export const createCadlLibrary = createTypeSpecLibrary;
export function createTypeSpecLibrary<
T extends { [code: string]: DiagnosticMessages },
E extends Record<string, any>,
>(lib: Readonly<TypeSpecLibraryDef<T, E>>): TypeSpecLibrary<T, E> {
State extends string = never,
>(lib: Readonly<TypeSpecLibraryDef<T, E, State>>): TypeSpecLibrary<T, E, State> {
let emitterOptionValidator: JSONSchemaValidator;

const { reportDiagnostic, createDiagnostic } = createDiagnosticCreator(lib.diagnostics, lib.name);

function createStateSymbol(name: string): symbol {
return Symbol.for(`${lib.name}.${name}`);
}
Expand All @@ -61,8 +77,11 @@ export function createTypeSpecLibrary<
if (caller) {
loadedUrls.add(caller);
}

return {
...lib,
diagnostics: lib.diagnostics,
stateKeys: createStateKeys(lib.name, lib.state),
reportDiagnostic,
createDiagnostic,
createStateSymbol,
Expand All @@ -82,6 +101,10 @@ export function createTypeSpecLibrary<
}
}

export function defineLinter(def: LinterDefinition): LinterDefinition {
return def;
}

export function paramMessage<T extends string[]>(
strings: readonly string[],
...keys: T
Expand Down
14 changes: 11 additions & 3 deletions packages/compiler/src/core/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
Diagnostic,
DiagnosticMessages,
LibraryInstance,
LinterDefinition,
LinterRule,
LinterRuleContext,
LinterRuleDiagnosticReport,
Expand Down Expand Up @@ -36,6 +37,11 @@ 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 @@ -44,7 +50,8 @@ export function createLinter(
const ref = diagnostics.pipe(parseRuleReference(extendingRuleSetName));
if (ref) {
const library = await resolveLibrary(ref.libraryName);
const extendingRuleSet = library?.definition?.linter?.ruleSets?.[ref.name];
const libLinterDefinition = library && getLinterDefinition(library);
const extendingRuleSet = libLinterDefinition?.ruleSets?.[ref.name];
if (extendingRuleSet) {
await extendRuleSet(extendingRuleSet);
} else {
Expand Down Expand Up @@ -139,8 +146,9 @@ export function createLinter(
tracer.trace("register-library", name);

const library = await loadLibrary(name);
if (library?.definition?.linter?.rules) {
for (const ruleDef of library.definition.linter.rules) {
const linter = library && getLinterDefinition(library);
if (linter?.rules) {
for (const ruleDef of linter.rules) {
const ruleId = `${name}/${ruleDef.name}`;
tracer.trace(
"register-library.rule",
Expand Down
1 change: 1 addition & 0 deletions packages/compiler/src/core/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,7 @@ export async function compile(
...resolution,
metadata,
definition: libDefinition,
linter: entrypoint?.esmExports.$linter,
};
}

Expand Down
Loading

0 comments on commit da99aa9

Please sign in to comment.