Skip to content

fix(protobuf): generate valid scalar map fields - #298

Open
Shubham-Padkonde wants to merge 1 commit into
accordproject:mainfrom
Shubham-Padkonde:fix/protobuf-map-fields
Open

Shubham-Padkonde wants to merge 1 commit into
accordproject:mainfrom
Shubham-Padkonde:fix/protobuf-map-fields

Conversation

@Shubham-Padkonde

Copy link
Copy Markdown

Related to #249.

Changes

Protobuf map fields now omit the illegal optional label and resolve Concerto scalar keys and values to their underlying Protobuf types. For example, an optional map from an Identifier extends String scalar to an Amount extends Double scalar now generates map<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 protoc compilation regression. Updated the affected Protobuf snapshot and existing map tests.

Validation

  • The four new model tests failed before the fix and pass after it. The compiler regression failed before the fix with Field labels (required/optional/repeated) are not allowed on map fields and passes after it.
  • npm run build and npm run test:esm pass.
  • 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%).
  • Unmodified base 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.
  • Node 24.15.0 on Windows; the Protobuf checks used protoc 36.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

  • DCO sign-off included in the commit.
  • Behavior covered by regression and compiler tests.
  • Commit message follows AP format.
  • No new public option or API; existing map output corrected.
  • Targets main from a fork branch.

Signed-off-by: Shubham Padkonde <shubhampadkonde12@gmail.com>

This branch has not been deployed

No deployments
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