Skip to content

fix(validators): handle pointer fields in RequiredIf (#334) - #335

Merged
inhere merged 1 commit into
gookit:masterfrom
SAY-5:fix/required-if-pointer-334
May 21, 2026
Merged

inhere merged 1 commit into
gookit:masterfrom
SAY-5:fix/required-if-pointer-334

Conversation

@SAY-5

@SAY-5 SAY-5 commented May 21, 2026

Copy link
Copy Markdown
Contributor

Fixes #334.

Validation.RequiredIf silently skipped its check whenever either side of the rule was a pointer:

  • Destination field is a pointer. When the rule references a *bool/*string field, reflect.ValueOf(dstVal).Kind() is reflect.Pointer. convTypeByBaseKind has no string-to-Pointer conversion, so dstVal == wantVal was never evaluated and the function fell through to the default as True, skip check branch.
  • Source field is a pointer to a zero value. val != nil && !IsEmpty(val) is true for *string("") (the pointer is non-nil), so the must be present and not empty contract was not enforced.

Unwrap pointers on both sides:

  • A nil pointer on the destination side counts as the field being absent: the rule does not trigger and the dependent field stays optional.
  • A non-nil pointer on the destination side is compared against the rule argument by its underlying kind, matching how a non-pointer field of the same type is handled today.
  • A new requiredIfValIsPresent helper applies the same nil-and-zero-via-pointer logic to the source field, so *string("") is treated the same as "".

The reproducer from the issue:

type Person struct {
    IsOld      *bool
    Experience *string `validate:"requiredIf:IsOld,true"`
}

Before this PR, both {IsOld: &true, Experience: &""} and {IsOld: &true} reported IsSuccess() == true. With this PR they correctly report false, while {IsOld: &false} and {IsOld: nil} still pass and {IsOld: &true, Experience: &"10y"} still passes.

Tests

  • TestIssues_334 in issues_test.go adds five subtests covering each pointer scenario in the issue. Two of them (pointer to empty string is not present, nil pointer is not present) fail on the old code with Result should be False and pass on the new code; the three previously-passing positive cases continue to pass.
  • go test ./... -count=1 -race passes locally for the main package and all three locale sub-packages.
  • go vet ./... and gofmt -l validators.go issues_test.go are clean.

RequiredIf silently skipped its check whenever either side of the rule
was a pointer:

  - When the destination field is a pointer (e.g. *bool), reflect.ValueOf
    returns a value of Kind Pointer. convTypeByBaseKind has no
    string-to-Pointer conversion, so dstVal == wantVal was never
    evaluated and the function fell through to default-True / skip.

  - When the source field is a pointer to a zero value (e.g.
    *string("")), val != nil && !IsEmpty(val) was true, so the
    rule's 'must be present and not empty' contract was not enforced.

Unwrap pointers on both sides: treat a nil pointer as the field being
absent (rule does not trigger, dependent field remains optional), and
treat a pointer to a zero value as the zero value itself for the
not-empty check. Adds five subtests covering each pointer scenario;
two of them fail on the old code and pass on the new code.

Closes gookit#334
@inhere
inhere merged commit ad5894a into gookit:master May 21, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RequiredIfValidation does not support pointers

2 participants