Skip to content
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.

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

Make RustReservedWordsSymbolProvider configurable #2382

Merged
merged 7 commits into from
Mar 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -263,3 +263,9 @@ message = "Add more client re-exports. Specifically, it re-exports `aws_smithy_h
references = ["smithy-rs#2437", "aws-sdk-rust#600"]
meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client" }
author = "ysaito1001"

[[smithy-rs]]
message = "Smithy members named `send` were previously renamed to `send_value` at codegen time. These will now be called `send` in the generated code."
references = ["smithy-rs#2382"]
meta = { "breaking" = true, "tada" = false, "bug" = true, "target" = "server" }
author = "jdisanti"
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

package software.amazon.smithy.rust.codegen.client.smithy

import software.amazon.smithy.rust.codegen.core.rustlang.RustReservedWordConfig
import software.amazon.smithy.rust.codegen.core.smithy.generators.StructureGenerator
import software.amazon.smithy.rust.codegen.core.smithy.generators.UnionGenerator

val ClientReservedWords = RustReservedWordConfig(
structureMemberMap = StructureGenerator.structureMemberNameMap +
mapOf(
"send" to "send_value",
// To avoid conflicts with the `make_operation` and `presigned` functions on generated inputs
"make_operation" to "make_operation_value",
"presigned" to "presigned_value",
"customize" to "customize_value",
// To avoid conflicts with the error metadata `meta` field
"meta" to "meta_value",
),
unionMemberMap = mapOf(
// Unions contain an `Unknown` variant. This exists to support parsing data returned from the server
// that represent union variants that have been added since this SDK was generated.
UnionGenerator.UnknownVariantName to "${UnionGenerator.UnknownVariantName}Value",
"${UnionGenerator.UnknownVariantName}Value" to "${UnionGenerator.UnknownVariantName}Value_",
),
enumMemberMap = mapOf(
// Unknown is used as the name of the variant containing unexpected values
"Unknown" to "UnknownValue",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

From a comment on #2377, these should have constants.

// Real models won't end in `_` so it's safe to stop here
"UnknownValue" to "UnknownValue_",
),
)
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class RustClientCodegenPlugin : ClientDecoratableBuildPlugin() {
.let { StreamingShapeMetadataProvider(it) }
// Rename shapes that clash with Rust reserved words & and other SDK specific features e.g. `send()` cannot
// be the name of an operation input
.let { RustReservedWordSymbolProvider(it) }
.let { RustReservedWordSymbolProvider(it, ClientReservedWords) }
// Allows decorators to inject a custom symbol provider
.let { codegenDecorator.symbolProvider(it) }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,23 @@ import software.amazon.smithy.model.shapes.UnionShape
import software.amazon.smithy.model.traits.EnumTrait
import software.amazon.smithy.rust.codegen.core.smithy.RustSymbolProvider
import software.amazon.smithy.rust.codegen.core.smithy.WrappingSymbolProvider
import software.amazon.smithy.rust.codegen.core.smithy.generators.UnionGenerator
import software.amazon.smithy.rust.codegen.core.smithy.renamedFrom
import software.amazon.smithy.rust.codegen.core.util.hasTrait
import software.amazon.smithy.rust.codegen.core.util.letIf

class RustReservedWordSymbolProvider(private val base: RustSymbolProvider) : WrappingSymbolProvider(base) {
data class RustReservedWordConfig(
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: RustReservedWordsConfig to match the rest of the uses?

/** Map of struct member names that should get renamed */
val structureMemberMap: Map<String, String>,
/** Map of union member names that should get renamed */
val unionMemberMap: Map<String, String>,
/** Map of enum member names that should get renamed */
val enumMemberMap: Map<String, String>,
)

class RustReservedWordSymbolProvider(
private val base: RustSymbolProvider,
private val reservedWordConfig: RustReservedWordConfig,
) : WrappingSymbolProvider(base) {
private val internal =
ReservedWordSymbolProvider.builder().symbolProvider(base)
.nameReservedWords(RustReservedWords)
Expand All @@ -33,35 +44,14 @@ class RustReservedWordSymbolProvider(private val base: RustSymbolProvider) : Wra
val reservedWordReplacedName = internal.toMemberName(shape)
val container = model.expectShape(shape.container)
return when {
container is StructureShape -> when (baseName) {
"build" -> "build_value"
"builder" -> "builder_value"
"default" -> "default_value"
"send" -> "send_value"
// To avoid conflicts with the `make_operation` and `presigned` functions on generated inputs
"make_operation" -> "make_operation_value"
"presigned" -> "presigned_value"
"customize" -> "customize_value"
// To avoid conflicts with the error metadata `meta` field
"meta" -> "meta_value"
else -> reservedWordReplacedName
}
container is StructureShape ->
reservedWordConfig.structureMemberMap.getOrDefault(baseName, reservedWordReplacedName)

container is UnionShape -> when (baseName) {
// Unions contain an `Unknown` variant. This exists to support parsing data returned from the server
// that represent union variants that have been added since this SDK was generated.
UnionGenerator.UnknownVariantName -> "${UnionGenerator.UnknownVariantName}Value"
"${UnionGenerator.UnknownVariantName}Value" -> "${UnionGenerator.UnknownVariantName}Value_"
else -> reservedWordReplacedName
}
container is UnionShape ->
reservedWordConfig.unionMemberMap.getOrDefault(baseName, reservedWordReplacedName)

container is EnumShape || container.hasTrait<EnumTrait>() -> when (baseName) {
// Unknown is used as the name of the variant containing unexpected values
"Unknown" -> "UnknownValue"
// Real models won't end in `_` so it's safe to stop here
"UnknownValue" -> "UnknownValue_"
else -> reservedWordReplacedName
}
container is EnumShape || container.hasTrait<EnumTrait>() ->
reservedWordConfig.enumMemberMap.getOrDefault(baseName, reservedWordReplacedName)

else -> error("unexpected container: $container")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,15 @@ open class StructureGenerator(
private val shape: StructureShape,
private val customizations: List<StructureCustomization>,
) {
companion object {
/** Reserved struct member names */
val structureMemberNameMap: Map<String, String> = mapOf(
"build" to "build_value",
"builder" to "builder_value",
"default" to "default_value",
)
}

private val errorTrait = shape.getTrait<ErrorTrait>()
protected val members: List<MemberShape> = shape.allMembers.values.toList()
private val accessorMembers: List<MemberShape> = when (errorTrait) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import software.amazon.smithy.model.shapes.UnionShape
import software.amazon.smithy.model.traits.ErrorTrait
import software.amazon.smithy.rust.codegen.core.rustlang.Attribute
import software.amazon.smithy.rust.codegen.core.rustlang.RustModule
import software.amazon.smithy.rust.codegen.core.rustlang.RustReservedWordConfig
import software.amazon.smithy.rust.codegen.core.rustlang.RustReservedWordSymbolProvider
import software.amazon.smithy.rust.codegen.core.rustlang.RustReservedWords
import software.amazon.smithy.rust.codegen.core.rustlang.Visibility
Expand Down Expand Up @@ -138,13 +139,21 @@ fun String.asSmithyModel(sourceLocation: String? = null, smithyVersion: String =
}

// Intentionally only visible to codegen-core since the other modules have their own symbol providers
internal fun testSymbolProvider(model: Model): RustSymbolProvider = SymbolVisitor(
internal fun testSymbolProvider(
model: Model,
rustReservedWordConfig: RustReservedWordConfig? = null,
): RustSymbolProvider = SymbolVisitor(
testRustSettings(),
model,
ServiceShape.builder().version("test").id("test#Service").build(),
TestRustSymbolProviderConfig,
).let { BaseSymbolMetadataProvider(it, additionalAttributes = listOf(Attribute.NonExhaustive)) }
.let { RustReservedWordSymbolProvider(it) }
.let {
RustReservedWordSymbolProvider(
it,
rustReservedWordConfig ?: RustReservedWordConfig(emptyMap(), emptyMap(), emptyMap()),
)
}

// Intentionally only visible to codegen-core since the other modules have their own contexts
internal fun testCodegenContext(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,108 @@ import software.amazon.smithy.rust.codegen.core.util.lookup
internal class RustReservedWordSymbolProviderTest {
Copy link
Contributor

Choose a reason for hiding this comment

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

Great tests.

private class TestSymbolProvider(model: Model) :
WrappingSymbolProvider(SymbolVisitor(testRustSettings(), model, null, TestRustSymbolProviderConfig))
private val emptyConfig = RustReservedWordConfig(emptyMap(), emptyMap(), emptyMap())

@Test
fun `structs are escaped`() {
val model = """
namespace test
structure Self {}
""".asSmithyModel()
val provider = RustReservedWordSymbolProvider(TestSymbolProvider(model))
val provider = RustReservedWordSymbolProvider(TestSymbolProvider(model), emptyConfig)
val symbol = provider.toSymbol(model.lookup("test#Self"))
symbol.name shouldBe "SelfValue"
}

private fun mappingTest(config: RustReservedWordConfig, model: Model, id: String, test: (String) -> Unit) {
val provider = RustReservedWordSymbolProvider(TestSymbolProvider(model), config)
val symbol = provider.toMemberName(model.lookup("test#Container\$$id"))
test(symbol)
}

@Test
fun `structs member names are mapped via config`() {
val config = emptyConfig.copy(
structureMemberMap = mapOf(
"name_to_map" to "mapped_name",
"NameToMap" to "MappedName",
),
)
var model = """
namespace test
structure Container {
name_to_map: String
}
""".asSmithyModel()
mappingTest(config, model, "name_to_map") { memberName ->
memberName shouldBe "mapped_name"
}

model = """
namespace test
enum Container {
NameToMap = "NameToMap"
}
""".asSmithyModel(smithyVersion = "2.0")
mappingTest(config, model, "NameToMap") { memberName ->
// Container was not a struct, so the field keeps its old name
memberName shouldBe "NameToMap"
}

model = """
namespace test
union Container {
NameToMap: String
}
""".asSmithyModel()
mappingTest(config, model, "NameToMap") { memberName ->
// Container was not a struct, so the field keeps its old name
memberName shouldBe "NameToMap"
}
}

@Test
fun `union member names are mapped via config`() {
val config = emptyConfig.copy(
unionMemberMap = mapOf(
"name_to_map" to "mapped_name",
"NameToMap" to "MappedName",
),
)

var model = """
namespace test
union Container {
NameToMap: String
}
""".asSmithyModel()
mappingTest(config, model, "NameToMap") { memberName ->
memberName shouldBe "MappedName"
}

model = """
namespace test
structure Container {
name_to_map: String
}
""".asSmithyModel()
mappingTest(config, model, "name_to_map") { memberName ->
// Container was not a union, so the field keeps its old name
memberName shouldBe "name_to_map"
}

model = """
namespace test
enum Container {
NameToMap = "NameToMap"
}
""".asSmithyModel(smithyVersion = "2.0")
mappingTest(config, model, "NameToMap") { memberName ->
// Container was not a union, so the field keeps its old name
memberName shouldBe "NameToMap"
}
}

@Test
fun `member names are escaped`() {
val model = """
Expand All @@ -42,7 +132,7 @@ internal class RustReservedWordSymbolProviderTest {
async: String
}
""".asSmithyModel()
val provider = RustReservedWordSymbolProvider(TestSymbolProvider(model))
val provider = RustReservedWordSymbolProvider(TestSymbolProvider(model), emptyConfig)
provider.toMemberName(
MemberShape.builder().id("namespace#container\$async").target("namespace#Integer").build(),
) shouldBe "r##async"
Expand All @@ -58,7 +148,15 @@ internal class RustReservedWordSymbolProviderTest {
namespace foo
@enum([{ name: "dontcare", value: "dontcare" }]) string Container
""".asSmithyModel()
val provider = RustReservedWordSymbolProvider(TestSymbolProvider(model))
val provider = RustReservedWordSymbolProvider(
TestSymbolProvider(model),
reservedWordConfig = emptyConfig.copy(
enumMemberMap = mapOf(
"Unknown" to "UnknownValue",
"UnknownValue" to "UnknownValue_",
),
),
)

fun expectEnumRename(original: String, expected: MaybeRenamed) {
val symbol = provider.toSymbol(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.StringShape
import software.amazon.smithy.model.traits.EnumTrait
import software.amazon.smithy.rust.codegen.core.rustlang.Attribute.Companion.AllowDeprecated
import software.amazon.smithy.rust.codegen.core.rustlang.RustReservedWordConfig
import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter
import software.amazon.smithy.rust.codegen.core.rustlang.Writable
import software.amazon.smithy.rust.codegen.core.rustlang.rust
Expand All @@ -30,6 +31,12 @@ import software.amazon.smithy.rust.codegen.core.util.lookup
import software.amazon.smithy.rust.codegen.core.util.orNull

class EnumGeneratorTest {
private val rustReservedWordConfig = RustReservedWordConfig(
enumMemberMap = mapOf("Unknown" to "UnknownValue"),
structureMemberMap = emptyMap(),
unionMemberMap = emptyMap(),
)

@Nested
inner class EnumMemberModelTests {
private val testModel = """
Expand All @@ -47,7 +54,7 @@ class EnumGeneratorTest {
])
string EnumWithUnknown
""".asSmithyModel()
private val symbolProvider = testSymbolProvider(testModel)
private val symbolProvider = testSymbolProvider(testModel, rustReservedWordConfig = rustReservedWordConfig)

private val enumTrait = testModel.lookup<StringShape>("test#EnumWithUnknown").expectTrait<EnumTrait>()

Expand Down Expand Up @@ -276,7 +283,7 @@ class EnumGeneratorTest {
""".asSmithyModel()

val shape = model.lookup<StringShape>("test#SomeEnum")
val provider = testSymbolProvider(model)
val provider = testSymbolProvider(model, rustReservedWordConfig = rustReservedWordConfig)
val project = TestWorkspace.testProject(provider)
project.moduleFor(shape) {
renderEnum(model, provider, shape)
Expand Down
Loading