TML-2893: replace legacy PSL parser with parse + ResolvedDocument semantic layer#830
TML-2893: replace legacy PSL parser with parse + ResolvedDocument semantic layer#830SevInf wants to merge 36 commits into
Conversation
|
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:
📝 WalkthroughWalkthroughThis PR replaces legacy PSL parser entrypoints with a parse-then-resolve pipeline, adds qualified-name and resolved-document syntax support, moves validation into resolver and interpreter layers, and updates SQL/Mongo authoring code plus tests to consume resolved inputs. ChangesPSL parse/resolve migration
Sequence Diagram(s)sequenceDiagram
participant Provider
participant parse
participant resolve
participant Interpreter
Provider->>parse: schema source
parse-->>Provider: document + parse diagnostics + sourceFile
Provider->>resolve: document, sourceFile, scalarTypes, descriptors
resolve-->>Provider: ResolvedDocument + diagnostics
Provider->>Interpreter: resolved document + source metadata
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/1-framework/2-authoring/psl-parser/src/parse.ts (1)
293-317:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle string-literal keys in comma/recovery checks too.
After accepting string-literal keys, follow-up logic still branches only on identifiers. At Line 271 and Line 313 this misses
"StringLiteral"keys, which can misparse malformed objects (e.g.{ a "k": 2 }) and surface misleading unterminated-object diagnostics instead of recovering to the next field.Suggested fix
@@ - } else if (cursor.peekKind() === 'Ident') { + } else if (cursor.peekKind() === 'Ident' || cursor.peekKind() === 'StringLiteral') { @@ - const followsWithKey = cursor.peekKind() === 'Ident' && cursor.peekKind(1) === 'Colon'; + const followsWithKey = + (cursor.peekKind() === 'Ident' || cursor.peekKind() === 'StringLiteral') && + cursor.peekKind(1) === 'Colon';🤖 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/1-framework/2-authoring/psl-parser/src/parse.ts` around lines 293 - 317, The parseObjectField function now accepts both identifier keys and string-literal keys (via parseStringLiteralExpr), but the recovery lookahead logic still only checks for identifiers. The followsWithKey condition at line 313 should be updated to also check for StringLiteral kind alongside Ident kind to properly detect when a string-literal key is followed by a colon, ensuring malformed objects with string-literal keys are correctly recovered and avoid misleading unterminated-object diagnostics.
🧹 Nitpick comments (2)
packages/2-sql/2-authoring/contract-psl/src/psl-attribute-parsing.ts (1)
15-20: ⚡ Quick winPrefer intent-expressive helper names over new explanatory block comments.
These new blocks are descriptive, but this package guideline prefers keeping TS code self-explanatory without added comments where avoidable. Consider removing/reducing these blocks and keeping intent in names/types.
As per coding guidelines:
**/*.{ts,tsx,js,jsx,mts,mjs}: Don't add comments if avoidable, prefer code that expresses its intent.Also applies to: 32-36, 53-59
🤖 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-attribute-parsing.ts` around lines 15 - 20, Remove or significantly reduce the explanatory JSDoc comment blocks to align with the package guideline that prefers self-explanatory code over comments. The function and parameter names should express the intent without needing extensive documentation. This applies at three locations in the file: the comment block for the function at lines 15-20 (likely a quote-stripping function), the comment block at lines 32-36, and the comment block at lines 53-59. Either delete these blocks entirely or replace them with minimal single-line descriptions if absolutely necessary, allowing the function names and type signatures to convey the intent instead.Source: Coding guidelines
packages/2-sql/2-authoring/contract-psl/src/interpreter.ts (1)
284-297: ⚡ Quick winPrefer target-configured default namespace over hardcoded
'public'.Line 292 hardcodes the implicit namespace to
'public', while later flow already usesinput.target.defaultNamespaceId(for coordinates and model patching). PassingdefaultNamespaceIdintoresolveNamespaceIdForSqlTargetkeeps namespace resolution single-sourced and avoids config drift.Proposed diff
function resolveNamespaceIdForSqlTarget(input: { readonly bucketName: string; readonly targetId: string; + readonly defaultNamespaceId: string; }): string | undefined { if (input.targetId !== 'postgres') { return undefined; } if (input.bucketName === UNSPECIFIED_PSL_NAMESPACE_ID) { - return 'public'; + return input.defaultNamespaceId; } if (input.bucketName === 'unbound') { return '__unbound__'; } return input.bucketName; }const resolvedNamespaceId = resolveNamespaceIdForSqlTarget({ bucketName: namespace.id, targetId: input.target.targetId, + defaultNamespaceId: input.target.defaultNamespaceId, });const resolvedId = resolveNamespaceIdForSqlTarget({ bucketName: ns.id, targetId: input.target.targetId, + defaultNamespaceId: input.target.defaultNamespaceId, });Also applies to: 2091-2094, 2114-2117
🤖 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/interpreter.ts` around lines 284 - 297, The resolveNamespaceIdForSqlTarget function hardcodes 'public' as the default namespace when input.bucketName equals UNSPECIFIED_PSL_NAMESPACE_ID, but this should instead use a configurable default namespace to keep namespace resolution single-sourced. Modify the function signature to accept a defaultNamespaceId parameter, and return that parameter value instead of the hardcoded 'public' string when the condition is met. Update all call sites of resolveNamespaceIdForSqlTarget (including at lines 2091-2094 and 2114-2117) to pass the appropriate defaultNamespaceId value from the target configuration when invoking this function.
🤖 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/resolve.ts`:
- Around line 522-526: The namedTypes.set() call at the annotation resolution
site is resolving the same annotation parameter twice, once in the
resolveFieldType() method call and again in the resolveTypeTarget() method call.
This causes duplicate PSL_UNRESOLVED_TYPE_REFERENCE diagnostics when an
unresolved type is encountered. Refactor to resolve the annotation once and
store the result, then reuse this resolved result when setting both the type and
target properties instead of calling both resolver methods separately with the
same annotation parameter.
In `@packages/2-mongo-family/2-authoring/contract-psl/src/psl-helpers.ts`:
- Around line 94-100: The stripQuotes function only removes double quotes but
PSL now supports single-quoted literals. When the getMapName function extracts
the map attribute argument and calls stripQuotes on the result from argText,
single-quoted values like 'users' will retain their quotes and break name
resolution. Update the stripQuotes function to handle both single quotes (') and
double quotes (") when stripping quotes from the extracted text. This same issue
affects the related code around lines 118-123 that extracts relation names,
which will be resolved by the same stripQuotes fix.
In `@packages/2-sql/2-authoring/contract-psl/README.md`:
- Line 42: The README has inconsistent descriptions of the PSL parsing pipeline.
Line 42 correctly references parse and resolve from
`@prisma-next/psl-parser/syntax`, but the earlier overview and provider flow
bullets still describe parse followed by interpret. Locate those earlier bullet
points describing the flow and update them to use parse and resolve terminology
instead of parse and interpret, ensuring the entire document describes a single
consistent pipeline approach.
In `@packages/2-sql/2-authoring/contract-psl/src/psl-field-resolution.ts`:
- Around line 52-55: When getPositionalArgumentExpr(input.defaultAttribute)
returns a falsy value in the psl-field-resolution.ts file, instead of silently
returning an empty object {}, emit a diagnostic error that explains the problem
(such as missing positional argument, named-only arguments used, or extra
arguments provided) to help users understand why their `@default` attribute is
invalid. Replace the empty return {} with a proper diagnostic emission followed
by an appropriate error result.
---
Outside diff comments:
In `@packages/1-framework/2-authoring/psl-parser/src/parse.ts`:
- Around line 293-317: The parseObjectField function now accepts both identifier
keys and string-literal keys (via parseStringLiteralExpr), but the recovery
lookahead logic still only checks for identifiers. The followsWithKey condition
at line 313 should be updated to also check for StringLiteral kind alongside
Ident kind to properly detect when a string-literal key is followed by a colon,
ensuring malformed objects with string-literal keys are correctly recovered and
avoid misleading unterminated-object diagnostics.
---
Nitpick comments:
In `@packages/2-sql/2-authoring/contract-psl/src/interpreter.ts`:
- Around line 284-297: The resolveNamespaceIdForSqlTarget function hardcodes
'public' as the default namespace when input.bucketName equals
UNSPECIFIED_PSL_NAMESPACE_ID, but this should instead use a configurable default
namespace to keep namespace resolution single-sourced. Modify the function
signature to accept a defaultNamespaceId parameter, and return that parameter
value instead of the hardcoded 'public' string when the condition is met. Update
all call sites of resolveNamespaceIdForSqlTarget (including at lines 2091-2094
and 2114-2117) to pass the appropriate defaultNamespaceId value from the target
configuration when invoking this function.
In `@packages/2-sql/2-authoring/contract-psl/src/psl-attribute-parsing.ts`:
- Around line 15-20: Remove or significantly reduce the explanatory JSDoc
comment blocks to align with the package guideline that prefers self-explanatory
code over comments. The function and parameter names should express the intent
without needing extensive documentation. This applies at three locations in the
file: the comment block for the function at lines 15-20 (likely a
quote-stripping function), the comment block at lines 32-36, and the comment
block at lines 53-59. Either delete these blocks entirely or replace them with
minimal single-line descriptions if absolutely necessary, allowing the function
names and type signatures to convey the intent instead.
🪄 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: 613ebe10-80ac-4127-89a1-47e8cbd55732
📒 Files selected for processing (73)
packages/1-framework/1-core/framework-components/src/control/psl-extension-block-validator.tspackages/1-framework/1-core/framework-components/src/exports/psl-ast.tspackages/1-framework/1-core/framework-components/src/shared/psl-extension-block.tspackages/1-framework/1-core/framework-components/test/psl-extension-block-validator.test.tspackages/1-framework/2-authoring/psl-parser/README.mdpackages/1-framework/2-authoring/psl-parser/package.jsonpackages/1-framework/2-authoring/psl-parser/src/exports/index.tspackages/1-framework/2-authoring/psl-parser/src/exports/parser.tspackages/1-framework/2-authoring/psl-parser/src/exports/syntax.tspackages/1-framework/2-authoring/psl-parser/src/parse.tspackages/1-framework/2-authoring/psl-parser/src/parser.tspackages/1-framework/2-authoring/psl-parser/src/resolve.tspackages/1-framework/2-authoring/psl-parser/src/syntax/ast/attributes.tspackages/1-framework/2-authoring/psl-parser/src/syntax/ast/declarations.tspackages/1-framework/2-authoring/psl-parser/src/syntax/ast/expressions.tspackages/1-framework/2-authoring/psl-parser/src/syntax/ast/type-annotation.tspackages/1-framework/2-authoring/psl-parser/src/tokenizer.tspackages/1-framework/2-authoring/psl-parser/test/parse-corpus-parity.test.tspackages/1-framework/2-authoring/psl-parser/test/parse-diagnostics.test.tspackages/1-framework/2-authoring/psl-parser/test/parse-document.test.tspackages/1-framework/2-authoring/psl-parser/test/parse-leaf.test.tspackages/1-framework/2-authoring/psl-parser/test/parser-enum2.test.tspackages/1-framework/2-authoring/psl-parser/test/parser.test.tspackages/1-framework/2-authoring/psl-parser/test/resolve-validation.test.tspackages/1-framework/2-authoring/psl-parser/test/resolve.test.tspackages/1-framework/2-authoring/psl-parser/tsdown.config.tspackages/1-framework/2-authoring/psl-printer/test/declarative-policy-select.round-trip.test.tspackages/1-framework/2-authoring/psl-printer/test/print-psl-from-ast.test.tspackages/2-mongo-family/2-authoring/contract-psl/src/interpreter.tspackages/2-mongo-family/2-authoring/contract-psl/src/provider.tspackages/2-mongo-family/2-authoring/contract-psl/src/psl-helpers.tspackages/2-mongo-family/2-authoring/contract-psl/test/interpreter.polymorphism.test.tspackages/2-mongo-family/2-authoring/contract-psl/test/interpreter.test.tspackages/2-mongo-family/2-authoring/contract-psl/test/provider.test.tspackages/2-sql/2-authoring/contract-psl/README.mdpackages/2-sql/2-authoring/contract-psl/src/interpreter.tspackages/2-sql/2-authoring/contract-psl/src/provider.tspackages/2-sql/2-authoring/contract-psl/src/psl-attribute-parsing.tspackages/2-sql/2-authoring/contract-psl/src/psl-column-resolution.tspackages/2-sql/2-authoring/contract-psl/src/psl-field-resolution.tspackages/2-sql/2-authoring/contract-psl/src/psl-relation-resolution.tspackages/2-sql/2-authoring/contract-psl/src/resolved-read-shims.tspackages/2-sql/2-authoring/contract-psl/test/composed-mutation-defaults.test.tspackages/2-sql/2-authoring/contract-psl/test/fixtures.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.control-policy.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.defaults.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.diagnostics.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.enum2.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.extensions.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.namespaces.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.polymorphism.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.relations.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.types.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.value-objects.test.tspackages/2-sql/2-authoring/contract-psl/test/provider.test.tspackages/2-sql/2-authoring/contract-psl/test/psl-attribute-parsing.test.tspackages/2-sql/2-authoring/contract-psl/test/psl-ts-namespace-parity.test.tspackages/2-sql/2-authoring/contract-psl/test/resolved-read-shims.test.tspackages/2-sql/2-authoring/contract-psl/test/ts-psl-parity.test.tspackages/3-extensions/postgres/test/psl-namespace-qualifier-routing.test.tspackages/3-mongo-target/1-mongo-target/test/mongo-runner.polymorphism.integration.test.tstest/integration/test/authoring/diagnostics/ambiguous-backrelation-list/expected-diagnostics.jsontest/integration/test/authoring/diagnostics/invalid-default-arguments/expected-diagnostics.jsontest/integration/test/authoring/diagnostics/namespace-without-pack/expected-diagnostics.jsontest/integration/test/authoring/diagnostics/orphaned-backrelation-list/expected-diagnostics.jsontest/integration/test/authoring/diagnostics/unsupported-default-cuid/expected-diagnostics.jsontest/integration/test/authoring/diagnostics/unsupported-navigation-list-attribute/expected-diagnostics.jsontest/integration/test/authoring/parity/ts-psl-parity.real-packs.test.tstest/integration/test/authoring/psl-index-type-options.integration.test.tstest/integration/test/mongo/migration-psl-authoring.test.tstest/integration/test/utils/parse-and-resolve.tstest/integration/test/value-objects/value-objects.integration.test.ts
💤 Files with no reviewable changes (11)
- packages/1-framework/1-core/framework-components/src/exports/psl-ast.ts
- packages/1-framework/2-authoring/psl-parser/src/exports/parser.ts
- packages/1-framework/2-authoring/psl-parser/src/parser.ts
- packages/1-framework/2-authoring/psl-parser/test/parser.test.ts
- packages/1-framework/2-authoring/psl-printer/test/declarative-policy-select.round-trip.test.ts
- packages/1-framework/1-core/framework-components/src/control/psl-extension-block-validator.ts
- packages/1-framework/1-core/framework-components/test/psl-extension-block-validator.test.ts
- packages/1-framework/2-authoring/psl-parser/test/parser-enum2.test.ts
- packages/1-framework/2-authoring/psl-parser/package.json
- packages/1-framework/2-authoring/psl-printer/test/print-psl-from-ast.test.ts
- packages/1-framework/2-authoring/psl-parser/src/exports/index.ts
| } | ||
|
|
||
| export function resolve(document: DocumentAst, options: ResolveOptions = {}): ResolvedDocument { | ||
| const resolver = new Resolver(reconstructSource(document.syntax)); |
There was a problem hiding this comment.
What the hell? You have a well-formed SourceFile everywhere you have DocumentAst, just pass the damn thing as an argument, don't re-concatenenate entire friggin source from bare tokens.
d34ccf3 to
a756b76
Compare
@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/emitter
@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 📦
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/resolve.ts`:
- Around line 446-497: The bucketMember() function separates declarations into
different bucket arrays (models, enums, compositeTypes, genericBlocks), and the
byNamespace building loop (lines 484-496) registers them in a fixed order by
iterating through each array separately, which doesn't preserve source
declaration order. To fix this, add an ordered declaration list to the
NamespaceBucket structure that captures all declarations in source order, modify
bucketMember() to append each member to this ordered list (while still
categorizing it appropriately), and then update the byNamespace construction to
iterate through the ordered list calling registerNames() in source order instead
of processing all models first, then enums, then composites.
- Around line 690-703: The code currently uses declaredKindAcrossNamespaces()
which only returns the first namespace match for a name, causing it to miss
conflicts when a name exists with different kinds (model vs enum) across
multiple namespaces. Instead of making separate calls to check if the name
equals 'model' or 'enum', you need to check for the existence of ALL kinds
across all namespaces and report conflicts when a name is declared as both a
model and an enum in different namespaces. Apply this fix at all three affected
locations: the model/enum conflict checks starting around line 690-703 in the
first block, the similar checks around lines 708-714, and the checks around
lines 919-922. Replace the approach of checking declaredKindAcrossNamespaces()
return value with logic that detects all declarations of a given name across all
namespaces and determines if there are conflicting kinds.
- Around line 886-889: In the 'list' case block where
ArrayLiteralAst.cast(value.syntax) is called, instead of silently returning when
the cast fails, emit a diagnostic error to reject non-array values. Replace the
current early return with a diagnostic that reports the error, indicating that
list extension parameters must be array literals, not scalar or identifier
values. This ensures malformed extension configuration is properly flagged
rather than silently accepted.
🪄 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: b6952e60-f615-4b53-a75e-69f5dec0cde8
📒 Files selected for processing (72)
packages/1-framework/1-core/framework-components/src/control/psl-extension-block-validator.tspackages/1-framework/1-core/framework-components/src/exports/psl-ast.tspackages/1-framework/1-core/framework-components/src/shared/psl-extension-block.tspackages/1-framework/1-core/framework-components/test/psl-extension-block-validator.test.tspackages/1-framework/2-authoring/psl-parser/README.mdpackages/1-framework/2-authoring/psl-parser/package.jsonpackages/1-framework/2-authoring/psl-parser/src/exports/index.tspackages/1-framework/2-authoring/psl-parser/src/exports/parser.tspackages/1-framework/2-authoring/psl-parser/src/exports/syntax.tspackages/1-framework/2-authoring/psl-parser/src/parse.tspackages/1-framework/2-authoring/psl-parser/src/parser.tspackages/1-framework/2-authoring/psl-parser/src/resolve.tspackages/1-framework/2-authoring/psl-parser/src/syntax/ast/attributes.tspackages/1-framework/2-authoring/psl-parser/src/syntax/ast/declarations.tspackages/1-framework/2-authoring/psl-parser/src/syntax/ast/expressions.tspackages/1-framework/2-authoring/psl-parser/src/syntax/ast/type-annotation.tspackages/1-framework/2-authoring/psl-parser/src/tokenizer.tspackages/1-framework/2-authoring/psl-parser/test/parse-corpus-parity.test.tspackages/1-framework/2-authoring/psl-parser/test/parse-diagnostics.test.tspackages/1-framework/2-authoring/psl-parser/test/parse-document.test.tspackages/1-framework/2-authoring/psl-parser/test/parse-leaf.test.tspackages/1-framework/2-authoring/psl-parser/test/parser.test.tspackages/1-framework/2-authoring/psl-parser/test/resolve-validation.test.tspackages/1-framework/2-authoring/psl-parser/test/resolve.test.tspackages/1-framework/2-authoring/psl-parser/tsdown.config.tspackages/1-framework/2-authoring/psl-printer/test/declarative-policy-select.round-trip.test.tspackages/1-framework/2-authoring/psl-printer/test/print-psl-from-ast.test.tspackages/2-mongo-family/2-authoring/contract-psl/src/interpreter.tspackages/2-mongo-family/2-authoring/contract-psl/src/provider.tspackages/2-mongo-family/2-authoring/contract-psl/src/psl-helpers.tspackages/2-mongo-family/2-authoring/contract-psl/test/interpreter.polymorphism.test.tspackages/2-mongo-family/2-authoring/contract-psl/test/interpreter.test.tspackages/2-mongo-family/2-authoring/contract-psl/test/provider.test.tspackages/2-sql/2-authoring/contract-psl/README.mdpackages/2-sql/2-authoring/contract-psl/src/interpreter.tspackages/2-sql/2-authoring/contract-psl/src/provider.tspackages/2-sql/2-authoring/contract-psl/src/psl-attribute-parsing.tspackages/2-sql/2-authoring/contract-psl/src/psl-column-resolution.tspackages/2-sql/2-authoring/contract-psl/src/psl-field-resolution.tspackages/2-sql/2-authoring/contract-psl/src/psl-relation-resolution.tspackages/2-sql/2-authoring/contract-psl/src/resolved-read-shims.tspackages/2-sql/2-authoring/contract-psl/test/composed-mutation-defaults.test.tspackages/2-sql/2-authoring/contract-psl/test/fixtures.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.control-policy.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.defaults.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.diagnostics.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.enum.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.extensions.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.namespaces.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.polymorphism.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.relations.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.types.test.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.value-objects.test.tspackages/2-sql/2-authoring/contract-psl/test/provider.test.tspackages/2-sql/2-authoring/contract-psl/test/psl-attribute-parsing.test.tspackages/2-sql/2-authoring/contract-psl/test/psl-ts-namespace-parity.test.tspackages/2-sql/2-authoring/contract-psl/test/resolved-read-shims.test.tspackages/2-sql/2-authoring/contract-psl/test/ts-psl-parity.test.tspackages/3-extensions/postgres/test/psl-namespace-qualifier-routing.test.tspackages/3-mongo-target/1-mongo-target/test/mongo-runner.polymorphism.integration.test.tstest/integration/test/authoring/diagnostics/ambiguous-backrelation-list/expected-diagnostics.jsontest/integration/test/authoring/diagnostics/invalid-default-arguments/expected-diagnostics.jsontest/integration/test/authoring/diagnostics/namespace-without-pack/expected-diagnostics.jsontest/integration/test/authoring/diagnostics/orphaned-backrelation-list/expected-diagnostics.jsontest/integration/test/authoring/diagnostics/unsupported-default-cuid/expected-diagnostics.jsontest/integration/test/authoring/diagnostics/unsupported-navigation-list-attribute/expected-diagnostics.jsontest/integration/test/authoring/parity/ts-psl-parity.real-packs.test.tstest/integration/test/authoring/psl-index-type-options.integration.test.tstest/integration/test/mongo/migration-psl-authoring.test.tstest/integration/test/utils/parse-and-resolve.tstest/integration/test/value-objects/value-objects.integration.test.ts
💤 Files with no reviewable changes (45)
- test/integration/test/authoring/diagnostics/unsupported-default-cuid/expected-diagnostics.json
- packages/1-framework/2-authoring/psl-parser/src/exports/parser.ts
- test/integration/test/authoring/diagnostics/ambiguous-backrelation-list/expected-diagnostics.json
- packages/1-framework/2-authoring/psl-parser/src/exports/index.ts
- test/integration/test/authoring/diagnostics/namespace-without-pack/expected-diagnostics.json
- test/integration/test/authoring/diagnostics/unsupported-navigation-list-attribute/expected-diagnostics.json
- packages/2-sql/2-authoring/contract-psl/test/interpreter.control-policy.test.ts
- packages/2-sql/2-authoring/contract-psl/test/provider.test.ts
- test/integration/test/authoring/diagnostics/orphaned-backrelation-list/expected-diagnostics.json
- test/integration/test/authoring/parity/ts-psl-parity.real-packs.test.ts
- test/integration/test/authoring/psl-index-type-options.integration.test.ts
- packages/1-framework/1-core/framework-components/test/psl-extension-block-validator.test.ts
- test/integration/test/value-objects/value-objects.integration.test.ts
- packages/2-sql/2-authoring/contract-psl/test/ts-psl-parity.test.ts
- packages/3-mongo-target/1-mongo-target/test/mongo-runner.polymorphism.integration.test.ts
- packages/1-framework/2-authoring/psl-parser/package.json
- packages/2-sql/2-authoring/contract-psl/test/interpreter.types.test.ts
- packages/1-framework/1-core/framework-components/src/control/psl-extension-block-validator.ts
- packages/3-extensions/postgres/test/psl-namespace-qualifier-routing.test.ts
- packages/1-framework/1-core/framework-components/src/exports/psl-ast.ts
- test/integration/test/authoring/diagnostics/invalid-default-arguments/expected-diagnostics.json
- packages/1-framework/2-authoring/psl-parser/src/parser.ts
- packages/2-sql/2-authoring/contract-psl/test/interpreter.enum.test.ts
- packages/2-sql/2-authoring/contract-psl/test/composed-mutation-defaults.test.ts
- test/integration/test/mongo/migration-psl-authoring.test.ts
- packages/2-sql/2-authoring/contract-psl/test/resolved-read-shims.test.ts
- packages/2-sql/2-authoring/contract-psl/test/fixtures.ts
- packages/1-framework/2-authoring/psl-parser/test/parser.test.ts
- packages/2-sql/2-authoring/contract-psl/test/interpreter.relations.test.ts
- packages/2-sql/2-authoring/contract-psl/test/interpreter.extensions.test.ts
- test/integration/test/utils/parse-and-resolve.ts
- packages/2-sql/2-authoring/contract-psl/src/psl-relation-resolution.ts
- packages/1-framework/2-authoring/psl-printer/test/declarative-policy-select.round-trip.test.ts
- packages/2-sql/2-authoring/contract-psl/test/psl-ts-namespace-parity.test.ts
- packages/2-sql/2-authoring/contract-psl/test/interpreter.value-objects.test.ts
- packages/2-sql/2-authoring/contract-psl/src/resolved-read-shims.ts
- packages/2-sql/2-authoring/contract-psl/test/psl-attribute-parsing.test.ts
- packages/2-sql/2-authoring/contract-psl/test/interpreter.polymorphism.test.ts
- packages/2-sql/2-authoring/contract-psl/test/interpreter.defaults.test.ts
- packages/2-sql/2-authoring/contract-psl/test/interpreter.test.ts
- packages/2-sql/2-authoring/contract-psl/test/interpreter.diagnostics.test.ts
- packages/2-sql/2-authoring/contract-psl/test/interpreter.namespaces.test.ts
- packages/2-sql/2-authoring/contract-psl/src/psl-column-resolution.ts
- packages/2-sql/2-authoring/contract-psl/src/psl-attribute-parsing.ts
- packages/2-sql/2-authoring/contract-psl/src/psl-field-resolution.ts
🚧 Files skipped from review as they are similar to previous changes (26)
- packages/1-framework/2-authoring/psl-parser/tsdown.config.ts
- packages/1-framework/2-authoring/psl-parser/test/parse-leaf.test.ts
- packages/1-framework/2-authoring/psl-parser/src/syntax/ast/expressions.ts
- packages/1-framework/2-authoring/psl-parser/test/parse-diagnostics.test.ts
- packages/1-framework/2-authoring/psl-parser/src/syntax/ast/declarations.ts
- packages/1-framework/2-authoring/psl-parser/test/parse-corpus-parity.test.ts
- packages/1-framework/2-authoring/psl-parser/src/syntax/ast/attributes.ts
- packages/1-framework/1-core/framework-components/src/shared/psl-extension-block.ts
- packages/2-mongo-family/2-authoring/contract-psl/test/interpreter.polymorphism.test.ts
- packages/1-framework/2-authoring/psl-parser/src/syntax/ast/type-annotation.ts
- packages/2-mongo-family/2-authoring/contract-psl/test/provider.test.ts
- packages/2-sql/2-authoring/contract-psl/README.md
- packages/1-framework/2-authoring/psl-parser/src/exports/syntax.ts
- packages/2-mongo-family/2-authoring/contract-psl/src/provider.ts
- packages/1-framework/2-authoring/psl-parser/test/resolve.test.ts
- packages/2-sql/2-authoring/contract-psl/src/provider.ts
- packages/1-framework/2-authoring/psl-parser/README.md
- packages/2-mongo-family/2-authoring/contract-psl/test/interpreter.test.ts
- packages/1-framework/2-authoring/psl-parser/test/parse-document.test.ts
- packages/1-framework/2-authoring/psl-parser/src/tokenizer.ts
- packages/2-mongo-family/2-authoring/contract-psl/src/psl-helpers.ts
- packages/1-framework/2-authoring/psl-parser/src/parse.ts
- packages/1-framework/2-authoring/psl-parser/test/resolve-validation.test.ts
- packages/1-framework/2-authoring/psl-printer/test/print-psl-from-ast.test.ts
- packages/2-mongo-family/2-authoring/contract-psl/src/interpreter.ts
- packages/2-sql/2-authoring/contract-psl/src/interpreter.ts
…eric blocks
Addresses the maintainer + CodeRabbit comments on the psl-parser substrate and
closes the rebase-surfaced enum gap. Consumers (SQL/Mongo contract-psl) adapt to
these API changes in a follow-up; they are transitionally red on the new
`resolve` signature, per-target scalars, and the enum-block shape.
- resolve(document, sourceFile, options?): take the SourceFile the caller
already has from parse() instead of reconstructing source text from green
tokens; derive every diagnostic Range from it. Removes reconstructSource.
- Per-target scalar types: ResolveOptions.scalarTypes (defaulting to the
exported DEFAULT_SCALAR_TYPES) so a target scalar like Mongo's ObjectId
resolves as { kind: 'scalar' } rather than { kind: 'unresolved' }.
- Inline the identifierText wrapper; add IdentifierAst.name() and use it.
- Resolve a named-type annotation once (reuse type.target) so a single
unresolved reference no longer double-reports PSL_UNRESOLVED_TYPE_REFERENCE.
- Bound the qualified-constructor/call lookahead (MAX_QUALIFIED_CALL_SEGMENTS)
so a pathological identifier chain can't drive an unbounded scan; behaviour is
identical for all well-formed qualified calls.
- Retire the native enum grammar (parseEnum/parseEnumValue/parseEnumMember,
enum removed from RESERVED_BLOCK_KEYWORDS). `enum {...}` now routes through the
generic-block path: @@type lands as a block attribute, `member = value` and
bare `member` land as key-value entries, so a registered enum descriptor
resolves the block into extensionBlocks. parseKeyValue now accepts a bare key
(no `=`) as a bare-member entry.
- Export argText from @prisma-next/psl-parser/syntax (raw CST token text) so
consumers stop duplicating it; StringLiteralExprAst.value() already unquotes
both quote styles.
- Fold the standalone parse-corpus-parity cases into parse-document.test.ts and
delete the file; add resolve-enum-routing coverage (enum→extensionBlocks,
duplicate-member diagnostic, per-target scalar, SourceFile-derived ranges).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/2-mongo-family/2-authoring/contract-psl/src/psl-helpers.ts (1)
140-147: 💤 Low value
quotedStringArgdoesn't decode escape sequences.The regex-based extraction in
quotedStringArgcaptures the inner text verbatim, but unlikestringLiteralValue()(which usesStringLiteralExprAst.cast().value()), it doesn't decode escape sequences like\"or\\. If a discriminator value contains escapes (e.g.,@@base(Post, "foo\"bar")), the returned string will include the literal backslash.Consider using
stringLiteralValue()for consistency:Proposed fix
export function quotedStringArg(attr: ResolvedAttribute, index: number): string | undefined { const value = attr.positionalArg(index); if (value === undefined) return undefined; - const trimmed = argText(value).trim(); - const match = trimmed.match(/^(['"])(.*)\1$/); - if (!match) return undefined; - return match[2] ?? ''; + return stringLiteralValue(value); }🤖 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-mongo-family/2-authoring/contract-psl/src/psl-helpers.ts` around lines 140 - 147, The quotedStringArg function extracts quoted string values using regex matching that captures literal text without decoding escape sequences, so strings containing escapes like \" or \\ will retain the backslash characters. Instead of using the regex-based extraction approach, refactor quotedStringArg to use the stringLiteralValue() function which properly handles escape sequence decoding, ensuring consistency with how escape sequences are handled elsewhere in the codebase and correctly parsing discriminator values that contain escaped characters.packages/2-mongo-family/2-authoring/contract-psl/test/interpreter.test.ts (1)
1837-1945: 💤 Low valueDuplicate test coverage for namespace block rejection.
The
describe('per-target namespace dispatch (FR16c)')block (lines 1837–1891) anddescribe('namespace block rejection')block (lines 1893–1944) test nearly identical scenarios with minor PSL variations (ObjectId@id@Map("_id")vsString@id``). If this duplication is intentional for coverage breadth, consider adding a brief comment explaining why both suites exist to avoid future cleanup confusion.🤖 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-mongo-family/2-authoring/contract-psl/test/interpreter.test.ts` around lines 1837 - 1945, The test suites `describe('per-target namespace dispatch (FR16c)')` and `describe('namespace block rejection')` contain nearly identical test cases with only minor PSL variations (ObjectId-based vs String-based models). If this duplication is intentional for comprehensive coverage, add a clarifying comment above the `describe('namespace block rejection')` block explaining the purpose of having both test suites and what distinct scenarios or edge cases the second suite is intended to cover beyond the first.
🤖 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/parse.ts`:
- Around line 479-489: Type annotations with trailing separators like `Int.` or
`supabase:` are not being caught by the parser. In the `parseQualifiedName` call
within `parseTypeAnnotation`, add a handler for trailing separators (in addition
to the existing `onExtraSegment` handler) to detect and report a diagnostic when
a separator appears at the end with no following segment. This handler should
use a diagnostic like `PSL_INVALID_QUALIFIED_TYPE` with an appropriate message
indicating the trailing separator is invalid.
- Around line 393-401: The `parseFunctionCall` function accepts a 'Dot' token in
the lookahead check at line 396 and commits to a FunctionCall node before
verifying the qualified name chain actually ends with a left paren. This causes
inputs like `a.b` to be parsed as FunctionCall nodes without parentheses, which
is invalid. After the `parseQualifiedName(cursor)` call, verify that
`cursor.peekKind() === 'LParen'` is true; if not, the qualified identifier is
not actually a function call, so either abort the node or return undefined to
reject it as a function call.
In `@packages/1-framework/2-authoring/psl-parser/src/resolve.ts`:
- Around line 292-299: The scalar type check at line 292 using
this.#scalarTypes.has(typeName) executes before the qualified namespace
resolution check at line 296, causing qualified names like ns.String to
incorrectly resolve as scalar types instead of performing namespace lookup. Move
the namespace qualifier resolution block (checking
annotation.namespaceName()?.name() and using nameTable.byNamespace) to execute
before the scalar type check, so that qualified names are properly resolved
through their namespace first.
---
Nitpick comments:
In `@packages/2-mongo-family/2-authoring/contract-psl/src/psl-helpers.ts`:
- Around line 140-147: The quotedStringArg function extracts quoted string
values using regex matching that captures literal text without decoding escape
sequences, so strings containing escapes like \" or \\ will retain the backslash
characters. Instead of using the regex-based extraction approach, refactor
quotedStringArg to use the stringLiteralValue() function which properly handles
escape sequence decoding, ensuring consistency with how escape sequences are
handled elsewhere in the codebase and correctly parsing discriminator values
that contain escaped characters.
In `@packages/2-mongo-family/2-authoring/contract-psl/test/interpreter.test.ts`:
- Around line 1837-1945: The test suites `describe('per-target namespace
dispatch (FR16c)')` and `describe('namespace block rejection')` contain nearly
identical test cases with only minor PSL variations (ObjectId-based vs
String-based models). If this duplication is intentional for comprehensive
coverage, add a clarifying comment above the `describe('namespace block
rejection')` block explaining the purpose of having both test suites and what
distinct scenarios or edge cases the second suite is intended to cover beyond
the first.
🪄 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: c9adc33a-65a5-435d-8a2e-e0c8c0bd0861
📒 Files selected for processing (33)
packages/1-framework/2-authoring/psl-parser/src/exports/syntax.tspackages/1-framework/2-authoring/psl-parser/src/parse.tspackages/1-framework/2-authoring/psl-parser/src/resolve.tspackages/1-framework/2-authoring/psl-parser/src/syntax/ast/attributes.tspackages/1-framework/2-authoring/psl-parser/src/syntax/ast/expressions.tspackages/1-framework/2-authoring/psl-parser/src/syntax/ast/identifier.tspackages/1-framework/2-authoring/psl-parser/src/syntax/ast/qualified-name.tspackages/1-framework/2-authoring/psl-parser/src/syntax/ast/type-annotation.tspackages/1-framework/2-authoring/psl-parser/src/syntax/syntax-kind.tspackages/1-framework/2-authoring/psl-parser/test/parse-diagnostics.test.tspackages/1-framework/2-authoring/psl-parser/test/parse-document.test.tspackages/1-framework/2-authoring/psl-parser/test/parse-leaf.test.tspackages/1-framework/2-authoring/psl-parser/test/resolve-enum-routing.test.tspackages/1-framework/2-authoring/psl-parser/test/resolve-validation.test.tspackages/1-framework/2-authoring/psl-parser/test/resolve.test.tspackages/1-framework/2-authoring/psl-parser/test/syntax/ast.test.tspackages/2-mongo-family/2-authoring/contract-psl/src/interpreter.tspackages/2-mongo-family/2-authoring/contract-psl/src/provider.tspackages/2-mongo-family/2-authoring/contract-psl/src/psl-helpers.tspackages/2-mongo-family/2-authoring/contract-psl/test/interpreter.polymorphism.test.tspackages/2-mongo-family/2-authoring/contract-psl/test/interpreter.test.tspackages/2-sql/2-authoring/contract-psl/README.mdpackages/2-sql/2-authoring/contract-psl/src/provider.tspackages/2-sql/2-authoring/contract-psl/src/psl-column-resolution.tspackages/2-sql/2-authoring/contract-psl/src/psl-field-resolution.tspackages/2-sql/2-authoring/contract-psl/src/resolved-read-shims.tspackages/2-sql/2-authoring/contract-psl/test/fixtures.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.enum.test.tspackages/2-sql/2-authoring/contract-psl/test/psl-attribute-parsing.test.tspackages/2-sql/2-authoring/contract-psl/test/resolved-read-shims.test.tspackages/3-extensions/postgres/test/psl-namespace-qualifier-routing.test.tspackages/3-mongo-target/1-mongo-target/test/mongo-runner.polymorphism.integration.test.tstest/integration/test/utils/parse-and-resolve.ts
✅ Files skipped from review due to trivial changes (3)
- packages/1-framework/2-authoring/psl-parser/src/syntax/ast/identifier.ts
- packages/1-framework/2-authoring/psl-parser/src/syntax/syntax-kind.ts
- packages/2-sql/2-authoring/contract-psl/README.md
🚧 Files skipped from review as they are similar to previous changes (15)
- packages/3-mongo-target/1-mongo-target/test/mongo-runner.polymorphism.integration.test.ts
- packages/3-extensions/postgres/test/psl-namespace-qualifier-routing.test.ts
- packages/1-framework/2-authoring/psl-parser/src/exports/syntax.ts
- packages/2-sql/2-authoring/contract-psl/test/fixtures.ts
- packages/2-mongo-family/2-authoring/contract-psl/test/interpreter.polymorphism.test.ts
- packages/2-sql/2-authoring/contract-psl/src/provider.ts
- test/integration/test/utils/parse-and-resolve.ts
- packages/2-sql/2-authoring/contract-psl/test/psl-attribute-parsing.test.ts
- packages/2-sql/2-authoring/contract-psl/test/resolved-read-shims.test.ts
- packages/2-sql/2-authoring/contract-psl/src/resolved-read-shims.ts
- packages/1-framework/2-authoring/psl-parser/test/resolve.test.ts
- packages/2-mongo-family/2-authoring/contract-psl/src/provider.ts
- packages/1-framework/2-authoring/psl-parser/test/resolve-validation.test.ts
- packages/2-sql/2-authoring/contract-psl/src/psl-column-resolution.ts
- packages/2-sql/2-authoring/contract-psl/src/psl-field-resolution.ts
| return findFirstChild(this.syntax, QualifiedNameAst.cast); | ||
| } | ||
|
|
||
| name(): IdentifierAst | undefined { |
There was a problem hiding this comment.
Remove this method, use qualifiedName where we need a name
| * string value of a string literal should use {@link StringLiteralExprAst.value} | ||
| * instead; `argText` is for the cases that need the unmodified source slice. | ||
| */ | ||
| export function argText(value: ExpressionAst): string { |
There was a problem hiding this comment.
This is not really sepcific to expression or AST nodes. Change to accept arbitrary SyntaxNode
| * type-annotation position diagnoses over-qualification, the attribute position | ||
| * diagnoses a trailing dot, and the call positions diagnose neither. | ||
| */ | ||
| interface QualifiedNameHooks { |
There was a problem hiding this comment.
Why do we need hooks? Qualified name parsing should be universal regardless of whether it is a attribute, type annotation or anything else.
…+ hooks, fix call/order/scalar bugs Address the 2nd PR #830 review round on the psl-parser internals. QualifiedName consolidation: the attribute name accessor now returns the inner QualifiedName (one source of truth for namespace + bare-name segments); the duplicate dot()/namespaceName()/name()-as-identifier accessors and the hand-built-tree fallbacks are gone. FunctionCallAst.name() is removed in favour of qualifiedName(). argText is generalised to accept an arbitrary SyntaxNode and relocated to the SyntaxNode-helper module. Hookless qualified-name parsing: parseQualifiedName no longer takes per-position diagnostic hooks; it returns structured findings (extra separators, trailing separator) and each position keeps its own wording. Type annotations now also flag a trailing separator (Int. / supabase:). parseFunctionCall bug: a dotted reference with no trailing paren (a.b) was committed as a paren-less FunctionCall. The entry lookahead is now bounded — a bare Ident( or a single namespace-qualified Ident.Ident( — so a.b falls through to the bare-identifier form instead. resolve: DEFAULT_SCALAR_TYPES is removed and scalarTypes is required — the target supplies the concrete scalar list, with no framework fallback. Declaration name registration walks the bucket in source order, so the first declaration wins and its diagnostic attaches to the source-first decl regardless of kind. list extension params reject a non-array value with a diagnostic instead of silently passing. Cross-namespace kind checks now test every namespace for the requested kind rather than the first name hit. A qualified ns.Type resolves against the namespace before the bare-scalar shortcut, so ns.String no longer binds to the built-in scalar. README: "CST AST types" -> "CST node types". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/1-framework/2-authoring/psl-parser/src/parse.ts (1)
331-346:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInclude string-literal keys in object-field recovery.
Line 335 still only re-enters the field loop for an
Identkey. Now that Line 363 accepts string-literal keys,{ "a": 1 "b": 2 }stops the object early, leaves"b": 2 }for the outer parser, and cascades diagnostics instead of reporting the missing comma and continuing.🐛 Proposed fix
+function isObjectKeyStart(kind: TokenKind): boolean { + return kind === 'Ident' || kind === 'StringLiteral'; +} + export function parseObjectLiteralExpr(cursor: Cursor): GreenNode | undefined { if (cursor.peekKind() !== 'LBrace') return undefined; @@ - } else if (cursor.peekKind() === 'Ident') { + } else if (isObjectKeyStart(cursor.peekKind())) { // A following identifier key with no separating comma re-enters the loop // (the next parseObjectField consumes the key, ≥1 token, guaranteeing // progress). Flag the missing comma at the gap after the previous field. @@ - if (cursor.peekKind() === 'Ident') { + if (cursor.peekKind() === 'Ident') { parseIdentifier(cursor); // identifier key } else if (cursor.peekKind() === 'StringLiteral') { @@ - const followsWithKey = cursor.peekKind() === 'Ident' && cursor.peekKind(1) === 'Colon'; + const followsWithKey = isObjectKeyStart(cursor.peekKind()) && cursor.peekKind(1) === 'Colon';Also applies to: 357-380
🤖 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/1-framework/2-authoring/psl-parser/src/parse.ts` around lines 331 - 346, The condition checking for missing commas between object-literal fields only checks for Ident token types, but string-literal keys are now accepted by parseObjectField. Extend the condition that currently checks cursor.peekKind() === 'Ident' to also check for the string-literal token type (likely 'String' or similar) so that missing commas are properly detected and reported when fields use string-literal keys, allowing the parser to continue parsing rather than terminating the object early. Apply this same fix to any similar logic in the range around lines 357-380 where comparable field parsing occurs.packages/2-sql/2-authoring/contract-psl/src/interpreter.ts (2)
2060-2071:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftNamespace collisions remain possible because relation state is keyed by bare model name.
Line 2060 (
modelNamespaceIds) and Line 2169 / Line 2224 / Line 2297 relation plumbing usemodel.name/modelNamekeys. With same model names in multiple namespaces, entries overwrite/merge and relations can attach to the wrong model.Also applies to: 2164-2170, 2224-2230, 2297-2301
🤖 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/interpreter.ts` around lines 2060 - 2071, The modelNamespaceIds Map uses only model.name as a key, which causes collisions when identical model names exist across multiple namespaces, allowing entries to overwrite each other and causing relations to attach to the wrong model. Change the key used in modelNamespaceIds.set() at line 2060 to be a composite key that includes both the namespace identifier (resolvedNamespaceId) and the model name, ensuring uniqueness. Then update all corresponding lookups of modelNamespaceIds in the relation plumbing code at lines 2164-2170, 2224-2230, and 2297-2301 to use the same composite key format when retrieving values from the map.
330-386: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftExtract target-specific namespace policy into adapters instead of branching on
targetId.This file now hard-branches on
targetIdfor namespace lowering/validation (Line 334, Line 353, Line 368), which conflicts with the repo’s target-adapter policy and will keep growing conditional complexity as targets evolve.As per coding guidelines,
**/*.{ts,tsx,js,jsx}: “Don't branch on target; use adapters:.agents/rules/no-target-branches.mdc.”🤖 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/interpreter.ts` around lines 330 - 386, The functions resolveNamespaceIdForSqlTarget and validateNamespaceBlocksForSqlTarget both branch directly on the targetId value ('postgres', 'sqlite') instead of using adapters. Extract the target-specific namespace resolution and validation logic into separate adapter implementations for each target type. Create adapter functions or a map-based lookup that routes to the appropriate implementation based on targetId, removing the explicit if-statements that check for specific target values. This will eliminate conditional complexity and follow the repo's target-adapter pattern as specified in the coding guidelines.Source: Coding guidelines
🧹 Nitpick comments (2)
packages/1-framework/2-authoring/psl-parser/test/support.ts (1)
9-19: ⚡ Quick winConsider consolidating duplicate scalar type definitions.
The
frameworkScalarTypesset here duplicatesDEFAULT_SQL_SCALAR_TYPESintest/integration/test/utils/parse-and-resolve.ts(lines 14-24). Both define identical scalar names. To improve maintainability, consider:
- Exporting
frameworkScalarTypesfrom a shared location (e.g., the parser package's public exports)- Having both test utilities import from that single source
This ensures framework scalar types remain synchronized across test infrastructure.
🤖 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/1-framework/2-authoring/psl-parser/test/support.ts` around lines 9 - 19, The frameworkScalarTypes set is duplicated in both test/support.ts and parse-and-resolve.ts, creating a maintenance risk. Move the frameworkScalarTypes definition to a shared location in the parser package's public exports (e.g., the main index or a constants file), then update both test files to import and use this single shared definition instead of maintaining duplicate scalar type lists. This ensures scalar type definitions remain synchronized across the test infrastructure.packages/2-sql/2-authoring/contract-psl/test/provider.test.ts (1)
670-707: 💤 Low valueConsider caching the base context to avoid repeated instantiation.
The test creates separate
createPostgresTestContext()instances on lines 691, 694, and 698. Caching the base context would improve clarity and avoid redundant instantiation.♻️ Suggested refactor
it('resolves the extension scalar as a field type without emitting PSL_UNSUPPORTED_FIELD_TYPE', async () => { const tempDir = await mkdtemp(join(tmpdir(), 'psl-provider-custom-scalar-')); tempDirs.push(tempDir); const schemaPath = join(tempDir, 'schema.prisma'); await writeFile( schemaPath, `model Item { id Int `@id` uid SqlUuid } `, 'utf-8', ); process.chdir(tempDir); const contract = prismaContract('./schema.prisma', baseOptions); + const baseContext = createPostgresTestContext(); const result = await contract.source.load( createPostgresTestContext({ resolvedInputs: [schemaPath], scalarTypeDescriptors: new Map([ - ...createPostgresTestContext().scalarTypeDescriptors, + ...baseContext.scalarTypeDescriptors, ['SqlUuid', 'pg/uuid@1'], ]), codecLookup: { - ...createPostgresTestContext().codecLookup, + ...baseContext.codecLookup, targetTypesFor: (id: string) => { if (id === 'pg/uuid@1') return ['uuid']; - return createPostgresTestContext().codecLookup.targetTypesFor(id); + return baseContext.codecLookup.targetTypesFor(id); }, }, }), );🤖 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/test/provider.test.ts` around lines 670 - 707, The test within the anonymous function passed to prismaContract creates multiple separate instances of createPostgresTestContext() to spread its scalarTypeDescriptors and codecLookup properties, and again to access targetTypesFor. Cache the base context as a single variable before the load call, then reuse that cached instance in all places where createPostgresTestContext() is currently called separately for accessing scalarTypeDescriptors, codecLookup, and the targetTypesFor method. This eliminates redundant instantiation and improves code clarity.
🤖 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/interpreter.ts`:
- Around line 260-263: The blockNames Set collects all extension block names
from namespaces, but the PSL_UNRESOLVED_TYPE_REFERENCE suppression that uses
this set (around line 270) should only apply to enum blocks, not all extension
blocks. Modify the loop where blockNames is populated to filter and only add
extension blocks that are enum blocks. This will ensure the unresolved type
suppression is limited to enum blocks only and won't mask real unknown-type
errors for other extension block types.
---
Outside diff comments:
In `@packages/1-framework/2-authoring/psl-parser/src/parse.ts`:
- Around line 331-346: The condition checking for missing commas between
object-literal fields only checks for Ident token types, but string-literal keys
are now accepted by parseObjectField. Extend the condition that currently checks
cursor.peekKind() === 'Ident' to also check for the string-literal token type
(likely 'String' or similar) so that missing commas are properly detected and
reported when fields use string-literal keys, allowing the parser to continue
parsing rather than terminating the object early. Apply this same fix to any
similar logic in the range around lines 357-380 where comparable field parsing
occurs.
In `@packages/2-sql/2-authoring/contract-psl/src/interpreter.ts`:
- Around line 2060-2071: The modelNamespaceIds Map uses only model.name as a
key, which causes collisions when identical model names exist across multiple
namespaces, allowing entries to overwrite each other and causing relations to
attach to the wrong model. Change the key used in modelNamespaceIds.set() at
line 2060 to be a composite key that includes both the namespace identifier
(resolvedNamespaceId) and the model name, ensuring uniqueness. Then update all
corresponding lookups of modelNamespaceIds in the relation plumbing code at
lines 2164-2170, 2224-2230, and 2297-2301 to use the same composite key format
when retrieving values from the map.
- Around line 330-386: The functions resolveNamespaceIdForSqlTarget and
validateNamespaceBlocksForSqlTarget both branch directly on the targetId value
('postgres', 'sqlite') instead of using adapters. Extract the target-specific
namespace resolution and validation logic into separate adapter implementations
for each target type. Create adapter functions or a map-based lookup that routes
to the appropriate implementation based on targetId, removing the explicit
if-statements that check for specific target values. This will eliminate
conditional complexity and follow the repo's target-adapter pattern as specified
in the coding guidelines.
---
Nitpick comments:
In `@packages/1-framework/2-authoring/psl-parser/test/support.ts`:
- Around line 9-19: The frameworkScalarTypes set is duplicated in both
test/support.ts and parse-and-resolve.ts, creating a maintenance risk. Move the
frameworkScalarTypes definition to a shared location in the parser package's
public exports (e.g., the main index or a constants file), then update both test
files to import and use this single shared definition instead of maintaining
duplicate scalar type lists. This ensures scalar type definitions remain
synchronized across the test infrastructure.
In `@packages/2-sql/2-authoring/contract-psl/test/provider.test.ts`:
- Around line 670-707: The test within the anonymous function passed to
prismaContract creates multiple separate instances of
createPostgresTestContext() to spread its scalarTypeDescriptors and codecLookup
properties, and again to access targetTypesFor. Cache the base context as a
single variable before the load call, then reuse that cached instance in all
places where createPostgresTestContext() is currently called separately for
accessing scalarTypeDescriptors, codecLookup, and the targetTypesFor method.
This eliminates redundant instantiation and improves code clarity.
🪄 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: 2f53212a-4dc4-4321-928c-7bc3c4bd0ab1
📒 Files selected for processing (37)
packages/1-framework/2-authoring/psl-parser/README.mdpackages/1-framework/2-authoring/psl-parser/src/exports/syntax.tspackages/1-framework/2-authoring/psl-parser/src/parse.tspackages/1-framework/2-authoring/psl-parser/src/resolve.tspackages/1-framework/2-authoring/psl-parser/src/syntax/ast-helpers.tspackages/1-framework/2-authoring/psl-parser/src/syntax/ast/attributes.tspackages/1-framework/2-authoring/psl-parser/src/syntax/ast/expressions.tspackages/1-framework/2-authoring/psl-parser/test/parse-document.test.tspackages/1-framework/2-authoring/psl-parser/test/parse-leaf.test.tspackages/1-framework/2-authoring/psl-parser/test/resolve-enum-routing.test.tspackages/1-framework/2-authoring/psl-parser/test/resolve-validation.test.tspackages/1-framework/2-authoring/psl-parser/test/resolve.test.tspackages/1-framework/2-authoring/psl-parser/test/support.tspackages/1-framework/2-authoring/psl-parser/test/syntax/ast.test.tspackages/2-mongo-family/2-authoring/contract-psl/src/interpreter.tspackages/2-mongo-family/2-authoring/contract-psl/src/provider.tspackages/2-mongo-family/2-authoring/contract-psl/src/psl-helpers.tspackages/2-mongo-family/2-authoring/contract-psl/test/interpreter.polymorphism.test.tspackages/2-mongo-family/2-authoring/contract-psl/test/interpreter.test.tspackages/2-mongo-family/2-authoring/contract-psl/test/provider.test.tspackages/2-sql/2-authoring/contract-psl/src/interpreter.tspackages/2-sql/2-authoring/contract-psl/src/provider.tspackages/2-sql/2-authoring/contract-psl/src/psl-attribute-parsing.tspackages/2-sql/2-authoring/contract-psl/src/psl-column-resolution.tspackages/2-sql/2-authoring/contract-psl/src/psl-field-resolution.tspackages/2-sql/2-authoring/contract-psl/src/psl-relation-resolution.tspackages/2-sql/2-authoring/contract-psl/src/psl-resolved-reader.tspackages/2-sql/2-authoring/contract-psl/test/fixtures.tspackages/2-sql/2-authoring/contract-psl/test/interpreter.diagnostics.test.tspackages/2-sql/2-authoring/contract-psl/test/provider.test.tspackages/2-sql/2-authoring/contract-psl/test/psl-attribute-parsing.test.tspackages/2-sql/2-authoring/contract-psl/test/psl-resolved-reader.test.tspackages/3-extensions/postgres/test/psl-namespace-qualifier-routing.test.tspackages/3-mongo-target/1-mongo-target/test/mongo-runner.polymorphism.integration.test.tspackages/3-targets/6-adapters/sqlite/src/core/control-mutation-defaults.tstest/integration/test/mongo/migration-psl-authoring.test.tstest/integration/test/utils/parse-and-resolve.ts
💤 Files with no reviewable changes (1)
- packages/1-framework/2-authoring/psl-parser/src/syntax/ast/expressions.ts
✅ Files skipped from review due to trivial changes (1)
- packages/3-targets/6-adapters/sqlite/src/core/control-mutation-defaults.ts
🚧 Files skipped from review as they are similar to previous changes (14)
- packages/2-sql/2-authoring/contract-psl/src/provider.ts
- packages/3-mongo-target/1-mongo-target/test/mongo-runner.polymorphism.integration.test.ts
- packages/1-framework/2-authoring/psl-parser/README.md
- packages/3-extensions/postgres/test/psl-namespace-qualifier-routing.test.ts
- test/integration/test/mongo/migration-psl-authoring.test.ts
- packages/2-sql/2-authoring/contract-psl/test/interpreter.diagnostics.test.ts
- packages/2-mongo-family/2-authoring/contract-psl/test/interpreter.test.ts
- packages/2-sql/2-authoring/contract-psl/test/psl-attribute-parsing.test.ts
- packages/2-mongo-family/2-authoring/contract-psl/src/psl-helpers.ts
- packages/2-sql/2-authoring/contract-psl/test/fixtures.ts
- packages/2-sql/2-authoring/contract-psl/src/psl-relation-resolution.ts
- packages/2-mongo-family/2-authoring/contract-psl/src/interpreter.ts
- packages/1-framework/2-authoring/psl-parser/test/parse-document.test.ts
- packages/2-sql/2-authoring/contract-psl/src/psl-field-resolution.ts
Introduce a target-agnostic resolved view over the fault-tolerant CST: a `resolve(document): ResolvedDocument` pass that walks the tree once and returns per-namespace, name-keyed `ReadonlyMap` stores (models, enums, composite types, extension blocks), a document-level `namedTypes` map, and a diagnostics list. Every field/named-type reference resolves to a `TypeTarget` (scalar / ref with a kind-ful `DeclCoord` / crossSpace carrying coordinates but no DeclKind / constructor / unresolved). Declaration stores are first-declaration-wins on duplicate, with a collision diagnostic; a dangling reference yields an unresolved target plus a diagnostic. Cross-space references are never flagged unresolved. Attributes are exposed structurally as `ResolvedAttribute`, with arg values as CST `ExpressionAst` nodes and positional/string accessors that subsume the old `getPositionalArgument` / `parseQuotedStringLiteral` helpers. Each resolved entity keeps a CST back-pointer (`syntax`). `parse` and `resolve` stay separate exported entries; the pass is non-throwing on malformed input. Adds two non-semantic diagnostic codes (`PSL_UNRESOLVED_TYPE_REFERENCE`, `PSL_DUPLICATE_DECLARATION`). Bare references resolve current-namespace-then-document; qualified `ns.Type` references resolve against that namespace exactly, matching the live old parser + interpreter resolution policy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
…iteralExprAst.value() All CST-node call sites now read string values via StringLiteralExprAst.cast(expr.syntax)?.value(), which decodes escape sequences. A getNamedArgumentExpr helper is added alongside the existing getPositionalArgumentExpr to give callers direct access to named-arg nodes. The text-only parseObjectLiteralStringMap path uses a private decodeRawStringLiteral that mirrors the same semantics (strip outer quotes + decode escapes) for contexts where no CST node is reachable. parseQuotedStringLiteral (no-decode quote-strip) is deleted; the one contrived test that pinned the no-decode behaviour is updated to expect the decoded output. Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
…ckTypes Add a name-keyed `blockTypes` section to each `ResolvedNamespace`, populated from every named generic block, and a `block` `TypeTarget` variant so a field/named-type reference whose name binds to a generic block resolves instead of being left unresolved. The resolver records *what a name refers to* without judging whether the block's keyword is a valid field type — that keyword is carried on `ResolvedBlockType` for the interpreter to judge. Block names join the namespace declaration name-space for first-wins collision handling (a block colliding with a model/enum/composite emits `PSL_DUPLICATE_DECLARATION`), but carry no `DeclKind` — their keyword lives on `ResolvedBlockType`, not in `DeclKind`. Bare references resolve against the current + ambient namespace; qualified `ns.Name` against the named namespace, mirroring the rules for true declarations. Genuine unknowns still resolve to `unresolved` + `PSL_UNRESOLVED_TYPE_REFERENCE`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Adapt the SQL + Mongo PSL interpreters to the resolver's `block` `TypeTarget`
variant. Enum-typed (and any named-block-typed) field references now resolve to
`{ kind: 'block'; namespaceId; name }` instead of `unresolved`, so:
- SQL `resolveColumnDescriptor` routes a `block` target to the enum descriptors
keyed by the block name; a block whose keyword cannot be stored falls through
to `PSL_UNSUPPORTED_FIELD_TYPE` (resolves-but-unstorable).
- SQL/Mongo `fieldTypeName` and SQL `namedTypeBaseName` gain a `block` case
returning the block name.
- The dead `PSL_UNRESOLVED_TYPE_REFERENCE` suppression band-aid
(`enumTypedFieldRanges` + `suppressedRanges`) is removed: enum fields no
longer emit that diagnostic, so there is nothing to suppress. Genuine unknown
types still emit `PSL_UNRESOLVED_TYPE_REFERENCE`.
Duplicate enum block names now emit `PSL_DUPLICATE_DECLARATION` (block names
join the namespace declaration name-space, first-wins) rather than silently
last-writer-winning; the corpus oracle moves accordingly.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
…lve; regen supabase 0.14.0 fixture Rebase reconciliation onto main (v0.14.0): - TML-2916's interpreter.namespaces.test.ts arrived on the parser-replacement branch still calling the deleted parsePslDocument; route it through parseAndResolve like its siblings (the bare-model→public defaulting it pins is unchanged). - Regenerate the supabase example contract for the 0.13.0→0.14.0 extension-pack version bump (#843). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
eb25673 to
3f76821
Compare
…ame printSyntax, drop dead ResolvedEnum, preserve arg indexes Address review feedback on the parser-replacement branch: - TypeAnnotationAst: rename `qualifiedName()` to `name()` (returns the inner QualifiedName) and delete the duplicate `name`/`colon`/`dot`/`spaceName`/ `namespaceName`/`isOverQualified` fallback wrappers — QualifiedName is now the single source for the name segments. Callers reach through `.name()`. - Rename the `argText` source-slice helper to `printSyntax` across the parser and both contract-psl consumers (the name no longer describes its only use). - Remove the dead `ResolvedEnum`/`ResolvedEnumValue` surface and `ResolvedNamespace.enums`: enum is a descriptor-driven generic block routed through `blockTypes`, so the enum map was unreachable. - Stop dropping malformed-value arguments in both `resolveTypeTarget` (constructor args) and `collectArgs` (attribute args): a missing value keeps its positional slot so `positionalArg` indexes never shift. Constructor holes surface as empty-text positional args falling back to the annotation span. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
| codecLookup: context.codecLookup, | ||
| scalarTypes: new Set(context.scalarTypeDescriptors.keys()), | ||
| }); | ||
| // `parse` does not itself fail on syntax errors — it recovers and |
| this.syntax = syntax; | ||
| } | ||
|
|
||
| positionalArg(index: number): ExpressionAst | undefined { |
There was a problem hiding this comment.
Can we hold named and positional args in separate fields on ResolvedArgs? Then lookup would be a plain index/key lookup
…alifiedName.name → identifier parseQualifiedName now owns its diagnostics — no findings return, no hooks, no per-position parametrization. A malformed chain (over-qualification, a trailing separator) reports one uniform code `PSL_INVALID_QUALIFIED_NAME` with the same message regardless of position (type annotation, call callee, attribute name). `parseTypeAnnotation`/`parseAttribute` drop their bespoke re-diagnosing. Also threads the `QualifiedNameAst.name()` → `identifier()` rename through its callers (resolver, SQL resolved-reader, tests) so the qualified-name's bare segment reads as `.identifier()`, distinct from `IdentifierAst.name()`. Byte-identical contracts (fixtures:check clean); the only diagnostic change is the sanctioned uniform code. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
| return undefined; | ||
| } | ||
| return entry.value; | ||
| export function getNamedArgumentExpr( |
There was a problem hiding this comment.
Make it a method on ResolvedAttribute
| * Replaces the legacy `getPositionalArgumentEntry` `{ value, span }` accessor: | ||
| * callers read the raw text via {@link printSyntax} and derive spans from the CST. | ||
| */ | ||
| export function getPositionalArgumentExpr( |
There was a problem hiding this comment.
This helper has no function, inline it
| * {@link printSyntax} (preserving the raw source text the legacy comma-split | ||
| * produced). | ||
| */ | ||
| export function parseFieldList(value: ExpressionAst): readonly string[] | undefined { |
There was a problem hiding this comment.
That's incorrect. Don't use printSyntax here. Ensure expression is bare identifer, use it's name().
| * positional slot — it maps to an empty-text positional arg whose span falls | ||
| * back to the surrounding annotation `span`, so downstream indexes never shift. | ||
| */ | ||
| export function buildConstructorCall( |
There was a problem hiding this comment.
Don't use the adapter, update consumers instead.
…ce symbol map Collapses the parallel intermediate structures the resolve pass built. Each namespace now has a single source-ordered, first-wins symbol table (model / composite / named-block) that both type resolution and buildNamespace read, so the name table and the resolved document agree by construction: - merge the collect + collision passes into one walk; one `declare` helper replaces the bucket `seen` loop and registerNames/registerName - drop byNamespace + blockTypesByNamespace (one ScopeSymbol map), the per-bucket models/compositeTypes/declarations arrays, and the buildNamespace blockTypes re-derivation + its name-table alignment dance - collapse resolveTypeTarget's four bare-name map lookups into two symbol lookups (decl-before-block, current-before-ambient precedence preserved) - namedTypes collision uses a Set (the DeclKind values were dead) - remove the dead EnumDeclarationAst branch / DeclKind 'enum' (the parser emits enums as generic blocks; nothing produced 'enum') Behavior-preserving: parity corpus + resolve/resolve-validation green with no fixture changes; contracts byte-identical (fixtures:check clean). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
The parser emits enums as generic blocks (parse.ts has no parseEnum), and the resolver no longer uses EnumDeclarationAst. The EnumDeclarationAst / EnumValueDeclarationAst classes, the EnumDeclaration / EnumValueDeclaration syntax kinds, and their exports are dead with no producer or consumer. Pure dead-code removal, no behavior change. Member-iteration tests that used an enum as one of several members now use a generic block so multi-member iteration coverage is preserved. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
…gnostic test to resolver - psl-resolved-reader imported SyntaxNode both as a type and aliased as a value (`SyntaxNode as SyntaxNodeClass`); the bare-cast ratchet counts the import `as`. Import the class once as a value (a class name is also a valid type) — no alias. - cli.emit-command.additional asserted the legacy interpreter diagnostic for a genuinely-unknown field type (`data Unsupported`). Per the rework-E change that routes unknown types through the resolver, it now emits PSL_UNRESOLVED_TYPE_REFERENCE at the 0-based parser line. Update the assertions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
…ify resolve() Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
| return nameTable.scopes.get(namespaceId)?.get(name)?.kind === refKind; | ||
| } | ||
|
|
||
| function nodeText(node: SyntaxNode): string { |
There was a problem hiding this comment.
You have printSyntax function for this
| } | ||
| if (enumTypeDescriptors.has(field.typeName)) { | ||
| return enumTypeDescriptors.get(field.typeName); | ||
| if (target.kind === 'block') { |
There was a problem hiding this comment.
Check that it's actually an enum block
| return enumTypeDescriptors.get(target.name); | ||
| } | ||
| return scalarTypeDescriptors.get(field.typeName); | ||
| if (target.kind === 'unresolved') { |
There was a problem hiding this comment.
This is a dead code, unresolved target would never be in any of those places.
…oords; SQL reads coord namespace Make ResolveOptions.defaultNamespaceId required and thread it into the Resolver. The resolver now stamps it (in place of the hardcoded UNSPECIFIED_PSL_NAMESPACE_ID) onto bare-name resolved coordinates via a #stamp helper, while keeping every bucket/lookup key at __unspecified__. The SQL interpreter reads the resolved coord namespace for the bare FK target namespace, routing it through resolveNamespaceIdForSqlTarget so a non-postgres target keeps omitting references.namespaceId (SQLite Option-1 reconciliation, byte-identical). Providers thread their target defaultNamespaceId; Mongo passes UNBOUND_NAMESPACE_ID (inert). Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Linked issue
Refs TML-2893. Builds on the fault-tolerant CST parser landed in #795. Supersedes the closed #818 (that was an interim per-slice PR; this is the whole project as one PR).
At a glance
Before this PR the only PSL semantic layer was
PslDocumentAstfrom the legacy regex parser, which half-resolved type references (left a name string each interpreter re-resolved by hand viaallPslModels.find(...)).resolveresolves every reference to a kind-ful coordinate, once, over the lossless CST.Summary
Replace the legacy line-and-regex PSL parser (
parsePslDocument→PslDocumentAst) with the fault-tolerant CST parser (parse) plus a new CST-backed, target-agnosticResolvedDocumentsemantic layer that both the SQL and Mongo interpreters consume. The repo carried two parsers for one language; this makesparsethe only parser, gives the interpreters one typed, fault-tolerant surface to author against, and deletes the legacy parser. The migration is byte-identical for every existing contract — with one deliberate, operator-approved exception (bare-name resolution scope, below).PslDocumentAst,psl-printer, and the introspection /contract-inferemit path are kept and untouched — they construct and printPslDocumentAstbut never parse, so they're independent of the parser swap. They migrate offPslDocumentAstin a follow-up.How it fits together
The target pipeline is
PSL text ─parse─▶ CST ─resolve─▶ ResolvedDocument ─▶ per-target interpreters. Five phases, substrate → consumers → cleanup:43e948349,561f23c89,a10a46c4b,fb5f626d2). A newresolve(document, options?)pass walks the CST once and returns aResolvedDocument: per-namespace name-keyedReadonlyMapstores (insertion-ordered, first-declaration-wins), kind-fulDeclCoordtype targets (scalar/ref/crossSpace/constructor/unresolved),ResolvedAttributeaccessors over CSTExpressionAstnodes, CST back-pointers, and a combineddiagnosticslist. Extension-block + named-type-collision validation are relocated here from the parser at verbatim message/code parity. Bare-name resolution is scoped to referrer + ambient (top-level/unspecified) namespaces — a deliberate divergence from the legacy interpreter's document-wide last-wins (see Reviewer notes).296de8c2d,2096c343a). The Mongocontract-pslinterpreter + provider consumeparse → resolve → ResolvedDocument; contracts stay byte-identical. The provider dropsresolve'sPSL_UNRESOLVED_TYPE_REFERENCE(ObjectIdis a valid Mongo scalar; the interpreter'sPSL_UNSUPPORTED_FIELD_TYPEstays authoritative).1c338f0be,a0afd0719,fa4b8389f). The feat(psl-parser): fault-tolerant recursive-descent parser (parse + SourceFile) under ./syntax #795 parser was not a complete drop-in. These close grammar gaps the legacy parser accepted —@@-block-attributes in generic blocks, namespace-qualified constructor calls (pgvector.Vector(1536)), single-quoted strings, double-quoted object-literal keys, qualified default-function calls (@default(temporal.updatedAt())), qualified@@-block attributes — restore the unsupported-top-level-block diagnostic, and suppress a double-emit. Each carries regression tests; theparse-diagnosticsparity corpus stays green.e7798df23,77b5699eb,616252f5b,d7ef600b9,5339190e4). The SQLcontract-pslpackage migrates leaf-inward (read-shims → attribute accessors → resolution helpers → interpreter+provider → tests) ontoResolvedDocument; 287/287 byte-identical. The diagnostic source is reconciled: resolver-owned name-collision/extension-block diagnostics are merged fromResolvedDocument.diagnostics, interpreter-owned target diagnostics stay verbatim — nothing dropped, nothing double-emitted.9a44775fb,2b61dcc95,d34ccf374). Deletesrc/parser.ts, theparsePslDocumentexport, the./parserentry, andvalidateExtensionBlock+ its validator file/export/test; retire the obsolete parser-own tests. Align 6 integration emit-parity diagnostic fixtures to the parser's 0-based line convention. Restore + migrate 6 integration/E2E tests (including the PSL→real-MongoDB-migration E2E) toparse+resolverather than deleting them — coverage preserved, assertions intact.Behavior changes & evidence
resolve/ResolvedDocumentsurface, exported frompackages/1-framework/2-authoring/psl-parser/src/exports/syntax.ts(implsrc/resolve.ts).parseandresolveare separate entry points. Coverage:test/resolve.test.ts,test/resolve-validation.test.ts.ResolvedDocument— SQLpackages/2-sql/2-authoring/contract-psl/src/interpreter.ts(287/287), Mongopackages/2-mongo-family/2-authoring/contract-psl/src/interpreter.ts(139/139). The TS↔PSL real-pack parity test (test/integration/test/authoring/parity/ts-psl-parity.real-packs.test.ts) is independent byte-identity evidence.ns.Name. Two tests updated to the qualified form asserting the same contract (interpreter.namespaces.test.ts,ts-psl-parity.test.ts).rg parsePslDocumentandrg -w validateExtensionBlockare empty repo-wide;PslDocumentAst+psl-printer+ the infer path remain present and green.Reviewer notes
mongodb-memory-serverdoes not run on the dev host (NixOS —UnknownLinuxDistro), which blocks all repo mongo tests identically (pre-existing/environmental). The 3 mongo-dependent migrated tests —migration-psl-authoring(PSL→real-DB migration E2E),mongo-runner.polymorphism.integration,value-objects.integration— typecheck green and mirror verified patterns, but need CI to confirm they pass. This is the one gate not exercised locally.expected-diagnostics.json) were updated to match. No diagnostic code, message, or contract-shape oracle was changed — only positions. Open question for review: whether user-facing CLI rendering should display 1-based line/column. That's a presentation-layer follow-up, not part of this migration; positions are currently internal to the diagnostics surface.parsepurely syntactic (semantic validity stays inresolve) and leaves the parity corpus green. Largest substrate diff isa0afd0719.buildConstructorCallinpsl-column-resolution.ts; the enum2extensionBlockFromResolvedadapter inresolved-read-shims.ts) reuse the existing constructor/enum2 machinery via raw CST token text. Byte-faithful; depend only on psl-parser types, not the deleted parse functions. Native migration is a follow-up.Testing performed
pnpm --filter @prisma-next/psl-parser test— 400+ (incl. the parse-diagnostics parity corpus + the resolve/validation suites + corpus-parity regression tests)pnpm --filter @prisma-next/sql-contract-psl test— 287/287 byte-identicalpnpm --filter @prisma-next/mongo-contract-psl test— 139/139 byte-identicalpnpm --filter @prisma-next/framework-components test— 388 (validator removal)pnpm --filter @prisma-next/postgres test— 79 (incl. migrated namespace-qualifier-routing)pnpm fixtures:check— greenpnpm build,pnpm lint:deps— greenmongodb-memory-server): the MongoDB E2E + mongo integration tests → CI gate (see Reviewer notes).Skill update
n/a — internal only. No CLI command/flag, public TS API, config field, or error-code change for end users. (
@prisma-next/psl-parser'sparse/resolveare internal authoring substrate;parsePslDocumentwas not a documented public surface.)Follow-ups
sqlSchemaIrToPslAst/contract-inferpath offPslDocumentAstonto CST construction, then deletePslDocumentAst+psl-printer(deliberate next project).attribute-accessors-b: migrate the last raw-string object-literal/@@indexreader in SQLpsl-attribute-parsingto CST natively.buildConstructorCall, the enum2 adapter) to consume CST natively.Alternatives considered
parse+resolveinto one call. Rejected — callers that only need syntax (round-trip, formatting) skip resolution; keeping them separate keepsparsepurely syntactic.ResolvedValuelayer for attribute args. Rejected — consumers already read raw args descriptor-driven, and the CST expression node carries the structured form; a normalized union would be unconsumed and often ambiguous without the descriptor.DeclCoordcoordinates. Rejected — coordinates avoid a cyclic object graph (so nodes freeze) and mirror the existingentries[kind][name]/ ContractRecord<string, …>keying.parsePslDocumentas a driver. Rejected — they carry coverage the unit suites don't (real-DB migration, real-pack loading, cross-package routing); migrated toparse+resolveinstead, preserving coverage.Checklist
git commit -s) per the DCO.TML-NNNN: <sentence-case title>form.n/a — internal only).Notes for the reviewer
The decision log for every judgment call made while building this (the bare-name policy ruling, the diagnostic reconciliation, the parser-gap audit, the 0-based-position ruling, the coverage-preservation correction) is in
projects/psl-parser-replacement/(spec, plan, per-slice reviews). The single most important reviewer check is byte-identity: no contract-shape oracle was weakened to pass — only the sanctioned set changed (the two qualified-reference rewrites, one diagnostic message update with the code unchanged, and 0-based position values).Summary by CodeRabbit
New Features
Improvements
Breaking Changes