-
Notifications
You must be signed in to change notification settings - Fork 27
feat: like primitive types #172
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
Conversation
Caution Review failedThe pull request is closed. WalkthroughThis update introduces new generic schema constructors— Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SchemaFactory
participant Schema
User->>SchemaFactory: Call BoolLike/IntLike/FloatLike/UintLike/StringLike[T](opts)
SchemaFactory->>Schema: Create Schema[T] with custom coercer and apply options
Schema-->>User: Return configured Schema[T]
User->>Schema: Parse/Validate input with Schema[T]
Schema-->>User: Result or error (with custom type T)
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ 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 (
|
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
boolean.go
(1 hunks)booleanLike_test.go
(1 hunks)floatLike_test.go
(1 hunks)intLike_test.go
(1 hunks)numbers.go
(4 hunks)string.go
(1 hunks)stringLike_test.go
(1 hunks)uintLike_test.go
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
boolean.go (2)
utilsOptions.go (1)
SchemaOption
(64-64)conf/Coercers.go (1)
Coercers
(260-260)
string.go (2)
utilsOptions.go (1)
SchemaOption
(64-64)conf/Coercers.go (1)
Coercers
(260-260)
numbers.go (2)
utilsOptions.go (1)
SchemaOption
(64-64)conf/Coercers.go (1)
Coercers
(260-260)
intLike_test.go (7)
numbers.go (2)
IntLike
(106-120)NumberSchema
(16-23)tutils/testIssueMessages.go (1)
VerifyDefaultIssueMessages
(14-32)utilsOptions.go (2)
WithCoercer
(66-70)Message
(13-19)internals/contexts.go (1)
ExecCtx
(53-57)internals/tests.go (6)
Required
(114-121)GT
(236-247)LT
(223-234)GTE
(210-221)LTE
(197-208)EQ
(184-195)zogSchema.go (1)
TestFunc
(47-49)zconst/consts.go (1)
TypeNumber
(42-42)
🔇 Additional comments (12)
numbers.go (4)
10-12
: Good type constraint refinement!The change from
constraints.Ordered
toconstraints.Integer | constraints.Float
is more precise and appropriate for numeric types, as it excludes strings that were previously allowed byOrdered
.
44-45
: Clear deprecation notice added.The deprecation comment clearly indicates the preferred alternative while maintaining backward compatibility.
50-64
: Verify precision implications for integer types.While the implementation is correct, using
FloatLike
with integer types could lead to precision loss for large values due to float64's limited mantissa (53 bits). For example,FloatLike[int64]
may not accurately represent all int64 values.Consider documenting this limitation or restricting the constraint to floating-point types only.
106-120
: LGTM! Consider documenting type conversion behavior.The implementation is correct. Note that when used with float types, decimal parts will be truncated, and overflow may occur when casting to smaller integer types.
floatLike_test.go (1)
1-532
: Excellent comprehensive test coverage!The test file thoroughly covers all aspects of the
FloatLike
function including parsing, validation, schema options, transformations, and edge cases. The table-driven test approach is well-structured and easy to maintain.uintLike_test.go (1)
1-530
: Thorough test coverage with good edge case handling!The test file comprehensively covers the
UintLike
function. Particularly good to see the negative value test case (line 28) which verifies proper error handling for invalid inputs.intLike_test.go (1)
1-530
: Comprehensive test suite with proper signed integer handling!The test file thoroughly covers the
IntLike
function including proper handling of negative values for signed integers. The test structure is consistent with other test files and provides excellent coverage.boolean.go (1)
44-58
: Clean implementation of BoolLike function!The generic
BoolLike
function follows the established pattern and correctly handles boolean type aliases. The constraintT ~bool
is appropriate and the coercion logic is straightforward.string.go (1)
65-79
: LGTM! Well-implemented generic function for string-like types.The
StringLike
function correctly implements the generic pattern for custom string types. The coercer chain (string coercion followed by type casting) is appropriate, and the implementation follows the established patterns in the codebase.booleanLike_test.go (1)
1-879
: Excellent comprehensive test coverage for BoolLike functionality.This test file provides thorough coverage of the
BoolLike
function with well-structured table-driven tests. The tests cover all major scenarios including parsing, validation, schema options, transformations, and error handling. The use of a customCustomBool
type effectively validates the generic functionality.stringLike_test.go (2)
1-647
: Excellent comprehensive test coverage for StringLike functionality.This test file provides thorough coverage of the
StringLike
function with well-structured tests covering all string-specific validations. The test patterns are consistent and comprehensive.
346-350
: UUID Test Case Is Valid Under Current RegexThe
uuidRegex
used byis.UUIDv4
matches any 8-4-4-4-12 hex pattern without enforcing the “4” version nibble. Therefore,"123e4567-e89b-12d3-a456-426614174000"
correctly passes the existing validation and the test behaves as expected. No changes are required.
func UintLike[T Numeric](opts ...SchemaOption) *NumberSchema[T] { | ||
s := &NumberSchema[T]{ | ||
coercer: func(data any) (any, error) { | ||
x, err := conf.Coercers.Uint(data) | ||
if err != nil { | ||
return nil, err | ||
} | ||
return T(x.(uint)), nil | ||
}, | ||
} | ||
for _, opt := range opts { | ||
opt(s) | ||
} | ||
return s | ||
} |
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.
Potential overflow issue with signed types.
The Numeric
constraint includes signed types, but coercing through uint
can cause unexpected behavior. For example, UintLike[int8]
with input 200 would overflow to -56.
Consider either:
- Creating a separate constraint for unsigned types only
- Adding overflow validation before the cast
- Documenting this behavior clearly
Example of the issue:
// This would result in -56 due to overflow
schema := UintLike[int8]()
var result int8
schema.Parse(200, &result)
🤖 Prompt for AI Agents
In numbers.go around lines 171 to 185, the UintLike function uses a Numeric
constraint that includes signed types, but coercion is done through uint which
can cause overflow issues when casting back to signed types. To fix this, either
create a separate constraint that only allows unsigned types for UintLike, or
add explicit overflow checks before casting to T to prevent invalid values, or
clearly document this behavior so users are aware of the potential overflow risk
when using signed types.
Deploying zog with
|
Latest commit: |
5ccbae0
|
Status: | ✅ Deploy successful! |
Preview URL: | https://44bfbf1f.zog-3a0.pages.dev |
Branch Preview URL: | https://feat-like-primitive-types.zog-3a0.pages.dev |
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: 1
🧹 Nitpick comments (2)
docs/docs/custom-schemas.md (2)
19-19
: Avoid weak intensifiers; tighten the wordingThe adverb “quite” adds little value here. Consider a crisper alternative.
-This is quite simple to do for the supported primitive types (string, number, boolean). +This is straightforward for the supported primitive types (string, number, boolean).
48-48
: Minor lint issue: hard tabs flagged by markdownlintTabs inside fenced code blocks still trigger
MD010
. Replace with spaces or disable the rule locally to keep CI green.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/docs/custom-schemas.md
(3 hunks)docs/docs/reference.md
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/docs/reference.md
🧰 Additional context used
🪛 LanguageTool
docs/docs/custom-schemas.md
[style] ~19-~19: As an alternative to the over-used intensifier ‘quite’, consider replacing this phrase.
Context: ...om Schemas for Primitive Types This is quite simple to do for the supported primitive types...
(EN_WEAK_ADJECTIVE)
🪛 markdownlint-cli2 (0.17.2)
docs/docs/custom-schemas.md
32-32: Hard tabs
Column: 1
(MD010, no-hard-tabs)
33-33: Hard tabs
Column: 1
(MD010, no-hard-tabs)
37-37: Hard tabs
Column: 1
(MD010, no-hard-tabs)
48-48: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🔇 Additional comments (1)
docs/docs/custom-schemas.md (1)
37-37
: Looks good – matches new API
z.StringLike[Env]()
demonstrates the intended usage of the new generic helper and reads well in the example.
closes #163
Summary by CodeRabbit
New Features
Tests
Documentation