-
Notifications
You must be signed in to change notification settings - Fork 195
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
Changes from 6 commits
d3c95a2
c1add65
3337f93
4f59076
bedd1f8
4097d45
a88b847
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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( | ||
structMemberMap = StructureGenerator.structMemberMap + | ||
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", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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( | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: |
||||||||||||||
/** Map of struct member names that should get renamed */ | ||||||||||||||
val structMemberMap: 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) | ||||||||||||||
|
@@ -33,34 +44,19 @@ 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 -> when (val mapped = reservedWordConfig.structMemberMap[baseName]) { | ||||||||||||||
null -> reservedWordReplacedName | ||||||||||||||
else -> mapped | ||||||||||||||
} | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
These can be slightly simplified. |
||||||||||||||
|
||||||||||||||
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 -> when (val mapped = reservedWordConfig.unionMemberMap[baseName]) { | ||||||||||||||
null -> reservedWordReplacedName | ||||||||||||||
else -> mapped | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
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>() -> when (val mapped = reservedWordConfig.enumMemberMap[baseName]) { | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmmm... Yeah, I think this is dead code. |
||||||||||||||
null -> reservedWordReplacedName | ||||||||||||||
else -> mapped | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
else -> error("unexpected container: $container") | ||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,18 +22,108 @@ import software.amazon.smithy.rust.codegen.core.util.lookup | |
internal class RustReservedWordSymbolProviderTest { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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( | ||
structMemberMap = 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 = """ | ||
|
@@ -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" | ||
|
@@ -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( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I consciously make the effort to use "structure" to refer to Smithy structure shapes, and use struct only to refer to Rust
struct
s.