feat: support @@validate in type declarations
#1868
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request involve multiple modifications across various files, primarily focused on enhancing validation logic and import structures. Key updates include renaming functions for field reference validation, expanding scope checks for typedef fields in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Validator
participant SchemaGenerator
User->>SchemaGenerator: Request schema generation
SchemaGenerator->>Validator: Validate fields
Validator->>Validator: Check field references
Validator->>SchemaGenerator: Return validation results
SchemaGenerator->>User: Return generated schema
sequenceDiagram
participant User
participant ZModelScopeProvider
User->>ZModelScopeProvider: Request member access scope
ZModelScopeProvider->>ZModelScopeProvider: Check typedef fields
ZModelScopeProvider->>User: Return member access scope
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
packages/schema/src/plugins/zod/utils/schema-gen.ts (2)
Line range hint
15-147: Consider breaking down the attribute processing logic for better maintainability.The
makeFieldSchemafunction handles multiple validation attributes in a large switch statement. Consider extracting the attribute processing logic into separate functions for better maintainability and testability.Here's a suggested approach:
type AttributeHandler = (attr: DataModelFieldAttribute, schema: string) => string; const attributeHandlers: Record<string, AttributeHandler> = { '@length': (attr, schema) => handleLengthAttribute(attr, schema), '@contains': (attr, schema) => handleContainsAttribute(attr, schema), // ... other handlers }; function handleLengthAttribute(attr: DataModelFieldAttribute, schema: string): string { const message = getAttrLiteralArg<string>(attr, 'message'); const messageArg = message ? `, { message: ${JSON.stringify(message)} }` : ''; const min = getAttrLiteralArg<number>(attr, 'min'); const max = getAttrLiteralArg<number>(attr, 'max'); return `${schema}${min ? `.min(${min}${messageArg})` : ''}${max ? `.max(${max}${messageArg})` : ''}`; }
Line range hint
196-204: Consider improving type safety in refineDecimal function.The
refineDecimalfunction could benefit from stronger type safety by:
- Using a type union for the
opparameter- Adding proper error handling for the Decimal conversion
Here's a suggested improvement:
type DecimalOp = 'gt' | 'gte' | 'lt' | 'lte'; function refineDecimal(op: DecimalOp, value: number, messageArg: string) { return `.refine((v): boolean => { if (v === null || v === undefined) return false; try { return new Decimal(v.toString()).${op}(new Decimal(value)); } catch { return false; } }${messageArg})`; }tests/integration/tests/enhancements/json/crud.test.ts (1)
219-223: Consider adding edge cases to strengthen schema validation tests.While the current test cases cover the basic scenarios, consider adding:
- Edge case: age = 19 (just above the validation threshold)
- Boundary test: missing address/city field
- Error message validation to ensure clear feedback
const schema = params.zodSchemas.models.ProfileSchema; expect(schema.safeParse({ age: 10, address: { city: 'NY' } })).toMatchObject({ success: false }); expect(schema.safeParse({ age: 20, address: { city: 'NYC' } })).toMatchObject({ success: false }); expect(schema.safeParse({ age: 20, address: { city: 'NY' } })).toMatchObject({ success: true }); +// Edge case: just above threshold +expect(schema.safeParse({ age: 19, address: { city: 'NY' } })).toMatchObject({ success: false }); +// Missing field tests +expect(schema.safeParse({ age: 20 })).toMatchObject({ success: false }); +expect(schema.safeParse({ age: 20, address: {} })).toMatchObject({ success: false }); +// Error message validation +const result = schema.safeParse({ age: 10, address: { city: 'NY' } }); +expect(result.success).toBe(false); +if (!result.success) { + expect(result.error.errors[0].message).toContain('Invalid input'); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
packages/schema/src/language-server/validator/function-invocation-validator.ts(2 hunks)packages/schema/src/language-server/zmodel-scope.ts(1 hunks)packages/schema/src/plugins/prisma/schema-generator.ts(1 hunks)packages/schema/src/plugins/zod/generator.ts(7 hunks)packages/schema/src/plugins/zod/utils/schema-gen.ts(1 hunks)packages/schema/src/utils/ast-utils.ts(1 hunks)packages/sdk/src/utils.ts(2 hunks)tests/integration/tests/enhancements/json/crud.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/schema/src/plugins/prisma/schema-generator.ts
🧰 Additional context used
🪛 GitHub Check: CodeQL
packages/schema/src/plugins/zod/generator.ts
[warning] 722-722: Improper code sanitization
Code construction depends on an improperly sanitized value.
🔇 Additional comments (17)
packages/schema/src/plugins/zod/utils/schema-gen.ts (2)
Line range hint 1-13: LGTM! Import cleanup looks good.
The imports have been streamlined to remove unused dependencies while maintaining all necessary imports for the current functionality.
Line range hint 1-220: Verify the impact of removing makeValidationRefinements.
The removal of makeValidationRefinements aligns with the PR objective to support @@validate in type declarations. Let's verify that all validation-related code has been properly migrated.
✅ Verification successful
Based on the gathered evidence, I can now provide a final response:
The removal of makeValidationRefinements is safe and properly replaced with @@validate support
The codebase shows:
- The old
makeValidationRefinementsonly exists ingenerator.tsand has been refactored to support the new@@validateattribute - Extensive test coverage for
@@validatefunctionality across multiple test files - Proper implementation of validation refinements through zod's
.refine()method - Clean integration with the type system as shown by the presence of validation-related imports and validators
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to the old validation system
# and verify the new validation system is in place
# Check for any remaining references to makeValidationRefinements
rg "makeValidationRefinements" --type ts
# Look for the new @@validate implementation
rg "@@validate" --type ts
# Check for any TODO or FIXME comments related to validation
rg -e "TODO.*validat" -e "FIXME.*validat" --type ts
Length of output: 8609
Script:
#!/bin/bash
# Let's check the implementation of the new validation system
# Check the generator.ts file for validation-related changes
rg -A 10 "compile.*@@validate" packages/schema/src/plugins/zod/generator.ts
# Check for any validation-related functions in the current schema-gen.ts
ast-grep --pattern 'function $FUNC_NAME($_) {
$$$
validate
$$$
}'
# Look for any validation-related imports that might have been added/removed
rg "import.*validat" --type ts
Length of output: 5471
packages/schema/src/language-server/validator/function-invocation-validator.ts (2)
18-18: LGTM: Import change aligns with type declaration support
The change from getDataModelFieldReference to getFieldReference appropriately broadens the scope of field reference validation to support both data model fields and type declaration fields.
99-99: LGTM: Field reference validation updated for type declarations
The validation logic now uses getFieldReference which enables validation of fields in both data models and type declarations, aligning with the PR objective to support @@validate in type declarations.
Let's verify that this change doesn't break existing data model field validations:
✅ Verification successful
Based on the gathered information, I can now provide the final response:
Field reference validation supports both data model and type declaration fields
The implementation of getFieldReference in packages/sdk/src/utils.ts explicitly handles both DataModelField and TypeDefField types:
export function getFieldReference(expr: Expression): DataModelField | TypeDefField | undefined {
if (isReferenceExpr(expr) && (isDataModelField(expr.target.ref) || isTypeDefField(expr.target.ref))) {
return expr.target.ref;
}
}The validation logic in the function invocation validator correctly uses this helper, ensuring compatibility with both data model and type declaration field validations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for existing data model field validations to ensure compatibility
ast-grep --pattern 'getFieldReference($expr)'
# Search for test cases covering both data model and type declaration validations
rg -l "@@validate.*typedef"
Length of output: 77
Script:
#!/bin/bash
# Let's first check where getFieldReference is defined and used
rg "getFieldReference" -A 2
# Then look for test files related to function validation
fd "function.*validator.test" --type f
# And examine the test content for validation scenarios
fd "function.*validator.test" --type f --exec cat {}
Length of output: 1442
Script:
#!/bin/bash
# Let's examine the implementation context around the validation
rg -B 5 -A 5 "if \(!getFieldReference\(firstArg\)\)" packages/schema/src/language-server/validator/function-invocation-validator.ts
# Check test files that might cover validation scenarios
fd "validator.test" --type f --exec cat {}
# Look for type declaration related validation tests
rg -l "@@validate.*type\s"
Length of output: 690
packages/schema/src/language-server/zmodel-scope.ts (2)
140-141: LGTM! Clear and descriptive comment.
The comment accurately explains the conditions under which typedef fields are added to the scope, which is essential for understanding the scope resolution rules.
147-147: LGTM! Verify the impact on existing type definitions.
The condition correctly expands field type checking to include typedef fields, which aligns with the PR objective of supporting validation in type declarations.
Let's verify the usage of typedef fields in the codebase:
✅ Verification successful
LGTM! The typedef field support is properly integrated.
The verification confirms that:
- The
isTypeDefFieldtype guard is properly imported and used consistently across the codebase - The condition is part of a well-structured scope resolution system that handles both data model fields and typedef fields
- The change aligns with existing typedef field handling in other parts of the codebase, particularly in:
- Field type resolution (
zmodel-linker.ts) - Scope creation (
zmodel-scope.ts) - Attribute validation (
attribute-application-validator.ts)
- Field type resolution (
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for existing typedef fields that might be affected by this change
# Search for typedef field declarations
ast-grep --pattern 'typedef $_ {
$$$
$field: $_
$$$
}'
# Search for member access expressions on typedef fields
ast-grep --pattern '$_.$field'
Length of output: 95
Script:
#!/bin/bash
# Let's try a different approach to find typedef and field usage
# Search for typedef declarations and their fields
rg -U "typedef\s+\w+\s*{[^}]*}" -A 5
# Search for references to typedef fields in the codebase
rg "\btypedef\b.*\bfield\b" -A 2
# Search for field access patterns in TypeScript files
fd -e ts -x rg -l "\.field\b" {}
# Look for type-related scope resolution code
rg "isTypeDefField|isDataModelField" -A 2
Length of output: 33654
packages/schema/src/utils/ast-utils.ts (3)
Line range hint 142-309: LGTM! Well-structured utility functions
The AST utility functions are well-organized with:
- Clear separation of concerns
- Proper error handling
- Comprehensive documentation
Line range hint 1-17: Verify the migration of field reference validation logic
The removal of expression-related imports and field reference functions suggests that this functionality has been moved. Since this change supports @@validate in type declarations, we should verify that the field reference validation logic is properly handled in the new location.
✅ Verification successful
Field reference validation has been properly migrated and enhanced
The verification shows that getFieldReference function has been successfully migrated to packages/sdk/src/utils.ts and enhanced to handle both DataModelField and TypeDefField types. The new implementation properly handles:
- Reference expressions through
isReferenceExpr - Member access expressions through
isMemberAccessExpr - Both data model fields and type definition fields
The removal of imports and the old implementation from ast-utils.ts is correct as the functionality has been consolidated in a more appropriate location with broader type support.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that field reference validation has been moved to a new location
# and that both DataModelField and TypeDefField are properly handled
# Look for the new field reference validation logic
ast-grep --pattern 'function getFieldReference($$$)'
# Look for validation handling of both field types
rg -l "(?s)TypeDefField.*DataModelField|DataModelField.*TypeDefField"
Length of output: 426
Script:
#!/bin/bash
# Let's examine the implementation of field reference validation in these files
# Look for field reference validation logic in sdk/utils.ts
rg -A 10 "getFieldReference|validateFieldReference" "packages/sdk/src/utils.ts"
# Look for field reference validation in schema-gen.ts which seems relevant
rg -A 10 "getFieldReference|validateFieldReference" "packages/schema/src/plugins/zod/utils/schema-gen.ts"
# Check for any validation-related functions in zmodel-linker.ts
rg -A 10 "validate.*Field|link.*Field" "packages/schema/src/language-server/zmodel-linker.ts"
# Look for the old implementation to compare
rg -A 10 "getDataModelFieldReference" .
Length of output: 1046
Line range hint 42-141: Review inheritance handling with type declarations
The inheritance handling code is complex and handles multiple edge cases. With the addition of @@validate support in type declarations, please verify:
- Delegate model inheritance still works correctly with validated type fields
- Attribute inheritance filtering properly handles new validation attributes
- Container linking maintains consistency with type declarations
✅ Verification successful
Based on the search results, I can now provide a conclusive response:
No changes needed for inheritance handling with @@validate
The inheritance implementation in mergeBaseModels is already correctly handling validation attributes because:
-
The
@@validateattribute is treated like any other model attribute and follows the standard inheritance rules as evidenced by test cases (e.g., inissue-971.test.tswhere validation rules are inherited through multiple levels). -
The
filterBaseAttributefunction already has a comprehensive list of uninheritable attributes (@@delegate,@@map,@@unique,@@index,@@fulltext), and@@validateis not among them, meaning it's correctly inheritable by design. -
The container linking logic in
cloneAstis attribute-agnostic and works uniformly for all attributes, including validation rules.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential inheritance issues with type declarations
# Look for delegate models with validated type fields
ast-grep --pattern 'model $_ {
$$$
@@delegate
$$$
@@validate
$$$
}'
# Check for validation attributes in inheritance chains
ast-grep --pattern 'model $child extends $parent {
$$$
@@validate
$$$
}'
Length of output: 160
Script:
#!/bin/bash
# Let's try a different approach to verify inheritance handling
# 1. Find all model declarations with inheritance
ast-grep --pattern 'model $_ extends $_'
# 2. Look for validation-related attributes in the codebase
rg "@@validate" -A 2 -B 2
# 3. Check for delegate models
ast-grep --pattern 'model $_ {
$$$
@@delegate
}'
# 4. Look for type declarations that might be inherited
ast-grep --pattern 'type $_ {
$$$
}'
Length of output: 25123
tests/integration/tests/enhancements/json/crud.test.ts (2)
196-217: LGTM! Well-structured test schema for the new @@validate feature.
The schema effectively combines field-level (@gte) and type-level (@@validate) validations to test complex validation rules in type declarations.
194-231: Verify integration with existing validation features.
The test demonstrates the new @@validate feature well, but let's ensure it properly integrates with other validation features:
✅ Verification successful
Based on the analysis of the validation tests, implementation, and stdlib definitions, I can now provide a final response:
The test implementation properly validates integration with existing features
The test case correctly demonstrates integration with:
- Field-level validation (
@length(2, 10)oncityfield) - Model-level validation (
@@validateonProfiletype) - Complex validation expressions combining multiple conditions (
age > 18 && length(address.city, 2, 2)) - JSON field validation through type definitions
- Proper error handling and validation rejection
The implementation aligns well with other validation tests in the codebase, particularly with:
- Field validation tests showing similar patterns
- Zod plugin integration tests demonstrating consistent behavior
- Core validation functions in stdlib supporting all used operations
The test coverage is comprehensive and no integration issues are found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential conflicts with other validation features
# Search for other validation-related tests
echo "Checking for related validation tests..."
rg -l "@@validate|@validate" "tests/"
# Search for validation-related code in the core
echo "Checking core validation implementations..."
rg -l "@@validate|@validate" "packages/schema/src/"
# Look for potential validation plugins
echo "Checking for validation plugins..."
rg -l "registerValidator|ValidationPlugin" "packages/"
Length of output: 1088
Script:
#!/bin/bash
# Let's analyze the validation tests to understand the integration points
# Check the content of field validation tests
echo "=== Field Validation Test Content ==="
rg -A 10 -B 10 "@@validate" "tests/integration/tests/enhancements/with-policy/field-validation.test.ts"
# Check zod plugin test content
echo "=== Zod Plugin Test Content ==="
rg -A 10 -B 10 "@@validate" "tests/integration/tests/plugins/zod.test.ts"
# Check validation implementation in zod generator
echo "=== Zod Generator Implementation ==="
rg -A 10 -B 10 "@@validate" "packages/schema/src/plugins/zod/generator.ts"
# Look for validation-related functions in stdlib
echo "=== Stdlib Validation Functions ==="
cat "packages/schema/src/res/stdlib.zmodel"
Length of output: 44698
packages/sdk/src/utils.ts (3)
31-31: LGTM!
The addition of isTypeDefField import aligns with the PR objective of supporting type declarations.
Line range hint 529-536: LGTM! Well-structured field reference resolution.
The getFieldReference function is well-implemented:
- Handles both direct references and member access expressions
- Supports both DataModelField and TypeDefField
- Clear and concise implementation
522-522: Verify type definition field references.
The modification to use getFieldReference looks good, making the function more generic by supporting both data model and type definition fields.
Let's verify the usage of type definition fields in @@id attributes:
✅ Verification successful
Type definition field references in @@id attributes are not used in the codebase.
Based on the search results, all @@id attributes in the codebase reference model fields, not type definition fields. The modification to support type definition fields through getFieldReference makes the code more generic and future-proof, but currently there are no instances where type definition fields are used in @@id attributes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for @@id attributes referencing type definition fields
# Expected: Find any instances where @@id attributes reference type definition fields
# Search for @@id attributes
rg -U "@@id\s*\(\s*\[\s*[\w\s,]+\s*\]\s*\)" -A 5
# Search for type definitions with fields that might be referenced in @@id
ast-grep --pattern 'type $name {
$$$
$field: $_
$$$
}'
Length of output: 20231
packages/schema/src/plugins/zod/generator.ts (3)
2-11: Imports are appropriately added and utilized
The newly added imports (ExpressionContext, TypeScriptExpressionTransformer, TypeScriptExpressionTransformerError, getAttributeArg, getAttributeArgLiteral) are necessary for the enhanced validation and schema generation logic introduced in this file. They are correctly imported and used throughout the code.
Line range hint 304-340: Base schema generation for type definitions is correctly implemented
The generateTypeDefSchema method has been updated to construct the base schema for type definitions properly. The schema accurately represents the fields of the type definition, ensuring that validation is correctly applied to all fields.
668-688: Refinement function creation is effectively implemented
The createRefineFunction method correctly generates refinement functions for applying @@validate rules to schemas. This enhances modularity by centralizing the refinement logic and improves maintainability by avoiding duplication.
No description provided.