Skip to content

refactor language/src/core#19

Open
awli wants to merge 7 commits intomainfrom
refactor-core
Open

refactor language/src/core#19
awli wants to merge 7 commits intomainfrom
refactor-core

Conversation

@awli
Copy link
Copy Markdown
Contributor

@awli awli commented Apr 30, 2026

What

Why

How

Test Plan

  • Existing tests pass (pnpm test)
  • New/updated tests cover the change
  • Linting and type checks pass (pnpm lint && pnpm typecheck)

Checklist

  • My code follows the project's coding style
  • I have reviewed my own diff
  • I have added/updated documentation as needed
  • This change does not introduce new warnings

awli added 7 commits April 29, 2026 16:07
…criminant API

The pair `{ discriminantField?, resolveSchemaForDiscriminant?() }` appeared
byte-identically in four places (FieldTypeBase, NamedBlockEntryType,
BlockFactory, NamedBlockFactory). Extracted into a new `MaybeDiscriminant`
interface in types.ts and have all four extend it.

Also rewrote `DiscriminantProvider` as `Required<MaybeDiscriminant>` — it's
the narrowed form produced by the `hasDiscriminant()` guard, so deriving it
keeps the two shapes in sync automatically.
…to Map

Introduce abstract `ExpressionBase` class that extends `AstNodeBase` and
provides the default `__describe()` implementation (`expression <emit>`).
All expression classes now extend `ExpressionBase`; the six subclasses that
carried a verbatim duplicate of the default body (`MemberExpression`,
`SubscriptExpression`, `BinaryExpression`, `UnaryExpression`,
`ComparisonExpression`, `TernaryExpression`) lose their explicit definition
and inherit it.

Remove the per-class `static readonly kindLabel` field. Its only consumer is
the `KIND_LABELS` map built at the bottom of expressions.ts (used by
primitives.ts for type-mismatch error messages). Moved all labels inline
into that map — one source of truth, no scattered static fields.
Four moves consolidating lint infrastructure where it belongs:

  core/analysis/lint.ts              → core/analysis/lint-engine.ts
  lint/schema-walker.ts              → core/analysis/schema-walker.ts
  lint/symbol-table.ts               → core/analysis/symbol-table.ts
  lint/position-index.ts             → core/analysis/position-index-pass.ts

The file formerly called `lint.ts` contains the `LintEngine`/`PassStore`/
`defineRule` framework — not lint rules. Rename makes that explicit.

`schema-walker.ts`, `symbol-table.ts`, and `position-index.ts` under `lint/`
were framework adapters (e.g., the `position-index.ts` pass populates the
key already defined in `core/analysis/position-index.ts`), not lint rules.
Moving them next to their key/interface definitions removes an awkward
split where `service.ts` imported the key from `core/analysis/` while the
pass that populated it lived in `lint/`. The `lint/` directory is now
purely rules + utilities.

Also make `core/analysis/index.ts` the canonical barrel for analysis
exports — it now covers what the package-level `src/index.ts` was
re-exporting via deep paths. Renamed position-index-pass.ts to avoid
clashing with the existing position-index.ts that holds the `storeKey` and
query helpers.

All imports across the package updated; full typecheck + test suite green.
…e.ts

Moved TemplateText, TemplateInterpolation, TemplateExpression, parseTemplateParts,
TEMPLATE_PART_KINDS, isTemplatePartKind, and all dedent/clean helpers
(~390 lines) out of expressions.ts into a new core/template.ts. The
template subsystem had zero coupling to other expression classes — it only
used the Expression type for its interpolation payload and extended
ExpressionBase — so the split is clean.

Also extracted the Expression interface and ExpressionBase abstract class
into expression-base.ts to avoid the runtime import cycle that would
otherwise result (template.ts needs ExpressionBase; expressions.ts
re-exports TemplateExpression). expression-base.ts is a leaf file imported
by both expressions.ts and template.ts.

expressions.ts re-exports the template primitives so existing consumers
keep working without import-path changes; public surface is unchanged.
dialect.ts had grown to ~1700 lines, with several chunks that did not
actually need to live inside the Dialect class (which holds no state —
no fields, no constructor). This commit extracts four cohesive slices:

  core/discriminant.ts — DiscriminantConfig + prescanDiscriminantValue.
    Used by Block/NamedBlock factories via import, now imported from the
    new home. Re-exported from dialect.ts for back-compat.

  core/cst-diagnostics.ts — collectAllCstDiagnostics + missingNodeRange
    + collectCstDiagnosticsInner. Free functions that walk the parser
    CST collecting ERROR/MISSING diagnostics.

  core/comment-attacher.ts — parseInlineComments, parseElementComments,
    splitContainerComments, attachToFirstTypedMapEntry,
    attachToFirstProcedureStatement, attachToLastProcedureStatement,
    plus the hasProcedureStatements type guard. Previously private
    methods on Dialect that only used `this` to call a one-line
    parseComment delegator — now call sharedParseCommentNode directly.

Also removed the dead `Dialect.emit` method (28 lines, zero callers
across the monorepo — confirmed by grep of packages/, dialect/, apps/).
emitDocument in core/emit.ts is the real entry point.

No behavior change. Typecheck + full test suite + build pass.
Every call to `Block()`, `NamedBlock()`, `CollectionBlock()`, `TypedMap()`
did two back-to-back passes:

  1. `addBuilderMethods(node)` — installed ~22 methods on the node,
     including `.describe()`, `.example()`, `.required()`, `.extend()`,
     `.omit()`, `.pick()`, `.withProperties()`, `.clone()`, etc.
  2. `overrideFactoryBuilderMethods(base)` — immediately overwrote 14 of
     those exact methods with factory-typed versions (needed so chains
     preserve the concrete factory type).

The first install of those 14 methods was pure waste — closures allocated,
properties assigned, then overwritten without ever being called. Multiply
by every schema field declaration and it adds up.

Added an opt-in `{ factory: true }` options argument to `addBuilderMethods`
that skips the 14 methods `overrideFactoryBuilderMethods` will replace.
Factories pass the flag; primitives (StringValue/NumberValue/etc.) and
Sequence, which want the FieldBuilder-flavored methods, continue to use
the default behavior.

No public-API change (the options arg is optional). Typecheck + full
monorepo test suite + build pass; 157 language tests green.
@awli awli requested a review from Kamehameya April 30, 2026 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant