Skip to content

Convert to time.Time before asserting in isNeField - #1619

Open
youdie006 wants to merge 1 commit into
go-playground:masterfrom
youdie006:nefield-convert-timetype
Open

Convert to time.Time before asserting in isNeField#1619
youdie006 wants to merge 1 commit into
go-playground:masterfrom
youdie006:nefield-convert-timetype

Conversation

@youdie006

Copy link
Copy Markdown

nefield panics on a named type whose underlying type is time.Time.

type MyTime time.Time

type Test struct {
	Start MyTime
	End   MyTime `validate:"nefield=Start"`
}
validate.Struct(Test{Start: now, End: now})
// panic: interface conversion: interface {} is validator.MyTime, not time.Time

isNeField's struct branch (baked_in.go:1072) guards on ConvertibleTo(timeType), which admits
any named type with a time.Time underlying — then asserts without converting:

if fieldType.ConvertibleTo(timeType) && currentField.Type().ConvertibleTo(timeType) {
	t := getValue(currentField).(time.Time)
	fieldTime := getValue(field).(time.Time)

getValue returns val.Interface() (validator.go:516), so the dynamic type is still MyTime
and the assertion fails.

It is the only one

Of the 28 getValue(...).(time.Time) sites in baked_in.go, 26 convert first. The two that do
not are this pair. isEqField:1402, isGteField:2326, isGtField:2370, isLteField:2554,
isLtField:2598 and isNeCrossStructField:1308 — the same not-equal semantics in the sibling
family — all do:

t := getValue(currentField.Convert(timeType)).(time.Time)

Same input through four of them, before the change:

eqfield   -> ok
ltfield   -> ok
necsfield -> ok
nefield   -> panic: interface conversion: interface {} is validator.MyTime, not time.Time

Change

Two lines, adding the Convert(timeType) the siblings already have. Convert is a no-op for a
plain time.Time, so nothing on the currently covered paths changes, and the v10 API is untouched.

Test added next to the other nefield tests. Verified red/green by reverting only baked_in.go
and keeping the test — it panics with interface conversion: interface {} is validator.namedTime.
It also pins the semantics rather than just the absence of a panic: equal named-time values must
fail the tag, different ones must pass.

go test -cover -race ./... passes across every package. gofmt -l reports doc.go and
translations/ko/ko_test.go both before and after, so this branch does not change it.

validator_test.go:5317-5414 covers nefield against time.Time, *time.Time and
time.Duration, which is why this shape was never exercised.


Disclosure: prepared with AI assistance; I verified the reproduction, the red/green runs and the
race suite myself.

The struct branch admits any type convertible to time.Time, but isNeField
asserted getValue(...) directly instead of converting first, so a named
type with a time.Time underlying panicked. Every other comparison
validator converts.
@youdie006
youdie006 requested a review from a team as a code owner August 30, 2026 23:43
@youdie006

Copy link
Copy Markdown
Author

The lint job is red on master too — 51 exhaustruct_v5 findings in benchmarks_test.go and
validator_test.go, none of them on the lines this PR adds (my test is validator_test.go
1943-1962; the flagged lines are 356…903 and 2207…10033). All six test jobs pass on Go 1.25 and
1.26 across Linux, macOS and Windows.

@nodivbyzero

Copy link
Copy Markdown
Contributor

The lint job is red on master too — 51 exhaustruct_v5 findings in benchmarks_test.go and validator_test.go, none of them on the lines this PR adds (my test is validator_test.go 1943-1962; the flagged lines are 356…903 and 2207…10033). All six test jobs pass on Go 1.25 and 1.26 across Linux, macOS and Windows.

I opened a PR to fix the linter issues: #1618

@nodivbyzero

Copy link
Copy Markdown
Contributor

Thank you for your contribution!
Please rebase your PR and I'll merge your changes once that's done.

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.

2 participants