fix(validation): preserve wildcard slice shape, nil traversal, and message priority#1417
Conversation
|
Context for reviewers: the core behavioral change is |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1417 +/- ##
==========================================
+ Coverage 67.82% 67.98% +0.16%
==========================================
Files 354 354
Lines 27345 27521 +176
==========================================
+ Hits 18546 18710 +164
- Misses 7952 7963 +11
- Partials 847 848 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
📝 WalkthroughWalkthroughThe changes enhance validated data handling to preserve original data types when expanding wildcard rules. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@validation/utils.go`:
- Around line 429-438: collectKeys currently calls reflect.ValueOf(v) without
guarding for nil, which panics for nullable leaves (e.g., map[string]any{"name":
nil} or []any{nil}); fix by checking for v == nil (or
reflect.ValueOf(v).IsValid() / rv.IsNil() when applicable) before using
rv.Kind() in collectKeys so nil leaves are treated as terminal values and not
recursed into, and add regression tests exercising DataBag.Keys()/collectKeys
with map[string]any{"name": nil} and []any{nil} to ensure no panic.
- Around line 117-123: dotGet currently calls reflect.ValueOf(data).Kind()
without guarding for nil/invalid values which can panic when an intermediate is
nil; update dotGet to first check for nil/invalid reflect.Value (e.g. rv :=
reflect.ValueOf(data); if !rv.IsValid() { return nil, false }) and also handle
nil interfaces/pointers by checking rv.IsNil() for kinds Interface/Ptr and
returning (nil, false) before calling Kind(), optionally unwrapping rv =
rv.Elem() for Interface/Ptr so subsequent logic (the slice/array branch) is
safe; refer to dotGet and the rv variable in utils.go when applying this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 89f86b12-ee26-4e8c-95a4-4067153e52f6
📒 Files selected for processing (6)
validation/engine.govalidation/engine_test.govalidation/rules_test.govalidation/utils.govalidation/utils_test.govalidation/validation_test.go
Summary
Validated()for wildcard fields now returns the original slice shape ([]any,[]int, etc.) instead of a string-keyed map, converting back to the typed source slice when safe.Message()defaults;integer,ucFirst, andlcFirstfilter aliases are added.Relate goravel/example#112
Why
Before this fix, validating any slice field with a wildcard rule (e.g.
scores.*) causedValidated()to returnmap[string]any{"0": 1, "1": 2}instead of the original slice. Traversal through nil values or typed slices like[]intalso panicked during wildcard expansion.Custom rule
Message()previously overrode any user-supplied message unconditionally, making per-field or per-rule message overrides impossible viaWithMessages. The priority order is now: explicitfield.rulemessage → explicitrulemessage → custom ruleMessage()default.