-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix handling for nested generic types #12
Conversation
* Fix handling for nested generic types. For non-defined generic types, the type arguments are appended to the generated schema names * Only keep necessary types/methods/functions as exported * Update README * Remove name and generic types arguments from CustomFn type. Their use was unclear and if required, can be derived from the type argument
Caution Review failedThe pull request is closed. WalkthroughThe updates significantly enhance the Zen library's ability to manage complex and generic data structures. Key improvements include robust support for self-referential types and refined APIs for custom type handling. With clearer documentation and streamlined interfaces, developers can efficiently convert various structures into Zod schemas, effectively boosting usability and overall clarity. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Converter
participant ZenLibrary
User->>Converter: AddType(UpdateMultiTCasesResponse)
Converter->>ZenLibrary: Validate Types
ZenLibrary-->>Converter: Types Validated
Converter->>User: Export Schema
User->>User: Write to File
Assessment against linked issues
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (4)
README.md (2)
7-7
: Correct the spelling.The word "self-referential" should be hyphenated.
- Zen supports self referential types and generic types. + Zen supports self-referential types and generic types.Tools
LanguageTool
[misspelling] ~7-~7: This word is normally spelled with a hyphen.
Context: ...c types. Other cyclic types (apart from self referential types) are not supported. ## Usage ``...(EN_COMPOUNDS_SELF_REFERENTIAL)
13-13
: Replace hard tabs with spaces.Hard tabs should be replaced with spaces for consistency.
- type User struct { - Name string `validate:"required"` - Nickname *string // pointers become optional - Age int `validate:"min=18"` - Height float64 `validate:"min=0,max=3"` - Tags []string `validate:"min=1"` - Favourites []struct { // nested structs are kept inline - Name string `validate:"required"` - } - Posts []Post // external structs are emitted as separate exports -} +type User struct { + Name string `validate:"required"` + Nickname *string // pointers become optional + Age int `validate:"min=18"` + Height float64 `validate:"min=0,max=3"` + Tags []string `validate:"min=1"` + Favourites []struct { // nested structs are kept inline + Name string `validate:"required"` + } + Posts []Post // external structs are emitted as separate exports +}Also applies to: 16-24, 30-31, 40-41, 52-52, 82-83, 86-87, 91-92, 97-98, 103-103, 119-120, 122-122, 131-131
Tools
Markdownlint
13-13: Column: 1
Hard tabs(MD010, no-hard-tabs)
zod.go (2)
12-14
: Improve the comment for clarity.The comment could be more concise and clear about the custom handler function map.
- // Initializes and returns a new converter instance. The custom handler function - // map should be keyed on the fully qualified type name (excluding generic type - // arguments), ie. package.typename. + // Initializes and returns a new converter instance. The custom handler function map + // should be keyed by the fully qualified type name (excluding generic type arguments), + // e.g., package.typename.
29-30
: Improve the panic message.The panic message could be more descriptive to help identify the issue.
- panic("input must be a struct") + panic("AddType: input must be a struct type")
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- README.md (4 hunks)
- custom/decimal/decimal.go (1 hunks)
- custom/optional/optional.go (1 hunks)
- zod.go (23 hunks)
- zod_test.go (2 hunks)
Additional context used
LanguageTool
README.md
[misspelling] ~7-~7: This word is normally spelled with a hyphen.
Context: ...c types. Other cyclic types (apart from self referential types) are not supported. ## Usage ``...(EN_COMPOUNDS_SELF_REFERENTIAL)
Markdownlint
README.md
13-13: Column: 1
Hard tabs(MD010, no-hard-tabs)
16-16: Column: 1
Hard tabs(MD010, no-hard-tabs)
17-17: Column: 1
Hard tabs(MD010, no-hard-tabs)
18-18: Column: 1
Hard tabs(MD010, no-hard-tabs)
19-19: Column: 1
Hard tabs(MD010, no-hard-tabs)
20-20: Column: 1
Hard tabs(MD010, no-hard-tabs)
21-21: Column: 1
Hard tabs(MD010, no-hard-tabs)
22-22: Column: 1
Hard tabs(MD010, no-hard-tabs)
23-23: Column: 1
Hard tabs(MD010, no-hard-tabs)
24-24: Column: 1
Hard tabs(MD010, no-hard-tabs)
30-30: Column: 1
Hard tabs(MD010, no-hard-tabs)
31-31: Column: 1
Hard tabs(MD010, no-hard-tabs)
40-40: Column: 1
Hard tabs(MD010, no-hard-tabs)
41-41: Column: 1
Hard tabs(MD010, no-hard-tabs)
52-52: Column: 1
Hard tabs(MD010, no-hard-tabs)
82-82: Column: 1
Hard tabs(MD010, no-hard-tabs)
83-83: Column: 1
Hard tabs(MD010, no-hard-tabs)
86-86: Column: 1
Hard tabs(MD010, no-hard-tabs)
87-87: Column: 1
Hard tabs(MD010, no-hard-tabs)
91-91: Column: 1
Hard tabs(MD010, no-hard-tabs)
92-92: Column: 1
Hard tabs(MD010, no-hard-tabs)
97-97: Column: 1
Hard tabs(MD010, no-hard-tabs)
98-98: Column: 1
Hard tabs(MD010, no-hard-tabs)
103-103: Column: 1
Hard tabs(MD010, no-hard-tabs)
119-119: Column: 1
Hard tabs(MD010, no-hard-tabs)
120-120: Column: 1
Hard tabs(MD010, no-hard-tabs)
122-122: Column: 1
Hard tabs(MD010, no-hard-tabs)
131-131: Column: 1
Hard tabs(MD010, no-hard-tabs)
Additional comments not posted (16)
custom/decimal/decimal.go (1)
11-13
: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
DecimalFunc
match the new signature.Verification successful
Verification successful. All function calls to
DecimalFunc
match the new signature.
DecimalFunc
is defined incustom/decimal/decimal.go
.- It is used in a test setup in
custom/decimal/decimal_test.go
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `DecimalFunc` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go -A 5 $'DecimalFunc'Length of output: 627
Script:
#!/bin/bash # Description: Search for direct calls to `DecimalFunc` in the entire codebase to ensure all usages match the new signature. # Test: Search for any direct function calls to `DecimalFunc`. rg --type go '\bDecimalFunc\b'Length of output: 232
custom/optional/optional.go (1)
12-14
: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
OptionalFunc
match the new signature.Verification successful
Function usage verified.
The search did not find any direct calls to
OptionalFunc
, and the only reference found in the test file is correctly integrated with thezen.NewConverter
call.
- No issues found with the function usage.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `OptionalFunc` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go -A 5 $'OptionalFunc'Length of output: 665
Script:
#!/bin/bash # Extract more context around the usage of OptionalFunc in the test file to verify the function calls match the new signature. rg --type go -A 10 -B 10 'OptionalFunc' custom/optional/optional_test.goLength of output: 563
Script:
#!/bin/bash # Search for any direct calls to OptionalFunc throughout the entire codebase. rg --type go 'OptionalFunc('Length of output: 107
Script:
#!/bin/bash # Search for any direct calls to OptionalFunc throughout the entire codebase with corrected regex pattern. rg --type go 'OptionalFunc\('Length of output: 31
README.md (1)
156-156
: LGTM!The function signature for custom type handlers has been updated correctly.
zod.go (8)
44-47
: LGTM!The method implementation is correct and follows the expected logic.
51-54
: LGTM!The method implementation is correct and follows the expected logic.
60-63
: LGTM!The function implementation is correct and follows the expected logic.
70-73
: LGTM!The function implementation is correct and follows the expected logic.
113-113
: LGTM!The type definition is correct and the simplification improves clarity.
115-117
: LGTM!The type definition is correct and the changes improve encapsulation.
318-321
: LGTM!The method implementation is correct and the removal of the
name
parameter simplifies the signature.
390-393
: LGTM!The method implementation is correct and the removal of the
name
parameter simplifies the signature.zod_test.go (5)
1979-1981
: LGTM!The modification simplifies the function without affecting its functionality.
2075-2078
: LGTM!The type definition is correct and enhances the type system's expressiveness.
2080-2080
: LGTM!The type definition is correct and provides a useful specialization of
GenericPair
.
2082-2084
: LGTM!The type definition is correct and enhances the type system's expressiveness.
2086-2109
: LGTM!The function implementation is correct and ensures the proper integration of generics into the type system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR and minor improvements of the code base (documentation, visibility scope). I haven't noticed anything wrong.
|
||
typeArgs := strings.Split(name[typeArgsIdx+1:len(name)-1], ",") | ||
for _, arg := range typeArgs { | ||
sb.WriteString(strings.ToTitle(arg[:1])) // Capitalize first letter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(no need to do anything, just thoughts): I have checked ToTitle() and ToUpper() difference, probably for ASCII text it does not matter; also because this function gets type names from go, so it probably can't have two commas in a row ",," resulting empty arg
obviously we need to fix GitHub Actions, probably updating actions for newer go? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/ci.yml (1 hunks)
Additional comments not posted (3)
.github/workflows/ci.yml (3)
42-42
: Verify the appropriateness of--verbose
and--timeout=3m
arguments.Ensure that these arguments are suitable for the workflow and do not negatively impact the linting process.
39-39
: Verify compatibility withgolangci-lint-action@v6
.Ensure that the new version does not introduce breaking changes and is compatible with the existing workflow.
41-41
: Verify compatibility withgolangci-lint v1.59
.Ensure that the new version does not introduce breaking changes and is compatible with the existing workflow.
Verification successful
Let's correct the
jq
command to properly filter the JSON response and verify the compatibility ofgolangci-lint v1.59
.
Let's list all available tags from the fetched data to confirm the existence of
v1.59
and identify the latest version if necessary.
Let's fetch the changelog for
v1.59.0
to verify if there are any breaking changes.
The update to
golangci-lint v1.59.0
is compatible with the existing workflow.The changelog for
v1.59.0
does not indicate any breaking changes that would affect the workflow.
- No breaking changes were found in the changelog for
v1.59.0
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify compatibility of `golangci-lint v1.59`. # Test: Check the changelog for breaking changes. Expect: No breaking changes that affect the workflow. curl -s https://api.github.com/repos/golangci/golangci-lint/releases | jq -r '.[0:5] | select(.tag_name == "v1.59") | .body'Length of output: 198
Script:
#!/bin/bash # Description: Verify compatibility of `golangci-lint v1.59`. # Test: Check the changelog for breaking changes. Expect: No breaking changes that affect the workflow. curl -s https://api.github.com/repos/golangci/golangci-lint/releases | jq -r '.[] | select(.tag_name == "v1.59") | .body'Length of output: 123
Script:
#!/bin/bash # Description: List all available tags from the golangci-lint releases. # Fetch the release data and list all tags. curl -s https://api.github.com/repos/golangci/golangci-lint/releases | jq -r '.[].tag_name'Length of output: 333
Script:
#!/bin/bash # Description: Verify compatibility of `golangci-lint v1.59.0`. # Test: Check the changelog for breaking changes. Expect: No breaking changes that affect the workflow. curl -s https://api.github.com/repos/golangci/golangci-lint/releases | jq -r '.[] | select(.tag_name == "v1.59.0") | .body'Length of output: 1318
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @hi-rai. Some small recommendations in the comments.
This fixes https://github.com/Hypersequent/tms-issues/issues/565
Summary by CodeRabbit
New Features
StructToZodSchema
for various data types.Converter
class for converting multiple types, improving functionality for complex scenarios.GenericPair
andPairMap
to enhance type safety and flexibility.Bug Fixes
Documentation
Tests
Chores