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.

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

Migrate to EnumShape #1932

Closed
wants to merge 6 commits into from
Closed

Migrate to EnumShape #1932

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8e10f8e
ModelTransformer for EnumShape
82marbag Nov 18, 2022
5d09638
Update EnumGenerator
82marbag Nov 18, 2022
bd524e8
Remove EnumTrait
82marbag Nov 18, 2022
3df6ad2
Fix tests
82marbag Nov 18, 2022
077d44d
Address comments
82marbag Dec 1, 2022
e2c0d14
Merge branch 'main' into 1671
82marbag Dec 1, 2022
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
23 changes: 17 additions & 6 deletions ...lient/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/CodegenVisitor.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@
package software.amazon.smithy.rust.codegen.client.smithy

import software.amazon.smithy.build.PluginContext
import software.amazon.smithy.codegen.core.CodegenException
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.knowledge.NullableIndex
import software.amazon.smithy.model.neighbor.Walker
import software.amazon.smithy.model.shapes.EnumShape
import software.amazon.smithy.model.shapes.ServiceShape
import software.amazon.smithy.model.shapes.Shape
import software.amazon.smithy.model.shapes.ShapeVisitor
Expand Down Expand Up @@ -39,7 +41,6 @@ import software.amazon.smithy.rust.codegen.core.smithy.transformers.EventStreamN
import software.amazon.smithy.rust.codegen.core.smithy.transformers.OperationNormalizer
import software.amazon.smithy.rust.codegen.core.smithy.transformers.RecursiveShapeBoxer
import software.amazon.smithy.rust.codegen.core.util.CommandFailed
import software.amazon.smithy.rust.codegen.core.util.getTrait
import software.amazon.smithy.rust.codegen.core.util.hasTrait
import software.amazon.smithy.rust.codegen.core.util.letIf
import software.amazon.smithy.rust.codegen.core.util.runCommand
Expand Down Expand Up @@ -106,6 +107,8 @@ class CodegenVisitor(
*/
internal fun baselineTransform(model: Model) =
model
// Convert string shapes with an @enum trait to an enum shape
.let { ModelTransformer.create().changeStringEnumsToEnumShapes(model, true) }
82marbag marked this conversation as resolved.
Show resolved Hide resolved
// Flattens mixins out of the model and removes them from the model
.let { ModelTransformer.create().flattenAndRemoveMixins(it) }
// Add errors attached at the service level to the models
Expand Down Expand Up @@ -206,13 +209,21 @@ class CodegenVisitor(
/**
* String Shape Visitor
*
* Although raw strings require no code generation, enums are actually `EnumTrait` applied to string shapes.
* Unnamed @enum shapes are not supported. If they could not be converted to EnumShape, this will fail.
*/
override fun stringShape(shape: StringShape) {
shape.getTrait<EnumTrait>()?.also { enum ->
rustCrate.useShapeWriter(shape) {
EnumGenerator(model, symbolProvider, this, shape, enum).render()
}
if (shape.hasTrait<EnumTrait>()) {
throw CodegenException("Unnamed @enum shapes are unsupported: $shape")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this error message accurate? My understanding from looking at this is that the model transformer might fail to convert an unnamed enum if e.g. one of its values cannot be safely synthesized into a valid name. So it's not like we don't support unnamed @enum shapes entirely, just the ones that we failed to convert when synthesizeEnumNames is set, right?. Aside: is that flag enabled by default?

If the above is correct, I'd fail here with a more informative error message, perhaps instructing the user to look in the log output above for the INFO messages that the transformer from awslabs/smithy printed indicating the exact reason why the enum could not be converted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, I've changed to a better log message, thanks!

}
super.stringShape(shape)
}

/**
* Enum Shape Visitor
*/
override fun enumShape(shape: EnumShape) {
rustCrate.useShapeWriter(shape) {
EnumGenerator(model, symbolProvider, this, shape).render()
}
}

Expand Down
39 changes: 39 additions & 0 deletions ...t/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/CodegenVisitorTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package software.amazon.smithy.rust.codegen.client.smithy
import io.kotest.matchers.shouldBe
import org.junit.jupiter.api.Test
import software.amazon.smithy.model.shapes.ShapeId
import software.amazon.smithy.model.traits.EnumTrait
import software.amazon.smithy.rust.codegen.client.smithy.customizations.ClientCustomizations
import software.amazon.smithy.rust.codegen.client.smithy.customize.CombinedCodegenDecorator
import software.amazon.smithy.rust.codegen.client.smithy.customize.NoOpEventStreamSigningDecorator
Expand Down Expand Up @@ -62,4 +63,42 @@ class CodegenVisitorTest {
val baselineModel = visitor.baselineTransform(model)
baselineModel.getShapesWithTrait(ShapeId.from("smithy.api#mixin")).isEmpty() shouldBe true
}

@Test
fun `baseline transform verify string enum converted to EnumShape`() {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I'd add a negative test too that asserts we throw an exception when an unnamed enum could not be converted to an enum shape.

val model = """
namespace com.example
use aws.protocols#restJson1
@restJson1
service Example {
operations: [ BasicOperation ]
}
operation BasicOperation {
input: Shape
}
structure Shape {
enum: BasicEnum
}
@enum([
{
value: "a0",
},
{
value: "a1",
}
])
string BasicEnum
""".asSmithyModel(smithyVersion = "2.0")
82marbag marked this conversation as resolved.
Show resolved Hide resolved
val (ctx, testDir) = generatePluginContext(model)
82marbag marked this conversation as resolved.
Show resolved Hide resolved
testDir.resolve("src").createDirectory()
testDir.resolve("src/main.rs").writeText("fn main() {}")
val codegenDecorator =
CombinedCodegenDecorator.fromClasspath(
ctx,
ClientCustomizations(),
)
val visitor = CodegenVisitor(ctx, codegenDecorator)
val baselineModel = visitor.baselineTransform(model)
baselineModel.getShapesWithTrait(EnumTrait.ID).isEmpty() shouldBe true
}
}
32 changes: 3 additions & 29 deletions ...in/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientInstantiatorTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ package software.amazon.smithy.rust.codegen.client.smithy.generators

import org.junit.jupiter.api.Test
import software.amazon.smithy.model.node.Node
import software.amazon.smithy.model.shapes.StringShape
import software.amazon.smithy.model.shapes.EnumShape
import software.amazon.smithy.rust.codegen.client.testutil.testCodegenContext
import software.amazon.smithy.rust.codegen.core.rustlang.RustModule
import software.amazon.smithy.rust.codegen.core.rustlang.rust
Expand All @@ -18,19 +18,12 @@ import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel
import software.amazon.smithy.rust.codegen.core.testutil.compileAndTest
import software.amazon.smithy.rust.codegen.core.testutil.unitTest
import software.amazon.smithy.rust.codegen.core.util.dq
import software.amazon.smithy.rust.codegen.core.util.expectTrait
import software.amazon.smithy.rust.codegen.core.util.lookup

internal class ClientInstantiatorTest {
private val model = """
namespace com.test
@enum([
{ value: "t2.nano" },
{ value: "t2.micro" },
])
string UnnamedEnum
@enum([
{
value: "t2.nano",
Expand All @@ -49,13 +42,13 @@ internal class ClientInstantiatorTest {

@Test
fun `generate named enums`() {
val shape = model.lookup<StringShape>("com.test#NamedEnum")
val shape = model.lookup<EnumShape>("com.test#NamedEnum")
val sut = clientInstantiator(codegenContext)
val data = Node.parse("t2.nano".dq())

val project = TestWorkspace.testProject()
project.withModule(RustModule.Model) {
EnumGenerator(model, symbolProvider, this, shape, shape.expectTrait()).render()
EnumGenerator(model, symbolProvider, this, shape).render()
unitTest("generate_named_enums") {
withBlock("let result = ", ";") {
sut.render(this, shape, data)
Expand All @@ -65,23 +58,4 @@ internal class ClientInstantiatorTest {
}
project.compileAndTest()
}

@Test
fun `generate unnamed enums`() {
val shape = model.lookup<StringShape>("com.test#UnnamedEnum")
val sut = clientInstantiator(codegenContext)
val data = Node.parse("t2.nano".dq())

val project = TestWorkspace.testProject()
project.withModule(RustModule.Model) {
EnumGenerator(model, symbolProvider, this, shape, shape.expectTrait()).render()
unitTest("generate_unnamed_enums") {
withBlock("let result = ", ";") {
sut.render(this, shape, data)
}
rust("""assert_eq!(result, UnnamedEnum("t2.nano".to_owned()));""")
}
}
project.compileAndTest()
}
}
8 changes: 3 additions & 5 deletions ...src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,17 @@ package software.amazon.smithy.rust.codegen.core.smithy
import software.amazon.smithy.codegen.core.CodegenException
import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.EnumShape
import software.amazon.smithy.model.shapes.MemberShape
import software.amazon.smithy.model.shapes.Shape
import software.amazon.smithy.model.shapes.StringShape
import software.amazon.smithy.model.shapes.StructureShape
import software.amazon.smithy.model.shapes.UnionShape
import software.amazon.smithy.model.traits.EnumDefinition
import software.amazon.smithy.model.traits.EnumTrait
import software.amazon.smithy.model.traits.StreamingTrait
import software.amazon.smithy.rust.codegen.core.rustlang.Attribute
import software.amazon.smithy.rust.codegen.core.rustlang.RustMetadata
import software.amazon.smithy.rust.codegen.core.rustlang.Visibility
import software.amazon.smithy.rust.codegen.core.util.hasTrait

/**
* Default delegator to enable easily decorating another symbol provider.
Expand Down Expand Up @@ -54,9 +53,8 @@ abstract class SymbolMetadataProvider(private val base: RustSymbolProvider) : Wr
is MemberShape -> memberMeta(shape)
is StructureShape -> structureMeta(shape)
is UnionShape -> unionMeta(shape)
is StringShape -> if (shape.hasTrait<EnumTrait>()) {
enumMeta(shape)
} else null
is EnumShape -> enumMeta(shape)
Copy link
Contributor

Choose a reason for hiding this comment

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

We can now make enumMeta accept EnumShape instead of StringShape.

is StringShape -> null

else -> null
}
Expand Down
13 changes: 5 additions & 8 deletions ...gen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolVisitor.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import software.amazon.smithy.model.shapes.BooleanShape
import software.amazon.smithy.model.shapes.ByteShape
import software.amazon.smithy.model.shapes.DocumentShape
import software.amazon.smithy.model.shapes.DoubleShape
import software.amazon.smithy.model.shapes.EnumShape
import software.amazon.smithy.model.shapes.FloatShape
import software.amazon.smithy.model.shapes.IntegerShape
import software.amazon.smithy.model.shapes.ListShape
Expand All @@ -36,7 +37,6 @@ import software.amazon.smithy.model.shapes.StructureShape
import software.amazon.smithy.model.shapes.TimestampShape
import software.amazon.smithy.model.shapes.UnionShape
import software.amazon.smithy.model.traits.EnumDefinition
import software.amazon.smithy.model.traits.EnumTrait
import software.amazon.smithy.model.traits.ErrorTrait
import software.amazon.smithy.rust.codegen.core.rustlang.RustType
import software.amazon.smithy.rust.codegen.core.rustlang.stripOuter
Expand Down Expand Up @@ -271,14 +271,11 @@ open class SymbolVisitor(
override fun longShape(shape: LongShape): Symbol = simpleShape(shape)
override fun floatShape(shape: FloatShape): Symbol = simpleShape(shape)
override fun doubleShape(shape: DoubleShape): Symbol = simpleShape(shape)
override fun stringShape(shape: StringShape): Symbol {
return if (shape.hasTrait<EnumTrait>()) {
val rustType = RustType.Opaque(shape.contextName(serviceShape).toPascalCase())
symbolBuilder(shape, rustType).locatedIn(Models).build()
} else {
simpleShape(shape)
}
override fun enumShape(shape: EnumShape): Symbol {
val rustType = RustType.Opaque(shape.contextName(serviceShape).toPascalCase())
return symbolBuilder(shape, rustType).locatedIn(Models).build()
}
override fun stringShape(shape: StringShape): Symbol = simpleShape(shape)

override fun listShape(shape: ListShape): Symbol {
val inner = this.toSymbol(shape.member)
Expand Down
53 changes: 31 additions & 22 deletions ...c/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ package software.amazon.smithy.rust.codegen.core.smithy.generators

import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.StringShape
import software.amazon.smithy.model.shapes.EnumShape
import software.amazon.smithy.model.traits.DeprecatedTrait
import software.amazon.smithy.model.traits.DocumentationTrait
import software.amazon.smithy.model.traits.EnumDefinition
import software.amazon.smithy.model.traits.EnumTrait
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.RustWriter
Expand All @@ -29,6 +29,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.expectRustMetadata
import software.amazon.smithy.rust.codegen.core.util.doubleQuote
import software.amazon.smithy.rust.codegen.core.util.dq
import software.amazon.smithy.rust.codegen.core.util.getTrait
import software.amazon.smithy.rust.codegen.core.util.hasTrait
import software.amazon.smithy.rust.codegen.core.util.orNull

/** Model that wraps [EnumDefinition] to calculate and cache values required to generate the Rust enum source. */
Expand Down Expand Up @@ -86,14 +87,26 @@ open class EnumGenerator(
private val model: Model,
private val symbolProvider: RustSymbolProvider,
private val writer: RustWriter,
protected val shape: StringShape,
protected val enumTrait: EnumTrait,
protected val shape: EnumShape,
) {
protected val symbol: Symbol = symbolProvider.toSymbol(shape)
protected val enumName: String = symbol.name
protected val meta = symbol.expectRustMetadata()
protected val sortedMembers: List<EnumMemberModel> =
enumTrait.values.sortedBy { it.value }.map { EnumMemberModel(it, symbolProvider) }
shape.allMembers.entries.sortedBy { it.key }.map {
val builder = EnumDefinition.builder()
.name(it.key)
.value(shape.enumValues[it.key])
.deprecated(it.value.getTrait<DeprecatedTrait>() != null)
82marbag marked this conversation as resolved.
Show resolved Hide resolved
.tags(it.value.tags)
if (it.value.hasTrait<DocumentationTrait>()) {
builder.documentation(it.value.expectTrait(DocumentationTrait::class.java).value)
}
EnumMemberModel(
builder.build(),
symbolProvider,
)
}
protected open var target: CodegenTarget = CodegenTarget.CLIENT

companion object {
Expand All @@ -108,24 +121,20 @@ open class EnumGenerator(
}

open fun render() {
if (enumTrait.hasNames()) {
// pub enum Blah { V1, V2, .. }
renderEnum()
writer.insertTrailingNewline()
// impl From<str> for Blah { ... }
renderFromForStr()
// impl FromStr for Blah { ... }
renderFromStr()
writer.insertTrailingNewline()
// impl Blah { pub fn as_str(&self) -> &str
implBlock()
writer.rustBlock("impl AsRef<str> for $enumName") {
rustBlock("fn as_ref(&self) -> &str") {
rust("self.as_str()")
}
// pub enum Blah { V1, V2, .. }
renderEnum()
writer.insertTrailingNewline()
// impl From<str> for Blah { ... }
renderFromForStr()
// impl FromStr for Blah { ... }
renderFromStr()
writer.insertTrailingNewline()
// impl Blah { pub fn as_str(&self) -> &str
implBlock()
writer.rustBlock("impl AsRef<str> for $enumName") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that we're touching this, we can rewrite it to make use of a single rust() call.

rustBlock("fn as_ref(&self) -> &str") {
rust("self.as_str()")
}
} else {
renderUnnamedEnum()
82marbag marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
4 changes: 2 additions & 2 deletions ...rc/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/Instantiator.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.model.shapes.BlobShape
import software.amazon.smithy.model.shapes.BooleanShape
import software.amazon.smithy.model.shapes.CollectionShape
import software.amazon.smithy.model.shapes.DocumentShape
import software.amazon.smithy.model.shapes.EnumShape
import software.amazon.smithy.model.shapes.ListShape
import software.amazon.smithy.model.shapes.MapShape
import software.amazon.smithy.model.shapes.MemberShape
Expand All @@ -28,7 +29,6 @@ import software.amazon.smithy.model.shapes.StringShape
import software.amazon.smithy.model.shapes.StructureShape
import software.amazon.smithy.model.shapes.TimestampShape
import software.amazon.smithy.model.shapes.UnionShape
import software.amazon.smithy.model.traits.EnumTrait
import software.amazon.smithy.model.traits.HttpPrefixHeadersTrait
import software.amazon.smithy.model.traits.StreamingTrait
import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency
Expand Down Expand Up @@ -279,7 +279,7 @@ open class Instantiator(

private fun renderString(writer: RustWriter, shape: StringShape, arg: StringNode) {
82marbag marked this conversation as resolved.
Show resolved Hide resolved
val data = writer.escape(arg.value).dq()
if (!shape.hasTrait<EnumTrait>()) {
if (shape !is EnumShape) {
82marbag marked this conversation as resolved.
Show resolved Hide resolved
writer.rust("$data.to_owned()")
} else {
val enumSymbol = symbolProvider.toSymbol(shape)
Expand Down
4 changes: 2 additions & 2 deletions ...n/software/amazon/smithy/rust/codegen/core/smithy/generators/http/HttpBindingGenerator.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.model.knowledge.HttpBindingIndex
import software.amazon.smithy.model.shapes.BlobShape
import software.amazon.smithy.model.shapes.CollectionShape
import software.amazon.smithy.model.shapes.DocumentShape
import software.amazon.smithy.model.shapes.EnumShape
import software.amazon.smithy.model.shapes.ListShape
import software.amazon.smithy.model.shapes.MapShape
import software.amazon.smithy.model.shapes.MemberShape
Expand All @@ -22,7 +23,6 @@ import software.amazon.smithy.model.shapes.SimpleShape
import software.amazon.smithy.model.shapes.StringShape
import software.amazon.smithy.model.shapes.StructureShape
import software.amazon.smithy.model.shapes.UnionShape
import software.amazon.smithy.model.traits.EnumTrait
import software.amazon.smithy.model.traits.MediaTypeTrait
import software.amazon.smithy.model.traits.TimestampFormatTrait
import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency
Expand Down Expand Up @@ -303,7 +303,7 @@ class HttpBindingGenerator(
rust("let body_str = std::str::from_utf8(body)?;")
}
}
if (targetShape.hasTrait<EnumTrait>()) {
if (targetShape is EnumShape) {
if (codegenTarget == CodegenTarget.SERVER) {
rust(
"Ok(#T::try_from(body_str)?)",
Expand Down
Loading