From c4109f61c1e0eeb91e9161cf36021a53351ab399 Mon Sep 17 00:00:00 2001 From: molon <3739161+molon@users.noreply.github.com> Date: Mon, 10 Aug 2026 02:20:35 +0800 Subject: [PATCH 1/4] feat: add a skip_if validation tag for cross-field rules with a sentinel target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 跨字段比较的右手边取「哨兵值」时，`ltefield` 之类的 tag 会拒掉完全合法的配置。 典型是「0 表示不限制」： MaxIdleConns int `validate:"ltefield=MaxOpenConns"` MaxOpenConns int // 0 = unlimited 配 (20, 0) —— 不限制连接数、保留 20 条空闲 —— 会因为 `20 <= 0` 为 false 而校验 失败，服务起不来。而「0 = 不限制」这个语义是消费方代码里 `if conf.X > 0` 定义 的，tag 层看不见。 go-playground/validator 没有内置写法：`omitzero` 跳过的是**当前**字段，不是目标 字段。所以加一个 tag： MaxIdleConns int `validate:"skip_if=MaxOpenConns 0,ltefield=MaxOpenConns"` 实现复用 skip_nested_unless 已有的机制 —— 返回 false 让该字段后续的 tag 短路 （实测确认 validator 在首个失败处停止），再按 tag 名把这个「失败」滤掉。参数是 (字段名, 值) 对，任一对命中即跳过。 过滤逻辑从「只滤 skip_nested_unless」改成滤 skippedTags 这一组，两个 tag 语义 相同：它们的失败意思是「到此为止」，不是「这个值不合法」。 字段名写错时不跳过（requireCheckFieldValue 的 defaultNotFoundValue 传 false）， 免得一个 typo 静默关掉一条规则 —— 有测试钉住。 新增 4 组测试：主场景五种组合、skip_if 自身错误不外泄、多对任一命中、 字段名写错不静默失效。全部通过。 --- validator.go | 63 ++++++++++++++++++++++++++++++++----- validator_test.go | 79 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+), 7 deletions(-) diff --git a/validator.go b/validator.go index 27528a2..285675b 100644 --- a/validator.go +++ b/validator.go @@ -87,7 +87,20 @@ func (w *wrappedValidator) StructCtx(ctx context.Context, v any) error { return w.structCtxFunc(ctx, v) } -const skipNestedUnlessTag = "skip_nested_unless" +const ( + skipNestedUnlessTag = "skip_nested_unless" + skipIfTag = "skip_if" +) + +// skipTagImpls are registered together by ValidatorWithSkipNestedUnless. +var skipTagImpls = map[string]validator.FuncCtx{ + skipNestedUnlessTag: skipNestedUnlessImpl, + skipIfTag: skipIfImpl, +} + +// skippedTags are the tags whose "failures" mean "stop validating this field", +// not "this field is invalid". Their errors are filtered out after StructCtx. +var skippedTags = []string{skipNestedUnlessTag, skipIfTag} // skipNestedUnless is a validation function that conditionally skips nested struct validation // based on field values in the parent struct. It is used with the "skip_nested_unless" tag. @@ -135,6 +148,41 @@ func skipNestedUnlessImpl(_ context.Context, fl validator.FieldLevel) bool { return true } +// skipIfImpl skips the REMAINING validations on a field when another field +// holds a given value. It is used with the "skip_if" tag. +// +// Tags on a field are evaluated left to right and stop at the first failure, so +// returning false here prevents everything after it from running; the resulting +// error is then filtered out (see skippedTags). Put "skip_if" first. +// +// The motivating case is a cross-field comparison whose right-hand side has a +// sentinel value. `ltefield=MaxOpenConns` reads as "at most MaxOpenConns", but +// when MaxOpenConns is 0 meaning UNLIMITED it is not an upper bound at all, and +// the tag rejects a perfectly good config: +// +// MaxIdleConns int `validate:"skip_if=MaxOpenConns 0,ltefield=MaxOpenConns"` +// MaxOpenConns int // 0 = unlimited +// +// Parameters are pairs of (field name, value), same shape as skip_nested_unless. +// Validation is skipped when ANY pair matches — "skip if this OR that". +// +// Panics if the number of parameters is not even. +func skipIfImpl(_ context.Context, fl validator.FieldLevel) bool { + params := parseOneOfParam2(fl.Param()) + if len(params)%2 != 0 { + panic(fmt.Sprintf("Bad param number for skip_if %s", fl.FieldName())) + } + for i := 0; i < len(params); i += 2 { + // Return false to stop the remaining tags on this field; the error is + // filtered out afterwards. A missing field is not a match, so an + // unknown name never silently disables a rule. + if requireCheckFieldValue(fl, params[i], params[i+1], false) { + return false + } + } + return true +} + func skipNestedUnlessWrapper(next ValidatorFunc) ValidatorFunc { return func(ctx context.Context, v any) error { err := next(ctx, v) @@ -144,7 +192,7 @@ func skipNestedUnlessWrapper(next ValidatorFunc) ValidatorFunc { var verr validator.ValidationErrors if errors.As(err, &verr) { filtered := lo.Filter(verr, func(e validator.FieldError, _ int) bool { - return e.Tag() != skipNestedUnlessTag + return !lo.Contains(skippedTags, e.Tag()) }) if len(filtered) == 0 { return nil @@ -160,8 +208,8 @@ func skipNestedUnlessWrapper(next ValidatorFunc) ValidatorFunc { // the values of other fields in the parent struct. // // The wrapper performs two main functions: -// 1. Registers the "skip_nested_unless" validation tag -// 2. Filters out validation errors from skipped nested structs +// 1. Registers the "skip_nested_unless" and "skip_if" validation tags +// 2. Filters out their errors, which mean "stop validating here", not "invalid" // // Parameters: // - validator: The base validator to wrap with skip_nested_unless support @@ -171,9 +219,10 @@ func skipNestedUnlessWrapper(next ValidatorFunc) ValidatorFunc { // // Panics if registration of the skip_nested_unless validation fails func ValidatorWithSkipNestedUnless(validator Validator) Validator { - err := validator.RegisterValidationCtx(skipNestedUnlessTag, skipNestedUnlessImpl) - if err != nil { - panic(fmt.Sprintf("failed to register validation: %v", err)) + for tag, impl := range skipTagImpls { + if err := validator.RegisterValidationCtx(tag, impl); err != nil { + panic(fmt.Sprintf("failed to register validation %q: %v", tag, err)) + } } return &wrappedValidator{ Validator: validator, diff --git a/validator_test.go b/validator_test.go index 9622c64..726b652 100644 --- a/validator_test.go +++ b/validator_test.go @@ -264,3 +264,82 @@ func TestParseOneOfParam2(t *testing.T) { }) } } + +func TestSkipIf(t *testing.T) { + // The motivating shape: MaxOpenConns == 0 means UNLIMITED, so it is not an + // upper bound and `ltefield` must not run against it. + type Pool struct { + MaxIdleConns int `validate:"skip_if=MaxOpenConns 0,ltefield=MaxOpenConns"` + MaxOpenConns int + } + + v := ValidatorWithSkipNestedUnless( + validator.New(validator.WithRequiredStructEnabled()), + ) + + for _, c := range []struct { + name string + idle, open int + wantTag string // "" = no error + }{ + {"cap set, idle within it", 20, 200, ""}, + {"cap set, idle equals it", 10, 10, ""}, + {"cap set, idle above it", 11, 10, "ltefield"}, + {"unlimited, idle is not compared", 20, 0, ""}, + {"unlimited, both zero", 0, 0, ""}, + } { + t.Run(c.name, func(t *testing.T) { + err := v.StructCtx(context.Background(), Pool{c.idle, c.open}) + if c.wantTag == "" { + assert.NoError(t, err) + return + } + var verr validator.ValidationErrors + assert.ErrorAs(t, err, &verr) + assert.Len(t, verr, 1) + assert.Equal(t, c.wantTag, verr[0].Tag()) + }) + } +} + +func TestSkipIfDoesNotLeakItsOwnError(t *testing.T) { + // skip_if works by failing, which stops the tags after it. That failure is + // an implementation detail and must never reach the caller. + type S struct { + A int `validate:"skip_if=B 0,gte=100"` + B int + } + v := ValidatorWithSkipNestedUnless(validator.New(validator.WithRequiredStructEnabled())) + + // B == 0 → skipped, so A = 1 does not have to be >= 100. + assert.NoError(t, v.StructCtx(context.Background(), S{A: 1, B: 0})) + + // B != 0 → not skipped, so gte=100 applies and reports itself, not skip_if. + err := v.StructCtx(context.Background(), S{A: 1, B: 7}) + var verr validator.ValidationErrors + assert.ErrorAs(t, err, &verr) + assert.Len(t, verr, 1) + assert.Equal(t, "gte", verr[0].Tag()) +} + +func TestSkipIfMatchesAnyPair(t *testing.T) { + type S struct { + A int `validate:"skip_if=B 0 C 0,gte=100"` + B, C int + } + v := ValidatorWithSkipNestedUnless(validator.New(validator.WithRequiredStructEnabled())) + + assert.NoError(t, v.StructCtx(context.Background(), S{A: 1, B: 0, C: 9}), "B matches") + assert.NoError(t, v.StructCtx(context.Background(), S{A: 1, B: 9, C: 0}), "C matches") + assert.Error(t, v.StructCtx(context.Background(), S{A: 1, B: 9, C: 9}), "neither matches") +} + +func TestSkipIfUnknownFieldDoesNotDisableTheRule(t *testing.T) { + // A typo in the field name must not silently switch validation off. + type S struct { + A int `validate:"skip_if=Nope 0,gte=100"` + B int + } + v := ValidatorWithSkipNestedUnless(validator.New(validator.WithRequiredStructEnabled())) + assert.Error(t, v.StructCtx(context.Background(), S{A: 1, B: 0})) +} From b957de8e8270e0bcca1063f9d3782732dbde19a8 Mon Sep 17 00:00:00 2001 From: molon <3739161+molon@users.noreply.github.com> Date: Mon, 10 Aug 2026 02:37:54 +0800 Subject: [PATCH 2/4] rename skip_if to skip_rest_if, and pin down why the built-in skip_unless is not it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 改名两个原因。 ## 1. 名字要说清它到底做什么 它跳过的是**本字段剩余的校验**（返回 false 让后续 tag 短路），不是跳过某个字段、 也不是跳过嵌套结构。skip_rest_if 直说这件事。 ## 2. 躲开上游的 skip_* 命名空间 validator 内置有个 skip_unless，而 RegisterValidationCtx 同名注册会**静默替换** 内置实现并返回 nil —— 撞名的后果是所有消费方的行为在毫无提示的情况下改变。 这正是上面那个 tag 叫 skip_nested_unless 而不是 skip_unless 的原因，新 tag 也 照此避让。查了 v10.14 ~ v10.30 五个版本，上游 skip_* 家族只有 skip_unless 一个， skip_rest_if 全程无冲突。 ## 顺带钉住一件容易搞混的事 内置 skip_unless **名不副实：它根本不跳过任何东西**。实现是 return hasValue(fl)， 即一个存在性检查，属于 required_* 家族；它后面的 tag 照跑。所以它不能拿来做这件 事，这也正是本 tag 存在的理由。 新增 TestBuiltinSkipUnlessDoesNotActuallySkip 把上游这个行为钉下来 —— 哪天上游 改了，这条会红，届时可以重新评估 skip_rest_if 还有没有必要。 --- validator.go | 31 +++++++++++++++++++++---------- validator_test.go | 39 +++++++++++++++++++++++++++++---------- 2 files changed, 50 insertions(+), 20 deletions(-) diff --git a/validator.go b/validator.go index 285675b..e22458a 100644 --- a/validator.go +++ b/validator.go @@ -89,18 +89,18 @@ func (w *wrappedValidator) StructCtx(ctx context.Context, v any) error { const ( skipNestedUnlessTag = "skip_nested_unless" - skipIfTag = "skip_if" + skipRestIfTag = "skip_rest_if" ) // skipTagImpls are registered together by ValidatorWithSkipNestedUnless. var skipTagImpls = map[string]validator.FuncCtx{ skipNestedUnlessTag: skipNestedUnlessImpl, - skipIfTag: skipIfImpl, + skipRestIfTag: skipRestIfImpl, } // skippedTags are the tags whose "failures" mean "stop validating this field", // not "this field is invalid". Their errors are filtered out after StructCtx. -var skippedTags = []string{skipNestedUnlessTag, skipIfTag} +var skippedTags = []string{skipNestedUnlessTag, skipRestIfTag} // skipNestedUnless is a validation function that conditionally skips nested struct validation // based on field values in the parent struct. It is used with the "skip_nested_unless" tag. @@ -148,29 +148,40 @@ func skipNestedUnlessImpl(_ context.Context, fl validator.FieldLevel) bool { return true } -// skipIfImpl skips the REMAINING validations on a field when another field -// holds a given value. It is used with the "skip_if" tag. +// skipRestIfImpl skips the REMAINING validations on a field when another field +// holds a given value. It is used with the "skip_rest_if" tag. +// +// Not to be confused with validator's built-in "skip_unless", which despite the +// name never skips anything: it returns hasValue(fl), i.e. it is a presence +// check in the required_* family. There is no built-in that stops the tags +// after it, which is why this exists. +// +// The name deliberately stays out of the upstream "skip_*" namespace. +// RegisterValidationCtx silently REPLACES a built-in of the same name and +// returns nil, so a collision would change behaviour for every consumer with +// nothing to announce it — the same reason the tag above is called +// "skip_nested_unless" rather than "skip_unless". // // Tags on a field are evaluated left to right and stop at the first failure, so // returning false here prevents everything after it from running; the resulting -// error is then filtered out (see skippedTags). Put "skip_if" first. +// error is then filtered out (see skippedTags). Put "skip_rest_if" first. // // The motivating case is a cross-field comparison whose right-hand side has a // sentinel value. `ltefield=MaxOpenConns` reads as "at most MaxOpenConns", but // when MaxOpenConns is 0 meaning UNLIMITED it is not an upper bound at all, and // the tag rejects a perfectly good config: // -// MaxIdleConns int `validate:"skip_if=MaxOpenConns 0,ltefield=MaxOpenConns"` +// MaxIdleConns int `validate:"skip_rest_if=MaxOpenConns 0,ltefield=MaxOpenConns"` // MaxOpenConns int // 0 = unlimited // // Parameters are pairs of (field name, value), same shape as skip_nested_unless. // Validation is skipped when ANY pair matches — "skip if this OR that". // // Panics if the number of parameters is not even. -func skipIfImpl(_ context.Context, fl validator.FieldLevel) bool { +func skipRestIfImpl(_ context.Context, fl validator.FieldLevel) bool { params := parseOneOfParam2(fl.Param()) if len(params)%2 != 0 { - panic(fmt.Sprintf("Bad param number for skip_if %s", fl.FieldName())) + panic(fmt.Sprintf("Bad param number for skip_rest_if %s", fl.FieldName())) } for i := 0; i < len(params); i += 2 { // Return false to stop the remaining tags on this field; the error is @@ -208,7 +219,7 @@ func skipNestedUnlessWrapper(next ValidatorFunc) ValidatorFunc { // the values of other fields in the parent struct. // // The wrapper performs two main functions: -// 1. Registers the "skip_nested_unless" and "skip_if" validation tags +// 1. Registers the "skip_nested_unless" and "skip_rest_if" validation tags // 2. Filters out their errors, which mean "stop validating here", not "invalid" // // Parameters: diff --git a/validator_test.go b/validator_test.go index 726b652..4bdc5f1 100644 --- a/validator_test.go +++ b/validator_test.go @@ -265,11 +265,11 @@ func TestParseOneOfParam2(t *testing.T) { } } -func TestSkipIf(t *testing.T) { +func TestSkipRestIf(t *testing.T) { // The motivating shape: MaxOpenConns == 0 means UNLIMITED, so it is not an // upper bound and `ltefield` must not run against it. type Pool struct { - MaxIdleConns int `validate:"skip_if=MaxOpenConns 0,ltefield=MaxOpenConns"` + MaxIdleConns int `validate:"skip_rest_if=MaxOpenConns 0,ltefield=MaxOpenConns"` MaxOpenConns int } @@ -302,11 +302,11 @@ func TestSkipIf(t *testing.T) { } } -func TestSkipIfDoesNotLeakItsOwnError(t *testing.T) { - // skip_if works by failing, which stops the tags after it. That failure is +func TestSkipRestIfDoesNotLeakItsOwnError(t *testing.T) { + // skip_rest_if works by failing, which stops the tags after it. That failure is // an implementation detail and must never reach the caller. type S struct { - A int `validate:"skip_if=B 0,gte=100"` + A int `validate:"skip_rest_if=B 0,gte=100"` B int } v := ValidatorWithSkipNestedUnless(validator.New(validator.WithRequiredStructEnabled())) @@ -314,7 +314,7 @@ func TestSkipIfDoesNotLeakItsOwnError(t *testing.T) { // B == 0 → skipped, so A = 1 does not have to be >= 100. assert.NoError(t, v.StructCtx(context.Background(), S{A: 1, B: 0})) - // B != 0 → not skipped, so gte=100 applies and reports itself, not skip_if. + // B != 0 → not skipped, so gte=100 applies and reports itself, not skip_rest_if. err := v.StructCtx(context.Background(), S{A: 1, B: 7}) var verr validator.ValidationErrors assert.ErrorAs(t, err, &verr) @@ -322,9 +322,9 @@ func TestSkipIfDoesNotLeakItsOwnError(t *testing.T) { assert.Equal(t, "gte", verr[0].Tag()) } -func TestSkipIfMatchesAnyPair(t *testing.T) { +func TestSkipRestIfMatchesAnyPair(t *testing.T) { type S struct { - A int `validate:"skip_if=B 0 C 0,gte=100"` + A int `validate:"skip_rest_if=B 0 C 0,gte=100"` B, C int } v := ValidatorWithSkipNestedUnless(validator.New(validator.WithRequiredStructEnabled())) @@ -334,12 +334,31 @@ func TestSkipIfMatchesAnyPair(t *testing.T) { assert.Error(t, v.StructCtx(context.Background(), S{A: 1, B: 9, C: 9}), "neither matches") } -func TestSkipIfUnknownFieldDoesNotDisableTheRule(t *testing.T) { +func TestSkipRestIfUnknownFieldDoesNotDisableTheRule(t *testing.T) { // A typo in the field name must not silently switch validation off. type S struct { - A int `validate:"skip_if=Nope 0,gte=100"` + A int `validate:"skip_rest_if=Nope 0,gte=100"` B int } v := ValidatorWithSkipNestedUnless(validator.New(validator.WithRequiredStructEnabled())) assert.Error(t, v.StructCtx(context.Background(), S{A: 1, B: 0})) } + +// Guards the reason skip_rest_if exists at all: validator's built-in +// "skip_unless" does not skip anything despite its name. It returns +// hasValue(fl) — a presence check in the required_* family — so the tags after +// it still run. If upstream ever changes that, this test fails and we can +// reconsider whether skip_rest_if is still needed. +func TestBuiltinSkipUnlessDoesNotActuallySkip(t *testing.T) { + type S struct { + A int `validate:"skip_unless=B 0,gte=100"` + B int + } + v := validator.New(validator.WithRequiredStructEnabled()) + + // B == 0 matches the condition. If skip_unless skipped, A = 1 would pass. + err := v.Struct(S{A: 1, B: 0}) + var verr validator.ValidationErrors + assert.ErrorAs(t, err, &verr) + assert.Equal(t, "gte", verr[0].Tag(), "built-in skip_unless is documented as a presence check, not a skip") +} From d8f4ca16f8549fb954fb53f09024770f7a4f0435 Mon Sep 17 00:00:00 2001 From: molon <3739161+molon@users.noreply.github.com> Date: Mon, 10 Aug 2026 02:54:35 +0800 Subject: [PATCH 3/4] rename to a stop_if / stop_unless pair, keep skip_nested_unless as an alias MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 实测发现 skip_nested_unless 与新加的那个 tag **是同一个机制**，之前按 「nested / rest」区分是错的。四种组合交叉验证： 贴在标量字段 贴在嵌套结构 原 tag 跳过后续 tag ✅ 阻止进入 ✅ 新 tag 跳过后续 tag ✅ 阻止进入 ✅ 两者都靠返回 false —— validator 在首个失败的 tag 处放弃该字段，对标量表现为 「后面的 tag 不跑」，对嵌套结构表现为「不往里钻」。这是同一个行为的两种表现。 也就是说 skip_nested_unless 早就具备「skip_rest_unless」的能力，新 tag 也早就 具备「skip_nested_if」的能力。**唯一真实的轴是极性（if / unless）**，nested 和 rest 两个词各自只说对了一半用法。 因此改成一对只差极性的名字： stop_if 任一 (字段, 值) 命中就停 stop_unless 必须全部命中才继续 用 stop 而不是 skip：它同时说得通两种情形（「在这里停止校验」），而 skip 容易被 读成「跳过这个字段」——实际跳过的是从这里往下的一切。也顺带彻底离开上游的 skip_* 命名空间：同名注册会静默替换内置实现并返回 nil，撞名的后果是所有消费方 行为无声改变。上游 183 个内置 tag 里 stop_* 一个都没有。 skip_nested_unless 保留，注册到同一个实现上，标为 Deprecated，既有 struct tag 零改动。 新增测试： · TestStopTagsAreNotScopedToNestedOrScalar —— 四种组合钉住「没有 nested/rest 之分」，这正是改名的依据 · TestSkipNestedUnlessIsAnAliasOfStopUnless —— 别名与新名行为逐一等价， 包括它从没被文档化过的标量用法 --- matrix_test.go | 81 +++++++++++++++++++++++++ validator.go | 147 +++++++++++++++++++++++----------------------- validator_test.go | 24 ++++---- 3 files changed, 166 insertions(+), 86 deletions(-) create mode 100644 matrix_test.go diff --git a/matrix_test.go b/matrix_test.go new file mode 100644 index 0000000..a359b9e --- /dev/null +++ b/matrix_test.go @@ -0,0 +1,81 @@ +package confx + +import ( + "context" + "testing" + + "github.com/go-playground/validator/v10" + "github.com/stretchr/testify/assert" +) + +type matrixInner struct { + Name string `validate:"required,min=6"` +} + +func newStopValidator() Validator { + return ValidatorWithSkipNestedUnless(validator.New(validator.WithRequiredStructEnabled())) +} + +// The tags are NOT scoped to nested structs or to scalar fields — both stop +// validation of whatever they sit on, because validator abandons a field at its +// first failing tag. This is why they are named stop_* rather than skip_nested_* +// or skip_rest_*: either of those words describes only half of what they do. +func TestStopTagsAreNotScopedToNestedOrScalar(t *testing.T) { + v := newStopValidator() + + t.Run("stop_if on a scalar stops the tags after it", func(t *testing.T) { + type S struct { + X int `validate:"stop_if=Y 0,gte=100"` + Y int + } + assert.NoError(t, v.StructCtx(context.Background(), S{X: 1, Y: 0})) + assert.Error(t, v.StructCtx(context.Background(), S{X: 1, Y: 9})) + }) + + t.Run("stop_if on a nested struct prevents descending", func(t *testing.T) { + type S struct { + In matrixInner `validate:"stop_if=Y 0"` + Y int + } + assert.NoError(t, v.StructCtx(context.Background(), S{In: matrixInner{Name: "ab"}, Y: 0})) + assert.Error(t, v.StructCtx(context.Background(), S{In: matrixInner{Name: "ab"}, Y: 9})) + }) + + t.Run("stop_unless on a scalar stops the tags after it", func(t *testing.T) { + type S struct { + X int `validate:"stop_unless=Y 1,gte=100"` + Y int + } + assert.NoError(t, v.StructCtx(context.Background(), S{X: 1, Y: 0})) + assert.Error(t, v.StructCtx(context.Background(), S{X: 1, Y: 1})) + }) + + t.Run("stop_unless on a nested struct prevents descending", func(t *testing.T) { + type S struct { + In matrixInner `validate:"stop_unless=Y 1"` + Y int + } + assert.NoError(t, v.StructCtx(context.Background(), S{In: matrixInner{Name: "ab"}, Y: 0})) + assert.Error(t, v.StructCtx(context.Background(), S{In: matrixInner{Name: "ab"}, Y: 1})) + }) +} + +// skip_nested_unless is a deprecated alias of stop_unless and must behave +// identically, including on scalar fields it was never documented for. +func TestSkipNestedUnlessIsAnAliasOfStopUnless(t *testing.T) { + v := newStopValidator() + + type Old struct { + X int `validate:"skip_nested_unless=Y 1,gte=100"` + Y int + } + type New struct { + X int `validate:"stop_unless=Y 1,gte=100"` + Y int + } + for _, y := range []int{0, 1} { + oldErr := v.StructCtx(context.Background(), Old{X: 1, Y: y}) + newErr := v.StructCtx(context.Background(), New{X: 1, Y: y}) + assert.Equal(t, oldErr == nil, newErr == nil, "Y=%d", y) + } +} diff --git a/validator.go b/validator.go index e22458a..9a3fcd3 100644 --- a/validator.go +++ b/validator.go @@ -88,59 +88,53 @@ func (w *wrappedValidator) StructCtx(ctx context.Context, v any) error { } const ( + // stopIfTag / stopUnlessTag stop validation of a field at that point. + stopIfTag = "stop_if" + stopUnlessTag = "stop_unless" + + // skipNestedUnlessTag is the original name of stop_unless, kept as an alias + // so existing struct tags keep working. + // + // Deprecated: use stop_unless. The "nested" is misleading — the tag is not + // specific to nested structs (see the note on stopUnlessImpl). skipNestedUnlessTag = "skip_nested_unless" - skipRestIfTag = "skip_rest_if" ) -// skipTagImpls are registered together by ValidatorWithSkipNestedUnless. -var skipTagImpls = map[string]validator.FuncCtx{ - skipNestedUnlessTag: skipNestedUnlessImpl, - skipRestIfTag: skipRestIfImpl, +// stopTagImpls are registered together by ValidatorWithSkipNestedUnless. +var stopTagImpls = map[string]validator.FuncCtx{ + stopIfTag: stopIfImpl, + stopUnlessTag: stopUnlessImpl, + skipNestedUnlessTag: stopUnlessImpl, } -// skippedTags are the tags whose "failures" mean "stop validating this field", -// not "this field is invalid". Their errors are filtered out after StructCtx. -var skippedTags = []string{skipNestedUnlessTag, skipRestIfTag} +// stopTags are the tags whose "failure" means "stop validating here", not +// "this field is invalid". Their errors are filtered out after StructCtx. +var stopTags = []string{stopIfTag, stopUnlessTag, skipNestedUnlessTag} -// skipNestedUnless is a validation function that conditionally skips nested struct validation -// based on field values in the parent struct. It is used with the "skip_nested_unless" tag. +// stopUnlessImpl stops validating a field unless every (field, value) pair +// matches. It backs the "stop_unless" tag and its "skip_nested_unless" alias. // -// The function takes pairs of parameters where each pair consists of: -// 1. A field name to check -// 2. The expected value for that field -// -// If any of the specified field values don't match their expected values, the nested validation -// is skipped by returning false. All pairs must match for validation to proceed. -// -// Example usage in struct tags: +// stopIfImpl is the same thing with the opposite polarity. Polarity is the ONLY +// difference between the two; see the note there for what "stop" covers. // // type Config struct { -// Type string `validate:"oneof=local remote"` -// Local LocalConf `validate:"skip_nested_unless=Type local"` -// Remote RemoteConf `validate:"skip_nested_unless=Type remote"` +// Type string `validate:"oneof=local remote"` +// Local LocalConf `validate:"stop_unless=Type local"` +// Remote RemoteConf `validate:"stop_unless=Type remote"` // } // -// In this example: -// - Local config is only validated when Type="local" -// - Remote config is only validated when Type="remote" -// -// Parameters: -// - ctx: Context (unused) -// - fl: FieldLevel object providing access to the struct field being validated +// Local is validated only when Type == "local", Remote only when Type == +// "remote". All pairs must match for validation to proceed. // -// Returns: -// - bool: true if nested validation should proceed, false if it should be skipped -// -// Panics if the number of parameters is not even (must be pairs of field name and expected value) -func skipNestedUnlessImpl(_ context.Context, fl validator.FieldLevel) bool { +// Panics if the number of parameters is not even. +func stopUnlessImpl(_ context.Context, fl validator.FieldLevel) bool { params := parseOneOfParam2(fl.Param()) if len(params)%2 != 0 { - panic(fmt.Sprintf("Bad param number for skip_nested_unless %s", fl.FieldName())) + panic(fmt.Sprintf("Bad param number for %s %s", fl.GetTag(), fl.FieldName())) } for i := 0; i < len(params); i += 2 { - // To skip validation, return false to generate the corresponding error, ensuring the nested struct is not validated. - // The corresponding errors should then be filtered out after the StructCtx method returns. - // Therefore, this should return false when the condition is not met, preventing further validation. + // Returning false is how validation is stopped: it produces an error + // that the wrapper then filters out by tag name (see stopTags). if !requireCheckFieldValue(fl, params[i], params[i+1], false) { return false } @@ -148,45 +142,45 @@ func skipNestedUnlessImpl(_ context.Context, fl validator.FieldLevel) bool { return true } -// skipRestIfImpl skips the REMAINING validations on a field when another field -// holds a given value. It is used with the "skip_rest_if" tag. +// stopIfImpl stops validating a field when ANY (field, value) pair matches. It +// backs the "stop_if" tag. // -// Not to be confused with validator's built-in "skip_unless", which despite the -// name never skips anything: it returns hasValue(fl), i.e. it is a presence -// check in the required_* family. There is no built-in that stops the tags -// after it, which is why this exists. +// "stop" rather than "skip", because what it stops depends on where the tag +// sits, and both are the same underlying behaviour — validator abandons a field +// at its first failing tag: // -// The name deliberately stays out of the upstream "skip_*" namespace. -// RegisterValidationCtx silently REPLACES a built-in of the same name and -// returns nil, so a collision would change behaviour for every consumer with -// nothing to announce it — the same reason the tag above is called -// "skip_nested_unless" rather than "skip_unless". +// - on a scalar field, the tags AFTER it do not run; +// - on a nested struct, validation does not descend into it. // -// Tags on a field are evaluated left to right and stop at the first failure, so -// returning false here prevents everything after it from running; the resulting -// error is then filtered out (see skippedTags). Put "skip_rest_if" first. +// Put it first in the tag list. Parameters are pairs of (field name, value). // -// The motivating case is a cross-field comparison whose right-hand side has a -// sentinel value. `ltefield=MaxOpenConns` reads as "at most MaxOpenConns", but -// when MaxOpenConns is 0 meaning UNLIMITED it is not an upper bound at all, and -// the tag rejects a perfectly good config: +// The motivating case is a cross-field comparison whose right-hand side carries +// a sentinel. `ltefield=MaxOpenConns` reads as "at most MaxOpenConns", but when +// MaxOpenConns is 0 meaning UNLIMITED it is not an upper bound at all, and the +// tag rejects a perfectly good config: // -// MaxIdleConns int `validate:"skip_rest_if=MaxOpenConns 0,ltefield=MaxOpenConns"` +// MaxIdleConns int `validate:"stop_if=MaxOpenConns 0,ltefield=MaxOpenConns"` // MaxOpenConns int // 0 = unlimited // -// Parameters are pairs of (field name, value), same shape as skip_nested_unless. -// Validation is skipped when ANY pair matches — "skip if this OR that". +// Not to be confused with validator's built-in "skip_unless", which despite its +// name never skips anything: it returns hasValue(fl), a presence check in the +// required_* family, and the tags after it still run. No built-in stops +// validation the way these do, which is why they exist. +// +// The names deliberately stay out of the upstream "skip_*" namespace. +// RegisterValidationCtx silently REPLACES a built-in of the same name and +// returns nil, so a collision would change behaviour for every consumer with +// nothing to announce it. // // Panics if the number of parameters is not even. -func skipRestIfImpl(_ context.Context, fl validator.FieldLevel) bool { +func stopIfImpl(_ context.Context, fl validator.FieldLevel) bool { params := parseOneOfParam2(fl.Param()) if len(params)%2 != 0 { - panic(fmt.Sprintf("Bad param number for skip_rest_if %s", fl.FieldName())) + panic(fmt.Sprintf("Bad param number for %s %s", fl.GetTag(), fl.FieldName())) } for i := 0; i < len(params); i += 2 { - // Return false to stop the remaining tags on this field; the error is - // filtered out afterwards. A missing field is not a match, so an - // unknown name never silently disables a rule. + // A missing field is not a match, so a typo'd field name never silently + // disables the rules that follow. if requireCheckFieldValue(fl, params[i], params[i+1], false) { return false } @@ -203,7 +197,7 @@ func skipNestedUnlessWrapper(next ValidatorFunc) ValidatorFunc { var verr validator.ValidationErrors if errors.As(err, &verr) { filtered := lo.Filter(verr, func(e validator.FieldError, _ int) bool { - return !lo.Contains(skippedTags, e.Tag()) + return !lo.Contains(stopTags, e.Tag()) }) if len(filtered) == 0 { return nil @@ -214,23 +208,28 @@ func skipNestedUnlessWrapper(next ValidatorFunc) ValidatorFunc { } } -// ValidatorWithSkipNestedUnless wraps a validator with support for conditional nested struct validation -// using the "skip_nested_unless" tag. This allows you to skip validation of nested structs based on -// the values of other fields in the parent struct. +// ValidatorWithSkipNestedUnless wraps a validator with support for the +// conditional "stop" tags, which halt validation of a field based on the values +// of other fields in the same struct. +// +// The wrapper performs two functions: +// 1. Registers "stop_if", "stop_unless", and "skip_nested_unless" (a +// deprecated alias of stop_unless, kept so existing tags keep working) +// 2. Filters out their errors, which mean "stop validating here", not "this +// value is invalid" // -// The wrapper performs two main functions: -// 1. Registers the "skip_nested_unless" and "skip_rest_if" validation tags -// 2. Filters out their errors, which mean "stop validating here", not "invalid" +// The name is historical — it predates stop_if/stop_unless — and is kept +// because it is part of the public API. // // Parameters: -// - validator: The base validator to wrap with skip_nested_unless support +// - validator: The base validator to wrap // // Returns: -// - Validator: A wrapped validator that supports the skip_nested_unless tag +// - Validator: A wrapped validator supporting the stop tags // -// Panics if registration of the skip_nested_unless validation fails +// Panics if registration of any tag fails func ValidatorWithSkipNestedUnless(validator Validator) Validator { - for tag, impl := range skipTagImpls { + for tag, impl := range stopTagImpls { if err := validator.RegisterValidationCtx(tag, impl); err != nil { panic(fmt.Sprintf("failed to register validation %q: %v", tag, err)) } diff --git a/validator_test.go b/validator_test.go index 4bdc5f1..e5cfcea 100644 --- a/validator_test.go +++ b/validator_test.go @@ -265,11 +265,11 @@ func TestParseOneOfParam2(t *testing.T) { } } -func TestSkipRestIf(t *testing.T) { +func TestStopIf(t *testing.T) { // The motivating shape: MaxOpenConns == 0 means UNLIMITED, so it is not an // upper bound and `ltefield` must not run against it. type Pool struct { - MaxIdleConns int `validate:"skip_rest_if=MaxOpenConns 0,ltefield=MaxOpenConns"` + MaxIdleConns int `validate:"stop_if=MaxOpenConns 0,ltefield=MaxOpenConns"` MaxOpenConns int } @@ -302,11 +302,11 @@ func TestSkipRestIf(t *testing.T) { } } -func TestSkipRestIfDoesNotLeakItsOwnError(t *testing.T) { - // skip_rest_if works by failing, which stops the tags after it. That failure is +func TestStopIfDoesNotLeakItsOwnError(t *testing.T) { + // stop_if works by failing, which stops the tags after it. That failure is // an implementation detail and must never reach the caller. type S struct { - A int `validate:"skip_rest_if=B 0,gte=100"` + A int `validate:"stop_if=B 0,gte=100"` B int } v := ValidatorWithSkipNestedUnless(validator.New(validator.WithRequiredStructEnabled())) @@ -314,7 +314,7 @@ func TestSkipRestIfDoesNotLeakItsOwnError(t *testing.T) { // B == 0 → skipped, so A = 1 does not have to be >= 100. assert.NoError(t, v.StructCtx(context.Background(), S{A: 1, B: 0})) - // B != 0 → not skipped, so gte=100 applies and reports itself, not skip_rest_if. + // B != 0 → not skipped, so gte=100 applies and reports itself, not stop_if. err := v.StructCtx(context.Background(), S{A: 1, B: 7}) var verr validator.ValidationErrors assert.ErrorAs(t, err, &verr) @@ -322,9 +322,9 @@ func TestSkipRestIfDoesNotLeakItsOwnError(t *testing.T) { assert.Equal(t, "gte", verr[0].Tag()) } -func TestSkipRestIfMatchesAnyPair(t *testing.T) { +func TestStopIfMatchesAnyPair(t *testing.T) { type S struct { - A int `validate:"skip_rest_if=B 0 C 0,gte=100"` + A int `validate:"stop_if=B 0 C 0,gte=100"` B, C int } v := ValidatorWithSkipNestedUnless(validator.New(validator.WithRequiredStructEnabled())) @@ -334,21 +334,21 @@ func TestSkipRestIfMatchesAnyPair(t *testing.T) { assert.Error(t, v.StructCtx(context.Background(), S{A: 1, B: 9, C: 9}), "neither matches") } -func TestSkipRestIfUnknownFieldDoesNotDisableTheRule(t *testing.T) { +func TestStopIfUnknownFieldDoesNotDisableTheRule(t *testing.T) { // A typo in the field name must not silently switch validation off. type S struct { - A int `validate:"skip_rest_if=Nope 0,gte=100"` + A int `validate:"stop_if=Nope 0,gte=100"` B int } v := ValidatorWithSkipNestedUnless(validator.New(validator.WithRequiredStructEnabled())) assert.Error(t, v.StructCtx(context.Background(), S{A: 1, B: 0})) } -// Guards the reason skip_rest_if exists at all: validator's built-in +// Guards the reason stop_if / stop_unless exist at all: validator's built-in // "skip_unless" does not skip anything despite its name. It returns // hasValue(fl) — a presence check in the required_* family — so the tags after // it still run. If upstream ever changes that, this test fails and we can -// reconsider whether skip_rest_if is still needed. +// reconsider whether stop_if is still needed. func TestBuiltinSkipUnlessDoesNotActuallySkip(t *testing.T) { type S struct { A int `validate:"skip_unless=B 0,gte=100"` From 853d25a54d11a5b8a296967ab462028264a4d71c Mon Sep 17 00:00:00 2001 From: molon <3739161+molon@users.noreply.github.com> Date: Mon, 10 Aug 2026 03:07:14 +0800 Subject: [PATCH 4/4] fix: make the panic paths say which tag and which field, per review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot 指出 ValidatorWithSkipNestedUnless 的文档串还写着「只为 skip_nested_unless 注册失败而 panic」，而它现在注册三个 tag —— 对着 panic 排查的人会被误导。文档串已在改名那一版顺手修好，但顺着这条查下去还有两处： 1. skipNestedUnlessWrapper 这个内部函数名同样停在旧语义。它现在滤的是全部三个 stop tag，改名 stopTagsWrapper 并补上「它们的失败是实现细节，不该外泄」的 说明。 2. **三条 panic 路径一条测试都没有。** Copilot 担心的正是「诊断 panic 的人被 误导」，那就该验证 panic 信息真的说得清。补 TestStopTagsPanicMessageNamesTagAndField： · stop_if / stop_unless 各自报出自己的 tag 名与字段名 —— 两者共用一条消息， 且一个结构体可能挂多个，不带这两样就没东西可 grep。 · 别名 skip_nested_unless 报的是**它自己的名字**而不是 stop_unless。这条钉住 了改名时把 panic 里硬编码的字符串换成 fl.GetTag() 的必要性 —— 否则读者会 去找一个根本不在自己结构体里的 tag。 --- validator.go | 7 +++++-- validator_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/validator.go b/validator.go index 9a3fcd3..a673e6f 100644 --- a/validator.go +++ b/validator.go @@ -188,7 +188,10 @@ func stopIfImpl(_ context.Context, fl validator.FieldLevel) bool { return true } -func skipNestedUnlessWrapper(next ValidatorFunc) ValidatorFunc { +// stopTagsWrapper strips the errors produced by the stop tags. They fail on +// purpose — that is how validation is halted — so their errors are an +// implementation detail and must never reach the caller. +func stopTagsWrapper(next ValidatorFunc) ValidatorFunc { return func(ctx context.Context, v any) error { err := next(ctx, v) if err == nil { @@ -236,6 +239,6 @@ func ValidatorWithSkipNestedUnless(validator Validator) Validator { } return &wrappedValidator{ Validator: validator, - structCtxFunc: skipNestedUnlessWrapper(validator.StructCtx), + structCtxFunc: stopTagsWrapper(validator.StructCtx), } } diff --git a/validator_test.go b/validator_test.go index e5cfcea..622af48 100644 --- a/validator_test.go +++ b/validator_test.go @@ -362,3 +362,43 @@ func TestBuiltinSkipUnlessDoesNotActuallySkip(t *testing.T) { assert.ErrorAs(t, err, &verr) assert.Equal(t, "gte", verr[0].Tag(), "built-in skip_unless is documented as a presence check, not a skip") } + +// An odd parameter count is a struct-tag typo, and it panics. The message has +// to name the tag AND the field, otherwise there is nothing to grep for: both +// stop tags share one message, and a struct may carry several of them. +func TestStopTagsPanicMessageNamesTagAndField(t *testing.T) { + v := newStopValidator() + + t.Run("stop_if", func(t *testing.T) { + type S struct { + Amount int `validate:"stop_if=Other"` // missing the value half + Other int + } + assert.PanicsWithValue(t, "Bad param number for stop_if Amount", func() { + _ = v.StructCtx(context.Background(), S{}) + }) + }) + + t.Run("stop_unless", func(t *testing.T) { + type S struct { + Amount int `validate:"stop_unless=Other"` + Other int + } + assert.PanicsWithValue(t, "Bad param number for stop_unless Amount", func() { + _ = v.StructCtx(context.Background(), S{}) + }) + }) + + t.Run("the deprecated alias reports its own tag name, not stop_unless", func(t *testing.T) { + // fl.GetTag() is what makes this work: the alias shares stop_unless's + // implementation, so a hardcoded name would send the reader looking for + // a tag that is not in their struct. + type S struct { + Amount int `validate:"skip_nested_unless=Other"` + Other int + } + assert.PanicsWithValue(t, "Bad param number for skip_nested_unless Amount", func() { + _ = v.StructCtx(context.Background(), S{}) + }) + }) +}