From 885eb9d31d02c5c59cc19179a7f30e9da87593c7 Mon Sep 17 00:00:00 2001 From: david-perez Date: Tue, 17 Jan 2023 13:45:16 +0100 Subject: [PATCH 01/13] Derive `Eq` and `Hash` --- CHANGELOG.next.toml | 4 +- .../client/smithy/RustClientCodegenPlugin.kt | 2 +- .../smithy/StreamingTraitSymbolProvider.kt | 21 +-- .../core/smithy/SymbolMetadataProvider.kt | 120 ++++++++++-------- .../smithy/PythonServerSymbolProvider.kt | 17 ++- .../ConstrainedShapeSymbolMetadataProvider.kt | 50 ++++++++ .../DeriveEqAndHashSymbolMetadataProvider.kt | 69 ++++++++++ .../PubCrateConstrainedShapeSymbolProvider.kt | 3 - .../server/smithy/RustCodegenServerPlugin.kt | 6 +- .../generators/ConstrainedBlobGenerator.kt | 3 +- .../ConstrainedCollectionGenerator.kt | 47 ++++--- .../generators/ConstrainedMapGenerator.kt | 16 +-- .../generators/ConstrainedNumberGenerator.kt | 1 + .../generators/ConstrainedStringGenerator.kt | 17 +-- .../PubCrateConstrainedCollectionGenerator.kt | 41 +++--- .../PubCrateConstrainedMapGenerator.kt | 49 ++++--- .../UnconstrainedCollectionGenerator.kt | 33 +++-- .../generators/UnconstrainedMapGenerator.kt | 58 +++++++-- .../aws-smithy-types/src/date_time/mod.rs | 2 +- rust-runtime/aws-smithy-types/src/lib.rs | 2 +- 20 files changed, 384 insertions(+), 177 deletions(-) create mode 100644 codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolMetadataProvider.kt create mode 100644 codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProvider.kt diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index fc4c4c2578..e3e0ea7994 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -9,4 +9,6 @@ # message = "Fix typos in module documentation for generated crates" # references = ["smithy-rs#920"] # meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"} -# author = "rcoh" \ No newline at end of file +# author = "rcoh" + +TODO DateTime, Blob now implement PartialEq, Hash diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/RustClientCodegenPlugin.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/RustClientCodegenPlugin.kt index 8750fa0c96..0189a4cb65 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/RustClientCodegenPlugin.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/RustClientCodegenPlugin.kt @@ -80,7 +80,7 @@ class RustClientCodegenPlugin : DecoratableBuildPlugin() { .let { StreamingShapeSymbolProvider(it, model) } // Add Rust attributes (like `#[derive(PartialEq)]`) to generated shapes .let { BaseSymbolMetadataProvider(it, model, additionalAttributes = listOf(NonExhaustive)) } - // Streaming shapes need different derives (e.g. they cannot derive Eq) + // Streaming shapes need different derives (e.g. they cannot derive `PartialEq`) .let { StreamingShapeMetadataProvider(it, model) } // Rename shapes that clash with Rust reserved words & and other SDK specific features e.g. `send()` cannot // be the name of an operation input diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/StreamingTraitSymbolProvider.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/StreamingTraitSymbolProvider.kt index 2e91ee5fba..34976309c5 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/StreamingTraitSymbolProvider.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/StreamingTraitSymbolProvider.kt @@ -8,7 +8,10 @@ package software.amazon.smithy.rust.codegen.core.smithy import software.amazon.smithy.codegen.core.Symbol import software.amazon.smithy.model.Model import software.amazon.smithy.model.shapes.BlobShape +import software.amazon.smithy.model.shapes.ListShape +import software.amazon.smithy.model.shapes.MapShape import software.amazon.smithy.model.shapes.MemberShape +import software.amazon.smithy.model.shapes.NumberShape import software.amazon.smithy.model.shapes.Shape import software.amazon.smithy.model.shapes.StringShape import software.amazon.smithy.model.shapes.StructureShape @@ -27,7 +30,7 @@ class StreamingShapeSymbolProvider(private val base: RustSymbolProvider, private WrappingSymbolProvider(base) { override fun toSymbol(shape: Shape): Symbol { val initial = base.toSymbol(shape) - // We are only targetting member shapes + // We are only targeting member shapes if (shape !is MemberShape) { return initial } @@ -49,7 +52,7 @@ class StreamingShapeSymbolProvider(private val base: RustSymbolProvider, private } /** - * SymbolProvider to drop the clone and PartialEq bounds in streaming shapes + * SymbolProvider to drop the `Clone` and `PartialEq` bounds in streaming shapes. * * Streaming shapes cannot be cloned and equality cannot be checked without reading the body. Because of this, these shapes * do not implement `Clone` or `PartialEq`. @@ -60,10 +63,6 @@ class StreamingShapeMetadataProvider( private val base: RustSymbolProvider, private val model: Model, ) : SymbolMetadataProvider(base) { - override fun memberMeta(memberShape: MemberShape): RustMetadata { - return base.toSymbol(memberShape).expectRustMetadata() - } - override fun structureMeta(structureShape: StructureShape): RustMetadata { val baseMetadata = base.toSymbol(structureShape).expectRustMetadata() return if (structureShape.hasStreamingMember(model)) { @@ -78,7 +77,11 @@ class StreamingShapeMetadataProvider( } else baseMetadata } - override fun enumMeta(stringShape: StringShape): RustMetadata { - return base.toSymbol(stringShape).expectRustMetadata() - } + override fun memberMeta(memberShape: MemberShape) = base.toSymbol(memberShape).expectRustMetadata() + override fun enumMeta(stringShape: StringShape) = base.toSymbol(stringShape).expectRustMetadata() + + override fun listMeta(listShape: ListShape) = base.toSymbol(listShape).expectRustMetadata() + override fun mapMeta(mapShape: MapShape) = base.toSymbol(mapShape).expectRustMetadata() + override fun stringMeta(stringShape: StringShape) = base.toSymbol(stringShape).expectRustMetadata() + override fun numberMeta(numberShape: NumberShape) = base.toSymbol(numberShape).expectRustMetadata() } diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt index 4f6f095e94..2b3e18fb72 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt @@ -8,7 +8,11 @@ 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.CollectionShape +import software.amazon.smithy.model.shapes.ListShape +import software.amazon.smithy.model.shapes.MapShape import software.amazon.smithy.model.shapes.MemberShape +import software.amazon.smithy.model.shapes.NumberShape import software.amazon.smithy.model.shapes.Shape import software.amazon.smithy.model.shapes.StringShape import software.amazon.smithy.model.shapes.StructureShape @@ -55,9 +59,14 @@ abstract class SymbolMetadataProvider(private val base: RustSymbolProvider) : Wr is MemberShape -> memberMeta(shape) is StructureShape -> structureMeta(shape) is UnionShape -> unionMeta(shape) + is ListShape -> listMeta(shape) + is MapShape -> mapMeta(shape) + is NumberShape -> numberMeta(shape) is StringShape -> if (shape.hasTrait()) { enumMeta(shape) - } else null + } else { + stringMeta(shape) + } else -> null } @@ -68,83 +77,92 @@ abstract class SymbolMetadataProvider(private val base: RustSymbolProvider) : Wr abstract fun structureMeta(structureShape: StructureShape): RustMetadata abstract fun unionMeta(unionShape: UnionShape): RustMetadata abstract fun enumMeta(stringShape: StringShape): RustMetadata + + abstract fun listMeta(listShape: ListShape): RustMetadata + abstract fun mapMeta(mapShape: MapShape): RustMetadata + abstract fun stringMeta(stringShape: StringShape): RustMetadata + abstract fun numberMeta(numberShape: NumberShape): RustMetadata +} + +fun containerDefaultMetadata( + shape: Shape, + model: Model, + additionalAttributes: List = emptyList(), +): RustMetadata { + val defaultDerives = setOf(RuntimeType.Debug, RuntimeType.PartialEq, RuntimeType.Clone) + + val isSensitive = shape.hasTrait() || + // Checking the shape's direct members for the sensitive trait should suffice. + // Whether their descendants, i.e. a member's member, is sensitive does not + // affect the inclusion/exclusion of the derived `Debug` trait of _this_ container + // shape; any sensitive descendant should still be printed as redacted. + shape.members().any { it.getMemberTrait(model, SensitiveTrait::class.java).isPresent } + + val setOfDerives = if (isSensitive) { + defaultDerives - RuntimeType.Debug + } else { + defaultDerives + } + return RustMetadata( + setOfDerives, + additionalAttributes, + Visibility.PUBLIC, + ) } /** - * The base metadata supports a list of attributes that are used by generators to decorate code. - * By default, we apply ```#[non_exhaustive]``` only to client structures since model changes should - * be considered as breaking only when generating server code. + * The base metadata supports a set of attributes that are used by generators to decorate code. + * + * By default we apply `#[non_exhaustive]` in [additionalAttributes] only to client structures since breaking model + * changes are fine when generating server code. */ class BaseSymbolMetadataProvider( base: RustSymbolProvider, private val model: Model, private val additionalAttributes: List, ) : SymbolMetadataProvider(base) { - private fun containerDefault(shape: Shape): RustMetadata { - val isSensitive = shape.hasTrait() || - // Checking the shape's direct members for the sensitive trait should suffice. - // Whether their descendants, i.e. a member's member, is sensitive does not - // affect the inclusion/exclusion of the derived Debug trait of _this_ container - // shape; any sensitive descendant should still be printed as redacted. - shape.members().any { it.getMemberTrait(model, SensitiveTrait::class.java).isPresent } - - val derives = if (isSensitive) { - defaultDerives - RuntimeType.Debug - } else { - defaultDerives - } - return RustMetadata( - derives, - additionalAttributes, - Visibility.PUBLIC, - ) - } - override fun memberMeta(memberShape: MemberShape): RustMetadata { - val container = model.expectShape(memberShape.container) - return when { - container.isStructureShape -> { + override fun memberMeta(memberShape: MemberShape): RustMetadata = + when (val container = model.expectShape(memberShape.container)) { + is StructureShape -> { // TODO(https://github.com/awslabs/smithy-rs/issues/943): Once streaming accessors are usable, // then also make streaming members `#[doc(hidden)]` if (memberShape.getMemberTrait(model, StreamingTrait::class.java).isPresent) { RustMetadata(visibility = Visibility.PUBLIC) } else { RustMetadata( - // At some point, visibility will be made PRIVATE, so make these `#[doc(hidden)]` for now + // At some point, visibility _may_ be made `PRIVATE`, so make these `#[doc(hidden)]` for now. visibility = Visibility.PUBLIC, additionalAttributes = listOf(Attribute.DocHidden), ) } } - container.isUnionShape || - container.isListShape || - container.isSetShape || - container.isMapShape - -> RustMetadata(visibility = Visibility.PUBLIC) - + is UnionShape, is CollectionShape, is MapShape -> RustMetadata(visibility = Visibility.PUBLIC) else -> TODO("Unrecognized container type: $container") } - } - override fun structureMeta(structureShape: StructureShape): RustMetadata { - return containerDefault(structureShape) - } + override fun structureMeta(structureShape: StructureShape) = containerDefaultMetadata(structureShape, model, additionalAttributes) + override fun unionMeta(unionShape: UnionShape) = containerDefaultMetadata(unionShape, model, additionalAttributes) - override fun unionMeta(unionShape: UnionShape): RustMetadata { - return containerDefault(unionShape) - } - - override fun enumMeta(stringShape: StringShape): RustMetadata { - return containerDefault(stringShape).withDerives( - RuntimeType.Hash, - // enums can be Eq because they can only contain ints and strings + override fun enumMeta(stringShape: StringShape): RustMetadata = + containerDefaultMetadata(stringShape, model, additionalAttributes).withDerives( + // Smithy's `enum` shapes can additionally be `Eq`, `PartialOrd`, `Ord`, and `Hash` because they can + // only contain strings. RuntimeType.Eq, - // enums can be PartialOrd/Ord because they can only contain ints and strings RuntimeType.PartialOrd, RuntimeType.Ord, + RuntimeType.Hash, ) - } + + // Only the server subproject uses these, so we provide a sane and conservative default implementation here so that + // the rest of symbol metadata providers can just delegate to it. + private val defaultRustMetadata = RustMetadata(visibility = Visibility.PRIVATE) + + override fun listMeta(listShape: ListShape) = defaultRustMetadata + override fun mapMeta(mapShape: MapShape) = defaultRustMetadata + override fun stringMeta(stringShape: StringShape) = defaultRustMetadata + override fun numberMeta(numberShape: NumberShape) = defaultRustMetadata companion object { private val defaultDerives by lazy { @@ -154,12 +172,10 @@ class BaseSymbolMetadataProvider( } private const val META_KEY = "meta" -fun Symbol.Builder.meta(rustMetadata: RustMetadata?): Symbol.Builder { - return this.putProperty(META_KEY, rustMetadata) -} +fun Symbol.Builder.meta(rustMetadata: RustMetadata?): Symbol.Builder = this.putProperty(META_KEY, rustMetadata) fun Symbol.expectRustMetadata(): RustMetadata = this.getProperty(META_KEY, RustMetadata::class.java).orElseThrow { CodegenException( - "Expected $this to have metadata attached but it did not. ", + "Expected `$this` to have metadata attached but it did not.", ) } diff --git a/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonServerSymbolProvider.kt b/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonServerSymbolProvider.kt index 1247d1e064..d7a28a79f9 100644 --- a/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonServerSymbolProvider.kt +++ b/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonServerSymbolProvider.kt @@ -8,7 +8,10 @@ package software.amazon.smithy.rust.codegen.server.python.smithy import software.amazon.smithy.codegen.core.Symbol import software.amazon.smithy.model.Model import software.amazon.smithy.model.shapes.BlobShape +import software.amazon.smithy.model.shapes.ListShape +import software.amazon.smithy.model.shapes.MapShape import software.amazon.smithy.model.shapes.MemberShape +import software.amazon.smithy.model.shapes.NumberShape import software.amazon.smithy.model.shapes.ServiceShape import software.amazon.smithy.model.shapes.Shape import software.amazon.smithy.model.shapes.StringShape @@ -88,10 +91,6 @@ class PythonServerSymbolVisitor( * Note that since streaming members can only be used on the root shape, this can only impact input and output shapes. */ class PythonStreamingShapeMetadataProvider(private val base: RustSymbolProvider, private val model: Model) : SymbolMetadataProvider(base) { - override fun memberMeta(memberShape: MemberShape): RustMetadata { - return base.toSymbol(memberShape).expectRustMetadata() - } - override fun structureMeta(structureShape: StructureShape): RustMetadata { val baseMetadata = base.toSymbol(structureShape).expectRustMetadata() return if (structureShape.hasStreamingMember(model)) { @@ -106,7 +105,11 @@ class PythonStreamingShapeMetadataProvider(private val base: RustSymbolProvider, } else baseMetadata } - override fun enumMeta(stringShape: StringShape): RustMetadata { - return base.toSymbol(stringShape).expectRustMetadata() - } + override fun memberMeta(memberShape: MemberShape) = base.toSymbol(memberShape).expectRustMetadata() + override fun enumMeta(stringShape: StringShape) = base.toSymbol(stringShape).expectRustMetadata() + + override fun listMeta(listShape: ListShape) = base.toSymbol(listShape).expectRustMetadata() + override fun mapMeta(mapShape: MapShape) = base.toSymbol(mapShape).expectRustMetadata() + override fun stringMeta(stringShape: StringShape) = base.toSymbol(stringShape).expectRustMetadata() + override fun numberMeta(numberShape: NumberShape) = base.toSymbol(numberShape).expectRustMetadata() } diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolMetadataProvider.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolMetadataProvider.kt new file mode 100644 index 0000000000..b12cffeed6 --- /dev/null +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolMetadataProvider.kt @@ -0,0 +1,50 @@ +package software.amazon.smithy.rust.codegen.server.smithy + +import software.amazon.smithy.model.Model +import software.amazon.smithy.model.shapes.ListShape +import software.amazon.smithy.model.shapes.MapShape +import software.amazon.smithy.model.shapes.MemberShape +import software.amazon.smithy.model.shapes.NumberShape +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.rust.codegen.core.rustlang.RustMetadata +import software.amazon.smithy.rust.codegen.core.rustlang.Visibility +import software.amazon.smithy.rust.codegen.core.smithy.RustSymbolProvider +import software.amazon.smithy.rust.codegen.core.smithy.SymbolMetadataProvider +import software.amazon.smithy.rust.codegen.core.smithy.containerDefaultMetadata +import software.amazon.smithy.rust.codegen.core.smithy.expectRustMetadata + +// TODO Docs +class ConstrainedShapeSymbolMetadataProvider( + private val base: RustSymbolProvider, + private val model: Model, + private val constrainedTypes: Boolean, +) : SymbolMetadataProvider(base) { + + override fun memberMeta(memberShape: MemberShape) = base.toSymbol(memberShape).expectRustMetadata() + override fun structureMeta(structureShape: StructureShape) = base.toSymbol(structureShape).expectRustMetadata() + override fun unionMeta(unionShape: UnionShape) = base.toSymbol(unionShape).expectRustMetadata() + override fun enumMeta(stringShape: StringShape) = base.toSymbol(stringShape).expectRustMetadata() + + private fun addDerivesAndAdjustVisibilityIfConstrained(shape: Shape): RustMetadata { + check(shape is ListShape || shape is MapShape || shape is StringShape || shape is NumberShape) + + val baseMetadata = base.toSymbol(shape).expectRustMetadata() + var derives = baseMetadata.derives + val additionalAttributes = baseMetadata.additionalAttributes.toMutableList() + + if (shape.canReachConstrainedShape(model, base)) { + derives += containerDefaultMetadata(shape, model).derives + } + + val visibility = Visibility.publicIf(constrainedTypes, Visibility.PUBCRATE) + return RustMetadata(derives, additionalAttributes, visibility) + } + + override fun listMeta(listShape: ListShape): RustMetadata = addDerivesAndAdjustVisibilityIfConstrained(listShape) + override fun mapMeta(mapShape: MapShape): RustMetadata = addDerivesAndAdjustVisibilityIfConstrained(mapShape) + override fun stringMeta(stringShape: StringShape): RustMetadata = addDerivesAndAdjustVisibilityIfConstrained(stringShape) + override fun numberMeta(numberShape: NumberShape): RustMetadata = addDerivesAndAdjustVisibilityIfConstrained(numberShape) +} diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProvider.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProvider.kt new file mode 100644 index 0000000000..62ab610140 --- /dev/null +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProvider.kt @@ -0,0 +1,69 @@ +package software.amazon.smithy.rust.codegen.server.smithy + +import software.amazon.smithy.model.Model +import software.amazon.smithy.model.shapes.DocumentShape +import software.amazon.smithy.model.shapes.DoubleShape +import software.amazon.smithy.model.shapes.FloatShape +import software.amazon.smithy.model.shapes.ListShape +import software.amazon.smithy.model.shapes.MapShape +import software.amazon.smithy.model.shapes.MemberShape +import software.amazon.smithy.model.shapes.NumberShape +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.StreamingTrait +import software.amazon.smithy.rust.codegen.core.rustlang.RustMetadata +import software.amazon.smithy.rust.codegen.core.smithy.DirectedWalker +import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType +import software.amazon.smithy.rust.codegen.core.smithy.RustSymbolProvider +import software.amazon.smithy.rust.codegen.core.smithy.SymbolMetadataProvider +import software.amazon.smithy.rust.codegen.core.smithy.expectRustMetadata +import software.amazon.smithy.rust.codegen.core.util.hasTrait + +// TODO Docs +// TODO Test +class DeriveEqAndHashSymbolMetadataProvider( + private val base: RustSymbolProvider, + val model: Model, +) : SymbolMetadataProvider(base) { + private val walker = DirectedWalker(model) + + private fun addDeriveEqAndHashIfPossible(shape: Shape): RustMetadata { + check(shape !is MemberShape) + val baseMetadata = base.toSymbol(shape).expectRustMetadata() + // TODO Justify + return if (walker.walkShapes(shape) + .any { it is FloatShape || it is DoubleShape || it is DocumentShape || it.hasTrait() } + ) { + baseMetadata + } else { + var ret = baseMetadata + if (ret.derives.contains(RuntimeType.PartialEq)) { + // We can only derive `Eq` if the type implements `PartialEq`. Not every shape that does not reach a + // floating point or a document shape does; for example, streaming shapes cannot be `PartialEq`, see + // [StreamingShapeMetadataProvider]. + ret = ret.withDerives(RuntimeType.Eq) + } + + // `std::collections::HashMap` does not implement `std::hash::Hash`: + // https://github.com/awslabs/smithy/issues/1567 + if (walker.walkShapes(shape).none { it is MapShape }) { + ret = ret.withDerives(RuntimeType.Hash) + } + + return ret + } + } + + override fun memberMeta(memberShape: MemberShape) = base.toSymbol(memberShape).expectRustMetadata() + + override fun structureMeta(structureShape: StructureShape) = addDeriveEqAndHashIfPossible(structureShape) + override fun unionMeta(unionShape: UnionShape) = addDeriveEqAndHashIfPossible(unionShape) + override fun enumMeta(stringShape: StringShape) = addDeriveEqAndHashIfPossible(stringShape) + + override fun listMeta(listShape: ListShape): RustMetadata = addDeriveEqAndHashIfPossible(listShape) + override fun mapMeta(mapShape: MapShape): RustMetadata = addDeriveEqAndHashIfPossible(mapShape) + override fun stringMeta(stringShape: StringShape): RustMetadata = addDeriveEqAndHashIfPossible(stringShape) + override fun numberMeta(numberShape: NumberShape): RustMetadata = addDeriveEqAndHashIfPossible(numberShape) +} diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/PubCrateConstrainedShapeSymbolProvider.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/PubCrateConstrainedShapeSymbolProvider.kt index 3674185520..800dc6c730 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/PubCrateConstrainedShapeSymbolProvider.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/PubCrateConstrainedShapeSymbolProvider.kt @@ -97,9 +97,6 @@ class PubCrateConstrainedShapeSymbolProvider( } is MemberShape -> { - require(model.expectShape(shape.container).isStructureShape) { - "This arm is only exercised by `ServerBuilderGenerator`" - } require(!shape.hasConstraintTraitOrTargetHasConstraintTrait(model, base)) { errorMessage(shape) } val targetShape = model.expectShape(shape.target) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/RustCodegenServerPlugin.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/RustCodegenServerPlugin.kt index 98e875f3cc..8a1dc17e54 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/RustCodegenServerPlugin.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/RustCodegenServerPlugin.kt @@ -73,8 +73,12 @@ class RustCodegenServerPlugin : SmithyBuildPlugin { .let { StreamingShapeSymbolProvider(it, model) } // Add Rust attributes (like `#[derive(PartialEq)]`) to generated shapes .let { BaseSymbolMetadataProvider(it, model, additionalAttributes = listOf()) } - // Streaming shapes need different derives (e.g. they cannot derive Eq) + // Constrained shapes generate newtypes that need the same derives we place on types generated from aggregate shapes. + .let { ConstrainedShapeSymbolMetadataProvider(it, model, constrainedTypes) } + // Streaming shapes need different derives (e.g. they cannot derive `PartialEq`) .let { StreamingShapeMetadataProvider(it, model) } + // Derive `Eq` and `Hash` if possible. + .let { DeriveEqAndHashSymbolMetadataProvider(it, model) } // 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, model) } diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedBlobGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedBlobGenerator.kt index acaee289ae..daef717f74 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedBlobGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedBlobGenerator.kt @@ -55,13 +55,14 @@ class ConstrainedBlobGenerator( val inner = RuntimeType.blob(codegenContext.runtimeConfig).toSymbol().rustType().render() val constraintViolation = constraintViolationSymbolProvider.toSymbol(shape) + // TODO Delegate RustMetadata entirely to the symbol provider val constrainedTypeVisibility = if (publicConstrainedTypes) { Visibility.PUBLIC } else { Visibility.PUBCRATE } val constrainedTypeMetadata = RustMetadata( - setOf(RuntimeType.Debug, RuntimeType.Clone, RuntimeType.PartialEq), + setOf(RuntimeType.Debug, RuntimeType.Clone, RuntimeType.PartialEq, RuntimeType.Eq, RuntimeType.Hash), visibility = constrainedTypeVisibility, ) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedCollectionGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedCollectionGenerator.kt index 9d8aeb1e46..54eb817cee 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedCollectionGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedCollectionGenerator.kt @@ -6,21 +6,22 @@ package software.amazon.smithy.rust.codegen.server.smithy.generators import software.amazon.smithy.codegen.core.Symbol +import software.amazon.smithy.codegen.core.SymbolProvider import software.amazon.smithy.model.shapes.CollectionShape import software.amazon.smithy.model.shapes.StructureShape import software.amazon.smithy.model.shapes.UnionShape import software.amazon.smithy.model.traits.LengthTrait import software.amazon.smithy.model.traits.Trait -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.RustWriter import software.amazon.smithy.rust.codegen.core.rustlang.Visibility import software.amazon.smithy.rust.codegen.core.rustlang.docs import software.amazon.smithy.rust.codegen.core.rustlang.documentShape import software.amazon.smithy.rust.codegen.core.rustlang.join import software.amazon.smithy.rust.codegen.core.rustlang.rust +import software.amazon.smithy.rust.codegen.core.rustlang.rustBlock import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType +import software.amazon.smithy.rust.codegen.core.smithy.expectRustMetadata import software.amazon.smithy.rust.codegen.core.util.PANIC import software.amazon.smithy.rust.codegen.core.util.orNull import software.amazon.smithy.rust.codegen.server.smithy.PubCrateConstraintViolationSymbolProvider @@ -72,11 +73,7 @@ class ConstrainedCollectionGenerator( val name = constrainedShapeSymbolProvider.toSymbol(shape).name val inner = "std::vec::Vec<#{ValueSymbol}>" val constraintViolation = constraintViolationSymbolProvider.toSymbol(shape) - val constrainedTypeVisibility = Visibility.publicIf(publicConstrainedTypes, Visibility.PUBCRATE) - val constrainedTypeMetadata = RustMetadata( - setOf(RuntimeType.Debug, RuntimeType.Clone, RuntimeType.PartialEq), - visibility = constrainedTypeVisibility, - ) + val constrainedSymbol = symbolProvider.toSymbol(shape) val codegenScope = arrayOf( "ValueSymbol" to constrainedShapeSymbolProvider.toSymbol(model.expectShape(shape.member.target)), @@ -87,33 +84,43 @@ class ConstrainedCollectionGenerator( writer.documentShape(shape, model) writer.docs(rustDocsConstrainedTypeEpilogue(name)) - constrainedTypeMetadata.render(writer) + val metadata = constrainedSymbol.expectRustMetadata() + metadata.render(writer) writer.rustTemplate( """ struct $name(pub(crate) $inner); """, *codegenScope, ) - if (constrainedTypeVisibility == Visibility.PUBCRATE) { - Attribute.AllowUnused.render(writer) - } - - writer.rustTemplate( - """ - impl $name { - /// ${rustDocsInnerMethod(inner)} - pub fn inner(&self) -> &$inner { - &self.0 - } + writer.rustBlock("impl $name") { + if (metadata.visibility == Visibility.PUBLIC) { + writer.rustTemplate( + """ + /// ${rustDocsInnerMethod(inner)} + pub fn inner(&self) -> &$inner { + &self.0 + } + """, + *codegenScope, + ) + } + writer.rustTemplate( + """ /// ${rustDocsIntoInnerMethod(inner)} pub fn into_inner(self) -> $inner { self.0 } #{ValidationFunctions:W} - } + """, + *codegenScope, + "ValidationFunctions" to constraintsInfo.map { it.validationFunctionDefinition(constraintViolation, inner) }.join("\n"), + ) + } + writer.rustTemplate( + """ impl #{TryFrom}<$inner> for $name { type Error = #{ConstraintViolation}; diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedMapGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedMapGenerator.kt index a97bf3da72..74dedb7096 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedMapGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedMapGenerator.kt @@ -11,13 +11,13 @@ import software.amazon.smithy.model.shapes.StructureShape import software.amazon.smithy.model.shapes.UnionShape import software.amazon.smithy.model.traits.LengthTrait 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.RustWriter import software.amazon.smithy.rust.codegen.core.rustlang.Visibility import software.amazon.smithy.rust.codegen.core.rustlang.docs import software.amazon.smithy.rust.codegen.core.rustlang.documentShape import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType +import software.amazon.smithy.rust.codegen.core.smithy.expectRustMetadata import software.amazon.smithy.rust.codegen.core.util.expectTrait import software.amazon.smithy.rust.codegen.server.smithy.PubCrateConstraintViolationSymbolProvider import software.amazon.smithy.rust.codegen.server.smithy.ServerCodegenContext @@ -60,12 +60,7 @@ class ConstrainedMapGenerator( val name = constrainedShapeSymbolProvider.toSymbol(shape).name val inner = "std::collections::HashMap<#{KeySymbol}, #{ValueSymbol}>" val constraintViolation = constraintViolationSymbolProvider.toSymbol(shape) - - val constrainedTypeVisibility = Visibility.publicIf(publicConstrainedTypes, Visibility.PUBCRATE) - val constrainedTypeMetadata = RustMetadata( - setOf(RuntimeType.Debug, RuntimeType.Clone, RuntimeType.PartialEq), - visibility = constrainedTypeVisibility, - ) + val constrainedSymbol = symbolProvider.toSymbol(shape) val codegenScope = arrayOf( "KeySymbol" to constrainedShapeSymbolProvider.toSymbol(model.expectShape(shape.key.target)), @@ -77,9 +72,12 @@ class ConstrainedMapGenerator( writer.documentShape(shape, model) writer.docs(rustDocsConstrainedTypeEpilogue(name)) - constrainedTypeMetadata.render(writer) + val metadata = constrainedSymbol.expectRustMetadata() + metadata.render(writer) writer.rustTemplate("struct $name(pub(crate) $inner);", *codegenScope) - if (constrainedTypeVisibility == Visibility.PUBCRATE) { + // TODO Use the same strategy as in `ConstrainedCollectionGenerator.kt`: `metadata.visibility == Visibility + // .PUBLIC` inside impl block + if (metadata.visibility == Visibility.PUBCRATE) { Attribute.AllowUnused.render(writer) } writer.rustTemplate( diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedNumberGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedNumberGenerator.kt index 6e85ac5c8f..9f2e58df56 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedNumberGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedNumberGenerator.kt @@ -76,6 +76,7 @@ class ConstrainedNumberGenerator( val constraintViolation = constraintViolationSymbolProvider.toSymbol(shape) val constraintsInfo = listOf(Range(rangeTrait).toTraitInfo(unconstrainedTypeName)) + // TODO Delegate RustMetadata entirely to the symbol provider val constrainedTypeVisibility = if (publicConstrainedTypes) { Visibility.PUBLIC } else { diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedStringGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedStringGenerator.kt index 2c7fcaa504..50b2152e8d 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedStringGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedStringGenerator.kt @@ -71,6 +71,7 @@ class ConstrainedStringGenerator( val inner = RustType.String.render() val constraintViolation = constraintViolationSymbolProvider.toSymbol(shape) + // TODO Delegate RustMetadata entirely to the symbol provider val constrainedTypeVisibility = if (publicConstrainedTypes) { Visibility.PUBLIC } else { @@ -157,7 +158,7 @@ class ConstrainedStringGenerator( """ ##[derive(Debug, PartialEq)] pub enum ${constraintViolation.name} { - #{Variants:W} + #{Variants:W} } """, "Variants" to constraintsInfo.map { it.constraintViolationVariant }.join(",\n"), @@ -198,12 +199,12 @@ class ConstrainedStringGenerator( } private data class Length(val lengthTrait: LengthTrait) : StringTraitInfo() { override fun toTraitInfo(): TraitInfo = TraitInfo( - { rust("Self::check_length(&value)?;") }, - { + tryFromCheck = { rust("Self::check_length(&value)?;") }, + constraintViolationVariant = { docs("Error when a string doesn't satisfy its `@length` requirements.") rust("Length(usize)") }, - { + asValidationExceptionField = { rust( """ Self::Length(length) => crate::model::ValidationExceptionField { @@ -213,7 +214,7 @@ private data class Length(val lengthTrait: LengthTrait) : StringTraitInfo() { """, ) }, - this::renderValidationFunction, + validationFunctionDefinition = this::renderValidationFunction, ) /** @@ -243,13 +244,13 @@ private data class Pattern(val symbol: Symbol, val patternTrait: PatternTrait) : val pattern = patternTrait.pattern return TraitInfo( - { rust("let value = Self::check_pattern(value)?;") }, - { + tryFromCheck = { rust("let value = Self::check_pattern(value)?;") }, + constraintViolationVariant = { docs("Error when a string doesn't satisfy its `@pattern`.") docs("Contains the String that failed the pattern.") rust("Pattern(String)") }, - { + asValidationExceptionField = { rust( """ Self::Pattern(string) => crate::model::ValidationExceptionField { diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/PubCrateConstrainedCollectionGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/PubCrateConstrainedCollectionGenerator.kt index a85b4c6107..09f9352cde 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/PubCrateConstrainedCollectionGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/PubCrateConstrainedCollectionGenerator.kt @@ -8,8 +8,14 @@ package software.amazon.smithy.rust.codegen.server.smithy.generators import software.amazon.smithy.model.shapes.CollectionShape import software.amazon.smithy.model.shapes.MapShape import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter +import software.amazon.smithy.rust.codegen.core.rustlang.conditionalBlock +import software.amazon.smithy.rust.codegen.core.rustlang.rust +import software.amazon.smithy.rust.codegen.core.rustlang.rustBlock +import software.amazon.smithy.rust.codegen.core.rustlang.rustBlockTemplate import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate +import software.amazon.smithy.rust.codegen.core.rustlang.withBlock import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType +import software.amazon.smithy.rust.codegen.core.smithy.isOptional import software.amazon.smithy.rust.codegen.core.smithy.module import software.amazon.smithy.rust.codegen.server.smithy.ServerCodegenContext import software.amazon.smithy.rust.codegen.server.smithy.canReachConstrainedShape @@ -54,14 +60,14 @@ class PubCrateConstrainedCollectionGenerator( val unconstrainedSymbol = unconstrainedShapeSymbolProvider.toSymbol(shape) val name = constrainedSymbol.name val innerShape = model.expectShape(shape.member.target) - val innerConstrainedSymbol = if (innerShape.isTransitivelyButNotDirectlyConstrained(model, symbolProvider)) { - pubCrateConstrainedShapeSymbolProvider.toSymbol(innerShape) + val innerMemberSymbol = if (innerShape.isTransitivelyButNotDirectlyConstrained(model, symbolProvider)) { + pubCrateConstrainedShapeSymbolProvider.toSymbol(shape.member) } else { - constrainedShapeSymbolProvider.toSymbol(innerShape) + constrainedShapeSymbolProvider.toSymbol(shape.member) } val codegenScope = arrayOf( - "InnerConstrainedSymbol" to innerConstrainedSymbol, + "InnerMemberSymbol" to innerMemberSymbol, "ConstrainedTrait" to RuntimeType.ConstrainedTrait, "UnconstrainedSymbol" to unconstrainedSymbol, "Symbol" to symbol, @@ -72,7 +78,7 @@ class PubCrateConstrainedCollectionGenerator( rustTemplate( """ ##[derive(Debug, Clone)] - pub(crate) struct $name(pub(crate) std::vec::Vec<#{InnerConstrainedSymbol}>); + pub(crate) struct $name(pub(crate) std::vec::Vec<#{InnerMemberSymbol}>); impl #{ConstrainedTrait} for $name { type Unconstrained = #{UnconstrainedSymbol}; @@ -130,22 +136,19 @@ class PubCrateConstrainedCollectionGenerator( val innerNeedsConversion = innerShape.typeNameContainsNonPublicType(model, symbolProvider, publicConstrainedTypes) - rustTemplate( - """ - impl #{From}<$name> for #{Symbol} { - fn from(v: $name) -> Self { - ${ - if (innerNeedsConversion) { - "v.0.into_iter().map(|item| item.into()).collect()" - } else { - "v.0" - } - } + rustBlockTemplate("impl #{From}<$name> for #{Symbol}", *codegenScope) { + rustBlock("fn from(v: $name) -> Self") { + if (innerNeedsConversion) { + withBlock("v.0.into_iter().map(|item| ", ").collect()") { + conditionalBlock("item.map(|item| ", ")", innerMemberSymbol.isOptional()) { + rust("item.into()") + } + } + } else { + rust("v.0") } } - """, - *codegenScope, - ) + } } } } diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/PubCrateConstrainedMapGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/PubCrateConstrainedMapGenerator.kt index abbce1d59e..9d5ad81125 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/PubCrateConstrainedMapGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/PubCrateConstrainedMapGenerator.kt @@ -9,8 +9,14 @@ import software.amazon.smithy.model.shapes.CollectionShape import software.amazon.smithy.model.shapes.MapShape import software.amazon.smithy.model.shapes.StringShape import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter +import software.amazon.smithy.rust.codegen.core.rustlang.conditionalBlock +import software.amazon.smithy.rust.codegen.core.rustlang.rust +import software.amazon.smithy.rust.codegen.core.rustlang.rustBlock +import software.amazon.smithy.rust.codegen.core.rustlang.rustBlockTemplate import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate +import software.amazon.smithy.rust.codegen.core.rustlang.withBlock import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType +import software.amazon.smithy.rust.codegen.core.smithy.isOptional import software.amazon.smithy.rust.codegen.core.smithy.module import software.amazon.smithy.rust.codegen.server.smithy.ServerCodegenContext import software.amazon.smithy.rust.codegen.server.smithy.canReachConstrainedShape @@ -54,15 +60,15 @@ class PubCrateConstrainedMapGenerator( val keyShape = model.expectShape(shape.key.target, StringShape::class.java) val valueShape = model.expectShape(shape.value.target) val keySymbol = constrainedShapeSymbolProvider.toSymbol(keyShape) - val valueSymbol = if (valueShape.isTransitivelyButNotDirectlyConstrained(model, symbolProvider)) { - pubCrateConstrainedShapeSymbolProvider.toSymbol(valueShape) + val valueMemberSymbol = if (valueShape.isTransitivelyButNotDirectlyConstrained(model, symbolProvider)) { + pubCrateConstrainedShapeSymbolProvider.toSymbol(shape.value) } else { - constrainedShapeSymbolProvider.toSymbol(valueShape) + constrainedShapeSymbolProvider.toSymbol(shape.value) } val codegenScope = arrayOf( "KeySymbol" to keySymbol, - "ValueSymbol" to valueSymbol, + "ValueMemberSymbol" to valueMemberSymbol, "ConstrainedTrait" to RuntimeType.ConstrainedTrait, "UnconstrainedSymbol" to unconstrainedSymbol, "Symbol" to symbol, @@ -73,7 +79,7 @@ class PubCrateConstrainedMapGenerator( rustTemplate( """ ##[derive(Debug, Clone)] - pub(crate) struct $name(pub(crate) std::collections::HashMap<#{KeySymbol}, #{ValueSymbol}>); + pub(crate) struct $name(pub(crate) std::collections::HashMap<#{KeySymbol}, #{ValueMemberSymbol}>); impl #{ConstrainedTrait} for $name { type Unconstrained = #{UnconstrainedSymbol}; @@ -117,22 +123,27 @@ class PubCrateConstrainedMapGenerator( val keyNeedsConversion = keyShape.typeNameContainsNonPublicType(model, symbolProvider, publicConstrainedTypes) val valueNeedsConversion = valueShape.typeNameContainsNonPublicType(model, symbolProvider, publicConstrainedTypes) - rustTemplate( - """ - impl #{From}<$name> for #{Symbol} { - fn from(v: $name) -> Self { - ${ if (keyNeedsConversion || valueNeedsConversion) { - val keyConversion = if (keyNeedsConversion) { ".into()" } else { "" } - val valueConversion = if (valueNeedsConversion) { ".into()" } else { "" } - "v.0.into_iter().map(|(k, v)| (k$keyConversion, v$valueConversion)).collect()" - } else { - "v.0" - } } + rustBlockTemplate("impl #{From}<$name> for #{Symbol}", *codegenScope) { + rustBlock("fn from(v: $name) -> Self") { + if (keyNeedsConversion || valueNeedsConversion) { + withBlock("v.0.into_iter().map(|(k, v)| {", "}).collect()") { + if (keyNeedsConversion) { + rust("let k = k.into();") + } + if (valueNeedsConversion) { + withBlock("let v = {", "};") { + conditionalBlock("v.map(|v| ", ")", valueMemberSymbol.isOptional()) { + rust("v.into()") + } + } + } + rust("(k, v)") + } + } else { + rust("v.0") } } - """, - *codegenScope, - ) + } } } } diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/UnconstrainedCollectionGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/UnconstrainedCollectionGenerator.kt index ef7b77c405..b5a9d45895 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/UnconstrainedCollectionGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/UnconstrainedCollectionGenerator.kt @@ -10,10 +10,13 @@ import software.amazon.smithy.model.shapes.StructureShape import software.amazon.smithy.model.shapes.UnionShape import software.amazon.smithy.rust.codegen.core.rustlang.RustType import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter +import software.amazon.smithy.rust.codegen.core.rustlang.conditionalBlock import software.amazon.smithy.rust.codegen.core.rustlang.rust import software.amazon.smithy.rust.codegen.core.rustlang.rustBlock 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.RuntimeType +import software.amazon.smithy.rust.codegen.core.smithy.isOptional import software.amazon.smithy.rust.codegen.core.smithy.makeMaybeConstrained import software.amazon.smithy.rust.codegen.core.smithy.module import software.amazon.smithy.rust.codegen.server.smithy.PubCrateConstraintViolationSymbolProvider @@ -65,14 +68,13 @@ class UnconstrainedCollectionGenerator( fun render() { check(shape.canReachConstrainedShape(model, symbolProvider)) - val innerShape = model.expectShape(shape.member.target) - val innerUnconstrainedSymbol = unconstrainedShapeSymbolProvider.toSymbol(innerShape) + val innerMemberSymbol = unconstrainedShapeSymbolProvider.toSymbol(shape.member) unconstrainedModuleWriter.withInlineModule(symbol.module()) { rustTemplate( """ ##[derive(Debug, Clone)] - pub(crate) struct $name(pub(crate) std::vec::Vec<#{InnerUnconstrainedSymbol}>); + pub(crate) struct $name(pub(crate) std::vec::Vec<#{InnerMemberSymbol}>); impl From<$name> for #{MaybeConstrained} { fn from(value: $name) -> Self { @@ -80,7 +82,7 @@ class UnconstrainedCollectionGenerator( } } """, - "InnerUnconstrainedSymbol" to innerUnconstrainedSymbol, + "InnerMemberSymbol" to innerMemberSymbol, "MaybeConstrained" to constrainedSymbol.makeMaybeConstrained(), ) @@ -99,26 +101,35 @@ class UnconstrainedCollectionGenerator( !innerShape.isDirectlyConstrained(symbolProvider) && innerShape !is StructureShape && innerShape !is UnionShape - val innerConstrainedSymbol = if (resolvesToNonPublicConstrainedValueType) { - pubCrateConstrainedShapeSymbolProvider.toSymbol(innerShape) + val constrainedMemberSymbol = if (resolvesToNonPublicConstrainedValueType) { + pubCrateConstrainedShapeSymbolProvider.toSymbol(shape.member) } else { - constrainedShapeSymbolProvider.toSymbol(innerShape) + constrainedShapeSymbolProvider.toSymbol(shape.member) } val innerConstraintViolationSymbol = constraintViolationSymbolProvider.toSymbol(innerShape) + val constrainValueWritable = writable { + conditionalBlock("inner.map(|inner| ", ").transpose()", constrainedMemberSymbol.isOptional()) { + rust("inner.try_into().map_err(|inner_violation| (idx, inner_violation))") + } + } + rustTemplate( """ - let res: Result, (usize, #{InnerConstraintViolationSymbol})> = value + let res: Result<#{Vec}<#{ConstrainedMemberSymbol}>, (usize, #{InnerConstraintViolationSymbol}) > = value .0 .into_iter() .enumerate() - .map(|(idx, inner)| inner.try_into().map_err(|inner_violation| (idx, inner_violation))) + .map(|(idx, inner)| { + #{ConstrainValueWritable:W} + }) .collect(); let inner = res.map_err(|(idx, inner_violation)| Self::Error::Member(idx, inner_violation))?; """, - "InnerConstrainedSymbol" to innerConstrainedSymbol, + "Vec" to RuntimeType.Vec, + "ConstrainedMemberSymbol" to constrainedMemberSymbol, "InnerConstraintViolationSymbol" to innerConstraintViolationSymbol, - "TryFrom" to RuntimeType.TryFrom, + "ConstrainValueWritable" to constrainValueWritable, ) } else { rust("let inner = value.0;") diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/UnconstrainedMapGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/UnconstrainedMapGenerator.kt index ac4d563bfb..d2f1ac643a 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/UnconstrainedMapGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/UnconstrainedMapGenerator.kt @@ -14,7 +14,10 @@ import software.amazon.smithy.rust.codegen.core.rustlang.join import software.amazon.smithy.rust.codegen.core.rustlang.rust import software.amazon.smithy.rust.codegen.core.rustlang.rustBlock import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate +import software.amazon.smithy.rust.codegen.core.rustlang.withBlock import software.amazon.smithy.rust.codegen.core.rustlang.writable +import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType +import software.amazon.smithy.rust.codegen.core.smithy.isOptional import software.amazon.smithy.rust.codegen.core.smithy.makeMaybeConstrained import software.amazon.smithy.rust.codegen.core.smithy.module import software.amazon.smithy.rust.codegen.server.smithy.PubCrateConstraintViolationSymbolProvider @@ -67,13 +70,13 @@ class UnconstrainedMapGenerator( check(shape.canReachConstrainedShape(model, symbolProvider)) val keySymbol = unconstrainedShapeSymbolProvider.toSymbol(keyShape) - val valueSymbol = unconstrainedShapeSymbolProvider.toSymbol(valueShape) + val valueMemberSymbol = unconstrainedShapeSymbolProvider.toSymbol(shape.value) unconstrainedModuleWriter.withInlineModule(symbol.module()) { rustTemplate( """ ##[derive(Debug, Clone)] - pub(crate) struct $name(pub(crate) std::collections::HashMap<#{KeySymbol}, #{ValueSymbol}>); + pub(crate) struct $name(pub(crate) std::collections::HashMap<#{KeySymbol}, #{ValueMemberSymbol}>); impl From<$name> for #{MaybeConstrained} { fn from(value: $name) -> Self { @@ -83,7 +86,7 @@ class UnconstrainedMapGenerator( """, "KeySymbol" to keySymbol, - "ValueSymbol" to valueSymbol, + "ValueMemberSymbol" to valueMemberSymbol, "MaybeConstrained" to constrainedSymbol.makeMaybeConstrained(), ) @@ -102,6 +105,11 @@ class UnconstrainedMapGenerator( !valueShape.isDirectlyConstrained(symbolProvider) && valueShape !is StructureShape && valueShape !is UnionShape + val constrainedMemberValueSymbol = if (resolvesToNonPublicConstrainedValueType) { + pubCrateConstrainedShapeSymbolProvider.toSymbol(shape.value) + } else { + constrainedShapeSymbolProvider.toSymbol(shape.value) + } val constrainedValueSymbol = if (resolvesToNonPublicConstrainedValueType) { pubCrateConstrainedShapeSymbolProvider.toSymbol(valueShape) } else { @@ -109,6 +117,7 @@ class UnconstrainedMapGenerator( } val constrainedKeySymbol = constrainedShapeSymbolProvider.toSymbol(keyShape) + val epilogueWritable = writable { rust("Ok((k, v))") } val constrainKeyWritable = writable { rustTemplate( "let k: #{ConstrainedKeySymbol} = k.try_into().map_err(Self::Error::Key)?;", @@ -116,17 +125,37 @@ class UnconstrainedMapGenerator( ) } val constrainValueWritable = writable { - rustTemplate( - """ - match #{ConstrainedValueSymbol}::try_from(v) { - Ok(v) => Ok((k, v)), - Err(inner_constraint_violation) => Err(Self::Error::Value(k, inner_constraint_violation)), + if (constrainedMemberValueSymbol.isOptional()) { + // The map is `@sparse`. + rustBlock("match v") { + rust("None => Ok((k, None)),") + withBlock("Some(v) =>", ",") { + // DRYing this up with the else branch below would make this less understandable. + rustTemplate( + """ + match #{ConstrainedValueSymbol}::try_from(v) { + Ok(v) => Ok((k, Some(v))), + Err(inner_constraint_violation) => Err(Self::Error::Value(k, inner_constraint_violation)), + } + """, + "ConstrainedValueSymbol" to constrainedValueSymbol, + "Epilogue" to epilogueWritable, + ) + } } - """, - "ConstrainedValueSymbol" to constrainedValueSymbol, - ) + } else { + rustTemplate( + """ + match #{ConstrainedValueSymbol}::try_from(v) { + Ok(v) => #{Epilogue:W}, + Err(inner_constraint_violation) => Err(Self::Error::Value(k, inner_constraint_violation)), + } + """, + "ConstrainedValueSymbol" to constrainedValueSymbol, + "Epilogue" to epilogueWritable, + ) + } } - val epilogueWritable = writable { rust("Ok((k, v))") } val constrainKVWritable = if ( isKeyConstrained(keyShape, symbolProvider) && @@ -143,7 +172,7 @@ class UnconstrainedMapGenerator( rustTemplate( """ - let res: Result, Self::Error> = value.0 + let res: Result<#{HashMap}<#{ConstrainedKeySymbol}, #{ConstrainedMemberValueSymbol}>, Self::Error> = value.0 .into_iter() .map(|(k, v)| { #{ConstrainKVWritable:W} @@ -151,8 +180,9 @@ class UnconstrainedMapGenerator( .collect(); let hm = res?; """, + "HashMap" to RuntimeType.HashMap, "ConstrainedKeySymbol" to constrainedKeySymbol, - "ConstrainedValueSymbol" to constrainedValueSymbol, + "ConstrainedMemberValueSymbol" to constrainedMemberValueSymbol, "ConstrainKVWritable" to constrainKVWritable, ) diff --git a/rust-runtime/aws-smithy-types/src/date_time/mod.rs b/rust-runtime/aws-smithy-types/src/date_time/mod.rs index 6c2600dc9f..accb788046 100644 --- a/rust-runtime/aws-smithy-types/src/date_time/mod.rs +++ b/rust-runtime/aws-smithy-types/src/date_time/mod.rs @@ -47,7 +47,7 @@ const NANOS_PER_SECOND_U32: u32 = 1_000_000_000; /// The [`aws-smithy-types-convert`](https://crates.io/crates/aws-smithy-types-convert) crate /// can be used for conversions to/from other libraries, such as /// [`time`](https://crates.io/crates/time) or [`chrono`](https://crates.io/crates/chrono). -#[derive(Debug, PartialEq, Clone, Copy)] +#[derive(Debug, PartialEq, Eq, Hash, Clone, Copy)] pub struct DateTime { seconds: i64, subsecond_nanos: u32, diff --git a/rust-runtime/aws-smithy-types/src/lib.rs b/rust-runtime/aws-smithy-types/src/lib.rs index f2729cefa1..1e45b94183 100644 --- a/rust-runtime/aws-smithy-types/src/lib.rs +++ b/rust-runtime/aws-smithy-types/src/lib.rs @@ -30,7 +30,7 @@ pub use error::Error; /// Binary Blob Type /// /// Blobs represent protocol-agnostic binary content. -#[derive(Debug, PartialEq, Clone)] +#[derive(Debug, PartialEq, Eq, Hash, Clone)] pub struct Blob { inner: Vec, } From cb1cd7300825ef769dbfb1e580ba831deab7a0c2 Mon Sep 17 00:00:00 2001 From: david-perez Date: Wed, 18 Jan 2023 13:33:25 +0100 Subject: [PATCH 02/13] DeriveEqAndHashSymbolMetadataProviderTest --- .../core/smithy/SymbolMetadataProvider.kt | 6 - .../ConstrainedShapeSymbolMetadataProvider.kt | 2 +- .../DeriveEqAndHashSymbolMetadataProvider.kt | 28 ++- ...riveEqAndHashSymbolMetadataProviderTest.kt | 230 ++++++++++++++++++ 4 files changed, 255 insertions(+), 11 deletions(-) create mode 100644 codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt index 2b3e18fb72..58b5904108 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt @@ -163,12 +163,6 @@ class BaseSymbolMetadataProvider( override fun mapMeta(mapShape: MapShape) = defaultRustMetadata override fun stringMeta(stringShape: StringShape) = defaultRustMetadata override fun numberMeta(numberShape: NumberShape) = defaultRustMetadata - - companion object { - private val defaultDerives by lazy { - setOf(RuntimeType.Debug, RuntimeType.PartialEq, RuntimeType.Clone) - } - } } private const val META_KEY = "meta" diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolMetadataProvider.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolMetadataProvider.kt index b12cffeed6..9e406d8be3 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolMetadataProvider.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolMetadataProvider.kt @@ -32,7 +32,7 @@ class ConstrainedShapeSymbolMetadataProvider( check(shape is ListShape || shape is MapShape || shape is StringShape || shape is NumberShape) val baseMetadata = base.toSymbol(shape).expectRustMetadata() - var derives = baseMetadata.derives + val derives = baseMetadata.derives.toMutableSet() val additionalAttributes = baseMetadata.additionalAttributes.toMutableList() if (shape.canReachConstrainedShape(model, base)) { diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProvider.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProvider.kt index 62ab610140..2b08785394 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProvider.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProvider.kt @@ -21,8 +21,26 @@ import software.amazon.smithy.rust.codegen.core.smithy.SymbolMetadataProvider import software.amazon.smithy.rust.codegen.core.smithy.expectRustMetadata import software.amazon.smithy.rust.codegen.core.util.hasTrait -// TODO Docs -// TODO Test +/** + * This symbol metadata provider adds derives to implement the [`Eq`] and [`Hash`] traits for shapes, whenever + * possible. + * + * These traits can be implemented by any shape _except_ if the shape's closure contains: + * + * 1. A `float`, `double`, or `document` shape: floating point types in Rust do not implement `Eq`. Similarly, + * [`document` shapes] may contain arbitrary JSON-like data containing floating point values. + * 2. A [@streaming] shape: all the streaming data would need to be buffered first to compare it. + * + * Additionally, the `Hash` trait cannot be implemented by shapes whose closure contains: + * + * 1. A `map` shape: we render `map` shapes as `std::collections::HashMap`, which _do not_ implement `Hash`. + * See https://github.com/awslabs/smithy/issues/1567. + * + * [`Eq`]: https://doc.rust-lang.org/std/cmp/trait.Eq.html + * [`Hash`]: https://doc.rust-lang.org/std/hash/trait.Hash.html + * [`document` shapes]: https://smithy.io/2.0/spec/simple-types.html#document + * [@streaming]: https://smithy.io/2.0/spec/streaming.html + */ class DeriveEqAndHashSymbolMetadataProvider( private val base: RustSymbolProvider, val model: Model, @@ -32,7 +50,7 @@ class DeriveEqAndHashSymbolMetadataProvider( private fun addDeriveEqAndHashIfPossible(shape: Shape): RustMetadata { check(shape !is MemberShape) val baseMetadata = base.toSymbol(shape).expectRustMetadata() - // TODO Justify + // See class-level documentation for why we filter these out. return if (walker.walkShapes(shape) .any { it is FloatShape || it is DoubleShape || it is DocumentShape || it.hasTrait() } ) { @@ -42,7 +60,9 @@ class DeriveEqAndHashSymbolMetadataProvider( if (ret.derives.contains(RuntimeType.PartialEq)) { // We can only derive `Eq` if the type implements `PartialEq`. Not every shape that does not reach a // floating point or a document shape does; for example, streaming shapes cannot be `PartialEq`, see - // [StreamingShapeMetadataProvider]. + // [StreamingShapeMetadataProvider]. This is just a defensive check in case other symbol providers + // want to remove the `PartialEq` trait, since we've also just checked that we do not reach a streaming + // shape. ret = ret.withDerives(RuntimeType.Eq) } diff --git a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt new file mode 100644 index 0000000000..7a51ed19e2 --- /dev/null +++ b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt @@ -0,0 +1,230 @@ +package software.amazon.smithy.rust.codegen.server.smithy + +import io.kotest.assertions.throwables.shouldThrowAny +import io.kotest.matchers.collections.shouldContainAll +import org.junit.jupiter.api.Assertions.* +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.Arguments +import org.junit.jupiter.params.provider.MethodSource +import org.junit.jupiter.params.provider.ValueSource +import software.amazon.smithy.model.shapes.ServiceShape +import software.amazon.smithy.model.shapes.Shape +import software.amazon.smithy.rust.codegen.core.smithy.BaseSymbolMetadataProvider +import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType +import software.amazon.smithy.rust.codegen.core.smithy.expectRustMetadata +import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel +import software.amazon.smithy.rust.codegen.core.util.lookup +import software.amazon.smithy.rust.codegen.server.smithy.testutil.serverTestSymbolProvider +import java.util.stream.Stream + +internal class DeriveEqAndHashSymbolMetadataProviderTest { + private val model= + """ + namespace test + + service TestService { + version: "123" + operations: [TestOperation, StreamingOperation, EventStreamOperation] + } + + operation TestOperation { + input: TestInputOutput + output: TestInputOutput + } + + operation StreamingOperation { + input: StreamingOperationInputOutput + output: StreamingOperationInputOutput + } + + operation EventStreamOperation { + input: EventStreamOperationInputOutput + output: EventStreamOperationInputOutput + } + + structure EventStreamOperationInputOutput { + @httpPayload + @required + union: StreamingUnion + } + + structure StreamingOperationInputOutput { + @httpPayload + @required + blobStream: BlobStream + } + + @streaming + blob BlobStream + + structure TestInputOutput { + hasFloat: HasFloat + hasDouble: HasDouble + hasDocument: HasDocument + containsFloat: ContainsFloat + containsDouble: ContainsDouble + containsDocument: ContainsDocument + + hasList: HasList + hasListWithMap: HasListWithMap + hasMap: HasMap + + eqAndHashStruct: EqAndHashStruct + } + + structure EqAndHashStruct { + blob: Blob + boolean: Boolean + string: String + byte: Byte + short: Short + integer: Integer + long: Long + enum: Enum + timestamp: Timestamp + + list: List + union: EqAndHashUnion + + // bigInteger: BigInteger + // bigDecimal: BigDecimal + } + + list List { + member: String + } + + list ListWithMap { + member: Map + } + + map Map { + key: String + value: String + } + + union EqAndHashUnion { + blob: Blob + boolean: Boolean + string: String + byte: Byte + short: Short + integer: Integer + long: Long + enum: Enum + timestamp: Timestamp + + list: List + } + + @streaming + union StreamingUnion { + eqAndHashStruct: EqAndHashStruct + } + + structure HasFloat { + float: Float + } + + structure HasDouble { + double: Double + } + + structure HasDocument { + document: Document + } + + structure HasList { + list: List + } + + structure HasListWithMap { + list: ListWithMap + } + + structure HasMap { + map: Map + } + + structure ContainsFloat { + hasFloat: HasFloat + } + + structure ContainsDouble { + hasDouble: HasDouble + } + + structure ContainsDocument { + containsDocument: HasDocument + } + + enum Enum { + DIAMOND + CLUB + HEART + SPADE + } + """.asSmithyModel(smithyVersion = "2.0") + private val serviceShape = model.lookup("test#TestService") + private val deriveEqAndHashSymbolMetadataProvider = serverTestSymbolProvider(model, serviceShape) + .let { BaseSymbolMetadataProvider(it, model, additionalAttributes = listOf()) } + .let { DeriveEqAndHashSymbolMetadataProvider(it, model) } + + companion object { + @JvmStatic + fun getShapes(): Stream { + val shapesWithNeitherEqNorHash = listOf( + "test#StreamingOperationInputOutput", + "test#EventStreamOperationInputOutput", + "test#StreamingUnion", + "test#TestInputOutput", + "test#HasFloat", + "test#HasDouble", + "test#HasDocument", + "test#ContainsFloat", + "test#ContainsDouble", + "test#ContainsDocument", + ) + + val shapesWithEqAndHash = listOf( + "test#EqAndHashStruct", + "test#EqAndHashUnion", + "test#Enum", + "test#HasList", + ) + + val shapesWithOnlyEq = listOf( + "test#HasListWithMap", + "test#HasMap", + ) + + return ( + shapesWithNeitherEqNorHash.map { Arguments.of(it, emptyList()) } + + shapesWithEqAndHash.map { Arguments.of(it, listOf(RuntimeType.Eq, RuntimeType.Hash)) } + + shapesWithOnlyEq.map { Arguments.of(it, listOf(RuntimeType.Eq)) } + ).stream() + } + } + + @ParameterizedTest(name = "(#{index}) Derive `Eq` and `Hash` when possible. Params = shape: {0}, expectedTraits: {1}") + @MethodSource("getShapes") + fun `it should derive Eq and Hash when possible`( + shapeId: String, + expectedTraits: Collection + ) { + val shape = model.lookup(shapeId) + val derives = deriveEqAndHashSymbolMetadataProvider.toSymbol(shape).expectRustMetadata().derives + derives shouldContainAll expectedTraits + } + + @ParameterizedTest + @ValueSource(strings = ["smithy.api#Blob", "test#BlobStream"]) + fun `blobs have no metadata`(shapeId: String) { + // Blobs have no metadata. `Eq` and `Hash` are derived on `aws_smithy_types::Blob` instead. + + val shape = model.lookup(shapeId) + shouldThrowAny { + deriveEqAndHashSymbolMetadataProvider.toSymbol(shape).expectRustMetadata() + } + } +} From 2a0f67cdb7bbf8dbc549b8865a229ebd63c9ae7a Mon Sep 17 00:00:00 2001 From: david-perez Date: Wed, 18 Jan 2023 14:14:12 +0100 Subject: [PATCH 03/13] Cleanup --- .../ConstrainedShapeSymbolMetadataProvider.kt | 6 ++- .../generators/ConstrainedBlobGenerator.kt | 32 +++++++------- .../generators/ConstrainedMapGenerator.kt | 42 ++++++++++++------- .../generators/ConstrainedNumberGenerator.kt | 25 ++--------- .../generators/ConstrainedStringGenerator.kt | 23 ++++------ ...riveEqAndHashSymbolMetadataProviderTest.kt | 13 ++++++ 6 files changed, 73 insertions(+), 68 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolMetadataProvider.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolMetadataProvider.kt index 9e406d8be3..ebe56e4091 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolMetadataProvider.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolMetadataProvider.kt @@ -16,7 +16,11 @@ import software.amazon.smithy.rust.codegen.core.smithy.SymbolMetadataProvider import software.amazon.smithy.rust.codegen.core.smithy.containerDefaultMetadata import software.amazon.smithy.rust.codegen.core.smithy.expectRustMetadata -// TODO Docs +/** + * This symbol metadata provider adds the usual derives on shapes that are constrained and hence generate newtypes. + * + * It also makes the newtypes `pub(crate)` when `publicConstrainedTypes` is disabled. + */ class ConstrainedShapeSymbolMetadataProvider( private val base: RustSymbolProvider, private val model: Model, diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedBlobGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedBlobGenerator.kt index daef717f74..db2623af76 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedBlobGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedBlobGenerator.kt @@ -18,8 +18,10 @@ import software.amazon.smithy.rust.codegen.core.rustlang.documentShape import software.amazon.smithy.rust.codegen.core.rustlang.join import software.amazon.smithy.rust.codegen.core.rustlang.render import software.amazon.smithy.rust.codegen.core.rustlang.rust +import software.amazon.smithy.rust.codegen.core.rustlang.rustBlock import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType +import software.amazon.smithy.rust.codegen.core.smithy.expectRustMetadata import software.amazon.smithy.rust.codegen.core.smithy.makeMaybeConstrained import software.amazon.smithy.rust.codegen.core.smithy.module import software.amazon.smithy.rust.codegen.core.smithy.rustType @@ -55,7 +57,6 @@ class ConstrainedBlobGenerator( val inner = RuntimeType.blob(codegenContext.runtimeConfig).toSymbol().rustType().render() val constraintViolation = constraintViolationSymbolProvider.toSymbol(shape) - // TODO Delegate RustMetadata entirely to the symbol provider val constrainedTypeVisibility = if (publicConstrainedTypes) { Visibility.PUBLIC } else { @@ -70,23 +71,26 @@ class ConstrainedBlobGenerator( writer.docs(rustDocsConstrainedTypeEpilogue(name)) constrainedTypeMetadata.render(writer) writer.rust("struct $name(pub(crate) $inner);") - if (constrainedTypeVisibility == Visibility.PUBCRATE) { - Attribute.AllowUnused.render(writer) - } - writer.rust( - """ - impl $name { - /// ${rustDocsInnerMethod(inner)} - pub fn inner(&self) -> &$inner { - &self.0 - } - + writer.rustBlock("impl $name") { + if (constrainedTypeVisibility == Visibility.PUBLIC) { + writer.rust( + """ + /// ${rustDocsInnerMethod(inner)} + pub fn inner(&self) -> &$inner { + &self.0 + } + """, + ) + } + writer.rust( + """ /// ${rustDocsIntoInnerMethod(inner)} pub fn into_inner(self) -> $inner { self.0 } - }""", - ) + """, + ) + } writer.renderTryFrom(inner, name, constraintViolation, constraintsInfo) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedMapGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedMapGenerator.kt index 74dedb7096..11d420a0a7 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedMapGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedMapGenerator.kt @@ -10,11 +10,13 @@ import software.amazon.smithy.model.shapes.MapShape import software.amazon.smithy.model.shapes.StructureShape import software.amazon.smithy.model.shapes.UnionShape import software.amazon.smithy.model.traits.LengthTrait -import software.amazon.smithy.rust.codegen.core.rustlang.Attribute import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter import software.amazon.smithy.rust.codegen.core.rustlang.Visibility import software.amazon.smithy.rust.codegen.core.rustlang.docs import software.amazon.smithy.rust.codegen.core.rustlang.documentShape +import software.amazon.smithy.rust.codegen.core.rustlang.rust +import software.amazon.smithy.rust.codegen.core.rustlang.rustBlock +import software.amazon.smithy.rust.codegen.core.rustlang.rustBlockTemplate import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType import software.amazon.smithy.rust.codegen.core.smithy.expectRustMetadata @@ -58,11 +60,12 @@ class ConstrainedMapGenerator( val lengthTrait = shape.expectTrait() val name = constrainedShapeSymbolProvider.toSymbol(shape).name - val inner = "std::collections::HashMap<#{KeySymbol}, #{ValueSymbol}>" + val inner = "#{HashMap}<#{KeySymbol}, #{ValueSymbol}>" val constraintViolation = constraintViolationSymbolProvider.toSymbol(shape) val constrainedSymbol = symbolProvider.toSymbol(shape) val codegenScope = arrayOf( + "HashMap" to RuntimeType.HashMap, "KeySymbol" to constrainedShapeSymbolProvider.toSymbol(model.expectShape(shape.key.target)), "ValueSymbol" to constrainedShapeSymbolProvider.toSymbol(model.expectShape(shape.value.target)), "From" to RuntimeType.From, @@ -75,25 +78,31 @@ class ConstrainedMapGenerator( val metadata = constrainedSymbol.expectRustMetadata() metadata.render(writer) writer.rustTemplate("struct $name(pub(crate) $inner);", *codegenScope) - // TODO Use the same strategy as in `ConstrainedCollectionGenerator.kt`: `metadata.visibility == Visibility - // .PUBLIC` inside impl block - if (metadata.visibility == Visibility.PUBCRATE) { - Attribute.AllowUnused.render(writer) - } - writer.rustTemplate( - """ - impl $name { - /// ${rustDocsInnerMethod(inner)} - pub fn inner(&self) -> &$inner { - &self.0 - } - + writer.rustBlockTemplate("impl $name", *codegenScope) { + if (metadata.visibility == Visibility.PUBLIC) { + writer.rustTemplate( + """ + /// ${rustDocsInnerMethod(inner)} + pub fn inner(&self) -> &$inner { + &self.0 + } + """, + *codegenScope, + ) + } + writer.rustTemplate( + """ /// ${rustDocsIntoInnerMethod(inner)} pub fn into_inner(self) -> $inner { self.0 } - } + """, + *codegenScope, + ) + } + writer.rustTemplate( + """ impl #{TryFrom}<$inner> for $name { type Error = #{ConstraintViolation}; @@ -147,6 +156,7 @@ class ConstrainedMapGenerator( type Unconstrained = #{UnconstrainedSymbol}; } """, + *codegenScope, "ConstrainedTrait" to RuntimeType.ConstrainedTrait, "UnconstrainedSymbol" to unconstrainedSymbol, ) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedNumberGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedNumberGenerator.kt index 9f2e58df56..6a7c550a68 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedNumberGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedNumberGenerator.kt @@ -26,6 +26,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.rustBlock import software.amazon.smithy.rust.codegen.core.rustlang.rustBlockTemplate import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType +import software.amazon.smithy.rust.codegen.core.smithy.expectRustMetadata import software.amazon.smithy.rust.codegen.core.smithy.makeMaybeConstrained import software.amazon.smithy.rust.codegen.core.smithy.module import software.amazon.smithy.rust.codegen.core.util.UNREACHABLE @@ -76,31 +77,13 @@ class ConstrainedNumberGenerator( val constraintViolation = constraintViolationSymbolProvider.toSymbol(shape) val constraintsInfo = listOf(Range(rangeTrait).toTraitInfo(unconstrainedTypeName)) - // TODO Delegate RustMetadata entirely to the symbol provider - val constrainedTypeVisibility = if (publicConstrainedTypes) { - Visibility.PUBLIC - } else { - Visibility.PUBCRATE - } - val constrainedTypeMetadata = RustMetadata( - - setOf( - RuntimeType.Debug, - RuntimeType.Clone, - RuntimeType.PartialEq, - RuntimeType.Eq, - RuntimeType.Hash, - ), - - visibility = constrainedTypeVisibility, - ) - writer.documentShape(shape, model) writer.docs(rustDocsConstrainedTypeEpilogue(name)) - constrainedTypeMetadata.render(writer) + val metadata = symbol.expectRustMetadata() + metadata.render(writer) writer.rust("struct $name(pub(crate) $unconstrainedTypeName);") - if (constrainedTypeVisibility == Visibility.PUBCRATE) { + if (metadata.visibility == Visibility.PUBCRATE) { Attribute.AllowUnused.render(writer) } writer.rustTemplate( diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedStringGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedStringGenerator.kt index 50b2152e8d..ce15b84cf5 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedStringGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedStringGenerator.kt @@ -24,6 +24,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.render import software.amazon.smithy.rust.codegen.core.rustlang.rust import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType +import software.amazon.smithy.rust.codegen.core.smithy.expectRustMetadata import software.amazon.smithy.rust.codegen.core.smithy.makeMaybeConstrained import software.amazon.smithy.rust.codegen.core.smithy.module import software.amazon.smithy.rust.codegen.core.smithy.testModuleForShape @@ -71,25 +72,12 @@ class ConstrainedStringGenerator( val inner = RustType.String.render() val constraintViolation = constraintViolationSymbolProvider.toSymbol(shape) - // TODO Delegate RustMetadata entirely to the symbol provider - val constrainedTypeVisibility = if (publicConstrainedTypes) { - Visibility.PUBLIC - } else { - Visibility.PUBCRATE - } - val constrainedTypeMetadata = RustMetadata( - setOf(RuntimeType.Debug, RuntimeType.Clone, RuntimeType.PartialEq, RuntimeType.Eq, RuntimeType.Hash), - visibility = constrainedTypeVisibility, - ) - - // Note that we're using the linear time check `chars().count()` instead of `len()` on the input value, since the - // Smithy specification says the `length` trait counts the number of Unicode code points when applied to string shapes. - // https://awslabs.github.io/smithy/1.0/spec/core/constraint-traits.html#length-trait writer.documentShape(shape, model) writer.docs(rustDocsConstrainedTypeEpilogue(name)) - constrainedTypeMetadata.render(writer) + val metadata = symbol.expectRustMetadata() + metadata.render(writer) writer.rust("struct $name(pub(crate) $inner);") - if (constrainedTypeVisibility == Visibility.PUBCRATE) { + if (metadata.visibility == Visibility.PUBCRATE) { Attribute.AllowUnused.render(writer) } writer.rust( @@ -223,6 +211,9 @@ private data class Length(val lengthTrait: LengthTrait) : StringTraitInfo() { */ @Suppress("UNUSED_PARAMETER") private fun renderValidationFunction(constraintViolation: Symbol, unconstrainedTypeName: String): Writable = { + // Note that we're using the linear time check `chars().count()` instead of `len()` on the input value, since the + // Smithy specification says the `length` trait counts the number of Unicode code points when applied to string shapes. + // https://awslabs.github.io/smithy/1.0/spec/core/constraint-traits.html#length-trait rust( """ fn check_length(string: &str) -> Result<(), $constraintViolation> { diff --git a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt index 7a51ed19e2..484949efdc 100644 --- a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt +++ b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt @@ -2,6 +2,7 @@ package software.amazon.smithy.rust.codegen.server.smithy import io.kotest.assertions.throwables.shouldThrowAny import io.kotest.matchers.collections.shouldContainAll +import io.kotest.matchers.collections.shouldNotContain import org.junit.jupiter.api.Assertions.* import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.Arguments @@ -227,4 +228,16 @@ internal class DeriveEqAndHashSymbolMetadataProviderTest { deriveEqAndHashSymbolMetadataProvider.toSymbol(shape).expectRustMetadata() } } + + @ParameterizedTest + // These don't implement `PartialEq` because they are not constrained, so they don't generate newtypes. If the + // symbol provider wrapped `ConstrainedShapeSymbolProvider` and they were constrained, they would generate + // newtypes, and they would hence implement `PartialEq`. + @ValueSource(strings = ["test#List", "test#Map", "test#ListWithMap"]) + fun `it should not derive Eq if shape does not implement PartialEq`(shapeId: String) { + val shape = model.lookup(shapeId) + val derives = deriveEqAndHashSymbolMetadataProvider.toSymbol(shape).expectRustMetadata().derives + derives shouldNotContain RuntimeType.PartialEq + derives shouldNotContain RuntimeType.Eq + } } From 7fb08a64d366aad9af40d84166f76d8f82084b41 Mon Sep 17 00:00:00 2001 From: david-perez Date: Wed, 18 Jan 2023 14:17:53 +0100 Subject: [PATCH 04/13] Cleanup --- CHANGELOG.next.toml | 6 +++++- .../server/smithy/ConstrainedShapeSymbolMetadataProvider.kt | 5 +++++ .../server/smithy/DeriveEqAndHashSymbolMetadataProvider.kt | 5 +++++ .../smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt | 5 +++++ 4 files changed, 20 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index e3e0ea7994..bc95e119b4 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -11,4 +11,8 @@ # meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"} # author = "rcoh" -TODO DateTime, Blob now implement PartialEq, Hash +[[smithy-rs]] +message = "`aws_smithy_types::date_time::DateTime`, `aws_smithy_types::Blob` now implement the `PartialEq` and `Hash` traits" +references = ["smithy-rs#2223"] +meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "all"} +author = "david-perez" diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolMetadataProvider.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolMetadataProvider.kt index ebe56e4091..13400b84ad 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolMetadataProvider.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolMetadataProvider.kt @@ -1,3 +1,8 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + package software.amazon.smithy.rust.codegen.server.smithy import software.amazon.smithy.model.Model diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProvider.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProvider.kt index 2b08785394..a507ab9ce0 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProvider.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProvider.kt @@ -1,3 +1,8 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + package software.amazon.smithy.rust.codegen.server.smithy import software.amazon.smithy.model.Model diff --git a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt index 484949efdc..7dac0c284c 100644 --- a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt +++ b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt @@ -1,3 +1,8 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + package software.amazon.smithy.rust.codegen.server.smithy import io.kotest.assertions.throwables.shouldThrowAny From e478aef7fc065b03ddec23327d9e5887b86b0bc6 Mon Sep 17 00:00:00 2001 From: david-perez Date: Wed, 18 Jan 2023 14:27:10 +0100 Subject: [PATCH 05/13] More cleanup --- .../core/smithy/StreamingTraitSymbolProvider.kt | 1 + .../codegen/core/smithy/SymbolMetadataProvider.kt | 4 ++++ .../python/smithy/PythonServerSymbolProvider.kt | 1 + .../ConstrainedShapeSymbolMetadataProvider.kt | 4 +++- .../DeriveEqAndHashSymbolMetadataProvider.kt | 2 ++ .../smithy/generators/ConstrainedBlobGenerator.kt | 15 +++------------ .../DeriveEqAndHashSymbolMetadataProviderTest.kt | 4 ++-- 7 files changed, 16 insertions(+), 15 deletions(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/StreamingTraitSymbolProvider.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/StreamingTraitSymbolProvider.kt index 34976309c5..3e1d082627 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/StreamingTraitSymbolProvider.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/StreamingTraitSymbolProvider.kt @@ -84,4 +84,5 @@ class StreamingShapeMetadataProvider( override fun mapMeta(mapShape: MapShape) = base.toSymbol(mapShape).expectRustMetadata() override fun stringMeta(stringShape: StringShape) = base.toSymbol(stringShape).expectRustMetadata() override fun numberMeta(numberShape: NumberShape) = base.toSymbol(numberShape).expectRustMetadata() + override fun blobMeta(blobShape: BlobShape) = base.toSymbol(blobShape).expectRustMetadata() } diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt index 58b5904108..a7017b504e 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt @@ -8,6 +8,7 @@ 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.BlobShape import software.amazon.smithy.model.shapes.CollectionShape import software.amazon.smithy.model.shapes.ListShape import software.amazon.smithy.model.shapes.MapShape @@ -62,6 +63,7 @@ abstract class SymbolMetadataProvider(private val base: RustSymbolProvider) : Wr is ListShape -> listMeta(shape) is MapShape -> mapMeta(shape) is NumberShape -> numberMeta(shape) + is BlobShape -> blobMeta(shape) is StringShape -> if (shape.hasTrait()) { enumMeta(shape) } else { @@ -82,6 +84,7 @@ abstract class SymbolMetadataProvider(private val base: RustSymbolProvider) : Wr abstract fun mapMeta(mapShape: MapShape): RustMetadata abstract fun stringMeta(stringShape: StringShape): RustMetadata abstract fun numberMeta(numberShape: NumberShape): RustMetadata + abstract fun blobMeta(blobShape: BlobShape): RustMetadata } fun containerDefaultMetadata( @@ -163,6 +166,7 @@ class BaseSymbolMetadataProvider( override fun mapMeta(mapShape: MapShape) = defaultRustMetadata override fun stringMeta(stringShape: StringShape) = defaultRustMetadata override fun numberMeta(numberShape: NumberShape) = defaultRustMetadata + override fun blobMeta(blobShape: BlobShape) = defaultRustMetadata } private const val META_KEY = "meta" diff --git a/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonServerSymbolProvider.kt b/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonServerSymbolProvider.kt index d7a28a79f9..2b38d94146 100644 --- a/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonServerSymbolProvider.kt +++ b/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonServerSymbolProvider.kt @@ -112,4 +112,5 @@ class PythonStreamingShapeMetadataProvider(private val base: RustSymbolProvider, override fun mapMeta(mapShape: MapShape) = base.toSymbol(mapShape).expectRustMetadata() override fun stringMeta(stringShape: StringShape) = base.toSymbol(stringShape).expectRustMetadata() override fun numberMeta(numberShape: NumberShape) = base.toSymbol(numberShape).expectRustMetadata() + override fun blobMeta(blobShape: BlobShape) = base.toSymbol(blobShape).expectRustMetadata() } diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolMetadataProvider.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolMetadataProvider.kt index 13400b84ad..01e8255ccc 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolMetadataProvider.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolMetadataProvider.kt @@ -6,6 +6,7 @@ package software.amazon.smithy.rust.codegen.server.smithy import software.amazon.smithy.model.Model +import software.amazon.smithy.model.shapes.BlobShape import software.amazon.smithy.model.shapes.ListShape import software.amazon.smithy.model.shapes.MapShape import software.amazon.smithy.model.shapes.MemberShape @@ -38,7 +39,7 @@ class ConstrainedShapeSymbolMetadataProvider( override fun enumMeta(stringShape: StringShape) = base.toSymbol(stringShape).expectRustMetadata() private fun addDerivesAndAdjustVisibilityIfConstrained(shape: Shape): RustMetadata { - check(shape is ListShape || shape is MapShape || shape is StringShape || shape is NumberShape) + check(shape is ListShape || shape is MapShape || shape is StringShape || shape is NumberShape || shape is BlobShape) val baseMetadata = base.toSymbol(shape).expectRustMetadata() val derives = baseMetadata.derives.toMutableSet() @@ -56,4 +57,5 @@ class ConstrainedShapeSymbolMetadataProvider( override fun mapMeta(mapShape: MapShape): RustMetadata = addDerivesAndAdjustVisibilityIfConstrained(mapShape) override fun stringMeta(stringShape: StringShape): RustMetadata = addDerivesAndAdjustVisibilityIfConstrained(stringShape) override fun numberMeta(numberShape: NumberShape): RustMetadata = addDerivesAndAdjustVisibilityIfConstrained(numberShape) + override fun blobMeta(blobShape: BlobShape) = addDerivesAndAdjustVisibilityIfConstrained(blobShape) } diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProvider.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProvider.kt index a507ab9ce0..5438447ed5 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProvider.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProvider.kt @@ -6,6 +6,7 @@ package software.amazon.smithy.rust.codegen.server.smithy import software.amazon.smithy.model.Model +import software.amazon.smithy.model.shapes.BlobShape import software.amazon.smithy.model.shapes.DocumentShape import software.amazon.smithy.model.shapes.DoubleShape import software.amazon.smithy.model.shapes.FloatShape @@ -91,4 +92,5 @@ class DeriveEqAndHashSymbolMetadataProvider( override fun mapMeta(mapShape: MapShape): RustMetadata = addDeriveEqAndHashIfPossible(mapShape) override fun stringMeta(stringShape: StringShape): RustMetadata = addDeriveEqAndHashIfPossible(stringShape) override fun numberMeta(numberShape: NumberShape): RustMetadata = addDeriveEqAndHashIfPossible(numberShape) + override fun blobMeta(blobShape: BlobShape): RustMetadata = addDeriveEqAndHashIfPossible(blobShape) } diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedBlobGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedBlobGenerator.kt index db2623af76..df948cc4e6 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedBlobGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedBlobGenerator.kt @@ -57,22 +57,13 @@ class ConstrainedBlobGenerator( val inner = RuntimeType.blob(codegenContext.runtimeConfig).toSymbol().rustType().render() val constraintViolation = constraintViolationSymbolProvider.toSymbol(shape) - val constrainedTypeVisibility = if (publicConstrainedTypes) { - Visibility.PUBLIC - } else { - Visibility.PUBCRATE - } - val constrainedTypeMetadata = RustMetadata( - setOf(RuntimeType.Debug, RuntimeType.Clone, RuntimeType.PartialEq, RuntimeType.Eq, RuntimeType.Hash), - visibility = constrainedTypeVisibility, - ) - writer.documentShape(shape, model) writer.docs(rustDocsConstrainedTypeEpilogue(name)) - constrainedTypeMetadata.render(writer) + val metadata = symbol.expectRustMetadata() + metadata.render(writer) writer.rust("struct $name(pub(crate) $inner);") writer.rustBlock("impl $name") { - if (constrainedTypeVisibility == Visibility.PUBLIC) { + if (metadata.visibility == Visibility.PUBLIC) { writer.rust( """ /// ${rustDocsInnerMethod(inner)} diff --git a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt index 7dac0c284c..f2a9248616 100644 --- a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt +++ b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt @@ -225,8 +225,8 @@ internal class DeriveEqAndHashSymbolMetadataProviderTest { @ParameterizedTest @ValueSource(strings = ["smithy.api#Blob", "test#BlobStream"]) - fun `blobs have no metadata`(shapeId: String) { - // Blobs have no metadata. `Eq` and `Hash` are derived on `aws_smithy_types::Blob` instead. + fun `unconstrained blobs have no metadata`(shapeId: String) { + // Unconstrained blobs have no metadata. `Eq` and `Hash` are derived on `aws_smithy_types::Blob` instead. val shape = model.lookup(shapeId) shouldThrowAny { From 40aa12f28795ffefd6415e2695e6f4914b2a785f Mon Sep 17 00:00:00 2001 From: david-perez Date: Wed, 18 Jan 2023 14:29:57 +0100 Subject: [PATCH 06/13] ./gradlew ktlintFormat --- .../smithy/generators/ConstrainedBlobGenerator.kt | 2 -- .../generators/ConstrainedCollectionGenerator.kt | 1 - .../smithy/generators/ConstrainedMapGenerator.kt | 1 - .../smithy/generators/ConstrainedNumberGenerator.kt | 1 - .../smithy/generators/ConstrainedStringGenerator.kt | 1 - .../DeriveEqAndHashSymbolMetadataProviderTest.kt | 11 +++++------ 6 files changed, 5 insertions(+), 12 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedBlobGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedBlobGenerator.kt index df948cc4e6..41fec1cec7 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedBlobGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedBlobGenerator.kt @@ -8,8 +8,6 @@ package software.amazon.smithy.rust.codegen.server.smithy.generators import software.amazon.smithy.codegen.core.Symbol import software.amazon.smithy.model.shapes.BlobShape import software.amazon.smithy.model.traits.LengthTrait -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.RustWriter import software.amazon.smithy.rust.codegen.core.rustlang.Visibility import software.amazon.smithy.rust.codegen.core.rustlang.Writable diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedCollectionGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedCollectionGenerator.kt index 54eb817cee..a29b12cec7 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedCollectionGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedCollectionGenerator.kt @@ -6,7 +6,6 @@ package software.amazon.smithy.rust.codegen.server.smithy.generators import software.amazon.smithy.codegen.core.Symbol -import software.amazon.smithy.codegen.core.SymbolProvider import software.amazon.smithy.model.shapes.CollectionShape import software.amazon.smithy.model.shapes.StructureShape import software.amazon.smithy.model.shapes.UnionShape diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedMapGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedMapGenerator.kt index 11d420a0a7..35b6187a0e 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedMapGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedMapGenerator.kt @@ -15,7 +15,6 @@ import software.amazon.smithy.rust.codegen.core.rustlang.Visibility import software.amazon.smithy.rust.codegen.core.rustlang.docs import software.amazon.smithy.rust.codegen.core.rustlang.documentShape import software.amazon.smithy.rust.codegen.core.rustlang.rust -import software.amazon.smithy.rust.codegen.core.rustlang.rustBlock import software.amazon.smithy.rust.codegen.core.rustlang.rustBlockTemplate import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedNumberGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedNumberGenerator.kt index 6a7c550a68..b7dc2b9fb8 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedNumberGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedNumberGenerator.kt @@ -13,7 +13,6 @@ import software.amazon.smithy.model.shapes.NumberShape import software.amazon.smithy.model.shapes.ShortShape import software.amazon.smithy.model.traits.RangeTrait 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.RustType import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter import software.amazon.smithy.rust.codegen.core.rustlang.Visibility diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedStringGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedStringGenerator.kt index ce15b84cf5..a8b56bd14f 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedStringGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedStringGenerator.kt @@ -12,7 +12,6 @@ import software.amazon.smithy.model.traits.LengthTrait import software.amazon.smithy.model.traits.PatternTrait import software.amazon.smithy.model.traits.Trait 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.RustType import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter import software.amazon.smithy.rust.codegen.core.rustlang.Visibility diff --git a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt index f2a9248616..1926400e60 100644 --- a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt +++ b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt @@ -8,7 +8,6 @@ package software.amazon.smithy.rust.codegen.server.smithy import io.kotest.assertions.throwables.shouldThrowAny import io.kotest.matchers.collections.shouldContainAll import io.kotest.matchers.collections.shouldNotContain -import org.junit.jupiter.api.Assertions.* import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.Arguments import org.junit.jupiter.params.provider.MethodSource @@ -24,7 +23,7 @@ import software.amazon.smithy.rust.codegen.server.smithy.testutil.serverTestSymb import java.util.stream.Stream internal class DeriveEqAndHashSymbolMetadataProviderTest { - private val model= + private val model = """ namespace test @@ -206,9 +205,9 @@ internal class DeriveEqAndHashSymbolMetadataProviderTest { return ( shapesWithNeitherEqNorHash.map { Arguments.of(it, emptyList()) } + - shapesWithEqAndHash.map { Arguments.of(it, listOf(RuntimeType.Eq, RuntimeType.Hash)) } + - shapesWithOnlyEq.map { Arguments.of(it, listOf(RuntimeType.Eq)) } - ).stream() + shapesWithEqAndHash.map { Arguments.of(it, listOf(RuntimeType.Eq, RuntimeType.Hash)) } + + shapesWithOnlyEq.map { Arguments.of(it, listOf(RuntimeType.Eq)) } + ).stream() } } @@ -216,7 +215,7 @@ internal class DeriveEqAndHashSymbolMetadataProviderTest { @MethodSource("getShapes") fun `it should derive Eq and Hash when possible`( shapeId: String, - expectedTraits: Collection + expectedTraits: Collection, ) { val shape = model.lookup(shapeId) val derives = deriveEqAndHashSymbolMetadataProvider.toSymbol(shape).expectRustMetadata().derives From 373a1ecb20b4d71086a1b5a578f26ab0d4197191 Mon Sep 17 00:00:00 2001 From: david-perez Date: Wed, 18 Jan 2023 14:35:08 +0100 Subject: [PATCH 07/13] Derive `Eq` and `Hash` wherever possible In server SDKs, these traits can be implemented by any shape _except_ if the shape's closure contains: 1. A `float`, `double`, or `document` shape: floating point types in Rust do not implement `Eq`. Similarly, [`document` shapes] may contain arbitrary JSON-like data containing floating point values. 2. A [@streaming] shape: all the streaming data would need to be buffered first to compare it. Additionally, the `Hash` trait cannot be implemented by shapes whose closure contains: 1. A `map` shape: we render `map` shapes as `std::collections::HashMap`, which _do not_ implement `Hash`. See https://github.com/awslabs/smithy/issues/1567. In **client SDKs, these traits cannot be derived on any code-generated Rust types corresponding to Smithy shapes**, since e.g. adding new optional members to a structure [is a backwards-compatible change], and doing so alters the semantics of these traits. However, this commit does implement these traits for the `aws_smithy_types::date_time::DateTime` and `aws_smithy_types::Blob` runtime types. This change is necessary to efficiently implement the `@uniqueItems` constraint trait in server SDKs. [`Eq`]: https://doc.rust-lang.org/std/cmp/trait.Eq.html [`Hash`]: https://doc.rust-lang.org/std/hash/trait.Hash.html [`document` shapes]: https://smithy.io/2.0/spec/simple-types.html#document [@streaming]: https://smithy.io/2.0/spec/streaming.html [is a backwards-compatible change]: https://smithy.io/2.0/guides/evolving-models.html#updating-structures From f1f5f8f490d7f74d71628442784b1d0498569815 Mon Sep 17 00:00:00 2001 From: david-perez Date: Wed, 18 Jan 2023 15:46:46 +0100 Subject: [PATCH 08/13] Fix tests --- .../DeriveEqAndHashSymbolMetadataProviderTest.kt | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt index 1926400e60..33ec037869 100644 --- a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt +++ b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt @@ -182,6 +182,7 @@ internal class DeriveEqAndHashSymbolMetadataProviderTest { "test#StreamingOperationInputOutput", "test#EventStreamOperationInputOutput", "test#StreamingUnion", + "test#BlobStream", "test#TestInputOutput", "test#HasFloat", "test#HasDouble", @@ -222,22 +223,11 @@ internal class DeriveEqAndHashSymbolMetadataProviderTest { derives shouldContainAll expectedTraits } - @ParameterizedTest - @ValueSource(strings = ["smithy.api#Blob", "test#BlobStream"]) - fun `unconstrained blobs have no metadata`(shapeId: String) { - // Unconstrained blobs have no metadata. `Eq` and `Hash` are derived on `aws_smithy_types::Blob` instead. - - val shape = model.lookup(shapeId) - shouldThrowAny { - deriveEqAndHashSymbolMetadataProvider.toSymbol(shape).expectRustMetadata() - } - } - @ParameterizedTest // These don't implement `PartialEq` because they are not constrained, so they don't generate newtypes. If the // symbol provider wrapped `ConstrainedShapeSymbolProvider` and they were constrained, they would generate // newtypes, and they would hence implement `PartialEq`. - @ValueSource(strings = ["test#List", "test#Map", "test#ListWithMap"]) + @ValueSource(strings = ["test#List", "test#Map", "test#ListWithMap", "smithy.api#Blob"]) fun `it should not derive Eq if shape does not implement PartialEq`(shapeId: String) { val shape = model.lookup(shapeId) val derives = deriveEqAndHashSymbolMetadataProvider.toSymbol(shape).expectRustMetadata().derives From fa929a64c1fcf84bdc9803c1cf439f5bfdeb10a1 Mon Sep 17 00:00:00 2001 From: david-perez Date: Wed, 18 Jan 2023 16:24:21 +0100 Subject: [PATCH 09/13] Update changelog --- CHANGELOG.next.toml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 12ea2158be..1ed5cdfb59 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -372,7 +372,13 @@ meta = { "breaking" = true, "tada" = false, "bug" = false } author = "ysaito1001" [[smithy-rs]] -message = "`aws_smithy_types::date_time::DateTime`, `aws_smithy_types::Blob` now implement the `PartialEq` and `Hash` traits" +message = "`aws_smithy_types::date_time::DateTime`, `aws_smithy_types::Blob` now implement the `Eq` and `Hash` traits" references = ["smithy-rs#2223"] meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "all"} author = "david-perez" + +[[smithy-rs]] +message = "Code-generated types for server SDKs now implement the `Eq` and `Hash` traits when possible" +references = ["smithy-rs#2223"] +meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "server"} +author = "david-perez" From d926b142d490f0ee538692dab0733caefc57709b Mon Sep 17 00:00:00 2001 From: david-perez Date: Wed, 18 Jan 2023 16:24:26 +0100 Subject: [PATCH 10/13] Derive `Eq` and `Hash` wherever possible In server SDKs, these traits can be implemented by any shape _except_ if the shape's closure contains: 1. A `float`, `double`, or `document` shape: floating point types in Rust do not implement `Eq`. Similarly, [`document` shapes] may contain arbitrary JSON-like data containing floating point values. 2. A [@streaming] shape: all the streaming data would need to be buffered first to compare it. Additionally, the `Hash` trait cannot be implemented by shapes whose closure contains: 1. A `map` shape: we render `map` shapes as `std::collections::HashMap`, which _do not_ implement `Hash`. See https://github.com/awslabs/smithy/issues/1567. In **client SDKs, these traits cannot be derived on any code-generated Rust types corresponding to Smithy shapes**, since e.g. adding new optional members to a structure [is a backwards-compatible change], and doing so alters the semantics of these traits. However, this commit does implement these traits for the `aws_smithy_types::date_time::DateTime` and `aws_smithy_types::Blob` runtime types. This change is necessary to efficiently implement the `@uniqueItems` constraint trait in server SDKs. This commit also introduces a constrained shape symbol metadata provider (`ConstrainedShapeSymbolMetadataProvider.kt`), to centralize generation of Rust metadata (derives, visibility) in one place, instead of each constrained type generator having to manually adjust metadata. Some constrained type methods are now conditionally generated based on visibility, instead of relying on `#[allow(dead_code)]`. [`Eq`]: https://doc.rust-lang.org/std/cmp/trait.Eq.html [`Hash`]: https://doc.rust-lang.org/std/hash/trait.Hash.html [`document` shapes]: https://smithy.io/2.0/spec/simple-types.html#document [@streaming]: https://smithy.io/2.0/spec/streaming.html [is a backwards-compatible change]: https://smithy.io/2.0/guides/evolving-models.html#updating-structures From f1f0b3ded359b2dbd1a6896ed5e1135486e73186 Mon Sep 17 00:00:00 2001 From: david-perez Date: Wed, 18 Jan 2023 17:10:58 +0100 Subject: [PATCH 11/13] Adjust Python --- .../server/python/smithy/PythonCodegenServerPlugin.kt | 8 +++++++- .../server/python/smithy/PythonServerSymbolProvider.kt | 2 +- rust-runtime/aws-smithy-http-server-python/src/types.rs | 4 ++-- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonCodegenServerPlugin.kt b/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonCodegenServerPlugin.kt index 3d8a57ef6b..76070f1650 100644 --- a/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonCodegenServerPlugin.kt +++ b/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonCodegenServerPlugin.kt @@ -17,7 +17,9 @@ import software.amazon.smithy.rust.codegen.core.smithy.EventStreamSymbolProvider import software.amazon.smithy.rust.codegen.core.smithy.SymbolVisitor import software.amazon.smithy.rust.codegen.core.smithy.SymbolVisitorConfig import software.amazon.smithy.rust.codegen.server.python.smithy.customizations.DECORATORS +import software.amazon.smithy.rust.codegen.server.smithy.ConstrainedShapeSymbolMetadataProvider import software.amazon.smithy.rust.codegen.server.smithy.ConstrainedShapeSymbolProvider +import software.amazon.smithy.rust.codegen.server.smithy.DeriveEqAndHashSymbolMetadataProvider import software.amazon.smithy.rust.codegen.server.smithy.customizations.ServerRequiredCustomizations import software.amazon.smithy.rust.codegen.server.smithy.customize.CombinedServerCodegenDecorator import java.util.logging.Level @@ -54,7 +56,7 @@ class PythonCodegenServerPlugin : SmithyBuildPlugin { } companion object { - /** SymbolProvider + /** * When generating code, smithy types need to be converted into Rust types—that is the core role of the symbol provider * * The Symbol provider is composed of a base [SymbolVisitor] which handles the core functionality, then is layered @@ -75,10 +77,14 @@ class PythonCodegenServerPlugin : SmithyBuildPlugin { .let { if (constrainedTypes) ConstrainedShapeSymbolProvider(it, model, serviceShape) else it } // Generate different types for EventStream shapes (e.g. transcribe streaming) .let { EventStreamSymbolProvider(symbolVisitorConfig.runtimeConfig, it, model, CodegenTarget.SERVER) } + // Constrained shapes generate newtypes that need the same derives we place on types generated from aggregate shapes. + .let { ConstrainedShapeSymbolMetadataProvider(it, model, constrainedTypes) } // Add Rust attributes (like `#[derive(PartialEq)]`) to generated shapes .let { BaseSymbolMetadataProvider(it, model, additionalAttributes = listOf()) } // Streaming shapes need different derives (e.g. they cannot derive Eq) .let { PythonStreamingShapeMetadataProvider(it, model) } + // Derive `Eq` and `Hash` if possible. + .let { DeriveEqAndHashSymbolMetadataProvider(it, model) } // 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, model) } diff --git a/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonServerSymbolProvider.kt b/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonServerSymbolProvider.kt index 2b38d94146..57e77ffe03 100644 --- a/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonServerSymbolProvider.kt +++ b/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonServerSymbolProvider.kt @@ -58,7 +58,7 @@ class PythonServerSymbolVisitor( val target = model.expectShape(shape.target) val container = model.expectShape(shape.container) - // We are only targetting non syntetic inputs and outputs. + // We are only targeting non-synthetic inputs and outputs. if (!container.hasTrait() && !container.hasTrait()) { return initial } diff --git a/rust-runtime/aws-smithy-http-server-python/src/types.rs b/rust-runtime/aws-smithy-http-server-python/src/types.rs index 1ce7779482..2fb127fb70 100644 --- a/rust-runtime/aws-smithy-http-server-python/src/types.rs +++ b/rust-runtime/aws-smithy-http-server-python/src/types.rs @@ -26,7 +26,7 @@ use crate::PyError; /// Python Wrapper for [aws_smithy_types::Blob]. #[pyclass] -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct Blob(aws_smithy_types::Blob); impl Blob { @@ -88,7 +88,7 @@ impl<'blob> From<&'blob Blob> for &'blob aws_smithy_types::Blob { /// Python Wrapper for [aws_smithy_types::date_time::DateTime]. #[pyclass] -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct DateTime(aws_smithy_types::date_time::DateTime); #[pyclass] From ad473540ba2773ae67edcae74811750a4ac6ed59 Mon Sep 17 00:00:00 2001 From: david-perez Date: Wed, 18 Jan 2023 17:25:45 +0100 Subject: [PATCH 12/13] Fix Python --- .../codegen/server/python/smithy/PythonCodegenServerPlugin.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonCodegenServerPlugin.kt b/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonCodegenServerPlugin.kt index 76070f1650..7e277185f7 100644 --- a/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonCodegenServerPlugin.kt +++ b/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonCodegenServerPlugin.kt @@ -77,10 +77,10 @@ class PythonCodegenServerPlugin : SmithyBuildPlugin { .let { if (constrainedTypes) ConstrainedShapeSymbolProvider(it, model, serviceShape) else it } // Generate different types for EventStream shapes (e.g. transcribe streaming) .let { EventStreamSymbolProvider(symbolVisitorConfig.runtimeConfig, it, model, CodegenTarget.SERVER) } - // Constrained shapes generate newtypes that need the same derives we place on types generated from aggregate shapes. - .let { ConstrainedShapeSymbolMetadataProvider(it, model, constrainedTypes) } // Add Rust attributes (like `#[derive(PartialEq)]`) to generated shapes .let { BaseSymbolMetadataProvider(it, model, additionalAttributes = listOf()) } + // Constrained shapes generate newtypes that need the same derives we place on types generated from aggregate shapes. + .let { ConstrainedShapeSymbolMetadataProvider(it, model, constrainedTypes) } // Streaming shapes need different derives (e.g. they cannot derive Eq) .let { PythonStreamingShapeMetadataProvider(it, model) } // Derive `Eq` and `Hash` if possible. From d6770597eda5d28ba8ce4ff4fe2b4b332da8423f Mon Sep 17 00:00:00 2001 From: david-perez Date: Thu, 19 Jan 2023 12:22:39 +0100 Subject: [PATCH 13/13] ./gradlew ktlintFormat --- .../server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt index 33ec037869..e347197276 100644 --- a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt +++ b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/DeriveEqAndHashSymbolMetadataProviderTest.kt @@ -5,7 +5,6 @@ package software.amazon.smithy.rust.codegen.server.smithy -import io.kotest.assertions.throwables.shouldThrowAny import io.kotest.matchers.collections.shouldContainAll import io.kotest.matchers.collections.shouldNotContain import org.junit.jupiter.params.ParameterizedTest