Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive input validation to ElectroDB entity attributes based on TypeSpec constraints. The emitter now generates validation functions at build time that enforce constraints like string length, integer types, datetime formats, and enum values.
Key changes:
- Introduces validation function generation based on TypeSpec decorators (@minlength, @maxlength, @minValue, @MaxValue, @pattern, etc.)
- Adds type-specific validators for integers, floats, datetime types, and enums
- Updates test suite to verify validation functions are generated correctly and enforce constraints as expected
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/emitter.ts | Implements validation constraint extraction and function generation with helper functions for type detection (integer, float, datetime), constraint gathering from property and scalar hierarchies, and validation function building via eval |
| test/entities.test.js | Updates existing entity tests to check for validation function presence and adds comprehensive validation test suites covering string length, integers, datetime, and enum validations with both positive and negative test cases |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (constraints.pattern) { | ||
| const escapedPattern = constraints.pattern.replace(/\\/g, "\\\\"); | ||
| checks.push( | ||
| `if (typeof value === "string" && !new RegExp("${escapedPattern}").test(value)) throw new Error("Value must match pattern ${escapedPattern}")`, | ||
| ); |
There was a problem hiding this comment.
The pattern escaping is insufficient and could lead to code injection. Only backslashes are escaped, but quotes and other special characters that could break out of the string literal are not handled. Consider using JSON.stringify to properly escape the pattern for use in a string literal, or use template literal escaping. For example, if a pattern contains a double quote, it would break the generated code.
| if (constraints.enumValues && constraints.enumValues.length > 0) { | ||
| const allowedValues = JSON.stringify(constraints.enumValues); | ||
| checks.push( | ||
| `if (!${allowedValues}.includes(value)) throw new Error("Value must be one of: ${constraints.enumValues.join(", ")}")`, | ||
| ); |
There was a problem hiding this comment.
The enum values in the error message are not escaped and could contain special characters that break the generated code or produce confusing error messages. While JSON.stringify is used for the array check, the error message uses join which doesn't escape special characters. Consider escaping the enum values in the error message or using JSON.stringify for the error message portion as well.
| /** | ||
| * Gets the base scalar name for a type (for datetime type identification). | ||
| */ | ||
| function getBaseScalarName(type: Scalar): string { | ||
| const dateTimeTypes = [ | ||
| "utcDateTime", | ||
| "offsetDateTime", | ||
| "plainDate", | ||
| "plainTime", | ||
| ]; | ||
|
|
||
| // Check the type itself first | ||
| if (dateTimeTypes.includes(type.name)) { | ||
| return type.name; | ||
| } | ||
|
|
||
| // Walk up the base scalar chain to find a datetime type | ||
| let baseType = type.baseScalar; | ||
| while (baseType) { | ||
| if (dateTimeTypes.includes(baseType.name)) { | ||
| return baseType.name; | ||
| } | ||
| baseType = baseType.baseScalar; | ||
| } | ||
|
|
||
| return type.name; | ||
| } |
There was a problem hiding this comment.
The getBaseScalarName function duplicates the dateTimeTypes array definition from isDateTimeType. Consider extracting this array to a module-level constant to avoid duplication and ensure consistency if the list needs to be updated in the future.
|
Maaan didn't know I am a typespec generator |
Lack of input validation can creep up on us - as typescript types won't easily allow string length or number type validation.