From a944060fc2381386e53942192b7be9e9eadbf4b1 Mon Sep 17 00:00:00 2001 From: Jack Robertson Date: Mon, 14 Sep 2026 16:23:29 +0100 Subject: [PATCH] Stop a property name shadowing the codec's argument `to_wire` read each property into a local named after it, so a document with a property called `value` emitted `value = value.value`, rebinding the struct the method was given. Every property after it then read off an Optional and raised NoMethodError. A property called `wire` clobbered the accumulator the same way. The locals are fixed names now, so no property can reach them. --- lib/openapi_kit/codegen/emit/codecs.rb | 14 +++--- spec/fixtures/schemas/kitchen_sink.yaml | 5 +++ spec/generated/translation_spec.rb | 15 +++++++ .../kitchen_sink/types/adversarial.rb | 43 ++++++++++++++++--- .../kitchen_sink/kitchen_sink/types/cat.rb | 4 +- .../kitchen_sink/kitchen_sink/types/dog.rb | 4 +- .../kitchen_sink/kitchen_sink/types/mod.rb | 40 ++++++++--------- .../kitchen_sink/types/mod_meta.rb | 4 +- .../kitchen_sink/kitchen_sink/types/user.rb | 4 +- spec/golden/server/server/types/mod.rb | 4 +- .../server/server/types/problem_details.rb | 4 +- 11 files changed, 95 insertions(+), 46 deletions(-) diff --git a/lib/openapi_kit/codegen/emit/codecs.rb b/lib/openapi_kit/codegen/emit/codecs.rb index 412c7a6..e520e94 100644 --- a/lib/openapi_kit/codegen/emit/codecs.rb +++ b/lib/openapi_kit/codegen/emit/codecs.rb @@ -139,9 +139,9 @@ def emit_property_to_wire(buffer, property) reader = property.identifier.to_s if Decode.optional_nullable?(required: property.required, meta: Model::Schema.meta(property.schema)) - buffer.line("#{reader} = value.#{reader}") - buffer.nest("if #{reader}.is_a?(::OpenAPIKit::Present)") do - buffer.line("inner = #{reader}.value") + buffer.line("field = value.#{reader}") + buffer.nest("if field.is_a?(::OpenAPIKit::Present)") do + buffer.line("inner = field.value") buffer.line("wire[#{key}] = inner.nil? ? nil : #{@registry.to_wire_expr(property.schema, value: "inner")}") end @@ -153,13 +153,13 @@ def emit_property_to_wire(buffer, property) return end - buffer.line("#{reader} = value.#{reader}") - dumped = @registry.to_wire_expr(property.schema, value: reader) + buffer.line("field = value.#{reader}") + dumped = @registry.to_wire_expr(property.schema, value: "field") if property.required - buffer.line("wire[#{key}] = #{reader}.nil? ? nil : #{dumped}") + buffer.line("wire[#{key}] = field.nil? ? nil : #{dumped}") else - buffer.line("wire[#{key}] = #{dumped} unless #{reader}.nil?") + buffer.line("wire[#{key}] = #{dumped} unless field.nil?") end end diff --git a/spec/fixtures/schemas/kitchen_sink.yaml b/spec/fixtures/schemas/kitchen_sink.yaml index 4b556fc..8c5c896 100644 --- a/spec/fixtures/schemas/kitchen_sink.yaml +++ b/spec/fixtures/schemas/kitchen_sink.yaml @@ -48,6 +48,11 @@ components: since: { type: string, format: date-time, default: "2020-01-01T00:00:00Z" } cleared: { type: string, nullable: true, default: null } rate: { type: number, default: 1 } + value: { type: integer, nullable: true } + wire: { type: string } + inner: { type: string, nullable: true } + field: { type: string, nullable: true } + trailing: { type: string } additionalProperties: { type: string, format: date-time } snake_cat: { type: object, required: [kind], properties: { kind: { type: string } } } snake_dog: { type: object, required: [kind], properties: { kind: { type: string } } } diff --git a/spec/generated/translation_spec.rb b/spec/generated/translation_spec.rb index 8b53403..e7113fd 100644 --- a/spec/generated/translation_spec.rb +++ b/spec/generated/translation_spec.rb @@ -77,6 +77,21 @@ def multi_file_document expect(generated.type("mod")).to include("const :dates, T.nilable(T::Array[::Time])") end + + it "encodes a property whose name would shadow the codec's own argument" do + generated = schemas(<<~YAML) + Mod: + type: object + properties: + value: { type: integer, nullable: true } + wire: { type: string } + field: { type: string, nullable: true } + trailing: { type: string } + YAML + + expect(generated.type("mod")).to include("field = value.value", "field = value.wire", + "field = value.field", "field = value.trailing") + end end describe "composition" do diff --git a/spec/golden/kitchen_sink/kitchen_sink/types/adversarial.rb b/spec/golden/kitchen_sink/kitchen_sink/types/adversarial.rb index a9f36ab..ec72f09 100644 --- a/spec/golden/kitchen_sink/kitchen_sink/types/adversarial.rb +++ b/spec/golden/kitchen_sink/kitchen_sink/types/adversarial.rb @@ -11,6 +11,11 @@ class Adversarial < T::Struct const :since, ::Time, factory: -> { ::OpenAPIKit::Codec::DateTime.from_wire("2020-01-01T00:00:00Z") } const :cleared, T.nilable(::String), default: nil const :rate, ::Float, factory: -> { ::OpenAPIKit::Codec::Float.from_wire(1) } + const :value, ::OpenAPIKit::Optional[T.nilable(::Integer)], default: ::OpenAPIKit::ABSENT + const :wire, T.nilable(::String) + const :inner, ::OpenAPIKit::Optional[T.nilable(::String)], default: ::OpenAPIKit::ABSENT + const :field, ::OpenAPIKit::Optional[T.nilable(::String)], default: ::OpenAPIKit::ABSENT + const :trailing, T.nilable(::String) const :additional_properties, T::Hash[::String, ::Time], factory: -> { {} } sig { params(other: T.untyped).returns(T::Boolean) } @@ -37,7 +42,12 @@ def self.from_wire(value) since: ::OpenAPIKit::Decode.defaulted(raw, "since", ::OpenAPIKit::Codec::DateTime.from_wire("2020-01-01T00:00:00Z")) { |v| ::OpenAPIKit::Codec::DateTime.from_wire(v) }, cleared: ::OpenAPIKit::Decode.defaulted(raw, "cleared", nil) { |v| ::OpenAPIKit::Codec::String.from_wire(v) }, rate: ::OpenAPIKit::Decode.defaulted(raw, "rate", ::OpenAPIKit::Codec::Float.from_wire(1)) { |v| ::OpenAPIKit::Codec::Float.from_wire(v) }, - additional_properties: ::OpenAPIKit::Decode.values(raw.except("serialize", "since", "cleared", "rate")) { |item| ::OpenAPIKit::Codec::DateTime.from_wire(item) }, + value: ::OpenAPIKit::Decode.optional_nullable(raw, "value") { |v| ::OpenAPIKit::Codec::Integer.from_wire(v) }, + wire: ::OpenAPIKit::Decode.optional(raw, "wire") { |v| ::OpenAPIKit::Codec::String.from_wire(v) }, + inner: ::OpenAPIKit::Decode.optional_nullable(raw, "inner") { |v| ::OpenAPIKit::Codec::String.from_wire(v) }, + field: ::OpenAPIKit::Decode.optional_nullable(raw, "field") { |v| ::OpenAPIKit::Codec::String.from_wire(v) }, + trailing: ::OpenAPIKit::Decode.optional(raw, "trailing") { |v| ::OpenAPIKit::Codec::String.from_wire(v) }, + additional_properties: ::OpenAPIKit::Decode.values(raw.except("serialize", "since", "cleared", "rate", "value", "wire", "inner", "field", "trailing")) { |item| ::OpenAPIKit::Codec::DateTime.from_wire(item) }, ) end @@ -45,12 +55,31 @@ def self.from_wire(value) def self.to_wire(value) wire = T.let({}, T::Hash[::String, ::OpenAPIKit::Wire]) wire["serialize"] = ::OpenAPIKit::Codec::String.to_wire(value.serialize_) - since = value.since - wire["since"] = ::OpenAPIKit::Codec::DateTime.to_wire(since) unless since.nil? - cleared = value.cleared - wire["cleared"] = ::OpenAPIKit::Codec::String.to_wire(cleared) unless cleared.nil? - rate = value.rate - wire["rate"] = ::OpenAPIKit::Codec::Float.to_wire(rate) unless rate.nil? + field = value.since + wire["since"] = ::OpenAPIKit::Codec::DateTime.to_wire(field) unless field.nil? + field = value.cleared + wire["cleared"] = ::OpenAPIKit::Codec::String.to_wire(field) unless field.nil? + field = value.rate + wire["rate"] = ::OpenAPIKit::Codec::Float.to_wire(field) unless field.nil? + field = value.value + if field.is_a?(::OpenAPIKit::Present) + inner = field.value + wire["value"] = inner.nil? ? nil : ::OpenAPIKit::Codec::Integer.to_wire(inner) + end + field = value.wire + wire["wire"] = ::OpenAPIKit::Codec::String.to_wire(field) unless field.nil? + field = value.inner + if field.is_a?(::OpenAPIKit::Present) + inner = field.value + wire["inner"] = inner.nil? ? nil : ::OpenAPIKit::Codec::String.to_wire(inner) + end + field = value.field + if field.is_a?(::OpenAPIKit::Present) + inner = field.value + wire["field"] = inner.nil? ? nil : ::OpenAPIKit::Codec::String.to_wire(inner) + end + field = value.trailing + wire["trailing"] = ::OpenAPIKit::Codec::String.to_wire(field) unless field.nil? wire.merge!(value.additional_properties.transform_values { |item| ::OpenAPIKit::Codec::DateTime.to_wire(item) }) wire end diff --git a/spec/golden/kitchen_sink/kitchen_sink/types/cat.rb b/spec/golden/kitchen_sink/kitchen_sink/types/cat.rb index 4f0fce9..6e7e538 100644 --- a/spec/golden/kitchen_sink/kitchen_sink/types/cat.rb +++ b/spec/golden/kitchen_sink/kitchen_sink/types/cat.rb @@ -39,8 +39,8 @@ def self.from_wire(value) def self.to_wire(value) wire = T.let({}, T::Hash[::String, ::OpenAPIKit::Wire]) wire["kind"] = ::OpenAPIKit::Codec::String.to_wire(value.kind) - lives = value.lives - wire["lives"] = ::OpenAPIKit::Codec::Integer.to_wire(lives) unless lives.nil? + field = value.lives + wire["lives"] = ::OpenAPIKit::Codec::Integer.to_wire(field) unless field.nil? wire end end diff --git a/spec/golden/kitchen_sink/kitchen_sink/types/dog.rb b/spec/golden/kitchen_sink/kitchen_sink/types/dog.rb index f44dd70..58e4ecd 100644 --- a/spec/golden/kitchen_sink/kitchen_sink/types/dog.rb +++ b/spec/golden/kitchen_sink/kitchen_sink/types/dog.rb @@ -39,8 +39,8 @@ def self.from_wire(value) def self.to_wire(value) wire = T.let({}, T::Hash[::String, ::OpenAPIKit::Wire]) wire["kind"] = ::OpenAPIKit::Codec::String.to_wire(value.kind) - breed = value.breed - wire["breed"] = ::OpenAPIKit::Codec::String.to_wire(breed) unless breed.nil? + field = value.breed + wire["breed"] = ::OpenAPIKit::Codec::String.to_wire(field) unless field.nil? wire end end diff --git a/spec/golden/kitchen_sink/kitchen_sink/types/mod.rb b/spec/golden/kitchen_sink/kitchen_sink/types/mod.rb index e633108..bbacd68 100644 --- a/spec/golden/kitchen_sink/kitchen_sink/types/mod.rb +++ b/spec/golden/kitchen_sink/kitchen_sink/types/mod.rb @@ -60,31 +60,31 @@ def self.to_wire(value) wire = T.let({}, T::Hash[::String, ::OpenAPIKit::Wire]) wire["id"] = ::OpenAPIKit::Codec::Integer.to_wire(value.id) wire["name"] = ::OpenAPIKit::Codec::String.to_wire(value.name) - status = value.status - wire["status"] = KitchenSink::Types::ModStatus::Codec.to_wire(status) unless status.nil? + field = value.status + wire["status"] = KitchenSink::Types::ModStatus::Codec.to_wire(field) unless field.nil? wire["updatedAt"] = ::OpenAPIKit::Codec::DateTime.to_wire(value.updated_at) - deleted_at = value.deleted_at - wire["deletedAt"] = deleted_at.nil? ? nil : ::OpenAPIKit::Codec::DateTime.to_wire(deleted_at) - summary = value.summary - wire["summary"] = ::OpenAPIKit::Codec::String.to_wire(summary) unless summary.nil? - bio = value.bio - if bio.is_a?(::OpenAPIKit::Present) - inner = bio.value + field = value.deleted_at + wire["deletedAt"] = field.nil? ? nil : ::OpenAPIKit::Codec::DateTime.to_wire(field) + field = value.summary + wire["summary"] = ::OpenAPIKit::Codec::String.to_wire(field) unless field.nil? + field = value.bio + if field.is_a?(::OpenAPIKit::Present) + inner = field.value wire["bio"] = inner.nil? ? nil : ::OpenAPIKit::Codec::String.to_wire(inner) end - page_size = value.page_size - wire["pageSize"] = ::OpenAPIKit::Codec::Integer.to_wire(page_size) unless page_size.nil? - owner = value.owner - if owner.is_a?(::OpenAPIKit::Present) - inner = owner.value + field = value.page_size + wire["pageSize"] = ::OpenAPIKit::Codec::Integer.to_wire(field) unless field.nil? + field = value.owner + if field.is_a?(::OpenAPIKit::Present) + inner = field.value wire["owner"] = inner.nil? ? nil : KitchenSink::Types::User::Codec.to_wire(inner) end - tags = value.tags - wire["tags"] = tags.map { |item| ::OpenAPIKit::Codec::String.to_wire(item) } unless tags.nil? - meta = value.meta - wire["meta"] = KitchenSink::Types::ModMeta::Codec.to_wire(meta) unless meta.nil? - extra = value.extra - wire["extra"] = extra.transform_values { |item| ::OpenAPIKit::Codec::Integer.to_wire(item) } unless extra.nil? + field = value.tags + wire["tags"] = field.map { |item| ::OpenAPIKit::Codec::String.to_wire(item) } unless field.nil? + field = value.meta + wire["meta"] = KitchenSink::Types::ModMeta::Codec.to_wire(field) unless field.nil? + field = value.extra + wire["extra"] = field.transform_values { |item| ::OpenAPIKit::Codec::Integer.to_wire(item) } unless field.nil? wire end end diff --git a/spec/golden/kitchen_sink/kitchen_sink/types/mod_meta.rb b/spec/golden/kitchen_sink/kitchen_sink/types/mod_meta.rb index 03b662a..070bd73 100644 --- a/spec/golden/kitchen_sink/kitchen_sink/types/mod_meta.rb +++ b/spec/golden/kitchen_sink/kitchen_sink/types/mod_meta.rb @@ -36,8 +36,8 @@ def self.from_wire(value) sig { override.params(value: KitchenSink::Types::ModMeta).returns(::OpenAPIKit::Wire) } def self.to_wire(value) wire = T.let({}, T::Hash[::String, ::OpenAPIKit::Wire]) - note = value.note - wire["note"] = ::OpenAPIKit::Codec::String.to_wire(note) unless note.nil? + field = value.note + wire["note"] = ::OpenAPIKit::Codec::String.to_wire(field) unless field.nil? wire end end diff --git a/spec/golden/kitchen_sink/kitchen_sink/types/user.rb b/spec/golden/kitchen_sink/kitchen_sink/types/user.rb index f72509e..2d7d7a7 100644 --- a/spec/golden/kitchen_sink/kitchen_sink/types/user.rb +++ b/spec/golden/kitchen_sink/kitchen_sink/types/user.rb @@ -42,8 +42,8 @@ def self.to_wire(value) wire = T.let({}, T::Hash[::String, ::OpenAPIKit::Wire]) wire["id"] = ::OpenAPIKit::Codec::Uuid.to_wire(value.id) wire["name"] = ::OpenAPIKit::Codec::String.to_wire(value.name) - friend = value.friend - wire["friend"] = KitchenSink::Types::User::Codec.to_wire(friend) unless friend.nil? + field = value.friend + wire["friend"] = KitchenSink::Types::User::Codec.to_wire(field) unless field.nil? wire end end diff --git a/spec/golden/server/server/types/mod.rb b/spec/golden/server/server/types/mod.rb index 2b86d3b..db8ed6a 100644 --- a/spec/golden/server/server/types/mod.rb +++ b/spec/golden/server/server/types/mod.rb @@ -42,8 +42,8 @@ def self.to_wire(value) wire = T.let({}, T::Hash[::String, ::OpenAPIKit::Wire]) wire["id"] = ::OpenAPIKit::Codec::Integer.to_wire(value.id) wire["name"] = ::OpenAPIKit::Codec::String.to_wire(value.name) - status = value.status - wire["status"] = Server::Types::ModStatus::Codec.to_wire(status) unless status.nil? + field = value.status + wire["status"] = Server::Types::ModStatus::Codec.to_wire(field) unless field.nil? wire end end diff --git a/spec/golden/server/server/types/problem_details.rb b/spec/golden/server/server/types/problem_details.rb index 63eb05c..a60f3fb 100644 --- a/spec/golden/server/server/types/problem_details.rb +++ b/spec/golden/server/server/types/problem_details.rb @@ -39,8 +39,8 @@ def self.from_wire(value) def self.to_wire(value) wire = T.let({}, T::Hash[::String, ::OpenAPIKit::Wire]) wire["title"] = ::OpenAPIKit::Codec::String.to_wire(value.title) - detail = value.detail - wire["detail"] = ::OpenAPIKit::Codec::String.to_wire(detail) unless detail.nil? + field = value.detail + wire["detail"] = ::OpenAPIKit::Codec::String.to_wire(field) unless field.nil? wire end end