Skip to content

Add static checker for validate.proto#474

Merged
timostamm merged 9 commits intomainfrom
tstamm/protovalidate-check
Mar 16, 2026
Merged

Add static checker for validate.proto#474
timostamm merged 9 commits intomainfrom
tstamm/protovalidate-check

Conversation

@timostamm
Copy link
Copy Markdown
Member

@timostamm timostamm commented Mar 11, 2026

validate.proto has grown to ~5100 lines with many repeated structural patterns across its rules.

Today, mistakes in these patterns - for example a CEL expression that doesn't compile or returns the wrong type - can only be caught by careful manual review or at runtime in downstream validation libraries.

This PR adds a static checker for the invariants of protovalidate. It compiles every (predefined).cel expression with the correct type environment and validates structural patterns. It runs via make lint-proto.

Note: This PR is part of an initiative to improve the Protovalidate SDLC. Follow-ups will add accompanying documentation.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 11, 2026

The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedMar 16, 2026, 4:57 PM

@timostamm
Copy link
Copy Markdown
Member Author

68f3a4c temporarily adds two mistakes. They are caught in the CI run:

go run ./tools/internal/protovalidate-check proto/protovalidate
buf/validate/validate.proto:626: buf.validate.FloatRules.gte: (predefined).cel[1]: type 'double' does not support field selection
buf/validate/validate.proto:3489: buf.validate.StringRules.uuid: (predefined).cel[0]: error parsing regexp: missing closing ]: `[0-9a-fA-F{12}$`

@timostamm timostamm requested a review from srikrsna-buf March 11, 2026 10:07
"buf.validate.AnyRules": singularType{singularKind: protoreflect.MessageKind, singularFullName: "google.protobuf.Any"},
"buf.validate.DurationRules": singularType{singularKind: protoreflect.MessageKind, singularFullName: "google.protobuf.Duration"},
"buf.validate.FieldMaskRules": singularType{singularKind: protoreflect.MessageKind, singularFullName: "google.protobuf.FieldMask"},
"buf.validate.TimestampRules": singularType{singularKind: protoreflect.MessageKind, singularFullName: "google.protobuf.Timestamp"},
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When adding a new standard rule, users will have to add a reference here as well. Maybe we should add this to the contributing guidelines or something similar? Can be in a followup PR.

Applies for other rule contributions as well. Useful for maintainers as well.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Standard rules are fields on one of the *Rules messages, and won't need an explicit mention here. For example, adding string.protobuf_dot_fqn (#465) doesn't need to update this.

This mapping reflects how the oneof type in FieldRules (link) relates to field types. For example, (buf.validate.field).string.max_len is only applicable to strings. Support for a new field type (e.g. field mask, #429) requires an update here.

Every implementation must maintain this mapping. It is one of the core invariants of protovalidate, and it's important for contributors, maintainers and new implementations to be aware. I've been thinking a lot on how to document or codify this best, but missed to call that out in the PR description. Added a note.

@timostamm
Copy link
Copy Markdown
Member Author

@srikrsna-buf, after merging in #479, I had to push up 8948dfc. Would you mind taking another look?

@timostamm timostamm merged commit 0d3a1ae into main Mar 16, 2026
8 checks passed
@timostamm timostamm deleted the tstamm/protovalidate-check branch March 16, 2026 17:20
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.

3 participants