Skip to content

Conversation

@stewSquared
Copy link

@stewSquared stewSquared commented Aug 13, 2025

PR Checklist (not all items are relevant to all PRs)

  • Added unit-tests (for runtime code)
  • Added bootstrapped code + smoke tests (when the rendering logic is modified)
  • Added build-plugins integration tests (when reflection loading is required at codegen-time)
  • Added alloy compliance tests (when simpleRestJson protocol behaviour is expanded/updated)
  • Updated dynamic module to match generated-code behaviour
  • Added documentation
  • Updated changelog

@stewSquared stewSquared changed the title Render fields marked nullable as smithy4s.Nullable rather than Option [0.19] Render fields marked nullable as smithy4s.Nullable rather than Option Aug 13, 2025
@stewSquared stewSquared changed the base branch from series/0.18 to series/0.19 August 13, 2025 17:39
@stewSquared stewSquared force-pushed the APIREG-321-render-nullable-as-nullable branch 2 times, most recently from ebd8470 to 4b79324 Compare August 20, 2025 20:22
val definition = contents.find(_.contains("final case class NullableExample")).get

assert(
definition.contains("nullableString: Nullable[String]"),
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, this test already passes before my changes to Renderer.scala and ToLine.scala. I don't understand this yet, because I don't know where else they would found and rendered as Nullable.

The other test is for sparse list and map types, which can have nullable elements. Those were previously rendered as Option.

@stewSquared stewSquared marked this pull request as ready for review August 20, 2025 20:24
@stewSquared stewSquared force-pushed the APIREG-321-render-nullable-as-nullable branch from 0b15191 to 4b79324 Compare August 20, 2025 20:57
@stewSquared stewSquared force-pushed the APIREG-321-render-nullable-as-nullable branch 2 times, most recently from 1cd004e to d697707 Compare August 21, 2025 20:02
@stewSquared stewSquared force-pushed the APIREG-321-render-nullable-as-nullable branch from d697707 to e3b9c40 Compare August 21, 2025 20:04
@kubukoz
Copy link
Member

kubukoz commented Aug 25, 2025

suggestion: let's do it in 0.18 with a @smithy4s.meta#asNullable trait? And we can make it the default in 0.19.

Could even have a metadata setting like smithy4sRenderDynamicHintBindings from #1816 to drive this more globally without breaking backwards compatibility.

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.

2 participants