Skip to content

fix: use symbol-provider for accessors in member-builder #4341

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

fix: use symbol-provider for accessors in member-builder #4341

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...k/src/main/kotlin/software/amazon/smithy/rustsdk/endpoints/OperationInputTestGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ class OperationInputTestGenerator(_ctx: ClientCodegenContext, private val test:
testOperationInput.operationParams.members.forEach { (key, value) ->
val member = operationInput.expectMember(key.value)
rustTemplate(
".${member.setterName()}(#{value})",
".${member.setterName(symbolProvider)}(#{value})",
"value" to instantiator.generate(member, value),
)
}
Expand Down
5 changes: 5 additions & 0 deletions ...software/amazon/smithy/rust/codegen/client/smithy/generators/ClientBuilderInstantiator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate
import software.amazon.smithy.rust.codegen.core.rustlang.writable
import software.amazon.smithy.rust.codegen.core.smithy.generators.BuilderGenerator
import software.amazon.smithy.rust.codegen.core.smithy.generators.BuilderInstantiator
import software.amazon.smithy.rust.codegen.core.smithy.generators.setterName

class ClientBuilderInstantiator(private val clientCodegenContext: ClientCodegenContext) : BuilderInstantiator {
override fun setField(
Expand All @@ -25,6 +26,10 @@ class ClientBuilderInstantiator(private val clientCodegenContext: ClientCodegenC
return setFieldWithSetter(builder, value, field)
}

override fun setterProvider(field: MemberShape): String {
return field.setterName(clientCodegenContext.symbolProvider)
}

/**
* For the client, we finalize builders with error correction enabled
*/
Expand Down
2 changes: 1 addition & 1 deletion ...kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientInstantiator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class ClientBuilderKindBehavior(val codegenContext: CodegenContext) : Instantiat
override fun hasFallibleBuilder(shape: StructureShape): Boolean =
BuilderGenerator.hasFallibleBuilder(shape, codegenContext.symbolProvider)

override fun setterName(memberShape: MemberShape): String = memberShape.setterName()
override fun setterName(memberShape: MemberShape): String = memberShape.setterName(codegenContext.symbolProvider)

override fun doesSetterTakeInOption(memberShape: MemberShape): Boolean = true
}
Expand Down
8 changes: 4 additions & 4 deletions ...ware/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentBuilderGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,11 +367,11 @@ class FluentBuilderGenerator(
else -> renderInputHelper(member, memberName, coreType)
}
// pure setter
val setterName = member.setterName()
val setterName = member.setterName(symbolProvider)
val optionalInputType = outerType.asOptional()
renderInputHelper(member, setterName, optionalInputType)

val getterName = member.getterName()
val getterName = member.getterName(symbolProvider)
renderGetterHelper(member, getterName, optionalInputType)
}
}
Expand Down Expand Up @@ -437,7 +437,7 @@ class FluentBuilderGenerator(
"""
Appends an item to `${member.memberName}`.
To override the contents of this collection use [`${member.setterName()}`](Self::${member.setterName()}).
To override the contents of this collection use [`${member.setterName(symbolProvider)}`](Self::${member.setterName(symbolProvider)}).
""",
)
documentShape(member, model)
Expand Down Expand Up @@ -467,7 +467,7 @@ class FluentBuilderGenerator(
"""
Adds a key-value pair to `${member.memberName}`.
To override the contents of this collection use [`${member.setterName()}`](Self::${member.setterName()}).
To override the contents of this collection use [`${member.setterName(symbolProvider)}`](Self::${member.setterName(symbolProvider)}).
""",
)
documentShape(member, model)
Expand Down
4 changes: 2 additions & 2 deletions ...tware/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ private fun generateOperationShapeDocs(
val builderInputDoc = memberShape.asFluentBuilderInputDoc(symbolProvider)
val builderInputLink = docLink("$fluentBuilderFullyQualifiedName::${symbolProvider.toMemberName(memberShape)}")
val builderSetterDoc = memberShape.asFluentBuilderSetterDoc(symbolProvider)
val builderSetterLink = docLink("$fluentBuilderFullyQualifiedName::${memberShape.setterName()}")
val builderSetterLink = docLink("$fluentBuilderFullyQualifiedName::${memberShape.setterName(symbolProvider)}")

val docTrait = memberShape.getMemberTrait(model, DocumentationTrait::class.java).orNull()
val docs =
Expand Down Expand Up @@ -440,7 +440,7 @@ internal fun MemberShape.asFluentBuilderInputDoc(symbolProvider: SymbolProvider)
* _NOTE: This function generates the setter type names that appear under **"The fluent builder is configurable:"**_
*/
private fun MemberShape.asFluentBuilderSetterDoc(symbolProvider: SymbolProvider): String {
val memberName = this.setterName()
val memberName = this.setterName(symbolProvider)
val outerType = symbolProvider.toSymbol(this).rustType()

return "$memberName(${outerType.asArgumentType(fullyQualified = false)})"
Expand Down
2 changes: 1 addition & 1 deletion ...e/amazon/smithy/rust/codegen/client/smithy/generators/protocol/ProtocolParserGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ class ProtocolParserGenerator(
val member = binding.member
val parsedValue = renderBindingParser(binding, operationShape, httpBindingGenerator, structuredDataParser)
if (parsedValue != null) {
withBlock("output = output.${member.setterName()}(", ");") {
withBlock("output = output.${member.setterName(symbolProvider)}(", ");") {
parsedValue(this)
}
}
Expand Down
23 changes: 13 additions & 10 deletions ...ain/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,14 @@ class OperationBuildError(private val runtimeConfig: RuntimeConfig) {
}
}

// Setter names will never hit a reserved word and therefore never need escaping.
Copy link
Collaborator

Choose a reason for hiding this comment

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

😅

fun MemberShape.setterName() = "set_${this.memberName.toSnakeCase()}"

// Getter names will never hit a reserved word and therefore never need escaping.
fun MemberShape.getterName() = "get_${this.memberName.toSnakeCase()}"
fun MemberShape.setterName(symbolProvider: SymbolProvider): String {
val unescaped = symbolProvider.toMemberName(this).removePrefix("r##")
return "set_${unescaped}"
}
fun MemberShape.getterName(symbolProvider: SymbolProvider): String {
val unescaped = symbolProvider.toMemberName(this).removePrefix("r##")
return "get_${unescaped}"
}

class BuilderGenerator(
private val model: Model,
Expand Down Expand Up @@ -192,7 +195,7 @@ class BuilderGenerator(
val memberName = member.memberName.toSnakeCase()
val setter =
if (symbolProvider.toSymbol(member).isOptional()) {
member.setterName()
member.setterName(symbolProvider)
} else {
memberName
}
Expand Down Expand Up @@ -311,7 +314,7 @@ class BuilderGenerator(

writer.documentShape(member, model)
writer.deprecatedShape(member)
writer.rustBlock("pub fn ${member.setterName()}(mut self, input: ${inputType.render(true)}) -> Self") {
writer.rustBlock("pub fn ${member.setterName(symbolProvider)}(mut self, input: ${inputType.render(true)}) -> Self") {
rust("self.$memberName = input; self")
}
}
Expand All @@ -333,7 +336,7 @@ class BuilderGenerator(

writer.documentShape(member, model)
writer.deprecatedShape(member)
writer.rustBlock("pub fn ${member.getterName()}(&self) -> &${inputType.render(true)}") {
writer.rustBlock("pub fn ${member.getterName(symbolProvider)}(&self) -> &${inputType.render(true)}") {
rust("&self.$memberName")
}
}
Expand Down Expand Up @@ -409,7 +412,7 @@ class BuilderGenerator(
) {
docs("Appends an item to `$memberName`.")
rust("///")
docs("To override the contents of this collection use [`${member.setterName()}`](Self::${member.setterName()}).")
docs("To override the contents of this collection use [`${member.setterName(symbolProvider)}`](Self::${member.setterName(symbolProvider)}).")
rust("///")
documentShape(member, model, autoSuppressMissingDocs = false)
deprecatedShape(member)
Expand All @@ -435,7 +438,7 @@ class BuilderGenerator(
) {
docs("Adds a key-value pair to `$memberName`.")
rust("///")
docs("To override the contents of this collection use [`${member.setterName()}`](Self::${member.setterName()}).")
docs("To override the contents of this collection use [`${member.setterName(symbolProvider)}`](Self::${member.setterName(symbolProvider)}).")
rust("///")
documentShape(member, model, autoSuppressMissingDocs = false)
deprecatedShape(member)
Expand Down
4 changes: 3 additions & 1 deletion .../kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderInstantiator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,14 @@ interface BuilderInstantiator {
mapErr: Writable? = null,
): Writable

fun setterProvider(field: MemberShape): String;

/** Set a field on a builder using the `$setterName` method. $value will be passed directly. */
fun setFieldWithSetter(
builder: String,
value: Writable,
field: MemberShape,
) = writable {
rustTemplate("$builder = $builder.${field.setterName()}(#{value})", "value" to value)
rustTemplate("$builder = $builder.${setterProvider(field)}(#{value})", "value" to value)
}
}
2 changes: 1 addition & 1 deletion ...in/software/amazon/smithy/rust/codegen/core/smithy/protocols/parse/CborParserGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ class CborParserGenerator(
rustBlock("${member.memberName.dq()} =>") {
val callBuilderSetMemberFieldWritable =
writable {
withBlock("builder.${member.setterName()}(", ")") {
withBlock("builder.${member.setterName(symbolProvider)}(", ")") {
conditionalBlock("Some(", ")", shouldWrapBuilderMemberSetterInputWithOption(member)) {
val symbol = symbolProvider.toSymbol(member)
if (symbol.isRustBoxed()) {
Expand Down
4 changes: 2 additions & 2 deletions ...mazon/smithy/rust/codegen/core/smithy/protocols/parse/EventStreamUnmarshallerGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ class EventStreamUnmarshallerGenerator(
}

private fun RustWriter.renderUnmarshallEventHeader(member: MemberShape) {
withBlock("builder = builder.${member.setterName()}(", ");") {
withBlock("builder = builder.${member.setterName(symbolProvider)}(", ");") {
conditionalBlock("Some(", ")", member.isOptional) {
when (val target = model.expectShape(member.target)) {
is BooleanShape -> rustTemplate("#{expect_fns}::expect_bool(header)?", *codegenScope)
Expand Down Expand Up @@ -285,7 +285,7 @@ class EventStreamUnmarshallerGenerator(
*codegenScope,
)
}
withBlock("builder = builder.${member.setterName()}(", ");") {
withBlock("builder = builder.${member.setterName(symbolProvider)}(", ");") {
conditionalBlock("Some(", ")", member.isOptional) {
when (target) {
is BlobShape -> {
Expand Down
6 changes: 3 additions & 3 deletions ...in/software/amazon/smithy/rust/codegen/core/smithy/protocols/parse/JsonParserGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,14 +262,14 @@ class JsonParserGenerator(
rustBlock("${jsonName(member).dq()} =>") {
when (codegenTarget) {
CodegenTarget.CLIENT -> {
withBlock("builder = builder.${member.setterName()}(", ");") {
withBlock("builder = builder.${member.setterName(symbolProvider)}(", ");") {
deserializeMember(member)
}
}

CodegenTarget.SERVER -> {
if (symbolProvider.toSymbol(member).isOptional()) {
withBlock("builder = builder.${member.setterName()}(", ");") {
withBlock("builder = builder.${member.setterName(symbolProvider)}(", ");") {
deserializeMember(member)
}
} else {
Expand All @@ -278,7 +278,7 @@ class JsonParserGenerator(
rust(
"""
{
builder = builder.${member.setterName()}(v);
builder = builder.${member.setterName(symbolProvider)}(v);
}
""",
)
Expand Down
4 changes: 2 additions & 2 deletions .../amazon/smithy/rust/codegen/core/smithy/protocols/parse/XmlBindingTraitParserGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ class XmlBindingTraitParserGenerator(
Ctx(tag = decoder, accum = "$builder.${symbolProvider.toMemberName(member)}.take()"),
)
}
rust("$builder = $builder.${member.setterName()}($temp);")
rust("$builder = $builder.${member.setterName(symbolProvider)}($temp);")
}
rustTemplate(
"_ => return Err(#{XmlDecodeError}::custom(\"expected ${member.xmlName()} tag\"))",
Expand Down Expand Up @@ -333,7 +333,7 @@ class XmlBindingTraitParserGenerator(
forceOptional = true,
)
}
rust("$builder = $builder.${member.setterName()}($temp);")
rust("$builder = $builder.${member.setterName(symbolProvider)}($temp);")
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions ...in/kotlin/software/amazon/smithy/rust/codegen/core/testutil/DefaultBuilderInstantiator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.writable
import software.amazon.smithy.rust.codegen.core.smithy.RustSymbolProvider
import software.amazon.smithy.rust.codegen.core.smithy.generators.BuilderGenerator
import software.amazon.smithy.rust.codegen.core.smithy.generators.BuilderInstantiator
import software.amazon.smithy.rust.codegen.core.smithy.generators.setterName

/**
* A Default instantiator that uses `builder.build()` in all cases. This exists to support tests in codegen-core
Expand All @@ -27,6 +28,10 @@ class DefaultBuilderInstantiator(private val checkFallibleBuilder: Boolean, priv
return setFieldWithSetter(builder, value, field)
}

override fun setterProvider(field: MemberShape): String {
return field.setterName(symbolProvider)
}

override fun finalizeBuilder(
builder: String,
shape: StructureShape,
Expand Down
2 changes: 1 addition & 1 deletion ...est/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/InstantiatorTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ class InstantiatorTest {
override fun hasFallibleBuilder(shape: StructureShape) =
BuilderGenerator.hasFallibleBuilder(shape, codegenContext.symbolProvider)

override fun setterName(memberShape: MemberShape) = memberShape.setterName()
override fun setterName(memberShape: MemberShape) = memberShape.setterName(codegenContext.symbolProvider)

override fun doesSetterTakeInOption(memberShape: MemberShape) = true
}
Expand Down
5 changes: 5 additions & 0 deletions ...kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerInstantiator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.generators.BuilderInstant
import software.amazon.smithy.rust.codegen.core.smithy.generators.Instantiator
import software.amazon.smithy.rust.codegen.core.smithy.generators.InstantiatorCustomization
import software.amazon.smithy.rust.codegen.core.smithy.generators.InstantiatorSection
import software.amazon.smithy.rust.codegen.core.smithy.generators.setterName
import software.amazon.smithy.rust.codegen.core.smithy.isOptional
import software.amazon.smithy.rust.codegen.core.smithy.protocols.parse.ReturnSymbolToParse
import software.amazon.smithy.rust.codegen.server.smithy.ServerCodegenContext
Expand Down Expand Up @@ -118,6 +119,10 @@ class ServerBuilderInstantiator(
}
}

override fun setterProvider(field: MemberShape): String {
return field.setterName(symbolProvider)
}

override fun finalizeBuilder(
builder: String,
shape: StructureShape,
Expand Down
10 changes: 5 additions & 5 deletions ...re/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -818,7 +818,7 @@ class ServerHttpBoundProtocolTraitImplGenerator(
rustTemplate(
"""
if let Some(value) = #{ParsedValue:W} {
input = input.${member.setterName()}($valueToSet)
input = input.${member.setterName(symbolProvider)}($valueToSet)
}
""",
"ParsedValue" to parsedValue,
Expand Down Expand Up @@ -1044,7 +1044,7 @@ class ServerHttpBoundProtocolTraitImplGenerator(
val deserializer = generateParseStrFn(binding, true)
rustTemplate(
"""
input = input.${binding.member.setterName()}(
input = input.${binding.member.setterName(symbolProvider)}(
#{deserializer}(m$index)?
);
""",
Expand Down Expand Up @@ -1145,7 +1145,7 @@ class ServerHttpBoundProtocolTraitImplGenerator(
rustTemplate(
"""
if !${memberName}_seen && k == "${it.locationName}" {
input = input.${it.member.setterName()}(
input = input.${it.member.setterName(symbolProvider)}(
#{deserializer}(&v)?
);
${memberName}_seen = true;
Expand Down Expand Up @@ -1259,7 +1259,7 @@ class ServerHttpBoundProtocolTraitImplGenerator(
unconstrainedShapeSymbolProvider
.toSymbol(queryParamsBinding.member)
.isOptional()
withBlock("input = input.${queryParamsBinding.member.setterName()}(", ");") {
withBlock("input = input.${queryParamsBinding.member.setterName(symbolProvider)}(", ");") {
conditionalBlock("Some(", ")", conditional = isOptional) {
write("query_params")
}
Expand All @@ -1277,7 +1277,7 @@ class ServerHttpBoundProtocolTraitImplGenerator(
rustBlock("if !$memberName.is_empty()") {
withBlock(
"input = input.${
binding.member.setterName()
binding.member.setterName(symbolProvider)
}(",
");",
) {
Expand Down
Loading