From 6c025bc6aa93094a1d74e539847f22f9aa1f0384 Mon Sep 17 00:00:00 2001 From: Martin Fleck Date: Tue, 13 Oct 2026 17:15:21 +0200 Subject: [PATCH] feat(core): refuse a build phase no listener is wired for; ban expected/actual An AST extension or a build-phase pass could declare any DocumentState, and one outside the phases the pipeline wires was accepted and never fired, which reads as the derivation never running. And an error class declaring both `expected` and `actual` is read by vitest as an assertion failure, which only a comment on ConflictError recorded. - Type `phase` on AstExtension and BuildPhasePass as a union of the wired phases, with a readonly `all()` namespace beside each, as IntegrityPhase - Rename the field from `state` to `phase`, the word the pipeline and the Langium hooks use, and the services' protected bucket members with it - Make both `register` methods throw on a phase outside `all()`, for a cast or a JavaScript caller the type cannot stop - Wire the pipeline's listeners from `all()`, so the wired set and the accepted set are one list - Add the ast-grep rule error-expected-and-actual over source and test trees, matching quoted, computed and abstract members too, and shrink ConflictError's comment to the constraint - Name that rule and the connection-logger rule in conventions, so CONTRIBUTING can describe check:ast-grep as the rules conventions name Relates to #178 --- ast-grep/rules/error-expected-and-actual.yml | 81 ++++++++++++ .../tests/error-expected-and-actual-test.yml | 24 ++++ docs/CONTRIBUTING.md | 5 +- docs/contributing/conventions.md | 11 ++ .../order-flow-ast-extension.ts | 4 +- .../ast-extension/ast-extension-service.ts | 115 +++++++++++------- .../build-phase-pass-service.ts | 46 ++++--- .../build-phase-pass/build-phase-pass.ts | 43 +++++-- .../build-pipeline-integration.ts | 53 ++------ .../residency/cst-residency-service.ts | 2 +- .../ast-extension-service.test.ts | 67 ++++++---- .../build-phase-pass-service.test.ts | 52 ++++---- .../test/langium/build-phases.types.test.ts | 50 ++++++++ .../build-pipeline-integration.test.ts | 2 +- packages/protocol/src/errors.ts | 11 +- 15 files changed, 396 insertions(+), 170 deletions(-) create mode 100644 ast-grep/rules/error-expected-and-actual.yml create mode 100644 ast-grep/tests/error-expected-and-actual-test.yml create mode 100644 packages/core/test/langium/build-phases.types.test.ts diff --git a/ast-grep/rules/error-expected-and-actual.yml b/ast-grep/rules/error-expected-and-actual.yml new file mode 100644 index 00000000..9abe7539 --- /dev/null +++ b/ast-grep/rules/error-expected-and-actual.yml @@ -0,0 +1,81 @@ +# vitest reads any thrown object that carries both `expected` and `actual` as an assertion +# failure, wherever on the prototype chain they sit: it diffs the pair and assigns the +# prettified values back, which throws on a getter and replaces the real failure with a +# `TypeError`. A class with both is one `throw` away from that whatever it extends, so the +# rule reads class bodies, not heritage, and a quoted or computed name counts as the name. +# A pair split across a hierarchy, a member declared on an interface or a type literal, +# or one added by assignment outside the class, escapes this rule; a static member never +# sits on a thrown instance but is flagged all the same. +id: error-expected-and-actual +language: typescript +severity: error +message: A class must not declare both `expected` and `actual`; a test reporter reads that pair as an assertion failure. Name them for what they hold, as ConflictError's baseVersion and actualVersion do. +files: + - packages/*/src/**/*.ts + - packages/*/test/**/*.ts + - examples/**/src/**/*.ts + - examples/**/test/**/*.ts +utils: + declares-expected: + any: + - kind: public_field_definition + has: + field: name + regex: '^(["'']?expected["'']?|\[["''`]expected["''`]\])$' + - kind: method_definition + has: + field: name + regex: '^(["'']?expected["'']?|\[["''`]expected["''`]\])$' + - kind: abstract_method_signature + has: + field: name + regex: '^(["'']?expected["'']?|\[["''`]expected["''`]\])$' + - kind: method_definition + all: + - has: { field: name, regex: ^constructor$ } + - has: + stopBy: end + any: + - kind: required_parameter + - kind: optional_parameter + all: + - has: { field: pattern, regex: ^expected$ } + - has: + any: + - kind: accessibility_modifier + - kind: override_modifier + - regex: ^readonly$ + declares-actual: + any: + - kind: public_field_definition + has: + field: name + regex: '^(["'']?actual["'']?|\[["''`]actual["''`]\])$' + - kind: method_definition + has: + field: name + regex: '^(["'']?actual["'']?|\[["''`]actual["''`]\])$' + - kind: abstract_method_signature + has: + field: name + regex: '^(["'']?actual["'']?|\[["''`]actual["''`]\])$' + - kind: method_definition + all: + - has: { field: name, regex: ^constructor$ } + - has: + stopBy: end + any: + - kind: required_parameter + - kind: optional_parameter + all: + - has: { field: pattern, regex: ^actual$ } + - has: + any: + - kind: accessibility_modifier + - kind: override_modifier + - regex: ^readonly$ +rule: + kind: class_body + all: + - has: { matches: declares-expected } + - has: { matches: declares-actual } diff --git a/ast-grep/tests/error-expected-and-actual-test.yml b/ast-grep/tests/error-expected-and-actual-test.yml new file mode 100644 index 00000000..a0908737 --- /dev/null +++ b/ast-grep/tests/error-expected-and-actual-test.yml @@ -0,0 +1,24 @@ +id: error-expected-and-actual +valid: + - 'class ConflictError extends ResponseError { get baseVersion() { return 1; } get actualVersion() { return 2; } }' + - 'class OnlyExpected extends Error { readonly expected = 1; }' + - 'class OnlyActual { constructor(public actual: number) {} }' + - 'interface Spec { expected: string; actual: string; }' + - 'class PlainParameters { constructor(expected: number, actual: number) {} }' + - 'class Sub extends Base { actual = 2; }' +invalid: + - 'class Mismatch extends Error { get expected() { return 1; } get actual() { return 2; } }' + - 'class Fields extends Error { readonly expected: string; actual?: string; }' + - 'class Parameters { constructor(public expected: number, private readonly actual: number) {} }' + - 'class Mixed extends ResponseError { readonly expected = 1; constructor(readonly actual: number) { super(); } }' + - 'class NoHeritage { expected = 1; actual = 2; }' + - 'class Quoted extends Error { "expected" = 1; get actual() { return 2; } }' + - 'class Computed extends Error { ["expected"] = 1; ["actual"] = 2; }' + - 'class ComputedGetter extends Error { get ["expected"]() { return 1; } get actual() { return 2; } }' + - 'class Setters extends Error { set expected(value: number) {} set actual(value: number) {} }' + - 'const Expression = class { expected = 1; actual = 2; };' + - 'abstract class Abstract extends Error { abstract get expected(): number; abstract get actual(): number; }' + - 'class Template extends Error { [`expected`] = 1; [`actual`] = 2; }' + - 'class Declared extends Error { declare expected: number; declare actual: number; }' + - 'class Accessors extends Error { accessor expected = 1; accessor actual = 2; }' + - 'class Modifiers extends Error { public expected = 1; protected actual = 2; }' diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index 2d8b65a2..63519408 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -119,8 +119,9 @@ TypeScript runs in strict mode (`tsconfig.base.json`). `npm run lint` runs Oxlint, native rules only; `oxlint.config.cjs` scopes the architecture rules to their packages. Two checks cover what Oxlint has no native rule for: `npm run check:phantom-deps` requires every import to be declared in its -workspace's manifest, and `npm run check:ast-grep` runs the localization and -test-fixture bans in `ast-grep/rules`, after their tests in `ast-grep/tests`. +workspace's manifest, and `npm run check:ast-grep` runs the rules in +`ast-grep/rules`, each a ban conventions names, after their tests in +`ast-grep/tests`. `npm run check:lint-policy` plants violations and permitted exceptions at real paths and runs the enforcing tool on each. diff --git a/docs/contributing/conventions.md b/docs/contributing/conventions.md index e2daabf4..83e3acca 100644 --- a/docs/contributing/conventions.md +++ b/docs/contributing/conventions.md @@ -112,6 +112,11 @@ and a test can drive time; a duration is a `stopwatch()`, never two `now()` readings, which a wall-clock step skews. Enforced by the ast-grep rule `time-through-clock` over framework and example source. +**No class declares both `expected` and `actual`**, because a test reporter +reads an error carrying that pair as an assertion failure and reports nothing +of the real one; name the two for what they hold. Enforced by the ast-grep +rule `error-expected-and-actual` over source and test trees. + **Typeguards and namespaces.** A typeguard the framework defines is a free function, `isFoo(x): x is Foo`, matching the generated guards adopters call. A namespace merged with a type holds factories, utilities and constants, never a @@ -310,6 +315,12 @@ command as an action instead. ## Logs and failures +**A connection is built with a logger**, so a malformed message or a failed +write on it reaches a log instead of vscode-jsonrpc's null logger; GLSP's +`createSocketConnection` takes none, so that one is built by hand. Enforced by +the ast-grep rule `connection-without-logger` over framework and example +source. + **A problem reaches each reader once.** The client hears of it through its rejection or notification, and the log through one line, written where the outcome is decided. Code that rethrows does not log, since whoever ends the diff --git a/examples/order-flow/server/src/language-server/order-flow-ast-extension.ts b/examples/order-flow/server/src/language-server/order-flow-ast-extension.ts index 600634a4..d66b3f2f 100644 --- a/examples/order-flow/server/src/language-server/order-flow-ast-extension.ts +++ b/examples/order-flow/server/src/language-server/order-flow-ast-extension.ts @@ -41,7 +41,7 @@ export class OrderFlowComputedPropertiesContribution implements AstExtensionCont registry.register({ id: '_writtenFields', nodeFilter: isTask, - state: DocumentState.ComputedScopes, + phase: DocumentState.ComputedScopes, compute: (node: Mutable) => { setHiddenProperty(node, '_writtenFields', collectWrittenFields(node)); } @@ -50,7 +50,7 @@ export class OrderFlowComputedPropertiesContribution implements AstExtensionCont registry.register({ id: '_effectSummary', nodeFilter: isTask, - state: DocumentState.ComputedScopes, + phase: DocumentState.ComputedScopes, compute: (node: Mutable) => { node._effectSummary = summariseEffects(node); } diff --git a/packages/core/src/langium/ast-extension/ast-extension-service.ts b/packages/core/src/langium/ast-extension/ast-extension-service.ts index e734e69d..8ece9b49 100644 --- a/packages/core/src/langium/ast-extension/ast-extension-service.ts +++ b/packages/core/src/langium/ast-extension/ast-extension-service.ts @@ -15,6 +15,35 @@ import { type ServerLanguageServices } from '../language-module.js'; import { Registry, type RegistryItem } from '../../util/registry.js'; import { type AstExtensionRegistry } from './ast-extension-contribution.js'; +/** + * The document-build phases an {@link AstExtension} may fire at. `Changed` has + * no AST yet, and `IndexedContent` carries the same per-document information + * as `Parsed`, so neither is one. + */ +export type AstExtensionPhase = + | DocumentState.Parsed + | DocumentState.ComputedScopes + | DocumentState.Linked + | DocumentState.IndexedReferences + | DocumentState.Validated; + +export namespace AstExtensionPhase { + /** + * Every {@link AstExtensionPhase}, in build order. `BuildPipelineIntegration` + * wires one `onDocumentPhase` listener per entry and `register` refuses a + * phase outside it, so the two cannot drift apart. A fresh array each call. + */ + export function all(): readonly AstExtensionPhase[] { + return [ + DocumentState.Parsed, + DocumentState.ComputedScopes, + DocumentState.Linked, + DocumentState.IndexedReferences, + DocumentState.Validated + ]; + } +} + /** * A registration that derives data from an AST node and attaches it to the * node at a declared document-build phase. @@ -29,13 +58,13 @@ import { type AstExtensionRegistry } from './ast-extension-contribution.js'; * are stored as non-enumerable own properties. Otherwise symbol providers, * folding, semantic tokens, and validators will treat them as real children. * - * **Phase semantics:** {@link state} declares the single phase at which + * **Phase semantics:** {@link phase} declares the single phase at which * {@link compute} fires. The framework wires one `onDocumentPhase` listener per - * useful state unconditionally, rather than deriving the set from the states - * registrations declare — so a registration's state must be one of those, and - * adding one is a framework change, not a registration-time choice. + * {@link AstExtensionPhase} unconditionally, rather than deriving the set from + * the phases registrations declare — so `register` refuses any other phase, + * and adding one is a framework change, not a registration-time choice. * - * **`.ref`-dependent derivations: declare `state: ComputedScopes`, not `Linked`.** + * **`.ref`-dependent derivations: declare `phase: ComputedScopes`, not `Linked`.** * Lazy reference resolution is available from `ComputedScopes` onward — the * global scope is built by `IndexedContent` (which runs before * `ComputedScopes`), so reading a reference's `.ref` inside a `ComputedScopes` @@ -44,8 +73,8 @@ import { type AstExtensionRegistry } from './ast-extension-contribution.js'; * {@link DocumentBuilderOptions.refreshCrossDocumentComputedScopes} is set, so * one that reads another document's node goes stale when that node changes. * - * **DO NOT** stage a `state: ComputedScopes` placeholder hoping to refine it - * via a separate `state: Linked` registration when a downstream consumer + * **DO NOT** stage a `phase: ComputedScopes` placeholder hoping to refine it + * via a separate `phase: Linked` registration when a downstream consumer * (typically a `ScopeExtension.addDescriptions` firing during link) * iterates the derived value to build a scope: `onDocumentPhase(Linked)` fires * AFTER Langium's `Linker.link()` has already walked the document, by which @@ -63,32 +92,26 @@ export interface AstExtension extends RegistryItem /** * Per-document early-out. When set, the framework only invokes the * registration's callback for documents the filter accepts. If every - * registration at a given state rejects the document, the framework skips + * registration at a given phase rejects the document, the framework skips * the `streamAllContents` walk entirely — important for large workspaces * where most documents carry none of the AST shapes a given extension * targets. * - * Default: no filter (registration applies to every document at its state). + * Default: no filter (registration applies to every document at its phase). */ documentFilter?: (document: LangiumDocument) => boolean; /** - * The single document-build state at which {@link compute} fires. + * The single document-build phase at which {@link compute} fires. * - * Useful values: * - `Parsed` — derivations from AST shape only, no `.ref` reads. - * - `ComputedScopes` — lazy `.ref` resolution available; the most common state. + * - `ComputedScopes` — lazy `.ref` resolution available; the most common. * - `Linked` — derivations needing fully resolved cross-references. * - `IndexedReferences` — cross-document reverse-lookup derivations. * - `Validated` — derivations depending on diagnostics (rare). - * - * Any other state type-checks but never fires: the framework wires a - * listener only for the states above. `Changed` has no AST yet, and - * `IndexedContent` carries the same per-document information as `Parsed`, - * so neither is wired. */ - state: DocumentState; + phase: AstExtensionPhase; /** - * Mutate `node` for this registration at {@link state}. The framework hands + * Mutate `node` for this registration at {@link phase}. The framework hands * `node` typed as {@link Mutable} so computed / synthetic property * assignments (`node._foo = ...`) compile without per-callback * `asMutable(...)` casts. @@ -112,18 +135,18 @@ export interface AstExtension extends RegistryItem * * Adopter overrides go through {@link DefaultAstExtensionService}; the * interface keeps the public API stable while internals (the bucket- - * by-state cache) stay `protected` on the default class. + * by-phase cache) stay `protected` on the default class. */ export interface AstExtensionService extends AstExtensionRegistry { /** * Run a single AST node through every registration declaring - * `phase` as its state. Honours `documentFilter` and `nodeFilter`. + * `phase`. Honours `documentFilter` and `nodeFilter`. * Primarily for tests that need to invoke an extension callback without * driving Langium's full document-build pipeline. */ extendNode(node: AstNode, document: LangiumDocument, phase: DocumentState): void; /** - * Re-run all `compute` callbacks declaring `phase` as their state against + * Re-run all `compute` callbacks declaring `phase` against * every node in the document. Single-pass. Called by * `BuildPipelineIntegration` from an `onDocumentPhase` listener; * not for general adopter use. @@ -148,18 +171,18 @@ export type AstExtensionServiceOptions = LogNameOptions; * owns the registry + per-node logic, not the listener lifecycle. * * **Performance:** `extendDocument` shares a single `streamAllContents` walk - * per document per wired phase; cost is O(registrations-for-state × nodes), - * and it early-outs when no registration targets the state or the document. + * per document per wired phase; cost is O(registrations-for-phase × nodes), + * and it early-outs when no registration targets the phase or the document. */ export class DefaultAstExtensionService implements AstExtensionService { protected readonly extensions = new Registry(); /** - * Per-state bucket of registrations targeting each state. Rebuilt lazily + * Per-phase bucket of registrations targeting each phase. Rebuilt lazily * on the first refresh after a registry mutation — staleness is detected * via reference identity of the registry's cached array (see {@link Registry.all}). */ - protected stateBuckets = new Map(); - protected stateBucketsFor: readonly AstExtension[] | undefined; + protected phaseBuckets = new Map(); + protected phaseBucketsFor: readonly AstExtension[] | undefined; protected readonly tracer: Tracer; @@ -189,6 +212,16 @@ export class DefaultAstExtensionService implements AstExtensionService { * which calls {@link extendDocument} for the wired phases. */ register(extension: AstExtension): Disposable { + // A phase outside the wired set type-checks only past a cast, and a + // registration there is accepted and never fires. + if (!AstExtensionPhase.all().includes(extension.phase)) { + throw new Error( + `AST extension '${extension.id}' registered at phase ${DocumentState[extension.phase]} would never fire. ` + + `Allowed phases are ${AstExtensionPhase.all() + .map(phase => DocumentState[phase]) + .join(', ')}.` + ); + } // Cast: TypeScript can't see that `T extends AstNode` makes the extension // assignable to the default-parameter form. The runtime contract is the same. return this.extensions.register(extension as AstExtension); @@ -196,10 +229,10 @@ export class DefaultAstExtensionService implements AstExtensionService { /** * Production code goes through {@link extendDocument} via the - * `onDocumentPhase(state)` listener wired by `BuildPipelineIntegration`. + * `onDocumentPhase(phase)` listener wired by `BuildPipelineIntegration`. */ extendNode(node: AstNode, document: LangiumDocument, phase: DocumentState): void { - const bucket = this.getStateBucket(phase); + const bucket = this.getPhaseBucket(phase); for (const extension of bucket) { if (extension.documentFilter && !extension.documentFilter(document)) { continue; @@ -228,7 +261,7 @@ export class DefaultAstExtensionService implements AstExtensionService { if (cancelToken?.isCancellationRequested) { return; } - const bucket = this.getStateBucket(phase); + const bucket = this.getPhaseBucket(phase); if (bucket.length === 0) { return; } @@ -261,33 +294,33 @@ export class DefaultAstExtensionService implements AstExtensionService { } /** - * Bucket-by-state view of {@link extensions}, rebuilt lazily when the + * Bucket-by-phase view of {@link extensions}, rebuilt lazily when the * registry's cached `all()` reference changes. Replaces per-call allocation * on the per-node hot path — for a workspace with many documents at the - * same state, the bucket is computed once and reused until the next + * same phase, the bucket is computed once and reused until the next * register / unregister. */ - protected getStateBucket(phase: DocumentState): readonly AstExtension[] { + protected getPhaseBucket(phase: DocumentState): readonly AstExtension[] { const current = this.extensions.all(); - if (this.stateBucketsFor !== current) { + if (this.phaseBucketsFor !== current) { const next = new Map(); for (const extension of current) { - let bucket = next.get(extension.state); + let bucket = next.get(extension.phase); if (!bucket) { bucket = []; - next.set(extension.state, bucket); + next.set(extension.phase, bucket); } bucket.push(extension); } - this.stateBuckets = next; - this.stateBucketsFor = current; + this.phaseBuckets = next; + this.phaseBucketsFor = current; } - return this.stateBuckets.get(phase) ?? EMPTY_STATE_BUCKET; + return this.phaseBuckets.get(phase) ?? EMPTY_PHASE_BUCKET; } } -/** Shared empty result so `getStateBucket` doesn't allocate per call on the cold path. */ -const EMPTY_STATE_BUCKET: readonly AstExtension[] = Object.freeze([]); +/** Shared empty result so `getPhaseBucket` doesn't allocate per call on the cold path. */ +const EMPTY_PHASE_BUCKET: readonly AstExtension[] = Object.freeze([]); /** * Assign a non-enumerable own property — for synthetic children that must NOT diff --git a/packages/core/src/langium/build-phase-pass/build-phase-pass-service.ts b/packages/core/src/langium/build-phase-pass/build-phase-pass-service.ts index 115b7dae..a07f898c 100644 --- a/packages/core/src/langium/build-phase-pass/build-phase-pass-service.ts +++ b/packages/core/src/langium/build-phase-pass/build-phase-pass-service.ts @@ -13,7 +13,7 @@ import { type CancellationToken, type Disposable } from 'vscode-languageserver'; import { type LogNameOptions } from '../diagnostics/logger.js'; import { type ServerSharedServices } from '../module.js'; import { Registry } from '../../util/registry.js'; -import { type BuildPhasePass, type BuildPhasePassRegistry } from './build-phase-pass.js'; +import { type BuildPhasePass, type BuildPhasePassRegistry, BuildPassPhase } from './build-phase-pass.js'; /** * Public contract for the shared build-phase-pass runner. Extends the @@ -31,7 +31,7 @@ import { type BuildPhasePass, type BuildPhasePassRegistry } from './build-phase- */ export interface BuildPhasePassService extends BuildPhasePassRegistry { /** - * Run every pass declaring `phase` as its state, in priority order, + * Run every pass declaring `phase`, in priority order, * sequentially — each pass is awaited before the next begins. The cancel * token is checked before each pass so a preempted build stops cleanly. * No-ops when no pass targets the phase. Called by @@ -52,8 +52,8 @@ export type BuildPhasePassServiceOptions = LogNameOptions; * `BuildPipelineIntegration`, * which calls {@link runPasses} from its `onBuildPhase` listeners. * - * **Performance:** {@link runPasses} shares the bucket-by-state cache pattern - * with the AST-extension service — the per-state pass list is rebuilt lazily + * **Performance:** {@link runPasses} shares the bucket-by-phase cache pattern + * with the AST-extension service — the per-phase pass list is rebuilt lazily * only after a register / unregister, so steady-state dispatch allocates * nothing and early-outs when no pass targets the phase. * @@ -64,9 +64,9 @@ export type BuildPhasePassServiceOptions = LogNameOptions; */ export class DefaultBuildPhasePassService implements BuildPhasePassService { protected readonly passes = new Registry(); - /** Per-state bucket of passes targeting each state, rebuilt lazily on the first run after a registry mutation. */ - protected stateBuckets = new Map(); - protected stateBucketsFor: readonly BuildPhasePass[] | undefined; + /** Per-phase bucket of passes targeting each phase, rebuilt lazily on the first run after a registry mutation. */ + protected phaseBuckets = new Map(); + protected phaseBucketsFor: readonly BuildPhasePass[] | undefined; protected readonly tracer: Tracer; @@ -84,11 +84,21 @@ export class DefaultBuildPhasePassService implements BuildPhasePassService { } register(pass: BuildPhasePass): Disposable { + // A phase outside the wired set type-checks only past a cast, and a pass + // there is accepted and never fires. + if (!BuildPassPhase.all().includes(pass.phase)) { + throw new Error( + `Build-phase pass '${pass.id}' registered at phase ${DocumentState[pass.phase]} would never fire. ` + + `Allowed phases are ${BuildPassPhase.all() + .map(phase => DocumentState[phase]) + .join(', ')}.` + ); + } return this.passes.register(pass); } async runPasses(documents: readonly LangiumDocument[], phase: DocumentState, cancelToken: CancellationToken): Promise { - const bucket = this.getStateBucket(phase); + const bucket = this.getPhaseBucket(phase); if (bucket.length === 0) { return; } @@ -114,28 +124,28 @@ export class DefaultBuildPhasePassService implements BuildPhasePassService { } /** - * Bucket-by-state view of {@link passes}, rebuilt lazily when the registry's + * Bucket-by-phase view of {@link passes}, rebuilt lazily when the registry's * cached `all()` reference changes. Each bucket preserves the registry's * priority order (`Registry.all()` is already priority-sorted). */ - protected getStateBucket(phase: DocumentState): readonly BuildPhasePass[] { + protected getPhaseBucket(phase: DocumentState): readonly BuildPhasePass[] { const current = this.passes.all(); - if (this.stateBucketsFor !== current) { + if (this.phaseBucketsFor !== current) { const next = new Map(); for (const pass of current) { - let bucket = next.get(pass.state); + let bucket = next.get(pass.phase); if (!bucket) { bucket = []; - next.set(pass.state, bucket); + next.set(pass.phase, bucket); } bucket.push(pass); } - this.stateBuckets = next; - this.stateBucketsFor = current; + this.phaseBuckets = next; + this.phaseBucketsFor = current; } - return this.stateBuckets.get(phase) ?? EMPTY_STATE_BUCKET; + return this.phaseBuckets.get(phase) ?? EMPTY_PHASE_BUCKET; } } -/** Shared empty result so `getStateBucket` doesn't allocate per call on the cold path. */ -const EMPTY_STATE_BUCKET: readonly BuildPhasePass[] = Object.freeze([]); +/** Shared empty result so `getPhaseBucket` doesn't allocate per call on the cold path. */ +const EMPTY_PHASE_BUCKET: readonly BuildPhasePass[] = Object.freeze([]); diff --git a/packages/core/src/langium/build-phase-pass/build-phase-pass.ts b/packages/core/src/langium/build-phase-pass/build-phase-pass.ts index afa8db33..55366a30 100644 --- a/packages/core/src/langium/build-phase-pass/build-phase-pass.ts +++ b/packages/core/src/langium/build-phase-pass/build-phase-pass.ts @@ -7,10 +7,43 @@ * SPDX-License-Identifier: MIT ********************************************************************************/ -import { type DocumentState, type LangiumCoreServices, type LangiumDocument, type URI } from '@hydranium/langium'; +import { DocumentState, type LangiumCoreServices, type LangiumDocument, type URI } from '@hydranium/langium'; import { type CancellationToken, type Disposable } from 'vscode-languageserver'; import { type RegistryItem } from '../../util/registry.js'; +/** + * The build phases a {@link BuildPhasePass} may fire at: every phase after + * `Changed`, which has no AST yet. `IndexedContent` is one, unlike for an + * `AstExtension`: per document it carries nothing `Parsed` does, but per + * batch it is the only phase meaning the global index is whole again, which + * batch work keyed on a complete index has nowhere else to run. + */ +export type BuildPassPhase = + | DocumentState.Parsed + | DocumentState.IndexedContent + | DocumentState.ComputedScopes + | DocumentState.Linked + | DocumentState.IndexedReferences + | DocumentState.Validated; + +export namespace BuildPassPhase { + /** + * Every {@link BuildPassPhase}, in build order. `BuildPipelineIntegration` + * wires one `onBuildPhase` listener per entry and `register` refuses a phase + * outside it, so the two cannot drift apart. A fresh array each call. + */ + export function all(): readonly BuildPassPhase[] { + return [ + DocumentState.Parsed, + DocumentState.IndexedContent, + DocumentState.ComputedScopes, + DocumentState.Linked, + DocumentState.IndexedReferences, + DocumentState.Validated + ]; + } +} + /** * A unit of whole-batch work run once per build when the batch reaches a * declared build phase, ordered by {@link RegistryItem.priority}. @@ -46,12 +79,8 @@ import { type RegistryItem } from '../../util/registry.js'; * rather than running the remaining passes against a doomed build. */ export interface BuildPhasePass extends RegistryItem { - /** - * The single build phase whose `onBuildPhase` fires this pass. The framework - * wires a listener for the same phase set as AST extension; a pass declaring - * any other state type-checks but never fires. - */ - state: DocumentState; + /** The single build phase whose `onBuildPhase` fires this pass. */ + phase: BuildPassPhase; /** * Run this pass against the whole built batch. May mutate and reparse * documents — the framework's integrity pass does — or derive cross-document diff --git a/packages/core/src/langium/document-builder/build-pipeline-integration.ts b/packages/core/src/langium/document-builder/build-pipeline-integration.ts index 18edb1fd..723c5661 100644 --- a/packages/core/src/langium/document-builder/build-pipeline-integration.ts +++ b/packages/core/src/langium/document-builder/build-pipeline-integration.ts @@ -10,46 +10,11 @@ import { type LangiumDocument, DocumentState } from '@hydranium/langium'; import { type CancellationToken } from 'vscode-languageserver'; import { IntegrityPhase } from '../integrity/integrity-rule.js'; -import { groupDocumentsByLanguage } from '../build-phase-pass/build-phase-pass.js'; +import { AstExtensionPhase } from '../ast-extension/ast-extension-service.js'; +import { BuildPassPhase, groupDocumentsByLanguage } from '../build-phase-pass/build-phase-pass.js'; import { type ServerSharedServices } from '../module.js'; import { labelPhaseListener } from './labeled-phase-listener.js'; -/** - * AST extension wiring covers every "useful" `DocumentState` an extension can - * target. `Changed` has no AST yet; `IndexedContent` carries the same - * per-document information as `Parsed`. No-op fires for documents with no - * extension at a given state are negligible (`notifyDocumentPhase` is a plain - * listener loop), so the framework wires the full set unconditionally rather - * than collecting declared states from per-language registrations. - */ -const AST_EXTENSION_PHASES: readonly DocumentState[] = [ - DocumentState.Parsed, - DocumentState.ComputedScopes, - DocumentState.Linked, - DocumentState.IndexedReferences, - DocumentState.Validated -]; - -/** - * Build phases at which the `BuildPhasePassService` is driven. A pass - * registered at any other state never fires (the listener is not wired), and - * `runPasses` no-ops at a phase with no registered pass, so wiring the full set - * unconditionally costs only an empty-bucket check per phase. - * - * `IndexedContent` belongs here and NOT in {@link AST_EXTENSION_PHASES}, so the - * two cannot be collapsed. Per document it carries nothing `Parsed` does; per - * BATCH it is the only phase meaning the global index is whole again, which - * batch work keyed on a complete index has nowhere else to run. - */ -const BUILD_PHASE_PASS_STATES: readonly DocumentState[] = [ - DocumentState.Parsed, - DocumentState.IndexedContent, - DocumentState.ComputedScopes, - DocumentState.Linked, - DocumentState.IndexedReferences, - DocumentState.Validated -]; - /** * Default priority of the framework integrity passes. Integrity mutates / cleans * the AST that every derived-state pass reads, so it is *foundational* and must @@ -132,17 +97,19 @@ export class DefaultBuildPipelineIntegration implements BuildPipelineIntegration for (const phase of IntegrityPhase.all()) { passes.register({ id: `framework:integrity:${IntegrityPhase.toString(phase).toLowerCase()}`, - state: phase, + phase, priority: integrityPriority, run: (documents, cancelToken) => this.callIntegrity(documents, phase, cancelToken) }); } - // One onBuildPhase listener per pass state, routed through the overridable + // One onBuildPhase listener per pass phase, routed through the overridable // `runBuildPhase` seam → priority-ordered, sequential, cancel-aware pass // dispatch. The returned promise is awaited by the build pipeline, so a - // mutating pass (integrity) reparses before the build advances. - for (const phase of BUILD_PHASE_PASS_STATES) { + // mutating pass (integrity) reparses before the build advances. The full + // set is wired unconditionally: `runPasses` no-ops at a phase with no + // pass, so an unused listener costs one empty-bucket check. + for (const phase of BuildPassPhase.all()) { builder.onBuildPhase( phase, labelPhaseListener( @@ -164,7 +131,9 @@ export class DefaultBuildPipelineIntegration implements BuildPipelineIntegration // unchanged by the build-phase-pass registry, which is batch-granular. The // listener calls the `extendAst` seam directly (the document-phase analog of // `runBuildPhase`); there is no per-phase handler tier — one parameterized seam. - for (const phase of AST_EXTENSION_PHASES) { + // Wired for every phase unconditionally rather than for the phases the + // per-language registrations declare: a no-op fire is a plain listener loop. + for (const phase of AstExtensionPhase.all()) { builder.onDocumentPhase( phase, labelPhaseListener( diff --git a/packages/core/src/langium/residency/cst-residency-service.ts b/packages/core/src/langium/residency/cst-residency-service.ts index 0aa96427..2b836022 100644 --- a/packages/core/src/langium/residency/cst-residency-service.ts +++ b/packages/core/src/langium/residency/cst-residency-service.ts @@ -282,7 +282,7 @@ export class DefaultCstResidencyService implements CstResidencyService { this.langiumDocs = services.workspace.LangiumDocuments; services.workspace.BuildPhasePassService.register({ id: 'framework:cst-residency:shed', - state: DocumentState.Validated, + phase: DocumentState.Validated, priority: options.priority ?? CST_RESIDENCY_PASS_PRIORITY, run: documents => this.onBuild(documents) }); diff --git a/packages/core/test/langium/ast-extension/ast-extension-service.test.ts b/packages/core/test/langium/ast-extension/ast-extension-service.test.ts index 5419193a..3e3504f7 100644 --- a/packages/core/test/langium/ast-extension/ast-extension-service.test.ts +++ b/packages/core/test/langium/ast-extension/ast-extension-service.test.ts @@ -13,6 +13,7 @@ import { CancellationToken } from 'vscode-languageserver'; import { Logger } from '@hydranium/protocol'; import { makeFakeClock } from '@hydranium/protocol/testing'; import { + type AstExtension, type AstExtensionService, DefaultAstExtensionService, setHiddenProperty @@ -60,13 +61,13 @@ describe('AstExtensionService.extendNode', () => { service.register({ id: 'foo', nodeFilter: isType('Foo'), - state: DocumentState.ComputedScopes, + phase: DocumentState.ComputedScopes, compute: node => void fooCalls.push(node) }); service.register({ id: 'bar', nodeFilter: isType('Bar'), - state: DocumentState.ComputedScopes, + phase: DocumentState.ComputedScopes, compute: node => void barCalls.push(node) }); @@ -85,21 +86,21 @@ describe('AstExtensionService.extendNode', () => { id: 'late', priority: 10, nodeFilter: isType('Foo'), - state: DocumentState.ComputedScopes, + phase: DocumentState.ComputedScopes, compute: () => void calls.push('late') }); service.register({ id: 'early', priority: -1, nodeFilter: isType('Foo'), - state: DocumentState.ComputedScopes, + phase: DocumentState.ComputedScopes, compute: () => void calls.push('early') }); service.register({ id: 'mid', priority: 5, nodeFilter: isType('Foo'), - state: DocumentState.ComputedScopes, + phase: DocumentState.ComputedScopes, compute: () => void calls.push('mid') }); @@ -114,13 +115,13 @@ describe('AstExtensionService.extendNode', () => { service.register({ id: 'first', nodeFilter: isType('Foo'), - state: DocumentState.ComputedScopes, + phase: DocumentState.ComputedScopes, compute: () => void calls.push('first') }); service.register({ id: 'second', nodeFilter: isType('Foo'), - state: DocumentState.ComputedScopes, + phase: DocumentState.ComputedScopes, compute: () => void calls.push('second') }); @@ -137,7 +138,7 @@ describe('AstExtensionService.extendDocument', () => { service.register({ id: 'refresh-foo', nodeFilter: isType('Foo'), - state: DocumentState.Linked, + phase: DocumentState.Linked, compute: node => void refreshCalls.push((node as FakeNode).id ?? '') }); @@ -160,7 +161,7 @@ describe('AstExtensionService.extendDocument', () => { id: 'filtered', nodeFilter: isType('Foo'), documentFilter: document => (document as unknown as { tag?: string }).tag === 'apply', - state: DocumentState.Linked, + phase: DocumentState.Linked, compute: () => void calls.push('ran') }); @@ -184,14 +185,14 @@ describe('AstExtensionService.extendDocument', () => { id: 'a', nodeFilter: isType('Foo'), documentFilter: () => false, - state: DocumentState.Linked, + phase: DocumentState.Linked, compute: () => undefined }); service.register({ id: 'b', nodeFilter: isType('Bar'), documentFilter: () => false, - state: DocumentState.Linked, + phase: DocumentState.Linked, compute: () => undefined }); @@ -222,20 +223,20 @@ describe('AstExtensionService.extendDocument', () => { id: 'rejected', nodeFilter: isType('Foo'), documentFilter: () => false, - state: DocumentState.Linked, + phase: DocumentState.Linked, compute: () => void calls.push('rejected') }); service.register({ id: 'accepted', nodeFilter: isType('Foo'), documentFilter: () => true, - state: DocumentState.Linked, + phase: DocumentState.Linked, compute: () => void calls.push('accepted') }); service.register({ id: 'no-filter', nodeFilter: isType('Foo'), - state: DocumentState.Linked, + phase: DocumentState.Linked, compute: () => void calls.push('no-filter') }); @@ -252,13 +253,13 @@ describe('AstExtensionService.extendDocument', () => { const handle = service.register({ id: 'temp', nodeFilter: isType('Foo'), - state: DocumentState.Linked, + phase: DocumentState.Linked, compute: () => void calls.push('temp') }); service.register({ id: 'permanent', nodeFilter: isType('Foo'), - state: DocumentState.Linked, + phase: DocumentState.Linked, compute: () => void calls.push('permanent') }); @@ -279,7 +280,7 @@ describe('AstExtensionService.extendDocument', () => { service.register({ id: 'linked-only', nodeFilter: isType('Foo'), - state: DocumentState.Linked, + phase: DocumentState.Linked, compute: () => void calls++ }); @@ -300,7 +301,7 @@ describe('AstExtensionService.extendDocument cancellation', () => { service.register({ id: 'refresh-foo', nodeFilter: isType('Foo'), - state: DocumentState.Linked, + phase: DocumentState.Linked, compute: () => void calls.push('ran') }); @@ -322,7 +323,7 @@ describe('AstExtensionService disposal', () => { const handle = service.register({ id: 'x', nodeFilter: isType('Foo'), - state: DocumentState.ComputedScopes, + phase: DocumentState.ComputedScopes, compute: () => void calls.push('x') }); service.extendNode(makeFakeAstNode({ $type: 'Foo' }), {} as LangiumDocument, DocumentState.ComputedScopes); @@ -334,21 +335,33 @@ describe('AstExtensionService disposal', () => { it('throws on duplicate id', () => { const service = makeService(); - service.register({ id: 'dup', nodeFilter: isType('Foo'), state: DocumentState.ComputedScopes, compute: () => undefined }); + service.register({ id: 'dup', nodeFilter: isType('Foo'), phase: DocumentState.ComputedScopes, compute: () => undefined }); expect(() => - service.register({ id: 'dup', nodeFilter: isType('Foo'), state: DocumentState.ComputedScopes, compute: () => undefined }) + service.register({ id: 'dup', nodeFilter: isType('Foo'), phase: DocumentState.ComputedScopes, compute: () => undefined }) ).toThrow(/Duplicate registry id: 'dup'/); }); }); +describe('AstExtensionService.register', () => { + it.each([ + ['Changed', DocumentState.Changed], + ['IndexedContent', DocumentState.IndexedContent] + ])('refuses phase %s, which the pipeline never notifies', (name, phase) => { + const service = makeService(); + // The type rejects the literal, so only a cast or a JavaScript adopter reaches the guard. + const extension = { id: 'never', nodeFilter: isType('Foo'), phase, compute: () => undefined } as unknown as AstExtension; + expect(() => service.register(extension)).toThrow(new RegExp(`'never' registered at phase ${name} would never fire`)); + }); +}); + describe('AstExtensionService.extendDocument profiling', () => { afterEach(() => Logger.setLevel('info')); it('profiles per-registration self-time and reports it line-based at debug level', () => { const { tracer, lines } = makeCapturingTracer(makeFakeClock()); const service = new DefaultAstExtensionService(makeNoopLanguageServices({ shared: { Tracer: tracer } })); - service.register({ id: 'extA', nodeFilter: isType('Foo'), state: DocumentState.Linked, compute: () => undefined }); - service.register({ id: 'extB', nodeFilter: isType('Foo'), state: DocumentState.Linked, compute: () => undefined }); + service.register({ id: 'extA', nodeFilter: isType('Foo'), phase: DocumentState.Linked, compute: () => undefined }); + service.register({ id: 'extB', nodeFilter: isType('Foo'), phase: DocumentState.Linked, compute: () => undefined }); Logger.setLevel('debug'); const doc = buildDocument(makeFakeAstNode({ $type: 'Root' }), [ @@ -365,7 +378,7 @@ describe('AstExtensionService.extendDocument profiling', () => { it('allocates no session at the default info level', () => { const { tracer, lines } = makeCapturingTracer(makeFakeClock()); const service = new DefaultAstExtensionService(makeNoopLanguageServices({ shared: { Tracer: tracer } })); - service.register({ id: 'extA', nodeFilter: isType('Foo'), state: DocumentState.Linked, compute: () => undefined }); + service.register({ id: 'extA', nodeFilter: isType('Foo'), phase: DocumentState.Linked, compute: () => undefined }); const doc = buildDocument(makeFakeAstNode({ $type: 'Root' }), [makeFakeAstNode({ $type: 'Foo', id: 'a' })]); service.extendDocument(doc, DocumentState.Linked); @@ -377,10 +390,10 @@ describe('AstExtensionService.extendDocument profiling', () => { describe('AstExtensionService contribution group consumption', () => { it('reads `services.ast.extensions` and calls each contribution at construction', () => { const calls: string[] = []; - const ext = (id: string) => ({ + const ext = (id: string): AstExtension => ({ id, nodeFilter: isType('Foo'), - state: DocumentState.ComputedScopes, + phase: DocumentState.ComputedScopes, compute: () => undefined }); const services = makeNoopLanguageServices({ @@ -414,7 +427,7 @@ describe('AstExtensionService contribution group consumption', () => { service.register({ id: 'sink', nodeFilter: isType('Foo'), - state: DocumentState.ComputedScopes, + phase: DocumentState.ComputedScopes, compute: () => void sink.push('sink') }); service.extendNode(makeFakeAstNode({ $type: 'Foo' }), {} as LangiumDocument, DocumentState.ComputedScopes); diff --git a/packages/core/test/langium/build-phase-pass/build-phase-pass-service.test.ts b/packages/core/test/langium/build-phase-pass/build-phase-pass-service.test.ts index e44eb48b..16ad89a0 100644 --- a/packages/core/test/langium/build-phase-pass/build-phase-pass-service.test.ts +++ b/packages/core/test/langium/build-phase-pass/build-phase-pass-service.test.ts @@ -18,6 +18,7 @@ import { DefaultBuildPhasePassService } from '../../../src/langium/build-phase-pass/build-phase-pass-service.js'; import { type CapturedLine, makeCapturingTracer, makeNoopSharedServices } from '../../../src/testing/index.js'; +import { type BuildPhasePass } from '../../../src/langium/build-phase-pass/build-phase-pass.js'; function makeService(buildPhasePasses?: unknown): BuildPhasePassService { return new DefaultBuildPhasePassService(makeNoopSharedServices({ buildPhasePasses })); @@ -26,11 +27,11 @@ function makeService(buildPhasePasses?: unknown): BuildPhasePassService { const tick = (): Promise => new Promise(resolve => setTimeout(resolve, 0)); describe('BuildPhasePassService dispatch', () => { - it('runs only passes whose `state` matches the phase', async () => { + it('runs only passes whose `phase` matches', async () => { const service = makeService(); const ran: string[] = []; - service.register({ id: 'linked', state: DocumentState.Linked, run: () => void ran.push('linked') }); - service.register({ id: 'parsed', state: DocumentState.Parsed, run: () => void ran.push('parsed') }); + service.register({ id: 'linked', phase: DocumentState.Linked, run: () => void ran.push('linked') }); + service.register({ id: 'parsed', phase: DocumentState.Parsed, run: () => void ran.push('parsed') }); await service.runPasses([], DocumentState.Linked, CancellationToken.None); expect(ran).toEqual(['linked']); @@ -39,9 +40,9 @@ describe('BuildPhasePassService dispatch', () => { it('runs passes in priority order, lower first', async () => { const service = makeService(); const ran: string[] = []; - service.register({ id: 'late', priority: 10, state: DocumentState.Linked, run: () => void ran.push('late') }); - service.register({ id: 'early', priority: -1, state: DocumentState.Linked, run: () => void ran.push('early') }); - service.register({ id: 'mid', priority: 5, state: DocumentState.Linked, run: () => void ran.push('mid') }); + service.register({ id: 'late', priority: 10, phase: DocumentState.Linked, run: () => void ran.push('late') }); + service.register({ id: 'early', priority: -1, phase: DocumentState.Linked, run: () => void ran.push('early') }); + service.register({ id: 'mid', priority: 5, phase: DocumentState.Linked, run: () => void ran.push('mid') }); await service.runPasses([], DocumentState.Linked, CancellationToken.None); expect(ran).toEqual(['early', 'mid', 'late']); @@ -50,8 +51,8 @@ describe('BuildPhasePassService dispatch', () => { it('breaks priority ties by registration order', async () => { const service = makeService(); const ran: string[] = []; - service.register({ id: 'first', state: DocumentState.Linked, run: () => void ran.push('first') }); - service.register({ id: 'second', state: DocumentState.Linked, run: () => void ran.push('second') }); + service.register({ id: 'first', phase: DocumentState.Linked, run: () => void ran.push('first') }); + service.register({ id: 'second', phase: DocumentState.Linked, run: () => void ran.push('second') }); await service.runPasses([], DocumentState.Linked, CancellationToken.None); expect(ran).toEqual(['first', 'second']); @@ -63,14 +64,14 @@ describe('BuildPhasePassService dispatch', () => { service.register({ id: 'async', priority: 0, - state: DocumentState.Linked, + phase: DocumentState.Linked, run: async () => { events.push('async:start'); await tick(); events.push('async:end'); } }); - service.register({ id: 'sync', priority: 1, state: DocumentState.Linked, run: () => void events.push('sync') }); + service.register({ id: 'sync', priority: 1, phase: DocumentState.Linked, run: () => void events.push('sync') }); await service.runPasses([], DocumentState.Linked, CancellationToken.None); expect(events).toEqual(['async:start', 'async:end', 'sync']); @@ -82,7 +83,7 @@ describe('BuildPhasePassService dispatch', () => { let received: { documents: readonly LangiumDocument[]; token: CancellationToken } | undefined; service.register({ id: 'capture', - state: DocumentState.Linked, + phase: DocumentState.Linked, run: (docs, token) => void (received = { documents: docs, token }) }); @@ -96,7 +97,7 @@ describe('BuildPhasePassService cancellation', () => { it('runs nothing when the entry token is already cancelled', async () => { const service = makeService(); const ran: string[] = []; - service.register({ id: 'a', state: DocumentState.Linked, run: () => void ran.push('a') }); + service.register({ id: 'a', phase: DocumentState.Linked, run: () => void ran.push('a') }); await service.runPasses([], DocumentState.Linked, CancellationToken.Cancelled); expect(ran).toEqual([]); @@ -116,13 +117,13 @@ describe('BuildPhasePassService cancellation', () => { service.register({ id: 'a', priority: 0, - state: DocumentState.Linked, + phase: DocumentState.Linked, run: () => { ran.push('a'); cancelled = true; } }); - service.register({ id: 'b', priority: 1, state: DocumentState.Linked, run: () => void ran.push('b') }); + service.register({ id: 'b', priority: 1, phase: DocumentState.Linked, run: () => void ran.push('b') }); await service.runPasses([], DocumentState.Linked, token); expect(ran).toEqual(['a']); @@ -132,16 +133,23 @@ describe('BuildPhasePassService cancellation', () => { describe('BuildPhasePassService registration', () => { it('throws on duplicate id', () => { const service = makeService(); - service.register({ id: 'dup', state: DocumentState.Linked, run: () => undefined }); - expect(() => service.register({ id: 'dup', state: DocumentState.Linked, run: () => undefined })).toThrow( + service.register({ id: 'dup', phase: DocumentState.Linked, run: () => undefined }); + expect(() => service.register({ id: 'dup', phase: DocumentState.Linked, run: () => undefined })).toThrow( /Duplicate registry id: 'dup'/ ); }); + it('refuses phase Changed, which the pipeline never notifies', () => { + const service = makeService(); + // The type rejects the literal, so only a cast or a JavaScript adopter reaches the guard. + const pass = { id: 'never', phase: DocumentState.Changed, run: () => undefined } as unknown as BuildPhasePass; + expect(() => service.register(pass)).toThrow(/'never' registered at phase Changed would never fire/); + }); + it('disposal removes the pass so subsequent runs skip it', async () => { const service = makeService(); const ran: string[] = []; - const handle = service.register({ id: 'x', state: DocumentState.Linked, run: () => void ran.push('x') }); + const handle = service.register({ id: 'x', phase: DocumentState.Linked, run: () => void ran.push('x') }); await service.runPasses([], DocumentState.Linked, CancellationToken.None); handle.dispose(); @@ -155,12 +163,12 @@ describe('BuildPhasePassService registration', () => { const service = makeService({ alpha: { registerBuildPhasePasses: (registry: BuildPhasePassService) => { - registry.register({ id: 'alpha-pass', priority: 0, state: DocumentState.Linked, run: () => void ran.push('alpha') }); + registry.register({ id: 'alpha-pass', priority: 0, phase: DocumentState.Linked, run: () => void ran.push('alpha') }); } }, beta: { registerBuildPhasePasses: (registry: BuildPhasePassService) => { - registry.register({ id: 'beta-pass', priority: 1, state: DocumentState.Linked, run: () => void ran.push('beta') }); + registry.register({ id: 'beta-pass', priority: 1, phase: DocumentState.Linked, run: () => void ran.push('beta') }); } } }); @@ -181,8 +189,8 @@ describe('BuildPhasePassService profiling', () => { it('profiles per-pass self-time and reports it line-based at debug level', async () => { const { service, lines } = capturingService(); - service.register({ id: 'passA', state: DocumentState.Linked, run: () => undefined }); - service.register({ id: 'passB', state: DocumentState.Linked, run: () => undefined }); + service.register({ id: 'passA', phase: DocumentState.Linked, run: () => undefined }); + service.register({ id: 'passB', phase: DocumentState.Linked, run: () => undefined }); Logger.setLevel('debug'); await service.runPasses([], DocumentState.Linked, CancellationToken.None); @@ -194,7 +202,7 @@ describe('BuildPhasePassService profiling', () => { it('allocates no session at the default info level', async () => { const { service, lines } = capturingService(); - service.register({ id: 'passA', state: DocumentState.Linked, run: () => undefined }); + service.register({ id: 'passA', phase: DocumentState.Linked, run: () => undefined }); await service.runPasses([], DocumentState.Linked, CancellationToken.None); expect(lines.map(line => line.message).filter(message => message.includes('[profile'))).toHaveLength(0); diff --git a/packages/core/test/langium/build-phases.types.test.ts b/packages/core/test/langium/build-phases.types.test.ts new file mode 100644 index 00000000..544f1f48 --- /dev/null +++ b/packages/core/test/langium/build-phases.types.test.ts @@ -0,0 +1,50 @@ +/******************************************************************************** + * Copyright (c) 2026 EclipseSource and others. + * + * This program and the accompanying materials are made available under the + * terms of the MIT License which is available in the project root. + * + * SPDX-License-Identifier: MIT + ********************************************************************************/ + +import { DocumentState } from '@hydranium/langium'; +import { describe, expect, it } from 'vitest'; +import { type AstExtension, AstExtensionPhase } from '../../src/langium/ast-extension/ast-extension-service.js'; +import { type BuildPhasePass, BuildPassPhase } from '../../src/langium/build-phase-pass/build-phase-pass.js'; + +describe('AstExtensionPhase', () => { + it('rejects a phase the pipeline never notifies at compile time', () => { + const extension: AstExtension = { + id: 'changed', + nodeFilter: (node): node is never => node === undefined, + // @ts-expect-error `Changed` has no AST yet, so no listener is wired for it + phase: DocumentState.Changed, + compute: () => undefined + }; + expect(AstExtensionPhase.all()).not.toContain(extension.phase); + }); + + it('all() leaves out Changed, which has no AST, and IndexedContent, which per document adds nothing to Parsed', () => { + expect(AstExtensionPhase.all()).not.toContain(DocumentState.Changed); + expect(AstExtensionPhase.all()).not.toContain(DocumentState.IndexedContent); + expect(AstExtensionPhase.all()).toContain(DocumentState.Parsed); + }); +}); + +describe('BuildPassPhase', () => { + it('rejects a phase the pipeline never notifies at compile time', () => { + const pass: BuildPhasePass = { + id: 'changed', + // @ts-expect-error `Changed` has no AST yet, so no listener is wired for it + phase: DocumentState.Changed, + run: () => undefined + }; + expect(BuildPassPhase.all()).not.toContain(pass.phase); + }); + + it('all() keeps IndexedContent, where the global index is whole, and leaves out Changed', () => { + expect(BuildPassPhase.all()).not.toContain(DocumentState.Changed); + expect(BuildPassPhase.all()).toContain(DocumentState.IndexedContent); + expect(BuildPassPhase.all()).toContain(DocumentState.Parsed); + }); +}); diff --git a/packages/core/test/langium/document-builder/build-pipeline-integration.test.ts b/packages/core/test/langium/document-builder/build-pipeline-integration.test.ts index c23b19df..449ac5a3 100644 --- a/packages/core/test/langium/document-builder/build-pipeline-integration.test.ts +++ b/packages/core/test/langium/document-builder/build-pipeline-integration.test.ts @@ -270,7 +270,7 @@ describe('BuildPipelineIntegration foundational pass ordering', () => { // it cleans/mutates the AST that derived adopter passes read — so it sits in // a negative foundational band rather than tying with the adopter's default 0 // (a tie would break by registration order, letting the adopter win). - passes.register({ id: 'adopter', state: DocumentState.Linked, run: () => void order.push('adopter') }); + passes.register({ id: 'adopter', phase: DocumentState.Linked, run: () => void order.push('adopter') }); const integrityService = { enforceBatch: () => { diff --git a/packages/protocol/src/errors.ts b/packages/protocol/src/errors.ts index ea70a0ab..4609ec84 100644 --- a/packages/protocol/src/errors.ts +++ b/packages/protocol/src/errors.ts @@ -133,18 +133,15 @@ export class ConflictError extends ResponseError { } /** - * The base version the caller authored against. - * - * Must not be renamed to `expected`, nor its sibling to `actual`: a test - * reporter reads an error carrying both as an assertion failure, and - * vitest's formatter then ASSIGNS to them, which throws on an accessor and - * replaces the real failure with a `TypeError`. + * The base version the caller authored against. Not `expected`, and its + * sibling not `actual`: a test reporter reads that pair as an assertion + * failure and reports nothing of the real one. */ get baseVersion(): ModelVersion { return this.data!.baseVersion; } - /** The server's version at the time of the throw. Not `actual` — see {@link baseVersion}. */ + /** The server's version at the time of the throw. Not `actual`, see {@link baseVersion}. */ get actualVersion(): TextVersion { return this.data!.actualVersion; }