From 8a2369e50a57f76ca293f848f19b4ee0723ae334 Mon Sep 17 00:00:00 2001 From: Oliver Sun Date: Thu, 14 May 2026 12:30:08 -0400 Subject: [PATCH 1/3] Update PROTOVALIDATE lint rule to check NaN --- .../internal/buflintvalidate/numeric.go | 87 ++++++++++++++++++- private/bufpkg/bufcheck/lint_test.go | 12 +++ .../lint/protovalidate/proto/number.proto | 37 ++++++++ 3 files changed, 134 insertions(+), 2 deletions(-) diff --git a/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/numeric.go b/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/numeric.go index 5e4d434c9b..10ffd792db 100644 --- a/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/numeric.go +++ b/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/numeric.go @@ -16,6 +16,7 @@ package buflintvalidate import ( "fmt" + "math" "github.com/bufbuild/buf/private/pkg/protoencoding" "google.golang.org/protobuf/reflect/protoreflect" @@ -24,8 +25,8 @@ import ( ) var fieldNumberToCheckNumberRulesFunc = map[int32]func(*adder, int32, protoreflect.Message) error{ - floatRulesFieldNumber: checkNumberRules[float32], - doubleRulesFieldNumber: checkNumberRules[float64], + floatRulesFieldNumber: checkFloatNumberRules[float32], + doubleRulesFieldNumber: checkFloatNumberRules[float64], int32RulesFieldNumber: checkNumberRules[int32], int64RulesFieldNumber: checkNumberRules[int64], uInt32RulesFieldNumber: checkNumberRules[uint32], @@ -55,6 +56,88 @@ func checkNumberRules[ ) } +// checkFloatNumberRules runs checkNumberRules and also flags NaN values in +// rule fields where NaN makes the rule unsatisfiable or a no-op. Any NaN +// comparison evaluates to false, so const/lt/lte/gt/gte/in cannot be +// satisfied and not_in entries containing NaN have no effect. +func checkFloatNumberRules[ + T float32 | float64, +]( + adder *adder, + numberRuleFieldNumber int32, + ruleMessage protoreflect.Message, +) error { + if err := checkNumberRules[T](adder, numberRuleFieldNumber, ruleMessage); err != nil { + return err + } + return checkFloatNaNRules[T](adder, numberRuleFieldNumber, ruleMessage) +} + +func checkFloatNaNRules[ + T float32 | float64, +]( + adder *adder, + ruleFieldNumber int32, + ruleMessage protoreflect.Message, +) error { + var err error + ruleMessage.Range(func(field protoreflect.FieldDescriptor, value protoreflect.Value) bool { + fieldNumber := int32(field.Number()) + switch string(field.Name()) { + case "const", "lt", "lte", "gt", "gte": + v, ok := value.Interface().(T) + if !ok { + err = fmt.Errorf("unable to cast value to type %T", v) + return false + } + if math.IsNaN(float64(v)) { + adder.addForPathf( + []int32{ruleFieldNumber, fieldNumber}, + "Field %q has %s set to NaN. Comparisons with NaN are always false, so this rule can never be satisfied.", + adder.fieldName(), + adder.getFieldRuleName(ruleFieldNumber, fieldNumber), + ) + } + case "in": + list := value.List() + for i := range list.Len() { + v, ok := list.Get(i).Interface().(T) + if !ok { + err = fmt.Errorf("unable to cast value to type %T", v) + return false + } + if math.IsNaN(float64(v)) { + adder.addForPathf( + []int32{ruleFieldNumber, fieldNumber, int32(i)}, + "Field %q has NaN in %s. Comparisons with NaN are always false, so this entry can never match.", + adder.fieldName(), + adder.getFieldRuleName(ruleFieldNumber, fieldNumber), + ) + } + } + case "not_in": + list := value.List() + for i := range list.Len() { + v, ok := list.Get(i).Interface().(T) + if !ok { + err = fmt.Errorf("unable to cast value to type %T", v) + return false + } + if math.IsNaN(float64(v)) { + adder.addForPathf( + []int32{ruleFieldNumber, fieldNumber, int32(i)}, + "Field %q has NaN in %s. Comparisons with NaN are always false, so this entry has no effect.", + adder.fieldName(), + adder.getFieldRuleName(ruleFieldNumber, fieldNumber), + ) + } + } + } + return true + }) + return err +} + func checkNumericRules[ T int32 | int64 | uint32 | uint64 | float32 | float64 | timestamppb.Timestamp | durationpb.Duration, ]( diff --git a/private/bufpkg/bufcheck/lint_test.go b/private/bufpkg/bufcheck/lint_test.go index 43ac25ee09..a326cec24a 100644 --- a/private/bufpkg/bufcheck/lint_test.go +++ b/private/bufpkg/bufcheck/lint_test.go @@ -741,6 +741,18 @@ func TestRunProtovalidate(t *testing.T) { bufanalysistesting.NewFileAnnotation(t, "number.proto", 317, 79, 317, 117, "PROTOVALIDATE"), bufanalysistesting.NewFileAnnotation(t, "number.proto", 326, 5, 326, 44, "PROTOVALIDATE"), bufanalysistesting.NewFileAnnotation(t, "number.proto", 329, 79, 329, 117, "PROTOVALIDATE"), + bufanalysistesting.NewFileAnnotation(t, "number.proto", 334, 24, 334, 62, "PROTOVALIDATE"), + bufanalysistesting.NewFileAnnotation(t, "number.proto", 336, 21, 336, 56, "PROTOVALIDATE"), + bufanalysistesting.NewFileAnnotation(t, "number.proto", 338, 22, 338, 58, "PROTOVALIDATE"), + bufanalysistesting.NewFileAnnotation(t, "number.proto", 340, 21, 340, 56, "PROTOVALIDATE"), + bufanalysistesting.NewFileAnnotation(t, "number.proto", 342, 22, 342, 58, "PROTOVALIDATE"), + bufanalysistesting.NewFileAnnotation(t, "number.proto", 346, 5, 346, 40, "PROTOVALIDATE"), + bufanalysistesting.NewFileAnnotation(t, "number.proto", 349, 25, 349, 64, "PROTOVALIDATE"), + bufanalysistesting.NewFileAnnotation(t, "number.proto", 351, 32, 351, 71, "PROTOVALIDATE"), + bufanalysistesting.NewFileAnnotation(t, "number.proto", 353, 29, 353, 65, "PROTOVALIDATE"), + bufanalysistesting.NewFileAnnotation(t, "number.proto", 357, 5, 357, 41, "PROTOVALIDATE"), + bufanalysistesting.NewFileAnnotation(t, "number.proto", 360, 41, 360, 92, "PROTOVALIDATE"), + bufanalysistesting.NewFileAnnotation(t, "number.proto", 362, 44, 362, 90, "PROTOVALIDATE"), bufanalysistesting.NewFileAnnotation(t, "oneof.proto", 13, 7, 13, 43, "PROTOVALIDATE"), bufanalysistesting.NewFileAnnotation(t, "oneof.proto", 19, 7, 19, 43, "PROTOVALIDATE"), bufanalysistesting.NewFileAnnotation(t, "repeated.proto", 25, 5, 25, 48, "PROTOVALIDATE"), diff --git a/private/bufpkg/bufcheck/testdata/lint/protovalidate/proto/number.proto b/private/bufpkg/bufcheck/testdata/lint/protovalidate/proto/number.proto index 5e645b51c8..70cfe0186a 100644 --- a/private/bufpkg/bufcheck/testdata/lint/protovalidate/proto/number.proto +++ b/private/bufpkg/bufcheck/testdata/lint/protovalidate/proto/number.proto @@ -328,3 +328,40 @@ message IntTest { google.protobuf.UInt64Value valid_wkt_uint64_no_constraint_example = 106 [(buf.validate.field).uint64.example = 1]; google.protobuf.UInt64Value invalid_wkt_uint64_no_constraint_example = 107 [(buf.validate.field).int64.example = 1]; } + +message FloatNaNTest { + // the next line should be annotated + float const_nan = 1 [(buf.validate.field).float.const = nan]; + // the next line should be annotated + float lt_nan = 2 [(buf.validate.field).float.lt = nan]; + // the next line should be annotated + float lte_nan = 3 [(buf.validate.field).float.lte = nan]; + // the next line should be annotated + float gt_nan = 4 [(buf.validate.field).float.gt = nan]; + // the next line should be annotated + float gte_nan = 5 [(buf.validate.field).float.gte = nan]; + float in_nan = 6 [ + (buf.validate.field).float.in = 1.0, + // the next line should be annotated + (buf.validate.field).float.in = nan + ]; + // the next line should be annotated + float not_in_nan = 7 [(buf.validate.field).float.not_in = nan]; + // the next line should be annotated + double double_const_nan = 8 [(buf.validate.field).double.const = nan]; + // the next line should be annotated + double double_lt_nan = 9 [(buf.validate.field).double.lt = nan]; + double double_in_nan = 10 [ + (buf.validate.field).double.in = 1.0, + // the next line should be annotated + (buf.validate.field).double.in = nan + ]; + // the next line should be annotated + repeated double repeated_lt_nan = 11 [(buf.validate.field).repeated.items.double.lt = nan]; + // the next line should be annotated + map map_value_lt_nan = 12 [(buf.validate.field).map.values.float.lt = nan]; + // valid: bound is normal, example unrelated to NaN check + float ok_finite = 13 [(buf.validate.field).float.lt = 1.0]; + // valid: not_in with non-NaN + float ok_not_in = 14 [(buf.validate.field).float.not_in = 1.0]; +} From 2d611e311bd3c9f2012b13af691cd30aa822debb Mon Sep 17 00:00:00 2001 From: Oliver Sun Date: Thu, 14 May 2026 16:27:42 -0400 Subject: [PATCH 2/3] collapse in and not_in --- .../internal/buflintvalidate/numeric.go | 26 +++++-------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/numeric.go b/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/numeric.go index 10ffd792db..780de3bc8d 100644 --- a/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/numeric.go +++ b/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/numeric.go @@ -98,7 +98,7 @@ func checkFloatNaNRules[ adder.getFieldRuleName(ruleFieldNumber, fieldNumber), ) } - case "in": + case "in", "not_in": list := value.List() for i := range list.Len() { v, ok := list.Get(i).Interface().(T) @@ -107,28 +107,16 @@ func checkFloatNaNRules[ return false } if math.IsNaN(float64(v)) { + effect := "can never match" + if field.Name() == "not_in" { + effect = "has no effect" + } adder.addForPathf( []int32{ruleFieldNumber, fieldNumber, int32(i)}, - "Field %q has NaN in %s. Comparisons with NaN are always false, so this entry can never match.", - adder.fieldName(), - adder.getFieldRuleName(ruleFieldNumber, fieldNumber), - ) - } - } - case "not_in": - list := value.List() - for i := range list.Len() { - v, ok := list.Get(i).Interface().(T) - if !ok { - err = fmt.Errorf("unable to cast value to type %T", v) - return false - } - if math.IsNaN(float64(v)) { - adder.addForPathf( - []int32{ruleFieldNumber, fieldNumber, int32(i)}, - "Field %q has NaN in %s. Comparisons with NaN are always false, so this entry has no effect.", + "Field %q has NaN in %s. Comparisons with NaN are always false, so this entry %s.", adder.fieldName(), adder.getFieldRuleName(ruleFieldNumber, fieldNumber), + effect, ) } } From ae8ae415cf54f3e470683e2cbac298623f7b698f Mon Sep 17 00:00:00 2001 From: Oliver Sun Date: Thu, 14 May 2026 17:21:34 -0400 Subject: [PATCH 3/3] changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c1f163253f..c7f2022121 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ - Add LSP completion for `buf.gen.yaml`, `buf.yaml`, and `buf.policy.yaml` files. - Add error for when a dependency is added to `buf.yaml` and is missing from `buf.lock`. +- Update the `PROTOVALIDATE` lint rule support checking `NaN` in `const`, `in`, `not_in`, + `gt`, `gte`, `lt` and `lte` rules for `float` and `double` fields. ## [v1.69.0] - 2026-04-29