feat(protogen): preserve stable field numbers and auto-emit reserved statements on regeneration#403
Merged
Merged
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #403 +/- ##
==========================================
+ Coverage 74.63% 74.78% +0.15%
==========================================
Files 88 88
Lines 9185 9249 +64
==========================================
+ Hits 6855 6917 +62
- Misses 1755 1756 +1
- Partials 575 576 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
0c89e3a to
6e4a53f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
reservedand may be reused #402Background / Motivation
When Tableau regenerates
.protofiles from Excel workbooks, it previously re-assigned field numbers strictly from 1..N in declaration order. This broke wire-format compatibility in two ways:This PR makes regeneration wire-compatible by preserving old field numbers and emitting
reservedstatements as needed.What changed
internal/protogen/exporter.go—sheetExporter.assignFieldNumbers:oldMD(previously generatedMessageDescriptor) is available, fields whosenamematches an existing field keep their old number, regardless of column order.oldMDbut no longer in the current sheet has its number added to areservedset.oldMDare preserved verbatim across regenerations.[1, maxUsedNumber]that are neither active nor already reserved are auto-reserved — this defends against older Tableau versions that deleted fields without reserving them.Supporting pieces:
formatReservedNumberscollapses a sorted number list into the compact proto syntax (2, 5, 7 to 9, 12).sheetExporter.printReservedemits a singlereserved …;line at the correct indentation.exportStruct,exportUnion,exportMessager, and the nested-message branch inexportField) now consume the returned reserved slice and print it.Tests
internal/protogen/exporter_test.go:Test_formatReservedNumberscovers empty / single / non-consecutive / consecutive / mixed / multi-range formatting.Test_sheetExporter_assignFieldNumbersusesprotodesc.NewFileto synthesizeMessageDescriptors with arbitrary active fields and reserved ranges, covering 10 semantic scenarios:oldMD == nil→ sequential assignment from 1.reserved Nright after active fields → new field skips it.reserved Nhigher than all active fields → all gaps below are auto-reserved; new field starts after N.reserved 2 to 4→ carried over verbatim.exportStruct/exportUnion/exportMessagertests extended withfield-number-compatibility-*sub-tests (insertion-in-the-middle, delete-and-add, sub-message nesting).Compatibility
.protofiles generated by older Tableau versions that left gaps withoutreserved: those gaps are now auto-reserved on the next regeneration.oldMDis discoverable (i.e., requiresPreserveFieldNumbers+ an existing generated proto in the registry), so fresh generations are unaffected.