Skip to content

[PEOPLE-41271] Separate the validation plan from Go emission in protoc-gen-govalidator - #159

Draft
Dmitry Rotay (DmitryRotay) wants to merge 4 commits into
masterfrom
PEOPLE-41271-validator-emission-seam
Draft

[PEOPLE-41271] Separate the validation plan from Go emission in protoc-gen-govalidator#159
Dmitry Rotay (DmitryRotay) wants to merge 4 commits into
masterfrom
PEOPLE-41271-validator-emission-seam

Conversation

@DmitryRotay

@DmitryRotay Dmitry Rotay (DmitryRotay) commented Aug 21, 2026

Copy link
Copy Markdown

Summary

Splits protoc-gen-govalidator into a validation plan and a Go emitter, so a second output language can reuse the plan instead of re-deriving it from the field options.

PEOPLE-41271

Generated Go is byte-identical

Built the plugin at master and at this commit, ran both with the same protoc, diffed the output trees:

Corpus Files Result
Full APISchema proto tree (880 protos, one invocation) 1,760 / 185,396 lines identical
valtest + example + playground 8 identical

plugin/testdata/*.golden also matches the checked-in valtest.validator.pb.go and geo.validator.pb.go exactly, apart from the protoc-gen-govalidator version line — those files were generated at v2.7.1 and the code reads v2.7.2, which predates this branch.

Problem description

Which checks a field needs, and in what order they run, existed only as the sequence of Go statements the plugin printed. Nothing else could read it.

That matters most in genStringValidator, which interleaves rewrites that permanently mutate the message field (NFC normalisation, URL breaking, trimming, the replacers, PUA stripping) with checks that read the mutated value. Order is data-changing, and the allow-list pattern is built as a side effect of walking those same rewrites. A C# emitter re-deriving that from the annotations would be free to diverge, silently.

Moved

  • plugin/validation_definitions.govalidation/plan/definitions.go — pure rename, only the package clause changed
  • Reading the field options — all thirteen get*Extension helpers
  • Length-bound resolution, URL scheme defaults, error-message assembly
  • Every prepareRegex / mergeRegex call, and the final pattern read
  • All panics on an invalid option — same messages, same order, now during plan construction

plugin.go drops 1,236 → 716 lines and no longer imports the validator package.

Added

  • validation/plan/plan.go — the vocabulary: 13 Step types (one per validator option) and 13 Op types for the ordered operations inside a string field
  • validation/plan/build.go — the builder
  • plugin/golden_test.go + testdata/ — there was no CI check that generated output stays put; now there is, via go test ./...

Preserved deliberately

  • Field-name-keyed pattern sharing across files — two fields with the same generated name accumulate each other's allowed characters. Plans are built per file immediately before that file is emitted, keeping visit order identical.
  • Emitter functions are a one-to-one transposition — same names, same g.P sequences, same comments.

Notes for review

  • Of 3,108 insertions, 1,634 are golden fixture text.
  • Regenerate goldens with go test ./protobuf/protoc-gen-govalidator/plugin/ -update. The request is assembled from the descriptors the generated Go packages register, so there is no checked-in descriptor set to keep in step.
  • The plan carries *protogen.Field on each node, so both emitters get naming from protogen. The seam is about rules, not names — the C# emitter derives its own casing from the descriptor.

Checklist

  • Is this PR a reasonable size?

    Large, but it is one mechanical split, and the diff is dominated by moved code and golden fixture.

  • List deployment sequence with all relevant PRs.

      1. This PR
      1. PEOPLE-41278 — C# generator and runtime, stacked on this branch
      1. PEOPLE-41270 — generate and publish from APISchema, after an s12-proto release and a protoc-docker image bump
  • For breaking changes: I have confirmed that other repos are not affected, or I have planned a deployment sequence to address any impact.

    No consumer-visible change — generated output is byte-identical.


Code Review Guidelines for Reviewers

  • Try to review in a timely manner. Opinions/nitpicks should not be
    blockers. Pair on a call for non-trivial feedback.
  • Overall design and approach should follow established patterns. Don't try
    to make the PR perfect.
  • Try to identify edge cases, race conditions, over-engineering, lack of
    test coverage and complexity.
  • If you don't feel qualified to review the code, pass it on to someone
    who is.

…c-gen-govalidator

Generated output is byte-identical: verified over the 1,760 files
protoc-gen-govalidator produces for the full APISchema proto tree.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review follow-ups: reset the pattern pool and regex names per run, hold the
goldens equal to the checked-in output, and keep a replacement's table value
verbatim rather than parsing it back to a rune.
@DmitryRotay
Dmitry Rotay (DmitryRotay) force-pushed the PEOPLE-41271-validator-emission-seam branch from d098ea3 to 38f02bd Compare September 1, 2026 01:51
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.

1 participant