feat(core): refuse a build phase no listener is wired for; ban expected/actual

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 }

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; }'

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.
 

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

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<Task>) => {
             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<Task>) => {
             node._effectSummary = summariseEffects(node);
          }

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<T extends AstNode = AstNode> 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<T extends AstNode = AstNode> 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<AstExtension>();
    /**
-    * 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<DocumentState, readonly AstExtension[]>();
-   protected stateBucketsFor: readonly AstExtension[] | undefined;
+   protected phaseBuckets = new Map<DocumentState, readonly AstExtension[]>();
+   protected phaseBucketsFor: readonly AstExtension[] | undefined;
 
    protected readonly tracer: Tracer;
 
@@ -189,17 +212,27 @@ export class DefaultAstExtensionService implements AstExtensionService {
     * which calls {@link extendDocument} for the wired phases.
     */
    register<T extends AstNode>(extension: AstExtension<T>): 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);
    }
 
    /**
     * 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<DocumentState, AstExtension[]>();
          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