Skip to content

Fix RBS Record translation to include optional fields#601

Open
Morriar wants to merge 1 commit into
mainfrom
at-fix-optional-shape-fields
Open

Fix RBS Record translation to include optional fields#601
Morriar wants to merge 1 commit into
mainfrom
at-fix-optional-shape-fields

Conversation

@Morriar
Copy link
Copy Markdown
Contributor

@Morriar Morriar commented May 12, 2026

Summary

  • RBS Record types with optional fields (e.g. { a: Integer, ?b: String }) were silently dropped during translation to RBI
  • The translator used type.fields which only returns required fields — switched to type.all_fields which includes both required and optional fields
  • Optional fields are now wrapped in T.nilable to represent that they may not be present
  • Already-nilable optional fields (e.g. ?c: Float?) are not double-wrapped thanks to Type.nilable simplification

@Morriar Morriar requested a review from a team as a code owner May 12, 2026 20:43
@Morriar Morriar force-pushed the at-fix-optional-shape-fields branch from 667315d to 25a6d5c Compare May 12, 2026 20:51
`type.fields` only returns required fields, silently dropping optional
ones. Switch to `type.all_fields` which returns both required and
optional fields. Optional fields are wrapped in `T.nilable`.
@Morriar Morriar force-pushed the at-fix-optional-shape-fields branch from 25a6d5c to 4d1375f Compare May 12, 2026 20:51
Comment on lines +85 to +88
Type.shape(type.all_fields.map do |name, (value, required)|
translated = translate(value)
[name, required ? translated : Type.nilable(translated)]
end.to_h)
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.

Suggested change
Type.shape(type.all_fields.map do |name, (value, required)|
translated = translate(value)
[name, required ? translated : Type.nilable(translated)]
end.to_h)
Type.shape(type.all_fields.to_h do |name, (value, required)|
translated = translate(value)
[name, required ? translated : Type.nilable(translated)]
end)

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