Skip to content

Commit e8d0f0f

Browse files
Support async callback in linter (#8942)
- Add 'exitRoot' final event for semantic walker and linter rules - Support 'async' in linter definition and async function as callback for 'exitRoot' event. --------- Co-authored-by: Timothee Guerin <[email protected]>
1 parent dc7045f commit e8d0f0f

File tree

7 files changed

+256
-23
lines changed

7 files changed

+256
-23
lines changed
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
changeKind: feature
3+
packages:
4+
- "@typespec/compiler"
5+
---
6+
7+
- Add 'exit' final event for linter rules
8+
- Support 'async' in linter definition and async function as callback for 'exit' event.

packages/compiler/src/core/linter.ts

Lines changed: 63 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { isPromise } from "../utils/misc.js";
12
import { DiagnosticCollector, compilerAssert, createDiagnosticCollector } from "./diagnostics.js";
23
import { getLocationContext } from "./helpers/location-context.js";
34
import { defineLinter } from "./library.js";
@@ -6,7 +7,7 @@ import { createUnusedUsingLinterRule } from "./linter-rules/unused-using.rule.js
67
import { createDiagnostic } from "./messages.js";
78
import type { Program } from "./program.js";
89
import { EventEmitter, mapEventEmitterToNodeListener, navigateProgram } from "./semantic-walker.js";
9-
import { startTimer, time } from "./stats.js";
10+
import { startTimer } from "./stats.js";
1011
import {
1112
Diagnostic,
1213
DiagnosticMessages,
@@ -26,7 +27,7 @@ type LinterLibraryInstance = { linter: LinterResolvedDefinition };
2627
export interface Linter {
2728
extendRuleSet(ruleSet: LinterRuleSet): Promise<readonly Diagnostic[]>;
2829
registerLinterLibrary(name: string, lib?: LinterLibraryInstance): void;
29-
lint(): LinterResult;
30+
lint(): Promise<LinterResult>;
3031
}
3132

3233
export interface LinterStats {
@@ -157,7 +158,25 @@ export function createLinter(
157158
return diagnostics.diagnostics;
158159
}
159160

160-
function lint(): LinterResult {
161+
async function lint(): Promise<LinterResult> {
162+
const syncLintResult = await lintInternal(false /* asyncRules */);
163+
const asyncLintResult = await lintInternal(true /* asyncRules */);
164+
165+
return {
166+
diagnostics: [...syncLintResult.diagnostics, ...asyncLintResult.diagnostics],
167+
stats: {
168+
runtime: {
169+
total: syncLintResult.stats.runtime.total + asyncLintResult.stats.runtime.total,
170+
rules: {
171+
...syncLintResult.stats.runtime.rules,
172+
...asyncLintResult.stats.runtime.rules,
173+
},
174+
},
175+
},
176+
};
177+
}
178+
179+
async function lintInternal(asyncRules: boolean): Promise<LinterResult> {
161180
const diagnostics = createDiagnosticCollector();
162181
const eventEmitter = new EventEmitter<SemanticNodeListener>();
163182
const stats: LinterStats = {
@@ -166,26 +185,61 @@ export function createLinter(
166185
rules: {},
167186
},
168187
};
188+
const filteredRules = new Map<string, LinterRule<string, any>>();
189+
for (const [ruleId, rule] of enabledRules) {
190+
if ((rule.async ?? false) === asyncRules) {
191+
filteredRules.set(ruleId, rule);
192+
}
193+
}
169194
tracer.trace(
170195
"lint",
171-
`Running linter with following rules:\n` +
172-
[...enabledRules.keys()].map((x) => ` - ${x}`).join("\n"),
196+
`Running ${asyncRules ? "async" : "sync"} linter with following rules:\n` +
197+
[...filteredRules.keys()].map((x) => ` - ${x}`).join("\n"),
173198
);
174199

175200
const timer = startTimer();
176-
for (const rule of enabledRules.values()) {
201+
const exitCallbacks = [];
202+
const EXIT_EVENT_NAME = "exit";
203+
const allPromises: Promise<any>[] = [];
204+
for (const rule of filteredRules.values()) {
177205
const createTiming = startTimer();
178206
const listener = rule.create(createLinterRuleContext(program, rule, diagnostics));
179207
stats.runtime.rules[rule.id] = createTiming.end();
180208
for (const [name, cb] of Object.entries(listener)) {
181209
const timedCb = (...args: any[]) => {
182-
const duration = time(() => (cb as any)(...args));
183-
stats.runtime.rules[rule.id] += duration;
210+
const timer = startTimer();
211+
const result = (cb as any)(...args);
212+
if (name === EXIT_EVENT_NAME && isPromise(result)) {
213+
compilerAssert(
214+
rule.async,
215+
`Linter rule "${rule.id}" is not marked as async but returned a promise from the "${name}" callback.`,
216+
);
217+
const rr = result.finally(() => {
218+
const duration = timer.end();
219+
stats.runtime.rules[rule.id] += duration;
220+
});
221+
allPromises.push(rr);
222+
} else {
223+
const duration = timer.end();
224+
stats.runtime.rules[rule.id] += duration;
225+
}
184226
};
185-
eventEmitter.on(name as any, timedCb);
227+
if (name === EXIT_EVENT_NAME) {
228+
// we need to trigger 'exit' callbacks explicitly after semantic walker is done
229+
exitCallbacks.push(timedCb);
230+
} else {
231+
eventEmitter.on(name as any, timedCb);
232+
}
186233
}
187234
}
188235
navigateProgram(program, mapEventEmitterToNodeListener(eventEmitter));
236+
for (const cb of exitCallbacks) {
237+
cb(program);
238+
}
239+
if (allPromises.length > 0) {
240+
await Promise.all(allPromises);
241+
}
242+
189243
stats.runtime.total = timer.end();
190244
return { diagnostics: diagnostics.diagnostics, stats };
191245
}

packages/compiler/src/core/program.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ async function createProgram(
325325
}
326326

327327
// Linter stage
328-
const lintResult = linter.lint();
328+
const lintResult = await linter.lint();
329329
runtimeStats.linter = lintResult.stats.runtime;
330330
program.reportDiagnostics(lintResult.diagnostics);
331331

packages/compiler/src/core/types.ts

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2323,7 +2323,7 @@ export interface LinterResolvedDefinition {
23232323
};
23242324
}
23252325

2326-
export interface LinterRuleDefinition<N extends string, DM extends DiagnosticMessages> {
2326+
interface LinterRuleDefinitionBase<N extends string, DM extends DiagnosticMessages> {
23272327
/** Rule name (without the library name) */
23282328
name: N;
23292329
/** Rule default severity. */
@@ -2334,16 +2334,40 @@ export interface LinterRuleDefinition<N extends string, DM extends DiagnosticMes
23342334
url?: string;
23352335
/** Messages that can be reported with the diagnostic. */
23362336
messages: DM;
2337+
}
2338+
2339+
interface LinterRuleDefinitionSync<N extends string, DM extends DiagnosticMessages>
2340+
extends LinterRuleDefinitionBase<N, DM> {
2341+
/** Whether this is an async rule. Default is false */
2342+
async?: false;
2343+
/** Creator */
2344+
create(
2345+
context: LinterRuleContext<DM>,
2346+
): SemanticNodeListener & { exit?: (context: Program) => void | undefined };
2347+
}
2348+
2349+
interface LinterRuleDefinitionAsync<N extends string, DM extends DiagnosticMessages>
2350+
extends LinterRuleDefinitionBase<N, DM> {
2351+
/** Whether this is an async rule. Default is false */
2352+
async: true;
23372353
/** Creator */
2338-
create(context: LinterRuleContext<DM>): SemanticNodeListener;
2354+
create(
2355+
context: LinterRuleContext<DM>,
2356+
): SemanticNodeListener & { exit?: (context: Program) => Promise<void | undefined> };
23392357
}
23402358

2359+
export type LinterRuleDefinition<N extends string, DM extends DiagnosticMessages> =
2360+
| LinterRuleDefinitionSync<N, DM>
2361+
| LinterRuleDefinitionAsync<N, DM>;
2362+
23412363
/** Resolved instance of a linter rule that will run. */
2342-
export interface LinterRule<N extends string, DM extends DiagnosticMessages>
2343-
extends LinterRuleDefinition<N, DM> {
2364+
export type LinterRule<N extends string, DM extends DiagnosticMessages> = LinterRuleDefinition<
2365+
N,
2366+
DM
2367+
> & {
23442368
/** Expanded rule id in format `<library-name>:<rule-name>` */
23452369
id: string;
2346-
}
2370+
};
23472371

23482372
/** Reference to a rule. In this format `<library name>:<rule/ruleset name>` */
23492373
export type RuleRef = `${string}/${string}`;

packages/compiler/src/testing/rule-tester.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,9 @@ export function createLinterRuleTester(
133133
const context = createLinterRuleContext(runner.program, rule, diagnostics);
134134
const listener = ruleDef.create(context);
135135
navigateProgram(runner.program, listener);
136+
if (listener.exit) {
137+
await listener.exit(runner.program);
138+
}
136139
// No diagnostics should have been reported to the program. If it happened the rule is calling reportDiagnostic directly and should NOT be doing that.
137140
expectDiagnosticEmpty(runner.program.diagnostics);
138141
return [res, diagnostics.diagnostics];

packages/compiler/src/utils/misc.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,3 +484,7 @@ class RekeyableMapImpl<K, V> implements RekeyableMap<K, V> {
484484
return true;
485485
}
486486
}
487+
488+
export function isPromise(value: unknown): value is Promise<unknown> {
489+
return !!value && typeof (value as any).then === "function";
490+
}

0 commit comments

Comments
 (0)