address-feedback: pr review round 1

This commit is contained in:
Aaron Prindle
2026-03-13 10:18:05 -07:00
parent bea6f92226
commit c8a857b72f
4 changed files with 26 additions and 21 deletions

View File

@@ -108,6 +108,7 @@ func Validate_ChainedValidation(ctx context.Context, op operation.Operation, fld
Value: "A", Validation: func(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *string) field.ErrorList {
errs := field.ErrorList{}
earlyReturn := false
errs = append(errs, validate.MaxLength(ctx, op, fldPath, obj, oldObj, 5)...)
if e := validate.RequiredPointer(ctx, op, fldPath, obj, oldObj); len(e) != 0 {
errs = append(errs, e...)
earlyReturn = true
@@ -115,7 +116,6 @@ func Validate_ChainedValidation(ctx context.Context, op operation.Operation, fld
if earlyReturn {
return errs
}
errs = append(errs, validate.MaxLength(ctx, op, fldPath, obj, oldObj, 5)...)
return errs
}},
})...)

View File

@@ -59,7 +59,7 @@ func TestBeta(t *testing.T) {
field.Required(field.NewPath("fieldA"), "").MarkBeta(),
})
// Invalid: mode A with FieldB set (forbidden), should be stability level alpha
// Invalid: mode A with FieldB set (forbidden), should be stability level beta
st.Value(&BetaStruct{D1: "A", FieldA: ptr.To("val"), FieldB: ptr.To("val")}).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByValidationStabilityLevel(), field.ErrorList{
field.Forbidden(field.NewPath("fieldB"), "").MarkBeta(),
})
@@ -67,7 +67,7 @@ func TestBeta(t *testing.T) {
// Valid: mode B with FieldB set
st.Value(&BetaStruct{D1: "B", FieldB: ptr.To("val")}).ExpectValid()
// Invalid: mode B with FieldB missing (required), should be stability level alpha
// Invalid: mode B with FieldB missing (required), should be stability level beta
st.Value(&BetaStruct{D1: "B"}).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByValidationStabilityLevel(), field.ErrorList{
field.Required(field.NewPath("fieldB"), "").MarkBeta(),
})

View File

@@ -338,6 +338,7 @@ func Validate_SameValueMixedPayloads(ctx context.Context, op operation.Operation
Value: "A", Validation: func(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *string) field.ErrorList {
errs := field.ErrorList{}
earlyReturn := false
errs = append(errs, validate.MinLength(ctx, op, fldPath, obj, oldObj, 3).MarkBeta()...)
if e := validate.RequiredPointer(ctx, op, fldPath, obj, oldObj).MarkAlpha(); len(e) != 0 {
errs = append(errs, e...)
earlyReturn = true
@@ -345,7 +346,6 @@ func Validate_SameValueMixedPayloads(ctx context.Context, op operation.Operation
if earlyReturn {
return errs
}
errs = append(errs, validate.MinLength(ctx, op, fldPath, obj, oldObj, 3).MarkBeta()...)
return errs
}},
})...)

View File

@@ -330,7 +330,6 @@ func (mtfv *discriminatorFieldValidator) generateMemberFieldValidation(context C
// carry different stability levels.
rulesByValue := make(map[string]Validations)
var values []string
uniformLevel := rules.rules[0].stabilityLevel
for _, rule := range rules.rules {
// Track unique discriminator values in order of first appearance.
if _, ok := rulesByValue[rule.value]; !ok {
@@ -346,29 +345,35 @@ func (mtfv *discriminatorFieldValidator) generateMemberFieldValidation(context C
v.Functions = marked
}
// Accumulate this rule's validations with others that share the same discriminator value.
existing := rulesByValue[rule.value]
existing.Add(v)
rulesByValue[rule.value] = existing
// Track whether all rules share the same level (for default-forbidden marking).
if rule.stabilityLevel != uniformLevel {
uniformLevel = ""
}
v.Add(rulesByValue[rule.value])
rulesByValue[rule.value] = v
}
slices.Sort(values)
discriminatorType := group.discriminatorMember.Type
// Mark the default-forbidden with the uniform level if all rules agree.
if uniformLevel != "" {
if mwf, ok := defaultForbidden.(MultiWrapperFunction); ok {
marked := make([]FunctionGen, len(mwf.Functions))
for i, f := range mwf.Functions {
marked[i] = f.WithStabilityLevel(uniformLevel)
}
mwf.Functions = marked
defaultForbidden = mwf
// When all rules share the same stability level, the default-forbidden
// (which fires for unrecognized discriminator values) should also be marked
// with that level so its errors carry the same stability annotation.
uniformLevel := rules.rules[0].stabilityLevel
for i := 1; i < len(rules.rules); i++ {
if rules.rules[i].stabilityLevel != uniformLevel {
uniformLevel = ""
break
}
}
if uniformLevel != "" {
mwf, ok := defaultForbidden.(MultiWrapperFunction)
if !ok {
return Validations{}, fmt.Errorf("internal error: defaultForbidden is not a MultiWrapperFunction")
}
marked := make([]FunctionGen, len(mwf.Functions))
for i, f := range mwf.Functions {
marked[i] = f.WithStabilityLevel(uniformLevel)
}
mwf.Functions = marked
defaultForbidden = mwf
}
var discriminatedRules []any
for _, val := range values {