tml-2956: PSL attribute-spec kit + @relation migration (1/3)#891
Open
SevInf wants to merge 26 commits into
Open
tml-2956: PSL attribute-spec kit + @relation migration (1/3)#891SevInf wants to merge 26 commits into
SevInf wants to merge 26 commits into
Conversation
…nference Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Add diagnosticCode to InterpretCtx and have interpretAttribute populate a leaf-facing context from the spec before each leaf parse, so a leaf emits with the attribute code rather than a hard-coded generic. Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Author the domain combinators @relation needs as ArgTypes over the expression AST, beside the attribute-spec engine, and export them from the package surface. enumOf types a homogeneous or mixed string/number set via a const tuple and a membership type-guard (no arktype: the value is already a parsed primitive and the check is a trivial closed set). fieldRef returns the bare name and carries scope metadata, deliberately leaving field-existence validation to the downstream interpreter. list lifts an element combinator over an array literal with nonEmpty/unique. Leaves emit through a shared helper stamped with ctx.diagnosticCode. Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
…fine specs The SQL @relation spec parses bare-identifier referential actions (onDelete: Cascade) whose set is validated downstream, so it needs a leaf that reads an IdentifierAst to its name with no parse-time check — fieldRef minus the resolution scope. Add identifierName() and export it. InferAttr previously constrained its parameter to AttributeSpec<unknown>, but Out sits contravariantly in AttributeSpec.refine, so any refine- carrying spec is not assignable to AttributeSpec<unknown> and the constraint rejected it. Drop the constraint; inference still recovers Out precisely. Adds a type-test for a refine-carrying spec. Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
…ttribute spec Replace the hand-written parseRelationAttribute with a sqlRelation AttributeSpec interpreted through the kit. The engine binds the positional-or-named name, fields/references field lists, map, and the bare-identifier onDelete/onUpdate; a refine holds the fields/references both-or-neither rule. Referential-action set validation stays downstream in normalizeReferentialAction (unchanged), preserving its code and span. interpretRelationAttribute assembles the InterpretCtx from interpreter state (declaring model, field, symbol table, source file) and maps the inferred output onto ParsedRelationAttribute. The three @relation call sites in interpreter.ts route through it; BuildModelNodeInput gains the source file and symbol table to assemble the context. Diagnostic codes and spans stay byte-identical for every @relation error path: the relations, many-to-many, and diagnostics suites pass unchanged and fixtures:check is clean. parseRelationAttribute is deleted; the helpers it used remain (other callers). Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
…n, review, briefs) Drive project/slice paper trail for the attribute-spec-kit slice: project spec + plan, slice spec + dispatch plan, dispatch briefs (D2/D3), and the code-review log. Transient project artifacts (removed at project close); no source references them. Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds attribute-spec parsing primitives and type inference utilities, re-exports them from ChangesAttribute-spec framework and relation migration
Foundation utility type exports
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
@prisma-next/extension-author-tools
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@prisma-next/extension-arktype-json
@prisma-next/middleware-cache
@prisma-next/mongo
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/extension-postgis
@prisma-next/postgres
@prisma-next/sql-orm-client
@prisma-next/sqlite
@prisma-next/extension-supabase
@prisma-next/target-mongo
@prisma-next/adapter-mongo
@prisma-next/driver-mongo
@prisma-next/contract
@prisma-next/utils
@prisma-next/config
@prisma-next/errors
@prisma-next/framework-components
@prisma-next/operations
@prisma-next/ts-render
@prisma-next/contract-authoring
@prisma-next/ids
@prisma-next/psl-parser
@prisma-next/psl-printer
@prisma-next/cli
@prisma-next/cli-telemetry
@prisma-next/config-loader
@prisma-next/emitter
@prisma-next/language-server
@prisma-next/migration-tools
prisma-next
@prisma-next/vite-plugin-contract-emit
@prisma-next/mongo-codec
@prisma-next/mongo-contract
@prisma-next/mongo-value
@prisma-next/mongo-contract-psl
@prisma-next/mongo-contract-ts
@prisma-next/mongo-emitter
@prisma-next/mongo-schema-ir
@prisma-next/mongo-query-ast
@prisma-next/mongo-orm
@prisma-next/mongo-query-builder
@prisma-next/mongo-lowering
@prisma-next/mongo-wire
@prisma-next/sql-contract
@prisma-next/sql-errors
@prisma-next/sql-operations
@prisma-next/sql-schema-ir
@prisma-next/sql-contract-psl
@prisma-next/sql-contract-ts
@prisma-next/sql-contract-emitter
@prisma-next/sql-lane-query-builder
@prisma-next/sql-relational-core
@prisma-next/sql-builder
@prisma-next/target-postgres
@prisma-next/target-sqlite
@prisma-next/adapter-postgres
@prisma-next/adapter-sqlite
@prisma-next/driver-postgres
@prisma-next/driver-sqlite
commit: |
size-limit report 📦
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/1-framework/2-authoring/psl-parser/src/attribute-spec/interpret.ts`:
- Around line 40-56: The named-argument loop in interpret should reject
duplicate keys instead of skipping them in silence. Update the duplicate check
around namedSeen in the attribute-spec parsing logic so that a repeated key
emits a structural diagnostic for the attribute and does not proceed as a
successful parse. Use the existing diagnostic pattern in interpret, and keep the
behavior aligned with the unknown-argument handling for spec.name and namedArgs.
In `@packages/2-sql/2-authoring/contract-psl/src/psl-relation-resolution.ts`:
- Around line 141-142: The relation list schema still permits duplicate column
names, which can let repeated FK entries flow into foreign key resolution.
Update the `fields` and `references` definitions in `psl-relation-resolution` to
enforce uniqueness by adding the list-level uniqueness constraint alongside the
existing `nonEmpty` validation. Keep the change localized to the relation
parsing schema so `foreignKeyNodes` only receives deduplicated names.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 32be81a6-69d0-4823-9438-44961008ee55
⛔ Files ignored due to path filters (6)
projects/typed-attribute-parsers/plan.mdis excluded by!projects/**projects/typed-attribute-parsers/slices/attribute-spec-kit/dispatches/02-combinators.mdis excluded by!projects/**projects/typed-attribute-parsers/slices/attribute-spec-kit/dispatches/03-migrate-relation.mdis excluded by!projects/**projects/typed-attribute-parsers/slices/attribute-spec-kit/plan.mdis excluded by!projects/**projects/typed-attribute-parsers/slices/attribute-spec-kit/spec.mdis excluded by!projects/**projects/typed-attribute-parsers/spec.mdis excluded by!projects/**
📒 Files selected for processing (17)
packages/1-framework/2-authoring/psl-parser/src/attribute-spec/combinators/diagnostic.tspackages/1-framework/2-authoring/psl-parser/src/attribute-spec/combinators/enum-of.tspackages/1-framework/2-authoring/psl-parser/src/attribute-spec/combinators/field-ref.tspackages/1-framework/2-authoring/psl-parser/src/attribute-spec/combinators/identifier-name.tspackages/1-framework/2-authoring/psl-parser/src/attribute-spec/combinators/list.tspackages/1-framework/2-authoring/psl-parser/src/attribute-spec/combinators/str.tspackages/1-framework/2-authoring/psl-parser/src/attribute-spec/field-attribute.tspackages/1-framework/2-authoring/psl-parser/src/attribute-spec/interpret.tspackages/1-framework/2-authoring/psl-parser/src/attribute-spec/optional.tspackages/1-framework/2-authoring/psl-parser/src/attribute-spec/types.tspackages/1-framework/2-authoring/psl-parser/src/exports/index.tspackages/1-framework/2-authoring/psl-parser/test/attribute-spec-combinators.test-d.tspackages/1-framework/2-authoring/psl-parser/test/attribute-spec-combinators.test.tspackages/1-framework/2-authoring/psl-parser/test/attribute-spec.test-d.tspackages/1-framework/2-authoring/psl-parser/test/attribute-spec.test.tspackages/2-sql/2-authoring/contract-psl/src/interpreter.tspackages/2-sql/2-authoring/contract-psl/src/psl-relation-resolution.ts
SevInf
commented
Jun 30, 2026
| * action routed to its normaliser — so a parse-time check would emit a second | ||
| * diagnostic for the same fault. Like `fieldRef` minus the resolution scope. | ||
| */ | ||
| export function identifierName(): ArgType<string> { |
Contributor
Author
There was a problem hiding this comment.
Why do we need this? Shouldn't referential action be useable via enumOf already?
SevInf
commented
Jun 30, 2026
| if (!(arg instanceof ArrayLiteralAst)) { | ||
| return notOk([leafDiagnostic(ctx, arg, `Expected a list of ${of.label}`)]); | ||
| } | ||
| const elements = [...arg.elements()]; |
Contributor
Author
There was a problem hiding this comment.
Why are we copying the list?
SevInf
commented
Jun 30, 2026
SevInf
commented
Jun 30, 2026
| const hasNamed = namedParsed.has(key); | ||
| if ( | ||
| hasPositional && | ||
| hasNamed && |
Contributor
Author
There was a problem hiding this comment.
Just don't allow duplicate arguments, it does not matter if their value is the same
SevInf
commented
Jun 30, 2026
| if (result.ok) namedParsed.set(key, result.value); | ||
| } | ||
|
|
||
| const positionalSeen = new Set<string>(); |
Contributor
Author
There was a problem hiding this comment.
This is way to complicated for what it is supposed to be doing? Why are we doing one pass over positional args, one pass over named arguments, then merging the two in a separate pass? Just produce a single map in one pass and report duplicates as you go.
Contributor
Author
There was a problem hiding this comment.
This is not addressed. Parsing is still done in 3 passes. With a separate maps for positional and named arguments, each bracnh handling it's duplication code, plus merging the 2 handling merge as well.
I want both positional and named passes to write intou output directly without intermediate maps and merge step. Named arg can conflict with positional arg key and it should produce an error.
SevInf
commented
Jun 30, 2026
Comment on lines
+8
to
+14
| type Simplify<T> = { [K in keyof T]: T[K] } & {}; | ||
|
|
||
| type UnionToIntersection<U> = (U extends unknown ? (k: U) => void : never) extends ( | ||
| k: infer I, | ||
| ) => void | ||
| ? I | ||
| : never; |
Contributor
Author
There was a problem hiding this comment.
I am pretty sure we already have these helpers defined in several places. Reuse them
SevInf
commented
Jun 30, 2026
| readonly key: string; | ||
| readonly type: Param<T>; | ||
| /** A trailing rest slot that consumes every remaining positional argument. */ | ||
| readonly variadic?: boolean; |
Contributor
Author
There was a problem hiding this comment.
Do we really have variadic positional arguments anywhere?
SevInf
commented
Jun 30, 2026
| * default when the argument is absent; without one, an absent argument leaves | ||
| * the output property unset. | ||
| */ | ||
| export function optional<T>(type: ArgType<T>, ...rest: [defaultValue: T] | []): OptionalParam<T> { |
Contributor
Author
There was a problem hiding this comment.
Why isn't this an ArgType on it's own?
Contributor
Author
There was a problem hiding this comment.
this was not addressed
SevInf
commented
Jun 30, 2026
| * The engine aggregates every error path's diagnostics; like the previous | ||
| * first-error parser, the caller skips the field when this returns `undefined`. | ||
| */ | ||
| export function interpretRelationAttribute(input: { |
Contributor
Author
There was a problem hiding this comment.
The only thing this function does is renaming map to constraintName? Remove it and use interpretAttribute result directly. Same is true for mongo code.
A named key supplied twice, or a key supplied both positionally and by name, is now always a structural error — previously a duplicate named key was silently dropped and a positional/named alias only conflicted when the values differed. Drop the now-dead argValuesEqual/isPlainRecord helpers. Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
list() spread arg.elements() into a second array only to read its length for the nonEmpty check. Iterate the generator once and track a count instead; behaviour is identical. Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
…eferences Mark the @relation fields and references lists unique:true so a repeated FK column name is rejected at parse and can never reach foreignKeyNodes. Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Extend enumOf to accept a bare identifier matching a string member, then route @relation onDelete/onUpdate through enumOf instead of the bespoke identifierName leaf. A bad action now errors at parse with the attribute code (PSL_INVALID_RELATION_ATTRIBUTE) rather than downstream PSL_UNSUPPORTED_REFERENTIAL_ACTION; normalizeReferentialAction becomes a pure token-to-action mapper. Delete identifierName and its export. Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
fieldRef now resolves a field name against its scoped model — self against the declaring model, referenced against the relation target — and emits the field-existence diagnostic at parse with the attribute code. A referenced target out of scope (cross-space) is still carried through unchecked. For @relation this short-circuits the parse before the downstream column resolution, so a missing field yields exactly one diagnostic; the downstream mapping is retained for column lookup. Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
…rity flags Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/2-sql/2-authoring/contract-psl/src/psl-relation-resolution.ts (1)
128-129: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve duplicate relation-list behavior.
Adding
unique: truerejects duplicate names infields/references, which tightens behavior relative to the legacy relation parser; the downstream test atpackages/2-sql/2-authoring/contract-psl/test/interpreter.relations.test.tsLines 389-417 now codifies that stricter behavior. Remove the uniqueness constraint here.Proposed adjustment
- fields: optional(list(fieldRef('self'), { nonEmpty: true, unique: true })), - references: optional(list(fieldRef('referenced'), { nonEmpty: true, unique: true })), + fields: optional(list(fieldRef('self'), { nonEmpty: true })), + references: optional(list(fieldRef('referenced'), { nonEmpty: true })),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/2-sql/2-authoring/contract-psl/src/psl-relation-resolution.ts` around lines 128 - 129, The relation-list validation in psl-relation-resolution should preserve the legacy ability to repeat names, so remove the unique: true constraint from both fields and references in the relation parser schema. Update the schema definition around the fields/references list handling in psl-relation-resolution so it only enforces nonEmpty where needed, and keep the existing relation resolution flow unchanged. Use the existing identifiers optional(list(...)), fieldRef('self'), and fieldRef('referenced') to locate the exact validation rules to adjust.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/2-sql/2-authoring/contract-psl/src/psl-relation-resolution.ts`:
- Around line 87-88: The referential-action validation in
normalizeReferentialAction is now bypassed by enumOf, causing unsupported
onDelete/onUpdate values to surface as PSL_INVALID_RELATION_ATTRIBUTE instead of
the legacy unsupported-action diagnostic. Restore the old diagnostic parity by
moving the unsupported-value check into the downstream normalizer path used by
normalizeReferentialAction, or add a per-argument diagnostic override in the
relation attribute parsing flow so the specific unsupported referential-action
code is preserved.
---
Duplicate comments:
In `@packages/2-sql/2-authoring/contract-psl/src/psl-relation-resolution.ts`:
- Around line 128-129: The relation-list validation in psl-relation-resolution
should preserve the legacy ability to repeat names, so remove the unique: true
constraint from both fields and references in the relation parser schema. Update
the schema definition around the fields/references list handling in
psl-relation-resolution so it only enforces nonEmpty where needed, and keep the
existing relation resolution flow unchanged. Use the existing identifiers
optional(list(...)), fieldRef('self'), and fieldRef('referenced') to locate the
exact validation rules to adjust.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: f5ca7493-831e-4d57-aaed-4fb80a8a2ce5
⛔ Files ignored due to path filters (2)
projects/typed-attribute-parsers/plan.mdis excluded by!projects/**projects/typed-attribute-parsers/slices/attribute-spec-kit/dispatches/04-address-review-r1.mdis excluded by!projects/**
📒 Files selected for processing (10)
packages/1-framework/2-authoring/psl-parser/src/attribute-spec/combinators/enum-of.tspackages/1-framework/2-authoring/psl-parser/src/attribute-spec/combinators/field-ref.tspackages/1-framework/2-authoring/psl-parser/src/attribute-spec/combinators/list.tspackages/1-framework/2-authoring/psl-parser/src/attribute-spec/interpret.tspackages/1-framework/2-authoring/psl-parser/src/exports/index.tspackages/1-framework/2-authoring/psl-parser/test/attribute-spec-combinators.test.tspackages/1-framework/2-authoring/psl-parser/test/attribute-spec.test.tspackages/2-sql/2-authoring/contract-psl/src/interpreter.tspackages/2-sql/2-authoring/contract-psl/src/psl-relation-resolution.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.relations.test.ts
💤 Files with no reviewable changes (1)
- packages/1-framework/2-authoring/psl-parser/src/exports/index.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/1-framework/2-authoring/psl-parser/src/attribute-spec/combinators/field-ref.ts
- packages/1-framework/2-authoring/psl-parser/src/attribute-spec/combinators/enum-of.ts
- packages/1-framework/2-authoring/psl-parser/src/attribute-spec/combinators/list.ts
- packages/1-framework/2-authoring/psl-parser/test/attribute-spec.test.ts
- packages/1-framework/2-authoring/psl-parser/test/attribute-spec-combinators.test.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/1-framework/2-authoring/psl-parser/src/attribute-spec/combinators/one-of.ts`:
- Around line 16-19: The oneOf() combinator currently accepts zero alternatives,
which allows construction of an impossible ArgType<never> that only fails at
runtime. Update the oneOf function signature in one-of.ts to require a non-empty
rest tuple for alts, so callers must pass at least one ArgType; keep the
existing label-building logic in oneOf and ensure the type still resolves
through OutOf<Alts[number]> for valid non-empty inputs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 8f4b5a02-bb68-4926-8296-3e5eee0629d8
⛔ Files ignored due to path filters (2)
projects/typed-attribute-parsers/plan.mdis excluded by!projects/**projects/typed-attribute-parsers/slices/attribute-spec-kit/dispatches/06-oneof-identifier.mdis excluded by!projects/**
📒 Files selected for processing (6)
packages/1-framework/2-authoring/psl-parser/src/attribute-spec/combinators/identifier.tspackages/1-framework/2-authoring/psl-parser/src/attribute-spec/combinators/one-of.tspackages/1-framework/2-authoring/psl-parser/src/exports/index.tspackages/1-framework/2-authoring/psl-parser/test/attribute-spec-combinators.test-d.tspackages/1-framework/2-authoring/psl-parser/test/attribute-spec-combinators.test.tspackages/2-sql/2-authoring/contract-psl/src/psl-relation-resolution.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/2-sql/2-authoring/contract-psl/src/psl-relation-resolution.ts
…literals enumOf no longer accepts a quoted string as a member. A string member is matched only when written as a bare identifier (e.g. Cascade); a quoted argument is rejected with the leaf diagnostic. Number members continue to match number literals. Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
… string-set follow-up Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Introduce oneOf(...alts) (ordered try-each, union output type, single aggregate diagnostic on total failure) and identifier(name) (pinned bare-identifier matcher with a literal output type). Rewire the relation attribute onDelete/onUpdate from enumOf to oneOf(identifier(...)), leaving the referential-action output union and normalizeReferentialAction mapping unchanged. Delete enumOf now that its behaviour is composed from the two primitives. Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
…ag ADR 231 reconcile at close-out Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Replace the bespoke enumOf combinator throughout ADR 231 with the shipped composition: enums are oneOf over per-member matchers -- identifier(name) for bare identifiers, pinned str(value)/num(value) for quoted-string/number members. Updates the relation code sample, the at-a-glance and language-server narratives, the scalars kit description, and the ADR 224 reference line; the rejected-alternative entry now records the dedicated enum leaf as the rejected option in favour of oneOf composition (principle #4). InferAttr union types are unchanged. Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Add a shared @prisma-next/utils/types module and consume it from the attribute-spec types instead of redefining the helpers locally. Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Type the rest parameter as a non-empty tuple so oneOf() with no alternatives is a compile error. Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
No attribute uses a variadic positional slot. Drop PositionalParam.variadic, the variadic branch in the output-type mapping, and the engine handling. YAGNI. Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Replace OptionalParam with OptionalArgType extending ArgType, collapsing the Param union to ArgType. optional() now spreads the wrapped type and adds the optionality marker; the engine detects optionality from the marker and parses the param directly. Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Replace the positional/named/resolve three-pass engine with one pass over the source arguments that builds the output map and reports duplicates, unknowns, and excess positionals inline, then applies optional defaults and missing-required diagnostics. Preserves diagnostic codes and keeps each error path’s span no coarser than before. Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Drop the ParsedRelationAttribute rename wrapper that mapped name->relationName and map->constraintName. interpretRelationAttribute now returns the spec output (SqlRelationOutput); the interpreter reads name and map directly. Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
…ification) Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
SevInf
force-pushed
the
tml-2956-typed-attribute-parsers
branch
from
b4613a1 to
48bcda1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
Slice 1 of 3 implementing ADR 231 — Declarative attribute specifications, scoped to the family interpreters. It stands up the declarative-attribute engine in
psl-parserand proves it end-to-end by migrating the SQL@relationattribute off its hand-written parser.Language-server integration (completion, go-to-definition over attribute arguments) is the follow-up this unblocks — and the reason the work lands under the Language Tools Support Prisma Next PSL project — but is not in scope here.
Why
Each family interpreter validated attribute arguments with hand-written code that pulled raw text out of the AST and re-checked its shape —
split(',')on field lists, ad-hoc quoted-string unwrapping, per-attribute unknown-argument rejection — duplicated across the SQL and Mongo interpreters. The output type drifted freely from the validation, and no other consumer (the language server) could read what an attribute accepts. ADR 231 replaces that with one declarativeAttributeSpecper attribute, read byinterpretAttribute, with the output type inferred from the spec.What's in this slice
psl-parser—ArgType,AttributeSpec,optional/fieldAttribute,interpretAttribute,InferAttr,InterpretCtx— consuming the parser'sExpressionAstCST directly.@relationneeds —str,enumOf,fieldRef(scope),list({ nonEmpty, unique }), plus a smallidentifierNameleaf for bare-identifier enum values.@relationmigrated to a declarativesqlRelationspec lowered throughinterpretAttribute; the hand-writtenparseRelationAttributeis deleted.Key decisions
ExpressionAstdirectly — no new intermediate argument representation. The CST already carries native literals and real spans.psl-parser, which already ownsExpressionAstand theSymbolTable. No new package.InterpretCtx.diagnosticCode— the engine threads the spec's code (herePSL_INVALID_RELATION_ATTRIBUTE) to every leaf, so codes stay byte-identical.onDelete/onUpdatekeepnormalizeReferentialActionas validator — they parse to the raw identifier name (no parse-time set check), preserving the existingPSL_UNSUPPORTED_REFERENTIAL_ACTIONcode + span.enumOfis intentionally not used for them.@relationleaf spans — strictly more precise, exercised by no fixture); messages may adopt the kit's phrasing; malformed inputs legacy tolerated by coincidence (a quotedonDelete: "Cascade") are now rejected. The span + message + stricter-rejection relaxations were explicitly ratified.Deliberately out of scope
Other SQL attributes (
@id,@unique,@@index,@default,@map,@@control,@@discriminator,@@base) → slice 2. All Mongo attributes → slice 3. The rest of ADR 231's combinator alphabet (int,bool,json,map,record,entityRef,codecRef,oneOf,funcCall,modelAttribute,blockAttribute) is added as later slices need it.@db.*native types and the TypeScript builder surface are out of the whole project.Validation
psl-parser: 504 tests pass (engine + combinators + type-level inference); typecheck + lint clean.@prisma-next/sql-contract-psl: 271 tests pass — the@relationrelations + diagnostics suites pass with zero test edits.pnpm fixtures:checkclean (no contract/migration drift); cross-packagepnpm typecheck143/143.Refs: TML-2956 · ADR 231 (#889).
Summary by CodeRabbit
nonEmpty/unique, optional values, andoneOfalternatives), plus field-level helpers and public exports to support authoring/interpretation.oneOf(identifier(...))usage.