Repository navigation
fix(protobuf): generate valid scalar map fields - #298
Open
Shubham-Padkonde wants to merge 1 commit into
Open
Shubham-Padkonde wants to merge 1 commit into
Shubham-Padkonde wants to merge 1 commit into
Conversation
Signed-off-by: Shubham Padkonde <shubhampadkonde12@gmail.com>
This branch has not been deployed
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.
Related to #249.
Changes
Protobuf map fields now omit the illegal
optionallabel and resolve Concerto scalar keys and values to their underlying Protobuf types. For example, an optional map from anIdentifier extends Stringscalar to anAmount extends Doublescalar now generatesmap<string, double> amounts = 1;.Map fields also use the property name instead of the map declaration name, so two properties using the same map type no longer generate duplicate field names. Field numbers are unchanged. Scalar lookup uses the map's declaring model, including when that map is imported by another namespace.
Added four regression tests using real models and a
protoccompilation regression. Updated the affected Protobuf snapshot and existing map tests.Validation
Field labels (required/optional/repeated) are not allowed on map fieldsand passes after it.npm run buildandnpm run test:esmpass.npm test: 701 passing, 52 pending; lint, dependency/lockfile checks, license checks and JSDoc pass. The command still exits nonzero on the repository-wide branch-coverage threshold (95.91% against 96%).af4fa2568c0af3e501614079f1f35039e88dcfaa, tested separately with the same dependencies and tools: 696 passing, 52 pending, and the same 95.91% branch coverage / threshold failure. The threshold is unchanged in this PR.protoc36.2. The 52 pending cases are existing skips or require unavailable external compilers.Flags
Generated map property names change to match their CTO property names; consumers of generated source/JSON field names should account for that correction. This does not renumber fields.
The full HR verification cases remain skipped for existing empty-enum and other generator limitations. This PR addresses the map-field defects; it does not claim every HR schema now compiles or add support for DateTime map keys.
Author Checklist
mainfrom a fork branch.