feat: enable alias declarations #2196
Conversation
📝 WalkthroughWalkthroughAlias declarations have been introduced and integrated throughout the language grammar, type system, semantic analysis, validation, code generation, and plugin infrastructure. The changes enable defining, referencing, and invoking aliases similarly to functions, with updates to parsing, scope/linking, validation, semantic highlighting, and test suites. Multiple utilities and validators were extended or generalized to support alias handling, and new test cases were added to verify correct behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Parser
participant Linker
participant ScopeProvider
participant Validator
participant CodeGen
participant TestSuite
User->>Parser: Define AliasDecl in model
Parser->>Linker: Parse and create AST with AliasDecl
Linker->>ScopeProvider: Resolve references (including AliasDecl)
ScopeProvider->>Linker: Provide scope for AliasDecl/member access
Linker->>Validator: Resolve invocations (AliasDecl or FunctionDecl)
Validator->>Validator: Validate type, context, and arguments
Validator->>CodeGen: Pass resolved AliasDecl/expressions
CodeGen->>User: Generate code (inlines alias expressions)
TestSuite->>Parser: Run tests with models using aliases
TestSuite->>Validator: Validate correct handling and errors
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
It’s conceptually more of a fragment than an alias... |
|
I partially resolve my issue by linking references inside alias to the first matching model field (this could be improved). Type resolution appears to be working well, and both Prisma model generation and policy rule generation are functioning as expected (tested on simple use cases). I will continue later this week. Before going further, I’d like to get the ZenStack team’s approval to make sure I’m not heading in the wrong direction. A few specific points I’d like to clarify:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (6)
packages/sdk/src/utils.ts (1)
460-468: Consider improving string-based auth detection in aliases.The
hasAuthInvocationfunction uses string matching on CST node text (text.includes('auth()')) to detect auth invocations within alias expressions. This approach could yield false positives (e.g., comments, string literals containing "auth()") and is brittle to formatting changes.Consider using AST traversal of the alias expression instead:
export function hasAuthInvocation(node: AstNode) { return streamAst(node).some((node) => { - const hasAuth = - isAuthInvocation(node) || - (isAliasDecl(node.$resolvedType?.decl) && - node.$resolvedType?.decl.expression?.$cstNode?.text.includes('auth()')); - return hasAuth; + if (isAuthInvocation(node)) { + return true; + } + // Check if this is an alias declaration with auth() in its expression + if (isAliasDecl(node.$resolvedType?.decl) && node.$resolvedType?.decl.expression) { + return streamAst(node.$resolvedType.decl.expression).some(isAuthInvocation); + } + return false; }); }packages/schema/src/plugins/enhancer/policy/utils.ts (1)
555-555: TODO indicates incomplete alias support in cross-model comparison.The TODO comment highlights that cross-model comparisons involving alias expressions are not yet supported. This is a significant limitation that should be addressed or at least documented clearly.
Would you like me to help implement the cross-model comparison support for alias expressions or create an issue to track this limitation?
packages/schema/src/language-server/zmodel-linker.ts (2)
500-519: Consider refactoring duplicate enum scope logicThe enum scope creation logic is duplicated. Both blocks do essentially the same thing but check different conditions.
- // if the field has enum declaration type, resolve the rest with that enum's fields on top of the scopes - if (getContainerOfType(node, isAliasDecl) && node.type.reference?.ref && isEnum(node.type.reference.ref)) { - const contextEnum = node.type.reference.ref as Enum; - const enumScope: ScopeProvider = (name) => contextEnum?.fields?.find((f) => f.name === name); - extraScopes = [enumScope, ...extraScopes]; - } - // make sure type is resolved first this.resolve(node.type, document, extraScopes); let scopes = extraScopes; // if the field has enum declaration type, resolve the rest with that enum's fields on top of the scopes - if (node.type.reference?.ref && isEnum(node.type.reference.ref)) { + if (node.type.reference?.ref && isEnum(node.type.reference.ref) && + (getContainerOfType(node, isAliasDecl) || true)) { const contextEnum = node.type.reference.ref as Enum; const enumScope: ScopeProvider = (name) => contextEnum.fields.find((f) => f.name === name); scopes = [enumScope, ...scopes]; }
534-558: Well-implemented alias resolution logicThe alias resolution correctly handles the context switching needed to resolve alias expressions in the containing model's scope. This addresses the scoping issues mentioned in the PR objectives.
Consider adding a comment to clarify why we check for
containingAliasfirst:private resolveAliasInvocation(node: InvocationExpr, document: LangiumDocument, extraScopes: ScopeProvider[]) { // function is resolved to matching alias declaration + // Check if we're already inside an alias (for nested alias calls) const containingAlias = getContainerOfType(node, isAliasDecl); const matchingAlias = containingAlias || this.findMatchingAlias(node);packages/schema/src/language-server/zmodel-scope.ts (2)
280-337: Consider refactoring for improved maintainabilityThe
getAliasMemberAccessScopemethod correctly handles member access resolution within aliases, but its complexity could be reduced.Consider extracting helper methods:
- Extract the invocation context finding logic (lines 293-296) into a separate method
- Extract the field lookup logic that's duplicated on lines 302-304 and 321-323
Example refactoring:
+ private findInvocationContext(node: AstNode): AstNode | undefined { + let context: AstNode | undefined = node.$container; + while (context && !isInvocationExpr(context)) { + context = context.$container; + } + return context; + } + + private findFieldByName(model: DataModel, fieldName: string): DataModelField | undefined { + return getModelFieldsWithBases(model).find(f => f.name === fieldName); + } private getAliasMemberAccessScope(context: ReferenceInfo): Scope { // ... existing code ... - // Check if this is used in an invocation context - let invocationContext: AstNode | undefined = node.$container; - while (invocationContext && !isInvocationExpr(invocationContext)) { - invocationContext = invocationContext.$container; - } + const invocationContext = this.findInvocationContext(node); if (invocationContext && isInvocationExpr(invocationContext)) { // Find the model where this invocation is used const containingModel = getContainerOfType(invocationContext, isDataModel); if (containingModel) { - const field = getModelFieldsWithBases(containingModel).find( - (f) => f.name === operandName - ); + const field = this.findFieldByName(containingModel, operandName); if (field && field.type.reference?.ref) { return this.createScopeForContainer(field.type.reference.ref, globalScope); }
339-351: Efficient scope combination implementationThe
combineScopesmethod correctly merges multiple scopes while avoiding duplicates.For better performance with larger scopes, consider using a Set or Map for duplicate detection:
private combineScopes(scopes: Scope[]): Scope { - const allElements: AstNodeDescription[] = []; + const elementMap = new Map<string, AstNodeDescription>(); for (const scope of scopes) { const elements = scope.getAllElements(); for (const element of elements) { - // Avoid duplicates - if (!allElements.some(e => e.name === element.name && e.type === element.type)) { - allElements.push(element); - } + const key = `${element.name}:${element.type}`; + if (!elementMap.has(key)) { + elementMap.set(key, element); + } } } - return new StreamScope(stream(allElements)); + return new StreamScope(stream(Array.from(elementMap.values()))); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
packages/language/src/generated/ast.tsis excluded by!**/generated/**,!**/generated/**packages/language/src/generated/grammar.tsis excluded by!**/generated/**,!**/generated/**packages/language/syntaxes/zmodel.tmLanguage.jsonis excluded by!**/*.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/*.yaml
📒 Files selected for processing (20)
packages/language/src/zmodel.langium(5 hunks)packages/language/syntaxes/zmodel.tmLanguage(1 hunks)packages/schema/src/language-server/validator/attribute-application-validator.ts(3 hunks)packages/schema/src/language-server/validator/expression-validator.ts(4 hunks)packages/schema/src/language-server/validator/function-invocation-validator.ts(9 hunks)packages/schema/src/language-server/validator/utils.ts(3 hunks)packages/schema/src/language-server/zmodel-linker.ts(8 hunks)packages/schema/src/language-server/zmodel-scope.ts(5 hunks)packages/schema/src/language-server/zmodel-semantic.ts(2 hunks)packages/schema/src/plugins/enhancer/policy/expression-writer.ts(2 hunks)packages/schema/src/plugins/enhancer/policy/policy-guard-generator.ts(1 hunks)packages/schema/src/plugins/enhancer/policy/utils.ts(5 hunks)packages/schema/src/plugins/prisma/schema-generator.ts(2 hunks)packages/schema/src/res/stdlib.zmodel(2 hunks)packages/schema/src/utils/ast-utils.ts(3 hunks)packages/schema/tests/schema/validation/attribute-validation.test.ts(2 hunks)packages/sdk/src/constants.ts(1 hunks)packages/sdk/src/typescript-expression-transformer.ts(4 hunks)packages/sdk/src/utils.ts(5 hunks)tests/integration/tests/plugins/policy.test.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (11)
packages/schema/src/language-server/validator/expression-validator.ts (1)
packages/schema/src/utils/ast-utils.ts (1)
isAuthInvocation(156-158)
packages/schema/src/language-server/validator/attribute-application-validator.ts (1)
packages/schema/src/language-server/validator/utils.ts (1)
mappedRawExpressionTypeToResolvedShape(106-121)
packages/sdk/src/typescript-expression-transformer.ts (1)
packages/sdk/src/utils.ts (1)
isAliasExpr(452-454)
packages/schema/src/plugins/enhancer/policy/utils.ts (2)
packages/schema/src/utils/ast-utils.ts (1)
isAliasInvocation(168-174)packages/sdk/src/utils.ts (1)
hasAuthInvocation(460-468)
packages/schema/src/plugins/prisma/schema-generator.ts (2)
packages/sdk/src/utils.ts (1)
isAliasExpr(452-454)packages/sdk/src/types.ts (1)
PluginError(115-119)
packages/sdk/src/utils.ts (1)
packages/schema/src/utils/ast-utils.ts (1)
isAuthInvocation(156-158)
packages/schema/tests/schema/validation/attribute-validation.test.ts (1)
packages/schema/tests/utils.ts (2)
loadModel(32-75)loadModelWithError(77-87)
packages/schema/src/language-server/validator/utils.ts (1)
packages/language/src/ast.ts (1)
ResolvedShape(10-10)
packages/schema/src/language-server/validator/function-invocation-validator.ts (1)
packages/sdk/src/utils.ts (2)
isFromStdlib(470-473)getFunctionExpressionContext(432-446)
packages/schema/src/utils/ast-utils.ts (2)
packages/language/src/ast.ts (2)
AstNode(4-4)AstNode(78-83)packages/schema/src/plugins/prisma/prisma-builder.ts (1)
Model(103-149)
tests/integration/tests/plugins/policy.test.ts (1)
packages/testtools/src/schema.ts (1)
loadSchema(173-249)
🔇 Additional comments (46)
packages/schema/src/plugins/enhancer/policy/policy-guard-generator.ts (1)
16-16: LGTM: Clean formatting improvementAdding a blank line after the import block improves code readability by clearly separating imports from the main code.
packages/sdk/src/constants.ts (1)
16-16: LGTM: Essential addition for alias supportThe
AliasFunctionexpression context is properly added to support the new alias declarations feature. The naming follows the existing enum pattern and is consistent with the broader implementation.packages/language/syntaxes/zmodel.tmLanguage (1)
23-23: LGTM: Proper syntax highlighting support for alias keywordThe addition of "alias" to the keyword pattern correctly enables syntax highlighting for the new alias declarations. The alphabetical placement between "abstract" and "attribute" maintains consistency.
packages/schema/src/plugins/enhancer/policy/expression-writer.ts (2)
8-8: LGTM: Necessary import for alias supportThe
isAliasDeclimport is correctly added to support alias declaration checking in the expression writer.
809-811: LGTM: Proper alias expression handlingThe alias handling logic correctly identifies alias declarations and recursively processes their underlying expressions. This enables seamless use of aliases in policy expressions by effectively "inlining" the alias definition at invocation sites.
packages/schema/src/res/stdlib.zmodel (2)
69-71: LGTM: Proper extension of expression context systemThe addition of
AliasFunctionto theExpressionContextenum correctly extends the context system to support alias declarations. The documentation is clear and follows the existing pattern.
83-83: LGTM: Correct context extension for auth() functionAdding
AliasFunctionto theauth()function's allowed expression contexts properly enables its use within alias declarations while maintaining the existing validation framework.packages/sdk/src/typescript-expression-transformer.ts (2)
27-27: LGTM: Import addition is appropriateThe import of
isAliasExprutility function is correctly added to support alias expression detection.
397-397: Improved error message with contextual informationThe addition of
${expr.$cstNode?.text}provides valuable context for debugging unresolved expressions.packages/schema/src/language-server/validator/attribute-application-validator.ts (2)
11-11: LGTM: Import addition supports alias validationThe
isAliasDeclimport is correctly added to enable alias declaration detection in attribute validation.
32-37: LGTM: Appropriate utility imports for type validationThe additional imports provide necessary functionality for type checking and validation in the alias context.
packages/schema/src/language-server/zmodel-semantic.ts (2)
2-2: LGTM: Import addition for semantic highlightingThe
isAliasDeclimport is correctly added to support alias declaration highlighting.
87-87: LGTM: Consistent semantic highlighting for aliasesTreating alias declarations the same as function declarations for semantic highlighting is appropriate since they are both callable entities.
packages/schema/src/language-server/validator/expression-validator.ts (5)
7-7: LGTM: Import addition for alias detectionThe
isAliasDeclimport is correctly added to support alias context checking.
25-25: LGTM: Import addition for container traversalThe
getContainerOfTypeimport is appropriate for checking if expressions are contained within alias declarations.
37-37: LGTM: Appropriate exclusion of auth() validation in alias contextsExcluding auth() validation when inside an alias declaration is logical since aliases may be used in different contexts where auth resolution requirements differ.
56-56: LGTM: Enhanced error message with contextual informationAdding
${expr.$cstNode?.text}provides valuable context for debugging unresolved expressions, consistent with similar improvements in other files.
54-54: Confirm logic fix for reference resolution error detectionThe updated condition now correctly reports errors when
hasReferenceResolutionErroris true (i.e., when any member‐ or reference‐resolution errors are present). I verified this is the only occurrence in
packages/schema/src/language-server/validator/expression-validator.ts, and no residual negated checks remain.packages/schema/tests/schema/validation/attribute-validation.test.ts (2)
409-409: LGTM: Updated error message reflects broader callable conceptThe change from "function" to "AbstractCallable" correctly reflects the generalization to include both functions and aliases as callable entities.
1395-1468: Excellent comprehensive test coverage for alias expressionsThe test suite thoroughly covers alias functionality including:
- Basic alias usage in policy rules and default values
- Nested alias expressions with auth() integration
- Complex alias compositions
- Type validation error cases
The tests demonstrate practical usage patterns and validate both positive and negative scenarios.
packages/schema/src/plugins/prisma/schema-generator.ts (2)
41-41: Import addition looks correct.The
isAliasExprutility import aligns with the new alias functionality and is used appropriately in the method below.
966-972: Alias expression handling implementation is solid.The logic correctly:
- Detects alias expressions using the imported utility
- Extracts the underlying expression from the alias's function reference
- Recursively processes the resolved expression
- Provides clear error messaging when the alias reference is missing
The recursive approach ensures that nested alias expressions would be properly resolved.
packages/sdk/src/utils.ts (3)
2-2: Import additions are appropriate.The new imports (
AbstractCallable,isAliasDecl,streamAst) support the alias functionality being added and are used correctly in the updated functions.Also applies to: 17-17, 44-44
432-432: Good generalization of function signature.Changing the parameter type from
FunctionDecltoAbstractCallableappropriately extends support to both functions and aliases, which makes sense since both can have expression contexts.
452-454: Simple and effective alias expression detection.The
isAliasExprfunction correctly identifies invocation expressions that reference alias declarations by checking the$typeof the referenced function.packages/schema/src/plugins/enhancer/policy/utils.ts (2)
17-17: Import updates support alias functionality.The new imports correctly bring in the
hasAuthInvocationutility and alias-related functions needed for the enhanced policy handling.Also applies to: 46-51
517-517: Enhanced auth detection for alias expressions.Switching from
isAuthInvocationtohasAuthInvocationproperly extends auth detection to include alias expressions that contain auth invocations, which is necessary for the alias feature.packages/schema/src/language-server/validator/utils.ts (3)
8-8: Import addition supports new type mapping functionality.The
ResolvedShapeimport is correctly added to support the newmappedRawExpressionTypeToResolvedShapefunction.
90-101: Enhanced builtin type mapping with proper handling.The addition of
'Object'and'Unsupported'cases correctly extends the type mapping function. The function structure is properly maintained with the switch statement covering all cases.
106-121: Well-structured expression type to resolved shape mapping.The new
mappedRawExpressionTypeToResolvedShapefunction provides a clean mapping from AST node types to their corresponding resolved shapes. The mappings are logical:
- Literal types map to their corresponding basic types
ObjectExprmaps to'Object'NullExprmaps to'Null'- Default fallback to
'Any'is appropriate for unknown typespackages/schema/src/utils/ast-utils.ts (2)
14-14: LGTM!The imports for
isAliasDeclandAliasDeclare properly added to support the new alias declaration feature.Also applies to: 18-18
319-327: LGTM!The
getAllLoadedAliasfunction follows the established pattern for retrieving declarations from all loaded documents and correctly filters forAliasDecltypes.tests/integration/tests/plugins/policy.test.ts (3)
114-173: LGTM!The test case comprehensively validates simple alias expressions in both default values and policy rules, with proper assertions on the generated policy guards.
175-272: LGTM!Excellent test coverage for complex alias expressions including enum comparisons, logical operators, and member access with collection predicates. The assertions correctly verify the generated Prisma filter objects.
274-330: LGTM!Good test coverage for member access in aliases, including edge cases with empty collections and non-matching data. The test correctly verifies that the generated policy filter is consistent regardless of the input context.
packages/language/src/zmodel.langium (2)
14-14: LGTM!The type system changes properly integrate alias declarations as first-class citizens in the language. The
AbstractCallableabstraction elegantly unifies functions and aliases for invocation expressions.Also applies to: 94-94, 97-97, 255-255
221-223: Verify the parameter design for alias declarationsThe alias declaration syntax includes parameters, but according to the PR objectives, the author questions whether parameters are beneficial since "references are resolved in the model scope where the rule is invoked."
Should alias declarations support parameters? If not, consider simplifying the syntax:
AliasDecl: - TRIPLE_SLASH_COMMENT* 'alias' name=RegularID '(' (params+=FunctionParam (',' params+=FunctionParam)*)? ')' '{' expression=Expression '}' (attributes+=InternalAttribute)*; + TRIPLE_SLASH_COMMENT* 'alias' name=RegularID '{' expression=Expression '}' (attributes+=InternalAttribute)*;The current implementation with parameters may lead to confusion if they don't actually create a new scope for the alias body.
Also applies to: 238-238
packages/schema/src/language-server/validator/function-invocation-validator.ts (3)
2-3: LGTM!The generalization from functions to callables is well-implemented with proper null checks and updated error messages.
Also applies to: 11-11, 52-60, 67-69
164-172: LGTM!The expression context handling correctly identifies alias declarations and returns the appropriate
AliasFunctioncontext.Also applies to: 80-80
157-161: LGTM!The refactoring correctly moves the function-specific checker logic inside the standard library check and properly updates the method signature to accept
AbstractCallable.Also applies to: 187-187
packages/schema/src/language-server/zmodel-linker.ts (3)
2-2: LGTM!The imports are properly organized to support alias declarations and invocations.
Also applies to: 30-30, 37-37, 61-65
199-201: LGTM!Good design decision to defer alias resolution until usage. This allows the alias expression to be resolved in the correct model context.
292-292: LGTM!The invocation resolution correctly differentiates between function and alias invocations, with appropriate type handling.
Also applies to: 309-312
packages/schema/src/language-server/zmodel-scope.ts (3)
4-4: LGTM!The import for
isAliasDeclis properly added and aligns with the new alias declaration feature.
121-126: Alias scope checks are correctly implementedThe additions properly detect when scope resolution occurs within an alias declaration and delegate to specialized methods. This ensures that alias expressions can reference model fields appropriately.
Consider caching the alias declaration lookup result to avoid repeated
getContainerOfTypecalls if performance becomes a concern in larger schemas.Also applies to: 135-141
259-278: Well-structured alias scope resolutionThe
getAliasScopemethod correctly aggregates fields from all data models, allowing alias expressions to reference any model field. The use ofgetModelFieldsWithBasesensures inherited fields are included.
packages/schema/src/language-server/validator/attribute-application-validator.ts
Outdated
Show resolved
Hide resolved
packages/schema/src/language-server/validator/function-invocation-validator.ts
Outdated
Show resolved
Hide resolved
| (isReferenceExpr(child) && isDataModelField(child.target.ref)) || | ||
| // TODO: field access from alias expression | ||
| isAliasInvocation(child) | ||
| ) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Field access detection extended for aliases with incomplete support.
The code correctly adds isAliasInvocation(child) to detect field access patterns, but the TODO comment on line 322 indicates that field access from alias expressions isn't fully implemented yet. This could lead to incomplete policy validation.
Please verify that the incomplete alias field access support doesn't introduce security vulnerabilities or incorrect policy enforcement:
🏁 Script executed:
#!/bin/bash
# Search for other locations where field access detection might need alias support
echo "=== Searching for field access patterns that might need alias support ==="
rg -A 3 -B 3 "isDataModelField.*target\.ref" --type ts
echo "=== Searching for policy-related field reference checks ==="
rg -A 3 -B 3 "isReferenceExpr.*isDataModelField" --type ts
echo "=== Searching for existing alias invocation usage ==="
rg -A 3 -B 3 "isAliasInvocation" --type tsLength of output: 13523
Incomplete alias field access handling may lead to missed policy checks
The addition of isAliasInvocation(child) in packages/schema/src/plugins/enhancer/policy/utils.ts correctly flags alias calls, but there is no logic to extract the underlying data‐model field. Our searches show:
- getFieldAccess in packages/schema/src/plugins/enhancer/policy/constraint-transformer.ts only handles reference and member accesses—no alias support.
- policy-guard-generator.ts and related guard logic never resolve alias invocations to actual fields.
Without extracting the actual DataModelField from an alias invocation, policy rules that use aliases can slip past validation.
Please address this by:
- In packages/schema/src/plugins/enhancer/policy/utils.ts (lines ~320–323), replace the bare
isAliasInvocation(child)check with code that resolves the alias and yields the child’s target field. - In packages/schema/src/plugins/enhancer/policy/constraint-transformer.ts within getFieldAccess, add a branch for
isAliasInvocation(expr)to resolve the alias and return the correct{ name: … }. - Update policy-guard-generator.ts (as needed) so that guards generated for alias-based expressions reference the actual field.
- Add unit tests covering alias‐based field accesses in policy rules to prevent regressions.
🤖 Prompt for AI Agents
In packages/schema/src/plugins/enhancer/policy/utils.ts around lines 320 to 323,
replace the simple isAliasInvocation(child) check with logic that resolves the
alias invocation to extract and return the underlying DataModelField. Then, in
packages/schema/src/plugins/enhancer/policy/constraint-transformer.ts within the
getFieldAccess function, add a case to handle isAliasInvocation expressions by
resolving the alias and returning the correct field name object. Also, update
policy-guard-generator.ts to ensure that guards generated for alias-based
expressions reference the resolved actual data-model field instead of the alias.
Finally, add unit tests that cover alias-based field accesses in policy rules to
ensure these changes prevent missed validations and regressions.
| export function isAliasInvocation(node: AstNode) { | ||
| // check if a matching alias exists | ||
| const allAlias = getContainerOfType(node, isModel)?.declarations.filter(isAliasDecl) ?? []; | ||
| // const aliasDecls = getAllLoadedAlias(this.langiumDocuments()); | ||
| return isInvocationExpr(node) && allAlias.some((alias) => alias.name === node.function.$refText); | ||
| // (!node.function.ref || !isFromStdlib(node.function.ref)) /* && isAliasDecl(node.function.ref) */ | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clean up commented code and consider cross-model alias resolution
The function has commented-out code that should be removed. Additionally, the current implementation only checks aliases within the containing model, which may not handle imported aliases correctly.
Consider this cleaner implementation:
export function isAliasInvocation(node: AstNode) {
- // check if a matching alias exists
- const allAlias = getContainerOfType(node, isModel)?.declarations.filter(isAliasDecl) ?? [];
- // const aliasDecls = getAllLoadedAlias(this.langiumDocuments());
- return isInvocationExpr(node) && allAlias.some((alias) => alias.name === node.function.$refText);
- // (!node.function.ref || !isFromStdlib(node.function.ref)) /* && isAliasDecl(node.function.ref) */
+ if (!isInvocationExpr(node)) {
+ return false;
+ }
+
+ // Check if the resolved reference is an alias declaration
+ if (node.function.ref && isAliasDecl(node.function.ref)) {
+ return true;
+ }
+
+ // Fallback: check by name in the current model
+ const allAlias = getContainerOfType(node, isModel)?.declarations.filter(isAliasDecl) ?? [];
+ return allAlias.some((alias) => alias.name === node.function.$refText);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function isAliasInvocation(node: AstNode) { | |
| // check if a matching alias exists | |
| const allAlias = getContainerOfType(node, isModel)?.declarations.filter(isAliasDecl) ?? []; | |
| // const aliasDecls = getAllLoadedAlias(this.langiumDocuments()); | |
| return isInvocationExpr(node) && allAlias.some((alias) => alias.name === node.function.$refText); | |
| // (!node.function.ref || !isFromStdlib(node.function.ref)) /* && isAliasDecl(node.function.ref) */ | |
| } | |
| export function isAliasInvocation(node: AstNode) { | |
| if (!isInvocationExpr(node)) { | |
| return false; | |
| } | |
| // Check if the resolved reference is an alias declaration | |
| if (node.function.ref && isAliasDecl(node.function.ref)) { | |
| return true; | |
| } | |
| // Fallback: check by name in the current model | |
| const allAlias = getContainerOfType(node, isModel)?.declarations.filter(isAliasDecl) ?? []; | |
| return allAlias.some((alias) => alias.name === node.function.$refText); | |
| } |
🤖 Prompt for AI Agents
In packages/schema/src/utils/ast-utils.ts around lines 168 to 174, remove the
commented-out code to clean up the function. Then, update the alias resolution
logic to check aliases not only in the containing model but also across imported
models to correctly handle cross-model alias references.
| // TODO: ensure referenceExpr from alias is resolved | ||
| return this.reference(expr as ReferenceExpr); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Address the TODO comment for alias reference resolution
This TODO indicates that ReferenceExpr from aliases may not be properly resolved, which could lead to transformation failures.
Do you want me to help implement the reference resolution logic for alias expressions, or should this be tracked as a separate issue?
🏁 Script executed:
#!/bin/bash
# Search for ReferenceExpr handling in alias contexts to understand the current state
ast-grep --pattern $'ReferenceExpr' | head -20
rg -A 5 "ReferenceExpr.*alias" Length of output: 1806
Implement alias reference resolution or track as a separate issue
It looks like the // TODO: ensure referenceExpr from alias is resolved in
packages/sdk/src/typescript-expression-transformer.ts (lines 92–93) remains unaddressed—there is no logic that unwraps or resolves alias definitions before calling this.reference(expr as ReferenceExpr). Without that, aliased references can fail during transformation.
Please choose one of the following:
- Implement alias resolution now
Add code to detect whenexproriginates from an alias (e.g., via a lookup or mapping function) and replace it with the underlyingReferenceExprbefore invokingthis.reference(...). - Track as a separate issue
Create a ticket to tackle alias-resolution in a dedicated PR, and update/remove the TODO to reference that issue.
Let me know which approach you’d like—and I can help with the implementation if you prefer.
🤖 Prompt for AI Agents
In packages/sdk/src/typescript-expression-transformer.ts around lines 92 to 93,
the TODO about resolving alias references before calling this.reference is not
implemented. To fix this, either add logic to detect if expr is an alias and
unwrap it to get the underlying ReferenceExpr before passing it to
this.reference, or create a separate issue to address alias resolution later and
update or remove the TODO comment to reference that issue accordingly.
|
Hi @Azzerty23 , thanks a lot for driving this effort. A sharing mechanism of policy rules is a long-desired feature. I think we have two main things to discuss:
For #1, my tendency is to conceptualize it as a function, because the concept already exists today, and a function can have its input's types defined so we can keep the ZModel a statically typed language as it is today). So it's basically: [keyword] isCurrentUser(user: User) {
user.id == auth().id
}
...
model User {
...
@@allow('all', isCurrentUser(this))
}
model Post {
...
@@allow('all', isCurrentUser(author))
}Love to hear your thoughts about this. The benefit is that the resolution is very clean and straightforward. The downside is you can only call the function where the type matches. It doesn't apply to other models that also have an "id" field. My feeling is that this should generally be fine, thanks to abstract inheritance and polymorphism. But please also verify if it works for your use cases. For #2, we can choose to use a new keyword like |
|
Hi @ymc9, thank you for your response! While implementing this, I realized what I created was more like a fragment, but that wasn’t driven by any specific use case. I agree that a function, as you described, makes much more sense. Fragments feel too restrictive for code reusability since their references rely on having the same names. I also agree with your second point about reusing the existing One question: do you think it would make sense to support parameters with default values for extra flexibility? (maybe as a future enhancement) |
Awesome! So we're very much on the same page. I suggest we keep parameter default out of the scope first to keep it simple. Additionally, please note that a significant portion of the implementation will need to be rewritten in v3 due to the entirely different mechanism for access policies. |
This is a first attempt at implementing
aliasdeclarations. The goal is to enable code reuse for both policy rules and default functions.Example of intended usage:
alias allowAll() { true } alias defaultTitle() { 'Default Title' } alias currentUser() { auth().id } alias ownPublishedPosts() { currentUser() == authorId && published } model Post { id Int @id @default(autoincrement()) title String @default(defaultTitle()) published Boolean @default(allowAll()) author User @relation(fields: [authorId], references: [id]) authorId String @default(auth().id) @@allow('read', allowAll()) @@allow('create,update,delete', currentUser() == authorId && published) @@deny('all', !ownPublishedPosts()) } model User { id String @id @default(cuid()) name String? posts Post[] @@allow('all', allowAll()) }However, during implementation, I realized that having a shared generic alias mechanism for both policy rules and default values might not be the best idea: policy rule expressions and default value expressions are not interchangeable, and it becomes tricky to handle validation rules properly.
I initially started working on
rule(#1959), but got sidetracked along the way... 😅Also, I might have done something completely off with the SDL (Langium) changes — I have zero experience in that area.
Currently, I'm generating functions from aliases inside the policy file. But I’m thinking of reverting to my original approach of inlining the expression instead, as it would make handling relations easier. For example, this:
Would become something like:
I’m also facing scoping issues where the expression doesn't allow referencing model fields:
alias ownPublishedPosts() { currentUser() == authorId && published } // Could not resolve reference to ReferenceTarget named 'authorId'. // Could not resolve reference to ReferenceTarget named 'published'. // invalid operand type for "&&" operator@ymc9, thanks a lot in advance for your help!
If this approach doesn’t make sense at all, please don’t hesitate to say so ;)