From 91ca287532a45bfce58b7f28d917119e30b8f968 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Thu, 24 Nov 2022 14:47:31 +0000 Subject: [PATCH 01/51] Refactor `ConstrainedMapGenerator` slightly --- .../rust/codegen/core/rustlang/RustType.kt | 11 ++++++++- .../generators/ConstrainedMapGenerator.kt | 24 +++++-------------- .../smithy/generators/LenghTraitCommon.kt | 15 ++++++++++++ 3 files changed, 31 insertions(+), 19 deletions(-) create mode 100644 codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/LenghTraitCommon.kt diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustType.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustType.kt index 120798258f..d10c8ea618 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustType.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustType.kt @@ -325,7 +325,16 @@ fun RustType.isCopy(): Boolean = when (this) { enum class Visibility { PRIVATE, PUBCRATE, - PUBLIC + PUBLIC; + + companion object { + fun publicIf(condition: Boolean, ifNot: Visibility): Visibility = + if (condition) { + PUBLIC + } else { + ifNot + } + } } /** 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 677350dd67..6379bdf36f 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 @@ -58,19 +58,7 @@ class ConstrainedMapGenerator( val inner = "std::collections::HashMap<#{KeySymbol}, #{ValueSymbol}>" val constraintViolation = constraintViolationSymbolProvider.toSymbol(shape) - val condition = if (lengthTrait.min.isPresent && lengthTrait.max.isPresent) { - "(${lengthTrait.min.get()}..=${lengthTrait.max.get()}).contains(&length)" - } else if (lengthTrait.min.isPresent) { - "${lengthTrait.min.get()} <= length" - } else { - "length <= ${lengthTrait.max.get()}" - } - - val constrainedTypeVisibility = if (publicConstrainedTypes) { - Visibility.PUBLIC - } else { - Visibility.PUBCRATE - } + val constrainedTypeVisibility = Visibility.publicIf(publicConstrainedTypes, Visibility.PUBCRATE) val constrainedTypeMetadata = RustMetadata( Attribute.Derives(setOf(RuntimeType.Debug, RuntimeType.Clone, RuntimeType.PartialEq)), visibility = constrainedTypeVisibility, @@ -97,27 +85,27 @@ class ConstrainedMapGenerator( pub fn inner(&self) -> &$inner { &self.0 } - + /// ${rustDocsIntoInnerMethod(inner)} pub fn into_inner(self) -> $inner { self.0 } } - + impl #{TryFrom}<$inner> for $name { type Error = #{ConstraintViolation}; - + /// ${rustDocsTryFromMethod(name, inner)} fn try_from(value: $inner) -> Result { let length = value.len(); - if $condition { + if ${lengthTrait.rustCondition()} { Ok(Self(value)) } else { Err(#{ConstraintViolation}::Length(length)) } } } - + impl #{From}<$name> for $inner { fn from(value: $name) -> Self { value.into_inner() diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/LenghTraitCommon.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/LenghTraitCommon.kt new file mode 100644 index 0000000000..f9b11f9d07 --- /dev/null +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/LenghTraitCommon.kt @@ -0,0 +1,15 @@ +package software.amazon.smithy.rust.codegen.server.smithy.generators + +import software.amazon.smithy.model.traits.LengthTrait + +fun LengthTrait.rustCondition(): String { + val condition = if (min.isPresent && max.isPresent) { + "(${min.get()}..=${max.get()}).contains(&length)" + } else if (min.isPresent) { + "${min.get()} <= length" + } else { + "length <= ${max.get()}" + } + + return condition +} From 4e7920280591c98989437e30c6796315368a0041 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Thu, 24 Nov 2022 15:21:17 +0000 Subject: [PATCH 02/51] Add `@length` list operation in `constraints.smithy` --- .../common-test-models/constraints.smithy | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/codegen-core/common-test-models/constraints.smithy b/codegen-core/common-test-models/constraints.smithy index 0bdfe6d2a1..fe4311b2c9 100644 --- a/codegen-core/common-test-models/constraints.smithy +++ b/codegen-core/common-test-models/constraints.smithy @@ -22,6 +22,7 @@ service ConstraintsService { QueryParamsTargetingMapOfEnumStringOperation, QueryParamsTargetingMapOfListOfLengthStringOperation, QueryParamsTargetingMapOfSetOfLengthStringOperation, + QueryParamsTargetingMapOfLengthListOfPatternStringOperation, QueryParamsTargetingMapOfListOfEnumStringOperation, QueryParamsTargetingMapOfPatternStringOperation, @@ -96,6 +97,13 @@ operation QueryParamsTargetingMapOfSetOfLengthStringOperation { errors: [ValidationException] } +@http(uri: "/query-params-targeting-map-of-length-list-of-pattern-string-operation", method: "POST") +operation QueryParamsTargetingMapOfLengthListOfPatternStringOperation { + input: QueryParamsTargetingMapOfLengthListOfPatternStringOperationInputOutput, + output: QueryParamsTargetingMapOfLengthListOfPatternStringOperationInputOutput, + errors: [ValidationException] +} + @http(uri: "/query-params-targeting-map-of-list-of-enum-string-operation", method: "POST") operation QueryParamsTargetingMapOfListOfEnumStringOperation { input: QueryParamsTargetingMapOfListOfEnumStringOperationInputOutput, @@ -276,6 +284,11 @@ structure QueryParamsTargetingMapOfSetOfLengthStringOperationInputOutput { mapOfSetOfLengthString: MapOfSetOfLengthString } +structure QueryParamsTargetingMapOfLengthListOfPatternStringOperationInputOutput { + @httpQueryParams + mapOfLengthListOfPatternString: MapOfLengthListOfPatternString +} + structure QueryParamsTargetingMapOfListOfEnumStringOperationInputOutput { @httpQueryParams mapOfListOfEnumString: MapOfListOfEnumString @@ -407,6 +420,11 @@ map MapOfSetOfLengthString { value: ListOfLengthString } +map MapOfLengthListOfPatternString { + key: PatternString, + value: LengthListOfPatternString +} + @length(min: 2, max: 8) list LengthListOfLengthString { member: LengthString @@ -492,6 +510,11 @@ list ListOfLengthPatternString { member: LengthPatternString } +@length(min:12, max: 39) +list LengthListOfPatternString { + member: PatternString +} + structure ConB { @required nice: String, From 000175c64316fcd0ddc73656a4a45f061fe5f645 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Thu, 24 Nov 2022 15:42:36 +0000 Subject: [PATCH 03/51] Add more setup required for rendering constraint `list`s --- .../rust/codegen/server/smithy/Constraints.kt | 6 ++++++ .../smithy/ValidateUnsupportedConstraints.kt | 9 ++++----- .../UnconstrainedCollectionGenerator.kt | 16 +++++++++++----- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt index 9acabf7532..b9f6b4ada3 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt @@ -45,6 +45,11 @@ fun Shape.hasConstraintTrait() = val supportedStringConstraintTraits: List> = listOf(LengthTrait::class.java, PatternTrait::class.java) +/** + * Supported constraint traits for the `list` and `set` shapes. + */ +val supportedCollectionConstraintTraits: List> = listOf(LengthTrait::class.java) + /** * We say a shape is _directly_ constrained if: * @@ -70,6 +75,7 @@ fun Shape.isDirectlyConstrained(symbolProvider: SymbolProvider): Boolean = when } is MapShape -> this.hasTrait() is StringShape -> this.hasTrait() || supportedStringConstraintTraits.any { this.hasTrait(it) } + is CollectionShape -> supportedCollectionConstraintTraits.any { this.hasTrait(it) } else -> false } diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt index 4535616bff..d827420e1d 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt @@ -8,7 +8,6 @@ package software.amazon.smithy.rust.codegen.server.smithy import software.amazon.smithy.model.Model import software.amazon.smithy.model.neighbor.Walker import software.amazon.smithy.model.shapes.BlobShape -import software.amazon.smithy.model.shapes.CollectionShape import software.amazon.smithy.model.shapes.EnumShape import software.amazon.smithy.model.shapes.MemberShape import software.amazon.smithy.model.shapes.OperationShape @@ -198,12 +197,12 @@ fun validateUnsupportedConstraints(model: Model, service: ServiceShape, codegenC .map { (shape, trait) -> UnsupportedConstraintOnShapeReachableViaAnEventStream(shape, trait) } .toSet() - // 4. Length trait on collection shapes or on blob shapes is used. It has not been implemented yet for these target types. + // 4. Length trait on blob shapes is used. It has not been implemented yet. // TODO(https://github.com/awslabs/smithy-rs/issues/1401) - val unsupportedLengthTraitOnCollectionOrOnBlobShapeSet = walker + val unsupportedLengthTraitOnBlobShapeSet = walker .walkShapes(service) .asSequence() - .filter { it is CollectionShape || it is BlobShape } + .filter { it is BlobShape } .filter { it.hasTrait() } .map { UnsupportedLengthTraitOnCollectionOrOnBlobShape(it, it.expectTrait()) } .toSet() @@ -230,7 +229,7 @@ fun validateUnsupportedConstraints(model: Model, service: ServiceShape, codegenC unsupportedConstraintOnMemberShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } + unsupportedLengthTraitOnStreamingBlobShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } + unsupportedConstraintOnShapeReachableViaAnEventStreamSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } + - unsupportedLengthTraitOnCollectionOrOnBlobShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } + + unsupportedLengthTraitOnBlobShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } + unsupportedRangeTraitOnShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } + unsupportedUniqueItemsTraitOnShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } 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 602cbb7aaf..e47892847d 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 @@ -16,6 +16,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.makeMaybeConstrained import software.amazon.smithy.rust.codegen.server.smithy.PubCrateConstraintViolationSymbolProvider import software.amazon.smithy.rust.codegen.server.smithy.ServerCodegenContext import software.amazon.smithy.rust.codegen.server.smithy.canReachConstrainedShape +import software.amazon.smithy.rust.codegen.server.smithy.isDirectlyConstrained import software.amazon.smithy.rust.codegen.server.smithy.traits.isReachableFromOperationInput /** @@ -48,6 +49,12 @@ class UnconstrainedCollectionGenerator( PubCrateConstraintViolationSymbolProvider(this) } } + private val constrainedShapeSymbolProvider = codegenContext.constrainedShapeSymbolProvider + private val constrainedSymbol = if (shape.isDirectlyConstrained(symbolProvider)) { + constrainedShapeSymbolProvider.toSymbol(shape) + } else { + pubCrateConstrainedShapeSymbolProvider.toSymbol(shape) + } fun render() { check(shape.canReachConstrainedShape(model, symbolProvider)) @@ -57,7 +64,6 @@ class UnconstrainedCollectionGenerator( val name = symbol.name val innerShape = model.expectShape(shape.member.target) val innerUnconstrainedSymbol = unconstrainedShapeSymbolProvider.toSymbol(innerShape) - val constrainedSymbol = pubCrateConstrainedShapeSymbolProvider.toSymbol(shape) val constraintViolationSymbol = constraintViolationSymbolProvider.toSymbol(shape) val constraintViolationName = constraintViolationSymbol.name val innerConstraintViolationSymbol = constraintViolationSymbolProvider.toSymbol(innerShape) @@ -67,16 +73,16 @@ class UnconstrainedCollectionGenerator( """ ##[derive(Debug, Clone)] pub(crate) struct $name(pub(crate) std::vec::Vec<#{InnerUnconstrainedSymbol}>); - + impl From<$name> for #{MaybeConstrained} { fn from(value: $name) -> Self { Self::Unconstrained(value) } } - + impl #{TryFrom}<$name> for #{ConstrainedSymbol} { type Error = #{ConstraintViolationSymbol}; - + fn try_from(value: $name) -> Result { let res: Result<_, (usize, #{InnerConstraintViolationSymbol})> = value .0 @@ -84,7 +90,7 @@ class UnconstrainedCollectionGenerator( .enumerate() .map(|(idx, inner)| inner.try_into().map_err(|inner_violation| (idx, inner_violation))) .collect(); - res.map(Self) + res.map(Self) .map_err(|(idx, inner_violation)| #{ConstraintViolationSymbol}(idx, inner_violation)) } } From 6299e29cf87cad87219151da9782d3d909fb483f Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Thu, 24 Nov 2022 17:00:25 +0000 Subject: [PATCH 04/51] Add initial draft of `ConstrainedCollectionGenerator` --- .../rust/codegen/server/smithy/Constraints.kt | 6 +- .../server/smithy/ServerCodegenVisitor.kt | 34 ++- .../ConstrainedCollectionGenerator.kt | 214 ++++++++++++++++++ 3 files changed, 248 insertions(+), 6 deletions(-) create mode 100644 codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedCollectionGenerator.kt diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt index b9f6b4ada3..653d0d3bb7 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt @@ -48,7 +48,11 @@ val supportedStringConstraintTraits: List> = listOf(LengthTrait /** * Supported constraint traits for the `list` and `set` shapes. */ -val supportedCollectionConstraintTraits: List> = listOf(LengthTrait::class.java) +val supportedCollectionConstraintTraits: List> = listOf( + LengthTrait::class.java, + // TODO(https://github.com/awslabs/smithy-rs/issues/1670): Not yet supported. + // UniqueItemsTrait::class.java +) /** * We say a shape is _directly_ constrained if: diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt index 8a3c5649bc..7d121a73dd 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt @@ -43,6 +43,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.transformers.RecursiveSha import software.amazon.smithy.rust.codegen.core.util.CommandFailed import software.amazon.smithy.rust.codegen.core.util.hasTrait import software.amazon.smithy.rust.codegen.core.util.runCommand +import software.amazon.smithy.rust.codegen.server.smithy.generators.ConstrainedCollectionGenerator import software.amazon.smithy.rust.codegen.server.smithy.generators.ConstrainedMapGenerator import software.amazon.smithy.rust.codegen.server.smithy.generators.ConstrainedStringGenerator import software.amazon.smithy.rust.codegen.server.smithy.generators.ConstrainedTraitForEnumGenerator @@ -288,11 +289,13 @@ open class ServerCodegenVisitor( override fun setShape(shape: SetShape) = collectionShape(shape) private fun collectionShape(shape: CollectionShape) { - if (shape.isReachableFromOperationInput() && shape.canReachConstrainedShape( + val renderUnconstrainedList = + shape.isReachableFromOperationInput() && shape.canReachConstrainedShape( model, codegenContext.symbolProvider, ) - ) { + + if (renderUnconstrainedList) { logger.info("[rust-server-codegen] Generating an unconstrained type for collection shape $shape") rustCrate.withModule(unconstrainedModule) unconstrainedModuleWriter@{ rustCrate.withModule(ModelsModule) modelsModuleWriter@{ @@ -305,9 +308,30 @@ open class ServerCodegenVisitor( } } - logger.info("[rust-server-codegen] Generating a constrained type for collection shape $shape") - rustCrate.withModule(constrainedModule) { - PubCrateConstrainedCollectionGenerator(codegenContext, this, shape).render() + if (!shape.isDirectlyConstrained(codegenContext.symbolProvider)) { + logger.info("[rust-server-codegen] Generating a constrained type for collection shape $shape") + rustCrate.withModule(constrainedModule) { + PubCrateConstrainedCollectionGenerator(codegenContext, this, shape).render() + } + } + } + + val isDirectlyConstrained = shape.isDirectlyConstrained(codegenContext.symbolProvider) + if (isDirectlyConstrained) { + rustCrate.withModule(ModelsModule) { + ConstrainedCollectionGenerator( + codegenContext, + this, + shape, + if (renderUnconstrainedList) codegenContext.unconstrainedShapeSymbolProvider.toSymbol(shape) else null, + ).render() + } + } + + if (isDirectlyConstrained || renderUnconstrainedList) { + rustCrate.withModule(ModelsModule) { + // TODO + // ListConstraintViolationGenerator(codegenContext, this,shape).render() } } } 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 new file mode 100644 index 0000000000..f4aefa8389 --- /dev/null +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedCollectionGenerator.kt @@ -0,0 +1,214 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.rust.codegen.server.smithy.generators + +import software.amazon.smithy.codegen.core.Symbol +import software.amazon.smithy.model.shapes.CollectionShape +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.rustTemplate +import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType +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 +import software.amazon.smithy.rust.codegen.server.smithy.ServerCodegenContext +import software.amazon.smithy.rust.codegen.server.smithy.supportedCollectionConstraintTraits + +/** + * [ConstrainedCollectionGenerator] generates a wrapper tuple newtype holding a constrained `std::vec::Vec`. + * This type can be built from unconstrained values, yielding a `ConstraintViolation` when the input does not satisfy + * the constraints. + * + * The [`length` trait] is the only constraint trait applicable to list shapes. + * + * If [unconstrainedSymbol] is provided, the `MaybeConstrained` trait is implemented for the constrained type, using the + * [unconstrainedSymbol]'s associated type as the associated type for the trait. + * + * [`length` trait]: https://awslabs.github.io/smithy/1.0/spec/core/constraint-traits.html#length-trait + */ +class ConstrainedCollectionGenerator( + val codegenContext: ServerCodegenContext, + val writer: RustWriter, + val shape: CollectionShape, + private val unconstrainedSymbol: Symbol? = null, +) { + private val model = codegenContext.model + private val constrainedShapeSymbolProvider = codegenContext.constrainedShapeSymbolProvider + private val publicConstrainedTypes = codegenContext.settings.codegenConfig.publicConstrainedTypes + private val constraintViolationSymbolProvider = + with(codegenContext.constraintViolationSymbolProvider) { + if (publicConstrainedTypes) { + this + } else { + PubCrateConstraintViolationSymbolProvider(this) + } + } + private val symbolProvider = codegenContext.symbolProvider + private val constraintsInfo: List = + supportedCollectionConstraintTraits + .mapNotNull { shape.getTrait(it).orNull() } + .map(CollectionTraitInfo::fromTrait) + .map(CollectionTraitInfo::toTraitInfo) + + fun render() { + assert(constraintsInfo.isNotEmpty()) { + "`ConstrainedCollectionGenerator` can only be invoked for constrained collections, but this shape was unconstrained" + } + + 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( + Attribute.Derives(setOf(RuntimeType.Debug, RuntimeType.Clone, RuntimeType.PartialEq)), + visibility = constrainedTypeVisibility, + ) + + val codegenScope = arrayOf( + "ValueSymbol" to constrainedShapeSymbolProvider.toSymbol(model.expectShape(shape.member.target)), + "From" to RuntimeType.From, + "TryFrom" to RuntimeType.TryFrom, + "ConstraintViolation" to constraintViolation, + ) + + writer.documentShape(shape, model, note = rustDocsNote(name)) + constrainedTypeMetadata.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 + } + + /// ${rustDocsIntoInnerMethod(inner)} + pub fn into_inner(self) -> $inner { + self.0 + } + } + + impl #{TryFrom}<$inner> for $name { + type Error = #{ConstraintViolation}; + + /// ${rustDocsTryFromMethod(name, inner)} + fn try_from(value: $inner) -> Result { + #{ConstraintChecks:W} + + Ok(Self(value)) + } + } + + impl #{From}<$name> for $inner { + fn from(value: $name) -> Self { + value.into_inner() + } + } + """, + *codegenScope, + "ConstraintChecks" to constraintsInfo.map { it.tryFromCheck }.join("\n"), + ) + + if (!publicConstrainedTypes && isValueConstrained(shape, model, symbolProvider)) { + writer.rustTemplate( + """ + impl #{From}<$name> for #{FullyUnconstrainedSymbol} { + fn from(value: $name) -> Self { + value + .into_inner() + .into_iter() + .map(|v| v.into()) + .collect() + } + } + """, + *codegenScope, + "FullyUnconstrainedSymbol" to symbolProvider.toSymbol(shape), + ) + } + + if (unconstrainedSymbol != null) { + writer.rustTemplate( + """ + impl #{ConstrainedTrait} for $name { + type Unconstrained = #{UnconstrainedSymbol}; + } + """, + "ConstrainedTrait" to RuntimeType.ConstrainedTrait(), + "UnconstrainedSymbol" to unconstrainedSymbol, + ) + } + } +} + +private sealed class CollectionTraitInfo { + data class Length(val lengthTrait: LengthTrait) : CollectionTraitInfo() { + override fun toTraitInfo(): TraitInfo = + TraitInfo( + tryFromCheck = { + rust("Self::check_length(value.len())?;") + }, + constraintViolationVariant = { + docs("Constraint violation error when the vector doesn't have the required length") + rust("Length(usize)") + }, + asValidationExceptionField = { + // This should be unused here I think + }, + validationFunctionDefinition = { constraintViolation -> + { + rustTemplate( + """ + fn check_length(length: usize) -> Result<(), #{ConstraintViolation}> { + if ${lengthTrait.rustCondition()} { + Ok(()) + } else { + Err(#{ConstraintViolation}::Length(length)) + } + } + """, + "ConstraintViolation" to constraintViolation, + ) + } + }, + ) + } + + companion object { + fun fromTrait(trait: Trait): CollectionTraitInfo = + when (trait) { + is LengthTrait -> { + Length(trait) + } + // TODO(https://github.com/awslabs/smithy-rs/issues/1670): Not implemented yet. + // is UniqueItemsTrait -> { + // UniqueItems(trait) + // } + else -> { + PANIC("CollectionTraitInfo.fromTrait called with unsupported trait $trait") + } + } + } + + abstract fun toTraitInfo(): TraitInfo +} From 617591d2ffec1c6325ba1e98d384b4e6a171403a Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Thu, 24 Nov 2022 17:00:49 +0000 Subject: [PATCH 05/51] Add license header to `LengthTraitCommon` --- .../codegen/server/smithy/generators/LenghTraitCommon.kt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/LenghTraitCommon.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/LenghTraitCommon.kt index f9b11f9d07..8a2763acb9 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/LenghTraitCommon.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/LenghTraitCommon.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.generators import software.amazon.smithy.model.traits.LengthTrait From d0a6faba8771835fe481727ebd2f949341dcdc93 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Fri, 25 Nov 2022 09:17:55 +0000 Subject: [PATCH 06/51] Add draft of collection constraint violation generation Copy `MapCostraintViolationGenerator` into `CollectionConstraintViolationGenerator` for now. --- .../server/smithy/ServerCodegenVisitor.kt | 4 +- .../CollectionConstraintViolationGenerator.kt | 121 ++++++++++++++++++ 2 files changed, 123 insertions(+), 2 deletions(-) create mode 100644 codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt index 7d121a73dd..ae791c9b2c 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt @@ -43,6 +43,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.transformers.RecursiveSha import software.amazon.smithy.rust.codegen.core.util.CommandFailed import software.amazon.smithy.rust.codegen.core.util.hasTrait import software.amazon.smithy.rust.codegen.core.util.runCommand +import software.amazon.smithy.rust.codegen.server.smithy.generators.CollectionConstraintViolationGenerator import software.amazon.smithy.rust.codegen.server.smithy.generators.ConstrainedCollectionGenerator import software.amazon.smithy.rust.codegen.server.smithy.generators.ConstrainedMapGenerator import software.amazon.smithy.rust.codegen.server.smithy.generators.ConstrainedStringGenerator @@ -330,8 +331,7 @@ open class ServerCodegenVisitor( if (isDirectlyConstrained || renderUnconstrainedList) { rustCrate.withModule(ModelsModule) { - // TODO - // ListConstraintViolationGenerator(codegenContext, this,shape).render() + CollectionConstraintViolationGenerator(codegenContext, this, shape).render() } } } diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt new file mode 100644 index 0000000000..bd03b8cb73 --- /dev/null +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt @@ -0,0 +1,121 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.rust.codegen.server.smithy.generators + +import software.amazon.smithy.model.shapes.MapShape +import software.amazon.smithy.model.shapes.StringShape +import software.amazon.smithy.model.traits.LengthTrait +import software.amazon.smithy.rust.codegen.core.rustlang.RustMetadata +import software.amazon.smithy.rust.codegen.core.rustlang.RustModule +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.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.util.getTrait +import software.amazon.smithy.rust.codegen.core.util.hasTrait +import software.amazon.smithy.rust.codegen.server.smithy.PubCrateConstraintViolationSymbolProvider +import software.amazon.smithy.rust.codegen.server.smithy.ServerCodegenContext +import software.amazon.smithy.rust.codegen.server.smithy.traits.isReachableFromOperationInput +import software.amazon.smithy.rust.codegen.server.smithy.validationErrorMessage + +class CollectionConstraintViolationGenerator( + codegenContext: ServerCodegenContext, + private val modelsModuleWriter: RustWriter, + val shape: MapShape, +) { + private val model = codegenContext.model + private val constrainedShapeSymbolProvider = codegenContext.constrainedShapeSymbolProvider + private val symbolProvider = codegenContext.symbolProvider + private val publicConstrainedTypes = codegenContext.settings.codegenConfig.publicConstrainedTypes + private val constraintViolationSymbolProvider = + with(codegenContext.constraintViolationSymbolProvider) { + if (publicConstrainedTypes) { + this + } else { + PubCrateConstraintViolationSymbolProvider(this) + } + } + + fun render() { + val keyShape = model.expectShape(shape.key.target, StringShape::class.java) + val valueShape = model.expectShape(shape.value.target) + val constraintViolationSymbol = constraintViolationSymbolProvider.toSymbol(shape) + val constraintViolationName = constraintViolationSymbol.name + + val constraintViolationCodegenScopeMutableList: MutableList> = mutableListOf() + if (isKeyConstrained(keyShape, symbolProvider)) { + constraintViolationCodegenScopeMutableList.add("KeyConstraintViolationSymbol" to constraintViolationSymbolProvider.toSymbol(keyShape)) + } + if (isValueConstrained(valueShape, model, symbolProvider)) { + constraintViolationCodegenScopeMutableList.add("ValueConstraintViolationSymbol" to constraintViolationSymbolProvider.toSymbol(valueShape)) + constraintViolationCodegenScopeMutableList.add("KeySymbol" to constrainedShapeSymbolProvider.toSymbol(keyShape)) + } + val constraintViolationCodegenScope = constraintViolationCodegenScopeMutableList.toTypedArray() + + val constraintViolationVisibility = if (publicConstrainedTypes) { + Visibility.PUBLIC + } else { + Visibility.PUBCRATE + } + modelsModuleWriter.withModule( + RustModule( + constraintViolationSymbol.namespace.split(constraintViolationSymbol.namespaceDelimiter).last(), + RustMetadata(visibility = constraintViolationVisibility), + ), + ) { + // TODO(https://github.com/awslabs/smithy-rs/issues/1401) We should really have two `ConstraintViolation` + // types here. One will just have variants for each constraint trait on the map shape, for use by the user. + // The other one will have variants if the shape's key or value is directly or transitively constrained, + // and is for use by the framework. + rustTemplate( + """ + ##[derive(Debug, PartialEq)] + pub${ if (constraintViolationVisibility == Visibility.PUBCRATE) " (crate) " else "" } enum $constraintViolationName { + ${if (shape.hasTrait()) "Length(usize)," else ""} + ${if (isKeyConstrained(keyShape, symbolProvider)) "##[doc(hidden)] Key(#{KeyConstraintViolationSymbol})," else ""} + ${if (isValueConstrained(valueShape, model, symbolProvider)) "##[doc(hidden)] Value(#{KeySymbol}, #{ValueConstraintViolationSymbol})," else ""} + } + """, + *constraintViolationCodegenScope, + ) + + if (shape.isReachableFromOperationInput()) { + rustBlock("impl $constraintViolationName") { + rustBlockTemplate( + "pub(crate) fn as_validation_exception_field(self, path: #{String}) -> crate::model::ValidationExceptionField", + "String" to RuntimeType.String, + ) { + rustBlock("match self") { + shape.getTrait()?.also { + rust( + """ + Self::Length(length) => crate::model::ValidationExceptionField { + message: format!("${it.validationErrorMessage()}", length, &path), + path, + }, + """, + ) + } + if (isKeyConstrained(keyShape, symbolProvider)) { + // Note how we _do not_ append the key's member name to the path. This is intentional, as + // per the `RestJsonMalformedLengthMapKey` test. Note keys are always strings. + // https://github.com/awslabs/smithy/blob/ee0b4ff90daaaa5101f32da936c25af8c91cc6e9/smithy-aws-protocol-tests/model/restJson1/validation/malformed-length.smithy#L296-L295 + rust("""Self::Key(key_constraint_violation) => key_constraint_violation.as_validation_exception_field(path),""") + } + if (isValueConstrained(valueShape, model, symbolProvider)) { + // `as_str()` works with regular `String`s and constrained string shapes. + rust("""Self::Value(key, value_constraint_violation) => value_constraint_violation.as_validation_exception_field(path + "/" + key.as_str()),""") + } + } + } + } + } + } + } +} From b71d1a97a52291ff65cdea6ea588da645de767e8 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Fri, 25 Nov 2022 09:30:28 +0000 Subject: [PATCH 07/51] Add `Visibility.toRustQualifier` --- .../amazon/smithy/rust/codegen/core/rustlang/RustType.kt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustType.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustType.kt index d10c8ea618..f7172b271b 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustType.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustType.kt @@ -335,6 +335,13 @@ enum class Visibility { ifNot } } + + fun toRustQualifier(): String = + when (this) { + PRIVATE -> "" + PUBCRATE -> "pub(crate)" + PUBLIC -> "pub" + } } /** From e684ce7200a1eb1167bdb2976c2f4faa5f740d62 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Fri, 25 Nov 2022 09:30:45 +0000 Subject: [PATCH 08/51] Implement `CollectionConstraintViolationGenerator` --- .../CollectionConstraintViolationGenerator.kt | 48 +++++++------------ 1 file changed, 18 insertions(+), 30 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt index bd03b8cb73..09fe11ee1c 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt @@ -5,8 +5,7 @@ package software.amazon.smithy.rust.codegen.server.smithy.generators -import software.amazon.smithy.model.shapes.MapShape -import software.amazon.smithy.model.shapes.StringShape +import software.amazon.smithy.model.shapes.CollectionShape import software.amazon.smithy.model.traits.LengthTrait import software.amazon.smithy.rust.codegen.core.rustlang.RustMetadata import software.amazon.smithy.rust.codegen.core.rustlang.RustModule @@ -21,13 +20,15 @@ import software.amazon.smithy.rust.codegen.core.util.getTrait import software.amazon.smithy.rust.codegen.core.util.hasTrait import software.amazon.smithy.rust.codegen.server.smithy.PubCrateConstraintViolationSymbolProvider import software.amazon.smithy.rust.codegen.server.smithy.ServerCodegenContext +import software.amazon.smithy.rust.codegen.server.smithy.canReachConstrainedShape import software.amazon.smithy.rust.codegen.server.smithy.traits.isReachableFromOperationInput import software.amazon.smithy.rust.codegen.server.smithy.validationErrorMessage class CollectionConstraintViolationGenerator( codegenContext: ServerCodegenContext, private val modelsModuleWriter: RustWriter, - val shape: MapShape, + val shape: CollectionShape, +// val constraintsInfo : List, ) { private val model = codegenContext.model private val constrainedShapeSymbolProvider = codegenContext.constrainedShapeSymbolProvider @@ -43,26 +44,21 @@ class CollectionConstraintViolationGenerator( } fun render() { - val keyShape = model.expectShape(shape.key.target, StringShape::class.java) - val valueShape = model.expectShape(shape.value.target) + val memberShape = model.expectShape(shape.member.target) val constraintViolationSymbol = constraintViolationSymbolProvider.toSymbol(shape) val constraintViolationName = constraintViolationSymbol.name val constraintViolationCodegenScopeMutableList: MutableList> = mutableListOf() - if (isKeyConstrained(keyShape, symbolProvider)) { - constraintViolationCodegenScopeMutableList.add("KeyConstraintViolationSymbol" to constraintViolationSymbolProvider.toSymbol(keyShape)) - } - if (isValueConstrained(valueShape, model, symbolProvider)) { - constraintViolationCodegenScopeMutableList.add("ValueConstraintViolationSymbol" to constraintViolationSymbolProvider.toSymbol(valueShape)) - constraintViolationCodegenScopeMutableList.add("KeySymbol" to constrainedShapeSymbolProvider.toSymbol(keyShape)) + val isMemberConstrained = memberShape.canReachConstrainedShape(model, symbolProvider) + + if (isMemberConstrained) { + constraintViolationCodegenScopeMutableList.add("MemberConstraintViolationSymbol" to constraintViolationSymbolProvider.toSymbol(memberShape)) } + val constraintViolationCodegenScope = constraintViolationCodegenScopeMutableList.toTypedArray() - val constraintViolationVisibility = if (publicConstrainedTypes) { - Visibility.PUBLIC - } else { - Visibility.PUBCRATE - } + val constraintViolationVisibility = Visibility.publicIf(publicConstrainedTypes, Visibility.PUBCRATE) + modelsModuleWriter.withModule( RustModule( constraintViolationSymbol.namespace.split(constraintViolationSymbol.namespaceDelimiter).last(), @@ -70,16 +66,15 @@ class CollectionConstraintViolationGenerator( ), ) { // TODO(https://github.com/awslabs/smithy-rs/issues/1401) We should really have two `ConstraintViolation` - // types here. One will just have variants for each constraint trait on the map shape, for use by the user. - // The other one will have variants if the shape's key or value is directly or transitively constrained, + // types here. One will just have variants for each constraint trait on the collection shape, for use by the user. + // The other one will have variants if the shape's member is directly or transitively constrained, // and is for use by the framework. rustTemplate( """ ##[derive(Debug, PartialEq)] - pub${ if (constraintViolationVisibility == Visibility.PUBCRATE) " (crate) " else "" } enum $constraintViolationName { + ${constraintViolationVisibility.toRustQualifier()} enum $constraintViolationName { ${if (shape.hasTrait()) "Length(usize)," else ""} - ${if (isKeyConstrained(keyShape, symbolProvider)) "##[doc(hidden)] Key(#{KeyConstraintViolationSymbol})," else ""} - ${if (isValueConstrained(valueShape, model, symbolProvider)) "##[doc(hidden)] Value(#{KeySymbol}, #{ValueConstraintViolationSymbol})," else ""} + ${if (isMemberConstrained) "##[doc(hidden)] Member(usize, #{MemberConstraintViolationSymbol})," else ""} } """, *constraintViolationCodegenScope, @@ -102,15 +97,8 @@ class CollectionConstraintViolationGenerator( """, ) } - if (isKeyConstrained(keyShape, symbolProvider)) { - // Note how we _do not_ append the key's member name to the path. This is intentional, as - // per the `RestJsonMalformedLengthMapKey` test. Note keys are always strings. - // https://github.com/awslabs/smithy/blob/ee0b4ff90daaaa5101f32da936c25af8c91cc6e9/smithy-aws-protocol-tests/model/restJson1/validation/malformed-length.smithy#L296-L295 - rust("""Self::Key(key_constraint_violation) => key_constraint_violation.as_validation_exception_field(path),""") - } - if (isValueConstrained(valueShape, model, symbolProvider)) { - // `as_str()` works with regular `String`s and constrained string shapes. - rust("""Self::Value(key, value_constraint_violation) => value_constraint_violation.as_validation_exception_field(path + "/" + key.as_str()),""") + if (isMemberConstrained) { + rust("""Self::Member(index, member_constraint_violation) => member_constraint_violation.as_validation_exception_field(path + "/" + &index.to_string()),""") } } } From c2554744355b765da17b9aa6774122ebd215df55 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Fri, 25 Nov 2022 10:35:43 +0000 Subject: [PATCH 09/51] Use `TraitInfo` on `CollectionConstraintViolationGenerator` --- .../server/smithy/ServerCodegenVisitor.kt | 5 +- .../CollectionConstraintViolationGenerator.kt | 72 +++++++++---------- .../ConstrainedCollectionGenerator.kt | 26 ++++--- 3 files changed, 54 insertions(+), 49 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt index ae791c9b2c..395f026fc4 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt @@ -44,6 +44,7 @@ import software.amazon.smithy.rust.codegen.core.util.CommandFailed import software.amazon.smithy.rust.codegen.core.util.hasTrait import software.amazon.smithy.rust.codegen.core.util.runCommand import software.amazon.smithy.rust.codegen.server.smithy.generators.CollectionConstraintViolationGenerator +import software.amazon.smithy.rust.codegen.server.smithy.generators.CollectionTraitInfo import software.amazon.smithy.rust.codegen.server.smithy.generators.ConstrainedCollectionGenerator import software.amazon.smithy.rust.codegen.server.smithy.generators.ConstrainedMapGenerator import software.amazon.smithy.rust.codegen.server.smithy.generators.ConstrainedStringGenerator @@ -317,6 +318,7 @@ open class ServerCodegenVisitor( } } + val constraintsInfo = CollectionTraitInfo.fromShape(shape) val isDirectlyConstrained = shape.isDirectlyConstrained(codegenContext.symbolProvider) if (isDirectlyConstrained) { rustCrate.withModule(ModelsModule) { @@ -324,6 +326,7 @@ open class ServerCodegenVisitor( codegenContext, this, shape, + constraintsInfo, if (renderUnconstrainedList) codegenContext.unconstrainedShapeSymbolProvider.toSymbol(shape) else null, ).render() } @@ -331,7 +334,7 @@ open class ServerCodegenVisitor( if (isDirectlyConstrained || renderUnconstrainedList) { rustCrate.withModule(ModelsModule) { - CollectionConstraintViolationGenerator(codegenContext, this, shape).render() + CollectionConstraintViolationGenerator(codegenContext, this, shape, constraintsInfo).render() } } } diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt index 09fe11ee1c..c1877ba8c8 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt @@ -6,32 +6,26 @@ package software.amazon.smithy.rust.codegen.server.smithy.generators import software.amazon.smithy.model.shapes.CollectionShape -import software.amazon.smithy.model.traits.LengthTrait import software.amazon.smithy.rust.codegen.core.rustlang.RustMetadata import software.amazon.smithy.rust.codegen.core.rustlang.RustModule 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.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.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.util.getTrait -import software.amazon.smithy.rust.codegen.core.util.hasTrait import software.amazon.smithy.rust.codegen.server.smithy.PubCrateConstraintViolationSymbolProvider import software.amazon.smithy.rust.codegen.server.smithy.ServerCodegenContext import software.amazon.smithy.rust.codegen.server.smithy.canReachConstrainedShape import software.amazon.smithy.rust.codegen.server.smithy.traits.isReachableFromOperationInput -import software.amazon.smithy.rust.codegen.server.smithy.validationErrorMessage class CollectionConstraintViolationGenerator( codegenContext: ServerCodegenContext, private val modelsModuleWriter: RustWriter, - val shape: CollectionShape, -// val constraintsInfo : List, + private val shape: CollectionShape, + private val constraintsInfo: List, ) { private val model = codegenContext.model - private val constrainedShapeSymbolProvider = codegenContext.constrainedShapeSymbolProvider private val symbolProvider = codegenContext.symbolProvider private val publicConstrainedTypes = codegenContext.settings.codegenConfig.publicConstrainedTypes private val constraintViolationSymbolProvider = @@ -47,16 +41,7 @@ class CollectionConstraintViolationGenerator( val memberShape = model.expectShape(shape.member.target) val constraintViolationSymbol = constraintViolationSymbolProvider.toSymbol(shape) val constraintViolationName = constraintViolationSymbol.name - - val constraintViolationCodegenScopeMutableList: MutableList> = mutableListOf() val isMemberConstrained = memberShape.canReachConstrainedShape(model, symbolProvider) - - if (isMemberConstrained) { - constraintViolationCodegenScopeMutableList.add("MemberConstraintViolationSymbol" to constraintViolationSymbolProvider.toSymbol(memberShape)) - } - - val constraintViolationCodegenScope = constraintViolationCodegenScopeMutableList.toTypedArray() - val constraintViolationVisibility = Visibility.publicIf(publicConstrainedTypes, Visibility.PUBCRATE) modelsModuleWriter.withModule( @@ -65,6 +50,18 @@ class CollectionConstraintViolationGenerator( RustMetadata(visibility = constraintViolationVisibility), ), ) { + var constraintViolationVariants = constraintsInfo.map { it.constraintViolationVariant } + if (isMemberConstrained) { + constraintViolationVariants += { + rustTemplate( + """ + ##[doc(hidden)] Member(usize, #{MemberConstraintViolationSymbol}) + """, + "MemberConstraintViolationSymbol" to constraintViolationSymbolProvider.toSymbol(memberShape), + ) + } + } + // TODO(https://github.com/awslabs/smithy-rs/issues/1401) We should really have two `ConstraintViolation` // types here. One will just have variants for each constraint trait on the collection shape, for use by the user. // The other one will have variants if the shape's member is directly or transitively constrained, @@ -73,36 +70,31 @@ class CollectionConstraintViolationGenerator( """ ##[derive(Debug, PartialEq)] ${constraintViolationVisibility.toRustQualifier()} enum $constraintViolationName { - ${if (shape.hasTrait()) "Length(usize)," else ""} - ${if (isMemberConstrained) "##[doc(hidden)] Member(usize, #{MemberConstraintViolationSymbol})," else ""} + #{ConstraintViolationVariants:W} } """, - *constraintViolationCodegenScope, + "ConstraintViolationVariants" to constraintViolationVariants.join(",\n"), ) if (shape.isReachableFromOperationInput()) { - rustBlock("impl $constraintViolationName") { - rustBlockTemplate( - "pub(crate) fn as_validation_exception_field(self, path: #{String}) -> crate::model::ValidationExceptionField", - "String" to RuntimeType.String, - ) { - rustBlock("match self") { - shape.getTrait()?.also { - rust( - """ - Self::Length(length) => crate::model::ValidationExceptionField { - message: format!("${it.validationErrorMessage()}", length, &path), - path, - }, - """, - ) - } - if (isMemberConstrained) { - rust("""Self::Member(index, member_constraint_violation) => member_constraint_violation.as_validation_exception_field(path + "/" + &index.to_string()),""") + var validationExceptionFields = constraintsInfo.map { it.asValidationExceptionField } + if (isMemberConstrained) { + validationExceptionFields += { rust("""Self::Member(index, member_constraint_violation) => member_constraint_violation.as_validation_exception_field(path + "/" + &index.toString())""") } + } + + rustTemplate( + """ + impl $constraintViolationName { + pub(crate) fn as_validation_exception_field(self, path: #{String}) -> crate::model::ValidationExceptionField { + match self { + #{AsValidationExceptionFields:W} } } } - } + """, + "String" to RuntimeType.String, + "AsValidationExceptionFields" to validationExceptionFields.join("\n"), + ) } } } 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 f4aefa8389..364d7ce729 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 @@ -24,6 +24,7 @@ import software.amazon.smithy.rust.codegen.core.util.orNull import software.amazon.smithy.rust.codegen.server.smithy.PubCrateConstraintViolationSymbolProvider import software.amazon.smithy.rust.codegen.server.smithy.ServerCodegenContext import software.amazon.smithy.rust.codegen.server.smithy.supportedCollectionConstraintTraits +import software.amazon.smithy.rust.codegen.server.smithy.validationErrorMessage /** * [ConstrainedCollectionGenerator] generates a wrapper tuple newtype holding a constrained `std::vec::Vec`. @@ -41,6 +42,7 @@ class ConstrainedCollectionGenerator( val codegenContext: ServerCodegenContext, val writer: RustWriter, val shape: CollectionShape, + private val constraintsInfo: List, private val unconstrainedSymbol: Symbol? = null, ) { private val model = codegenContext.model @@ -55,11 +57,6 @@ class ConstrainedCollectionGenerator( } } private val symbolProvider = codegenContext.symbolProvider - private val constraintsInfo: List = - supportedCollectionConstraintTraits - .mapNotNull { shape.getTrait(it).orNull() } - .map(CollectionTraitInfo::fromTrait) - .map(CollectionTraitInfo::toTraitInfo) fun render() { assert(constraintsInfo.isNotEmpty()) { @@ -161,7 +158,7 @@ class ConstrainedCollectionGenerator( } } -private sealed class CollectionTraitInfo { +internal sealed class CollectionTraitInfo { data class Length(val lengthTrait: LengthTrait) : CollectionTraitInfo() { override fun toTraitInfo(): TraitInfo = TraitInfo( @@ -173,7 +170,14 @@ private sealed class CollectionTraitInfo { rust("Length(usize)") }, asValidationExceptionField = { - // This should be unused here I think + rust( + """ + Self::Length(length) => crate::model::ValidationExceptionField { + message: format!("${lengthTrait.validationErrorMessage()}", length, &path), + path, + }, + """, + ) }, validationFunctionDefinition = { constraintViolation -> { @@ -195,7 +199,7 @@ private sealed class CollectionTraitInfo { } companion object { - fun fromTrait(trait: Trait): CollectionTraitInfo = + private fun fromTrait(trait: Trait): CollectionTraitInfo = when (trait) { is LengthTrait -> { Length(trait) @@ -208,6 +212,12 @@ private sealed class CollectionTraitInfo { PANIC("CollectionTraitInfo.fromTrait called with unsupported trait $trait") } } + + fun fromShape(shape: CollectionShape): List = + supportedCollectionConstraintTraits + .mapNotNull { shape.getTrait(it).orNull() } + .map(CollectionTraitInfo::fromTrait) + .map(CollectionTraitInfo::toTraitInfo) } abstract fun toTraitInfo(): TraitInfo From c109bb5b57baf1ff2016ef068833b26899276b3a Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Fri, 25 Nov 2022 14:04:18 +0000 Subject: [PATCH 10/51] Update docs on `ConstrainedCollectionGenerator` --- .../smithy/generators/ConstrainedCollectionGenerator.kt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) 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 364d7ce729..0159f61163 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 @@ -31,12 +31,14 @@ import software.amazon.smithy.rust.codegen.server.smithy.validationErrorMessage * This type can be built from unconstrained values, yielding a `ConstraintViolation` when the input does not satisfy * the constraints. * - * The [`length` trait] is the only constraint trait applicable to list shapes. + * The [`length`] and [`uniqueItems`] traits are the only constraint traits applicable to list shapes. + * The [`uniqueItems`] trait has not been implemented yet. * * If [unconstrainedSymbol] is provided, the `MaybeConstrained` trait is implemented for the constrained type, using the * [unconstrainedSymbol]'s associated type as the associated type for the trait. * - * [`length` trait]: https://awslabs.github.io/smithy/1.0/spec/core/constraint-traits.html#length-trait + * [`length`]: https://smithy.io/2.0/spec/constraint-traits.html#length-trait + * [`uniqueItem`]: https://smithy.io/2.0/spec/constraint-traits.html#smithy-api-uniqueitems-trait */ class ConstrainedCollectionGenerator( val codegenContext: ServerCodegenContext, From 4ebc39abe88a753a02f4bdd49b8cb01ef1633382 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Fri, 25 Nov 2022 14:32:07 +0000 Subject: [PATCH 11/51] Improve formatting on rust code --- .../generators/CollectionConstraintViolationGenerator.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt index c1877ba8c8..79bd4ba97c 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt @@ -55,7 +55,8 @@ class CollectionConstraintViolationGenerator( constraintViolationVariants += { rustTemplate( """ - ##[doc(hidden)] Member(usize, #{MemberConstraintViolationSymbol}) + ##[doc(hidden)] + Member(usize, #{MemberConstraintViolationSymbol}) """, "MemberConstraintViolationSymbol" to constraintViolationSymbolProvider.toSymbol(memberShape), ) From eaa4e6f24c17885fb99c0017fcaaf66acb27d73e Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Fri, 25 Nov 2022 14:34:09 +0000 Subject: [PATCH 12/51] Don't generate `ConstraintViolation` enum twice --- .../UnconstrainedCollectionGenerator.kt | 40 ------------------- 1 file changed, 40 deletions(-) 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 e47892847d..a62fe24b90 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 @@ -17,7 +17,6 @@ import software.amazon.smithy.rust.codegen.server.smithy.PubCrateConstraintViola import software.amazon.smithy.rust.codegen.server.smithy.ServerCodegenContext import software.amazon.smithy.rust.codegen.server.smithy.canReachConstrainedShape import software.amazon.smithy.rust.codegen.server.smithy.isDirectlyConstrained -import software.amazon.smithy.rust.codegen.server.smithy.traits.isReachableFromOperationInput /** * Generates a Rust type for a constrained collection shape that is able to hold values for the corresponding @@ -65,7 +64,6 @@ class UnconstrainedCollectionGenerator( val innerShape = model.expectShape(shape.member.target) val innerUnconstrainedSymbol = unconstrainedShapeSymbolProvider.toSymbol(innerShape) val constraintViolationSymbol = constraintViolationSymbolProvider.toSymbol(shape) - val constraintViolationName = constraintViolationSymbol.name val innerConstraintViolationSymbol = constraintViolationSymbolProvider.toSymbol(innerShape) unconstrainedModuleWriter.withModule(RustModule(module, RustMetadata(visibility = Visibility.PUBCRATE))) { @@ -103,43 +101,5 @@ class UnconstrainedCollectionGenerator( "TryFrom" to RuntimeType.TryFrom, ) } - - val constraintViolationVisibility = if (publicConstrainedTypes) { - Visibility.PUBLIC - } else { - Visibility.PUBCRATE - } - modelsModuleWriter.withModule( - RustModule( - constraintViolationSymbol.namespace.split(constraintViolationSymbol.namespaceDelimiter).last(), - RustMetadata(visibility = constraintViolationVisibility), - ), - ) { - // The first component of the tuple struct is the index in the collection where the first constraint - // violation was found. - rustTemplate( - """ - ##[derive(Debug, PartialEq)] - pub struct $constraintViolationName( - pub(crate) usize, - pub(crate) #{InnerConstraintViolationSymbol} - ); - """, - "InnerConstraintViolationSymbol" to innerConstraintViolationSymbol, - ) - - if (shape.isReachableFromOperationInput()) { - rustTemplate( - """ - impl $constraintViolationName { - pub(crate) fn as_validation_exception_field(self, path: #{String}) -> crate::model::ValidationExceptionField { - self.1.as_validation_exception_field(format!("{}/{}", path, &self.0)) - } - } - """, - "String" to RuntimeType.String, - ) - } - } } } From ba85018ecd4df6b920996f880f5772b1e5060bb4 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Fri, 25 Nov 2022 14:57:22 +0000 Subject: [PATCH 13/51] Make symbol provider work with constrained lists --- .../server/smithy/ConstrainedShapeSymbolProvider.kt | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolProvider.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolProvider.kt index 92ff5faf77..cb899d8831 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolProvider.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolProvider.kt @@ -54,8 +54,8 @@ class ConstrainedShapeSymbolProvider( ) : WrappingSymbolProvider(base) { private val nullableIndex = NullableIndex.of(model) - private fun publicConstrainedSymbolForMapShape(shape: Shape): Symbol { - check(shape is MapShape) + private fun publicConstrainedSymbolForMapOrCollectionShape(shape: Shape): Symbol { + check(shape is MapShape || shape is CollectionShape) val rustType = RustType.Opaque(shape.contextName(serviceShape).toPascalCase()) return symbolBuilder(shape, rustType).locatedIn(Models).build() @@ -74,7 +74,7 @@ class ConstrainedShapeSymbolProvider( is MapShape -> { if (shape.isDirectlyConstrained(base)) { check(shape.hasTrait()) { "Only the `length` constraint trait can be applied to maps" } - publicConstrainedSymbolForMapShape(shape) + publicConstrainedSymbolForMapOrCollectionShape(shape) } else { val keySymbol = this.toSymbol(shape.key) val valueSymbol = this.toSymbol(shape.value) @@ -85,11 +85,9 @@ class ConstrainedShapeSymbolProvider( } } is CollectionShape -> { - // TODO(https://github.com/awslabs/smithy-rs/issues/1401) Both arms return the same because we haven't - // implemented any constraint trait on collection shapes yet. if (shape.isDirectlyConstrained(base)) { - val inner = this.toSymbol(shape.member) - symbolBuilder(shape, RustType.Vec(inner.rustType())).addReference(inner).build() + // TODO: Perform some check on the constraint traits that the list is tagged with. + publicConstrainedSymbolForMapOrCollectionShape(shape) } else { val inner = this.toSymbol(shape.member) symbolBuilder(shape, RustType.Vec(inner.rustType())).addReference(inner).build() From 94de9175907ea0f6f51757b553a81d2df960a9d9 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Fri, 25 Nov 2022 14:57:43 +0000 Subject: [PATCH 14/51] Fix call to `ConstraintViolation::Member` --- .../smithy/generators/UnconstrainedCollectionGenerator.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 a62fe24b90..80119549ab 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 @@ -89,7 +89,7 @@ class UnconstrainedCollectionGenerator( .map(|(idx, inner)| inner.try_into().map_err(|inner_violation| (idx, inner_violation))) .collect(); res.map(Self) - .map_err(|(idx, inner_violation)| #{ConstraintViolationSymbol}(idx, inner_violation)) + .map_err(|(idx, inner_violation)| #{ConstraintViolationSymbol}::Member(idx, inner_violation)) } } """, From b8b029ff8fceb1d0754fb45b68f3af86c6044a41 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Fri, 25 Nov 2022 14:58:26 +0000 Subject: [PATCH 15/51] Render constraint validation functions for lists --- .../server/smithy/generators/ConstrainedCollectionGenerator.kt | 3 +++ 1 file changed, 3 insertions(+) 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 0159f61163..2ca98711ca 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 @@ -105,6 +105,8 @@ class ConstrainedCollectionGenerator( pub fn into_inner(self) -> $inner { self.0 } + + #{ValidationFunctions:W} } impl #{TryFrom}<$inner> for $name { @@ -126,6 +128,7 @@ class ConstrainedCollectionGenerator( """, *codegenScope, "ConstraintChecks" to constraintsInfo.map { it.tryFromCheck }.join("\n"), + "ValidationFunctions" to constraintsInfo.map { it.validationFunctionDefinition(constraintViolation) }.join("\n"), ) if (!publicConstrainedTypes && isValueConstrained(shape, model, symbolProvider)) { From 8be886c68c1a064e6045bad35047b83200cd8316 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Fri, 25 Nov 2022 14:58:46 +0000 Subject: [PATCH 16/51] Fix call to `usize::to_string` --- .../smithy/generators/CollectionConstraintViolationGenerator.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt index 79bd4ba97c..d3bfc8b5df 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt @@ -80,7 +80,7 @@ class CollectionConstraintViolationGenerator( if (shape.isReachableFromOperationInput()) { var validationExceptionFields = constraintsInfo.map { it.asValidationExceptionField } if (isMemberConstrained) { - validationExceptionFields += { rust("""Self::Member(index, member_constraint_violation) => member_constraint_violation.as_validation_exception_field(path + "/" + &index.toString())""") } + validationExceptionFields += { rust("""Self::Member(index, member_constraint_violation) => member_constraint_violation.as_validation_exception_field(path + "/" + &index.to_string())""") } } rustTemplate( From e41418927943fe5945e9a495cd4e8cfb3229fdbb Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Fri, 25 Nov 2022 15:24:09 +0000 Subject: [PATCH 17/51] Use map json customisation for collections as well --- .../protocols/serialize/JsonSerializerGenerator.kt | 7 +++++-- .../smithy/rust/codegen/server/smithy/Constraints.kt | 1 + ...foreIteratingOverMapOrCollectionJsonCustomization.kt} | 9 +++++---- .../codegen/server/smithy/protocols/ServerAwsJson.kt | 4 ++-- .../codegen/server/smithy/protocols/ServerRestJson.kt | 4 ++-- 5 files changed, 15 insertions(+), 10 deletions(-) rename codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/customizations/{BeforeIteratingOverMapJsonCustomization.kt => BeforeIteratingOverMapOrCollectionJsonCustomization.kt} (78%) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/JsonSerializerGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/JsonSerializerGenerator.kt index 82be9e7818..d779e0750c 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/JsonSerializerGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/JsonSerializerGenerator.kt @@ -57,7 +57,7 @@ sealed class JsonSerializerSection(name: String) : Section(name) { data class ServerError(val structureShape: StructureShape, val jsonObject: String) : JsonSerializerSection("ServerError") /** Mutate a map prior to it being serialized. **/ - data class BeforeIteratingOverMap(val shape: MapShape, val valueExpression: ValueExpression) : JsonSerializerSection("BeforeIteratingOverMap") + data class BeforeIteratingOverMapOrCollection(val shape: Shape, val valueExpression: ValueExpression) : JsonSerializerSection("BeforeIteratingOverMapOrCollection") /** Mutate the input object prior to finalization. */ data class InputStruct(val structureShape: StructureShape, val jsonObject: String) : JsonSerializerSection("InputStruct") @@ -423,6 +423,9 @@ class JsonSerializerGenerator( private fun RustWriter.serializeCollection(context: Context) { val itemName = safeName("item") + for (customization in customizations) { + customization.section(JsonSerializerSection.BeforeIteratingOverMapOrCollection(context.shape, context.valueExpression))(this) + } rustBlock("for $itemName in ${context.valueExpression.asRef()}") { serializeMember(MemberContext.collectionMember(context, itemName)) } @@ -432,7 +435,7 @@ class JsonSerializerGenerator( val keyName = safeName("key") val valueName = safeName("value") for (customization in customizations) { - customization.section(JsonSerializerSection.BeforeIteratingOverMap(context.shape, context.valueExpression))(this) + customization.section(JsonSerializerSection.BeforeIteratingOverMapOrCollection(context.shape, context.valueExpression))(this) } rustBlock("for ($keyName, $valueName) in ${context.valueExpression.asRef()}") { val keyExpression = "$keyName.as_str()" diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt index 653d0d3bb7..c2beb90a40 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt @@ -103,6 +103,7 @@ fun MemberShape.targetCanReachConstrainedShape(model: Model, symbolProvider: Sym model.expectShape(this.target).canReachConstrainedShape(model, symbolProvider) fun Shape.hasPublicConstrainedWrapperTupleType(model: Model, publicConstrainedTypes: Boolean): Boolean = when (this) { + is CollectionShape -> publicConstrainedTypes && supportedCollectionConstraintTraits.any(this::hasTrait) is MapShape -> publicConstrainedTypes && this.hasTrait() is StringShape -> !this.hasTrait() && (publicConstrainedTypes && supportedStringConstraintTraits.any(this::hasTrait)) is MemberShape -> model.expectShape(this.target).hasPublicConstrainedWrapperTupleType(model, publicConstrainedTypes) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/customizations/BeforeIteratingOverMapJsonCustomization.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/customizations/BeforeIteratingOverMapOrCollectionJsonCustomization.kt similarity index 78% rename from codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/customizations/BeforeIteratingOverMapJsonCustomization.kt rename to codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/customizations/BeforeIteratingOverMapOrCollectionJsonCustomization.kt index 820f5bc8b7..ecc56b602c 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/customizations/BeforeIteratingOverMapJsonCustomization.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/customizations/BeforeIteratingOverMapOrCollectionJsonCustomization.kt @@ -14,12 +14,13 @@ import software.amazon.smithy.rust.codegen.server.smithy.ServerCodegenContext import software.amazon.smithy.rust.codegen.server.smithy.workingWithPublicConstrainedWrapperTupleType /** - * A customization to, just before we iterate over a _constrained_ map shape in a JSON serializer, unwrap the wrapper - * newtype and take a shared reference to the actual `std::collections::HashMap` within it. + * A customization to, just before we iterate over a _constrained_ map or collection shape in a JSON serializer, + * unwrap the wrapper newtype and take a shared reference to the actual value within it. + * That value will be a `std::collections::HashMap` for map shapes, and a `std::vec::Vec` for collection shapes. */ -class BeforeIteratingOverMapJsonCustomization(private val codegenContext: ServerCodegenContext) : JsonSerializerCustomization() { +class BeforeIteratingOverMapOrCollectionJsonCustomization(private val codegenContext: ServerCodegenContext) : JsonSerializerCustomization() { override fun section(section: JsonSerializerSection): Writable = when (section) { - is JsonSerializerSection.BeforeIteratingOverMap -> writable { + is JsonSerializerSection.BeforeIteratingOverMapOrCollection -> writable { if (workingWithPublicConstrainedWrapperTupleType( section.shape, codegenContext.model, diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerAwsJson.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerAwsJson.kt index 815608fb24..887b51d20a 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerAwsJson.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerAwsJson.kt @@ -21,7 +21,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.protocols.serialize.JsonS import software.amazon.smithy.rust.codegen.core.smithy.protocols.serialize.StructuredDataSerializerGenerator import software.amazon.smithy.rust.codegen.core.util.hasTrait import software.amazon.smithy.rust.codegen.server.smithy.ServerCodegenContext -import software.amazon.smithy.rust.codegen.server.smithy.customizations.BeforeIteratingOverMapJsonCustomization +import software.amazon.smithy.rust.codegen.server.smithy.customizations.BeforeIteratingOverMapOrCollectionJsonCustomization import software.amazon.smithy.rust.codegen.server.smithy.generators.protocol.ServerAwsJsonProtocol import software.amazon.smithy.rust.codegen.server.smithy.generators.protocol.ServerProtocol @@ -90,6 +90,6 @@ class ServerAwsJsonSerializerGenerator( codegenContext, httpBindingResolver, ::awsJsonFieldName, - customizations = listOf(ServerAwsJsonError(awsJsonVersion), BeforeIteratingOverMapJsonCustomization(codegenContext)), + customizations = listOf(ServerAwsJsonError(awsJsonVersion), BeforeIteratingOverMapOrCollectionJsonCustomization(codegenContext)), ), ) : StructuredDataSerializerGenerator by jsonSerializerGenerator diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerRestJson.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerRestJson.kt index a913b806d2..b20c932e2c 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerRestJson.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerRestJson.kt @@ -13,7 +13,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.protocols.restJsonFieldNa import software.amazon.smithy.rust.codegen.core.smithy.protocols.serialize.JsonSerializerGenerator import software.amazon.smithy.rust.codegen.core.smithy.protocols.serialize.StructuredDataSerializerGenerator import software.amazon.smithy.rust.codegen.server.smithy.ServerCodegenContext -import software.amazon.smithy.rust.codegen.server.smithy.customizations.BeforeIteratingOverMapJsonCustomization +import software.amazon.smithy.rust.codegen.server.smithy.customizations.BeforeIteratingOverMapOrCollectionJsonCustomization import software.amazon.smithy.rust.codegen.server.smithy.generators.protocol.ServerRestJsonProtocol /** @@ -50,6 +50,6 @@ class ServerRestJsonSerializerGenerator( codegenContext, httpBindingResolver, ::restJsonFieldName, - customizations = listOf(BeforeIteratingOverMapJsonCustomization(codegenContext)), + customizations = listOf(BeforeIteratingOverMapOrCollectionJsonCustomization(codegenContext)), ), ) : StructuredDataSerializerGenerator by jsonSerializerGenerator From 6c50000c654b5a5ed0d14bc8955e600b9adc9aad Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Mon, 28 Nov 2022 09:34:17 +0000 Subject: [PATCH 18/51] Update to use overhauled module system --- .../rust/codegen/server/smithy/ServerCodegenVisitor.kt | 2 +- .../CollectionConstraintViolationGenerator.kt | 10 ++-------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt index 74aed2b08d..ee82e56f17 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt @@ -300,7 +300,7 @@ open class ServerCodegenVisitor( if (!shape.isDirectlyConstrained(codegenContext.symbolProvider)) { logger.info("[rust-server-codegen] Generating a constrained type for collection shape $shape") - rustCrate.withModule(constrainedModule) { + rustCrate.withModule(ConstrainedModule) { PubCrateConstrainedCollectionGenerator(codegenContext, this, shape).render() } } diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt index d3bfc8b5df..a294ec4d11 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt @@ -6,14 +6,13 @@ package software.amazon.smithy.rust.codegen.server.smithy.generators import software.amazon.smithy.model.shapes.CollectionShape -import software.amazon.smithy.rust.codegen.core.rustlang.RustMetadata -import software.amazon.smithy.rust.codegen.core.rustlang.RustModule 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.join 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.module import software.amazon.smithy.rust.codegen.server.smithy.PubCrateConstraintViolationSymbolProvider import software.amazon.smithy.rust.codegen.server.smithy.ServerCodegenContext import software.amazon.smithy.rust.codegen.server.smithy.canReachConstrainedShape @@ -44,12 +43,7 @@ class CollectionConstraintViolationGenerator( val isMemberConstrained = memberShape.canReachConstrainedShape(model, symbolProvider) val constraintViolationVisibility = Visibility.publicIf(publicConstrainedTypes, Visibility.PUBCRATE) - modelsModuleWriter.withModule( - RustModule( - constraintViolationSymbol.namespace.split(constraintViolationSymbol.namespaceDelimiter).last(), - RustMetadata(visibility = constraintViolationVisibility), - ), - ) { + modelsModuleWriter.withInlineModule(constraintViolationSymbol.module()) { var constraintViolationVariants = constraintsInfo.map { it.constraintViolationVariant } if (isMemberConstrained) { constraintViolationVariants += { From acc3c98b766aaccbc23fd05cfae23b628b2109a1 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Mon, 28 Nov 2022 10:31:29 +0000 Subject: [PATCH 19/51] Add constraint traits check for collection shapes --- .../smithy/ConstrainedShapeSymbolProvider.kt | 18 ++++++++++++++++-- .../rust/codegen/server/smithy/Constraints.kt | 16 ++++++++++------ .../smithy/ValidateUnsupportedConstraints.kt | 10 ---------- 3 files changed, 26 insertions(+), 18 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolProvider.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolProvider.kt index 7efe43c2f8..c2afc0ac6d 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolProvider.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolProvider.kt @@ -68,7 +68,7 @@ class ConstrainedShapeSymbolProvider( // (constraint trait precedence). val target = model.expectShape(shape.target) val targetSymbol = this.toSymbol(target) - // Handle boxing first so we end up with `Option>`, not `Box>`. + // Handle boxing first, so we end up with `Option>`, not `Box>`. handleOptionality(handleRustBoxing(targetSymbol, shape), shape, nullableIndex, base.config().nullabilityCheckMode) } is MapShape -> { @@ -86,7 +86,7 @@ class ConstrainedShapeSymbolProvider( } is CollectionShape -> { if (shape.isDirectlyConstrained(base)) { - // TODO: Perform some check on the constraint traits that the list is tagged with. + assert(constraintCollectionCheck(shape)) { "Only the `length` constraint trait can be applied to lists" } publicConstrainedSymbolForMapOrCollectionShape(shape) } else { val inner = this.toSymbol(shape.member) @@ -104,4 +104,18 @@ class ConstrainedShapeSymbolProvider( else -> base.toSymbol(shape) } } + + /** + * Checks that the collection: + * - Has at least 1 supported constraint applied to it, and + * - That it has no unsupported constraints applied. + * + * This check is relatively expensive, so we only run it on `assert`s. + */ + private fun constraintCollectionCheck(shape: CollectionShape): Boolean { + val supportedConstraintTraits = supportedCollectionConstraintTraits.mapNotNull { shape.getTrait(it) }.toSet() + val allConstraintTraits = allConstraintTraits.mapNotNull { shape.getTrait(it) }.toSet() + + return supportedConstraintTraits.isNotEmpty() && allConstraintTraits.subtract(supportedConstraintTraits).isEmpty() + } } diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt index 66b5471aa6..ef798be1f1 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt @@ -36,12 +36,16 @@ import software.amazon.smithy.rust.codegen.core.util.hasTrait * we support it or not_. */ fun Shape.hasConstraintTrait() = - hasTrait() || - hasTrait() || - hasTrait() || - hasTrait() || - hasTrait() || - hasTrait() + allConstraintTraits.any(this::hasTrait) + +val allConstraintTraits = setOf( + LengthTrait::class.java, + PatternTrait::class.java, + RangeTrait::class.java, + UniqueItemsTrait::class.java, + EnumTrait::class.java, + RequiredTrait::class.java, +) val supportedStringConstraintTraits: List> = listOf(LengthTrait::class.java, PatternTrait::class.java) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt index d827420e1d..0e5d93de20 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt @@ -16,9 +16,7 @@ import software.amazon.smithy.model.shapes.SetShape import software.amazon.smithy.model.shapes.Shape import software.amazon.smithy.model.shapes.ShapeId import software.amazon.smithy.model.shapes.UnionShape -import software.amazon.smithy.model.traits.EnumTrait import software.amazon.smithy.model.traits.LengthTrait -import software.amazon.smithy.model.traits.PatternTrait import software.amazon.smithy.model.traits.RangeTrait import software.amazon.smithy.model.traits.RequiredTrait import software.amazon.smithy.model.traits.StreamingTrait @@ -106,14 +104,6 @@ private data class UnsupportedUniqueItemsTraitOnShape(val shape: Shape, val uniq data class LogMessage(val level: Level, val message: String) data class ValidationResult(val shouldAbort: Boolean, val messages: List) -private val allConstraintTraits = setOf( - LengthTrait::class.java, - PatternTrait::class.java, - RangeTrait::class.java, - UniqueItemsTrait::class.java, - EnumTrait::class.java, - RequiredTrait::class.java, -) private val unsupportedConstraintsOnMemberShapes = allConstraintTraits - RequiredTrait::class.java fun validateOperationsWithConstrainedInputHaveValidationExceptionAttached(model: Model, service: ServiceShape): ValidationResult { From 6c1a2c2a640675847d5116f59f26d378de371b19 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Mon, 28 Nov 2022 11:01:23 +0000 Subject: [PATCH 20/51] Remove test checking that `@length` is not used in `list`s --- ...ValidateUnsupportedConstraintsAreNotUsedTest.kt | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt index 4c3fa0efe1..f682d38769 100644 --- a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt +++ b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt @@ -151,7 +151,7 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest { } @Test - fun `it should detect when the length trait on collection shapes or on blob shapes is used`() { + fun `it should detect when the length trait on blob shapes is used`() { val model = """ $baseModel @@ -161,18 +161,12 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest { blob: LengthBlob } - @length(min: 1) - list LengthCollection { - member: String - } - @length(min: 1) blob LengthBlob """.asSmithyModel() val validationResult = validateModel(model) - validationResult.messages shouldHaveSize 2 - validationResult.messages.forSome { it.message shouldContain "The list shape `test#LengthCollection` has the constraint trait `smithy.api#length` attached" } + validationResult.messages shouldHaveSize 1 validationResult.messages.forSome { it.message shouldContain "The blob shape `test#LengthBlob` has the constraint trait `smithy.api#length` attached" } } @@ -200,11 +194,11 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest { val model = """ $baseModel - + structure TestInputOutput { uniqueItemsList: UniqueItemsList } - + @uniqueItems list UniqueItemsList { member: String From c190d361efd39b84cdd270e3a938f0f1bd691265 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Mon, 28 Nov 2022 11:18:07 +0000 Subject: [PATCH 21/51] Refactor use of `shape.isDirectlyConstrained` --- .../rust/codegen/server/smithy/ServerCodegenVisitor.kt | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt index ee82e56f17..a49e418fd5 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt @@ -284,6 +284,7 @@ open class ServerCodegenVisitor( model, codegenContext.symbolProvider, ) + val isDirectlyConstrained = shape.isDirectlyConstrained(codegenContext.symbolProvider) if (renderUnconstrainedList) { logger.info("[rust-server-codegen] Generating an unconstrained type for collection shape $shape") @@ -298,7 +299,7 @@ open class ServerCodegenVisitor( } } - if (!shape.isDirectlyConstrained(codegenContext.symbolProvider)) { + if (!isDirectlyConstrained) { logger.info("[rust-server-codegen] Generating a constrained type for collection shape $shape") rustCrate.withModule(ConstrainedModule) { PubCrateConstrainedCollectionGenerator(codegenContext, this, shape).render() @@ -307,7 +308,6 @@ open class ServerCodegenVisitor( } val constraintsInfo = CollectionTraitInfo.fromShape(shape) - val isDirectlyConstrained = shape.isDirectlyConstrained(codegenContext.symbolProvider) if (isDirectlyConstrained) { rustCrate.withModule(ModelsModule) { ConstrainedCollectionGenerator( @@ -333,13 +333,15 @@ open class ServerCodegenVisitor( model, codegenContext.symbolProvider, ) + val isDirectlyConstrained = shape.isDirectlyConstrained(codegenContext.symbolProvider) + if (renderUnconstrainedMap) { logger.info("[rust-server-codegen] Generating an unconstrained type for map $shape") rustCrate.withModule(UnconstrainedModule) { UnconstrainedMapGenerator(codegenContext, this, shape).render() } - if (!shape.isDirectlyConstrained(codegenContext.symbolProvider)) { + if (!isDirectlyConstrained) { logger.info("[rust-server-codegen] Generating a constrained type for map $shape") rustCrate.withModule(ConstrainedModule) { PubCrateConstrainedMapGenerator(codegenContext, this, shape).render() @@ -347,7 +349,6 @@ open class ServerCodegenVisitor( } } - val isDirectlyConstrained = shape.isDirectlyConstrained(codegenContext.symbolProvider) if (isDirectlyConstrained) { rustCrate.withModule(ModelsModule) { ConstrainedMapGenerator( From ad2d5e93dfec55a52f7506399f319fc5f0677517 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Mon, 28 Nov 2022 11:38:55 +0000 Subject: [PATCH 22/51] Fix failing `UnconstrainedCollectionGeneratorTest` test --- .../smithy/generators/UnconstrainedCollectionGeneratorTest.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/UnconstrainedCollectionGeneratorTest.kt b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/UnconstrainedCollectionGeneratorTest.kt index 50543d6726..58b0d27c33 100644 --- a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/UnconstrainedCollectionGeneratorTest.kt +++ b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/UnconstrainedCollectionGeneratorTest.kt @@ -68,6 +68,8 @@ class UnconstrainedCollectionGeneratorTest { this@modelsModuleWriter, it, ).render() + + CollectionConstraintViolationGenerator(codegenContext, this@modelsModuleWriter, it, listOf()).render() } this@unconstrainedModuleWriter.unitTest( @@ -79,7 +81,7 @@ class UnconstrainedCollectionGeneratorTest { let list_a_unconstrained = list_a_unconstrained::ListAUnconstrained(vec![list_b_unconstrained]); let expected_err = - crate::model::list_a::ConstraintViolation(0, crate::model::list_b::ConstraintViolation( + crate::model::list_a::ConstraintViolation::Member(0, crate::model::list_b::ConstraintViolation::Member( 0, crate::model::structure_c::ConstraintViolation::MissingString, )); From 818477ec226ed71fd1e367039c7396f5b544412f Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Mon, 28 Nov 2022 11:54:44 +0000 Subject: [PATCH 23/51] Fix test --- .../smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt index f682d38769..98855c3527 100644 --- a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt +++ b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt @@ -157,7 +157,6 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest { $baseModel structure TestInputOutput { - collection: LengthCollection, blob: LengthBlob } From 715f96b95bf76a63d3b8064647c2865e3af1c519 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Mon, 28 Nov 2022 13:11:08 +0000 Subject: [PATCH 24/51] Actually fix the test --- .../smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt index 98855c3527..b387d9e0fe 100644 --- a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt +++ b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt @@ -166,7 +166,7 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest { val validationResult = validateModel(model) validationResult.messages shouldHaveSize 1 - validationResult.messages.forSome { it.message shouldContain "The blob shape `test#LengthBlob` has the constraint trait `smithy.api#length` attached" } + validationResult.messages[0].message shouldContain "The blob shape `test#LengthBlob` has the constraint trait `smithy.api#length` attached" } @Test From d2c59da42db5ee9f8525c90413bb060de6471414 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Mon, 28 Nov 2022 14:20:18 +0000 Subject: [PATCH 25/51] Update changelog --- CHANGELOG.next.toml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 469b82d380..fa836f63f9 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -242,13 +242,15 @@ message = """ * The `length` trait on `string` shapes. * The `length` trait on `map` shapes. +* The `length` trait on `list` shapes. +* The `length` trait on `set` shapes. * The `pattern` trait on `string` shapes. Upon receiving a request that violates the modeled constraints, the server SDK will reject it with a message indicating why. Unsupported (constraint trait, target shape) combinations will now fail at code generation time, whereas previously they were just ignored. This is a breaking change to raise awareness in service owners of their server SDKs behaving differently than what was modeled. To continue generating a server SDK with unsupported constraint traits, set `codegenConfig.ignoreUnsupportedConstraints` to `true` in your `smithy-build.json`. """ -references = ["smithy-rs#1199", "smithy-rs#1342", "smithy-rs#1401", "smithy-rs#1998"] +references = ["smithy-rs#1199", "smithy-rs#1342", "smithy-rs#1401", "smithy-rs#1998", "smithy-rs#2028"] meta = { "breaking" = true, "tada" = true, "bug" = false, "target" = "server" } author = "david-perez" From 2d221333d7ef6d90f3ad0f5b8ffa16303b64dd91 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Mon, 28 Nov 2022 16:16:09 +0000 Subject: [PATCH 26/51] Fix `constrainedCollectionCheck` --- .../server/smithy/ConstrainedShapeSymbolProvider.kt | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolProvider.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolProvider.kt index c2afc0ac6d..cc42d3f3d2 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolProvider.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolProvider.kt @@ -26,6 +26,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.locatedIn import software.amazon.smithy.rust.codegen.core.smithy.rustType import software.amazon.smithy.rust.codegen.core.smithy.symbolBuilder import software.amazon.smithy.rust.codegen.core.util.hasTrait +import software.amazon.smithy.rust.codegen.core.util.orNull import software.amazon.smithy.rust.codegen.core.util.toPascalCase /** @@ -86,7 +87,7 @@ class ConstrainedShapeSymbolProvider( } is CollectionShape -> { if (shape.isDirectlyConstrained(base)) { - assert(constraintCollectionCheck(shape)) { "Only the `length` constraint trait can be applied to lists" } + assert(constrainedCollectionCheck(shape)) { "Only the `length` constraint trait can be applied to lists" } publicConstrainedSymbolForMapOrCollectionShape(shape) } else { val inner = this.toSymbol(shape.member) @@ -112,9 +113,9 @@ class ConstrainedShapeSymbolProvider( * * This check is relatively expensive, so we only run it on `assert`s. */ - private fun constraintCollectionCheck(shape: CollectionShape): Boolean { - val supportedConstraintTraits = supportedCollectionConstraintTraits.mapNotNull { shape.getTrait(it) }.toSet() - val allConstraintTraits = allConstraintTraits.mapNotNull { shape.getTrait(it) }.toSet() + private fun constrainedCollectionCheck(shape: CollectionShape): Boolean { + val supportedConstraintTraits = supportedCollectionConstraintTraits.mapNotNull { shape.getTrait(it).orNull() }.toSet() + val allConstraintTraits = allConstraintTraits.mapNotNull { shape.getTrait(it).orNull() }.toSet() return supportedConstraintTraits.isNotEmpty() && allConstraintTraits.subtract(supportedConstraintTraits).isEmpty() } From 1eb8650aba50e387b2153aa6982620454034979f Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Mon, 28 Nov 2022 16:44:22 +0000 Subject: [PATCH 27/51] Add tests for `ConstrainedCollectionGenerator` --- .../ConstrainedCollectionGeneratorTest.kt | 163 ++++++++++++++++++ 1 file changed, 163 insertions(+) create mode 100644 codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedCollectionGeneratorTest.kt diff --git a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedCollectionGeneratorTest.kt b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedCollectionGeneratorTest.kt new file mode 100644 index 0000000000..85789439c4 --- /dev/null +++ b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedCollectionGeneratorTest.kt @@ -0,0 +1,163 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.rust.codegen.server.smithy.generators + +import io.kotest.matchers.string.shouldContain +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.ExtensionContext +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.Arguments +import org.junit.jupiter.params.provider.ArgumentsProvider +import org.junit.jupiter.params.provider.ArgumentsSource +import software.amazon.smithy.model.Model +import software.amazon.smithy.model.node.ArrayNode +import software.amazon.smithy.model.shapes.CollectionShape +import software.amazon.smithy.model.shapes.ListShape +import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter +import software.amazon.smithy.rust.codegen.core.rustlang.rustBlock +import software.amazon.smithy.rust.codegen.core.smithy.ModelsModule +import software.amazon.smithy.rust.codegen.core.testutil.TestWorkspace +import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel +import software.amazon.smithy.rust.codegen.core.testutil.compileAndTest +import software.amazon.smithy.rust.codegen.core.testutil.unitTest +import software.amazon.smithy.rust.codegen.core.util.lookup +import software.amazon.smithy.rust.codegen.server.smithy.ServerCodegenContext +import software.amazon.smithy.rust.codegen.server.smithy.testutil.serverTestCodegenContext +import software.amazon.smithy.rust.codegen.server.smithy.transformers.ShapesReachableFromOperationInputTagger +import java.util.stream.Stream + +class ConstrainedCollectionGeneratorTest { + data class TestCase(val model: Model, val validList: ArrayNode, val invalidList: ArrayNode) + + class ConstrainedListGeneratorTestProvider : ArgumentsProvider { + private val testCases = listOf( + // Min and max. + Triple("@length(min: 11, max: 12)", 11, 13), + // Min equal to max. + Triple("@length(min: 11, max: 11)", 11, 12), + // Only min. + Triple("@length(min: 11)", 15, 10), + // Only max. + Triple("@length(max: 11)", 11, 12), + ).map { + val validList = List(it.second, Int::toString) + val invalidList = List(it.third, Int::toString) + + Triple(it.first, ArrayNode.fromStrings(validList), ArrayNode.fromStrings(invalidList)) + }.map { (trait, validList, invalidList) -> + TestCase( + """ + namespace test + + $trait + list ConstrainedList { + member: String + } + + $trait + set ConstrainedSet { + member: String + } + """.asSmithyModel().let(ShapesReachableFromOperationInputTagger::transform), + validList, + invalidList, + ) + } + + override fun provideArguments(context: ExtensionContext?): Stream = + testCases.map { Arguments.of(it) }.stream() + } + + @ParameterizedTest + @ArgumentsSource(ConstrainedListGeneratorTestProvider::class) + fun `it should generate constrained collection types`(testCase: TestCase) { + val constrainedListShape = testCase.model.lookup("test#ConstrainedList") + + val codegenContext = serverTestCodegenContext(testCase.model) + val symbolProvider = codegenContext.symbolProvider + + val project = TestWorkspace.testProject(symbolProvider) + + project.withModule(ModelsModule) { + render(codegenContext, this, constrainedListShape) + + val instantiator = serverInstantiator(codegenContext) + rustBlock("##[cfg(test)] fn build_valid_list() -> std::vec::Vec") { + instantiator.render(this, constrainedListShape, testCase.validList) + } + rustBlock("##[cfg(test)] fn build_invalid_list() -> std::vec::Vec") { + instantiator.render(this, constrainedListShape, testCase.invalidList) + } + + unitTest( + name = "try_from_success", + test = """ + let list = build_valid_list(); + let _constrained: ConstrainedList = list.try_into().unwrap(); + """, + ) + unitTest( + name = "try_from_fail", + test = """ + let list = build_invalid_list(); + let constrained_res: Result = list.try_into(); + constrained_res.unwrap_err(); + """, + ) + unitTest( + name = "inner", + test = """ + let list = build_valid_list(); + let constrained = ConstrainedList::try_from(list.clone()).unwrap(); + + assert_eq!(constrained.inner(), &list); + """, + ) + unitTest( + name = "into_inner", + test = """ + let list = build_valid_list(); + let constrained = ConstrainedList::try_from(list.clone()).unwrap(); + + assert_eq!(constrained.into_inner(), list); + """, + ) + } + + project.compileAndTest() + } + + @Test + fun `type should not be constructible without using a constructor`() { + val model = """ + namespace test + + @length(min: 1, max: 69) + list ConstrainedList { + member: String + } + """.asSmithyModel().let(ShapesReachableFromOperationInputTagger::transform) + val constrainedCollectionShape = model.lookup("test#ConstrainedList") + + val writer = RustWriter.forModule(ModelsModule.name) + + val codegenContext = serverTestCodegenContext(model) + render(codegenContext, writer, constrainedCollectionShape) + + // Check that the wrapped type is `pub(crate)`. + writer.toString() shouldContain "pub struct ConstrainedList(pub(crate) std::vec::Vec);" + } + + private fun render( + codegenContext: ServerCodegenContext, + writer: RustWriter, + constrainedCollectionShape: CollectionShape, + ) { + val constraintsInfo = CollectionTraitInfo.fromShape(constrainedCollectionShape) + ConstrainedCollectionGenerator(codegenContext, writer, constrainedCollectionShape, constraintsInfo).render() + CollectionConstraintViolationGenerator(codegenContext, writer, constrainedCollectionShape, constraintsInfo).render() + } +} From 072df4040f09d04ec4f580e9458590b65eec36ee Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Mon, 28 Nov 2022 16:57:54 +0000 Subject: [PATCH 28/51] Make tests work for when sets are implemented --- .../ConstrainedCollectionGeneratorTest.kt | 100 ++++++++++-------- 1 file changed, 57 insertions(+), 43 deletions(-) diff --git a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedCollectionGeneratorTest.kt b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedCollectionGeneratorTest.kt index 85789439c4..f14ac17284 100644 --- a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedCollectionGeneratorTest.kt +++ b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedCollectionGeneratorTest.kt @@ -16,6 +16,7 @@ import software.amazon.smithy.model.Model import software.amazon.smithy.model.node.ArrayNode import software.amazon.smithy.model.shapes.CollectionShape import software.amazon.smithy.model.shapes.ListShape +import software.amazon.smithy.model.shapes.SetShape import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter import software.amazon.smithy.rust.codegen.core.rustlang.rustBlock import software.amazon.smithy.rust.codegen.core.smithy.ModelsModule @@ -23,12 +24,14 @@ import software.amazon.smithy.rust.codegen.core.testutil.TestWorkspace import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel import software.amazon.smithy.rust.codegen.core.testutil.compileAndTest import software.amazon.smithy.rust.codegen.core.testutil.unitTest +import software.amazon.smithy.rust.codegen.core.util.UNREACHABLE import software.amazon.smithy.rust.codegen.core.util.lookup import software.amazon.smithy.rust.codegen.server.smithy.ServerCodegenContext import software.amazon.smithy.rust.codegen.server.smithy.testutil.serverTestCodegenContext import software.amazon.smithy.rust.codegen.server.smithy.transformers.ShapesReachableFromOperationInputTagger import java.util.stream.Stream +@Suppress("DEPRECATION") class ConstrainedCollectionGeneratorTest { data class TestCase(val model: Model, val validList: ArrayNode, val invalidList: ArrayNode) @@ -74,57 +77,68 @@ class ConstrainedCollectionGeneratorTest { @ParameterizedTest @ArgumentsSource(ConstrainedListGeneratorTestProvider::class) fun `it should generate constrained collection types`(testCase: TestCase) { - val constrainedListShape = testCase.model.lookup("test#ConstrainedList") + val constrainedListShape = testCase.model.lookup("test#ConstrainedList") + // TODO(https://github.com/awslabs/smithy-rs/issues/1401): a `set` shape is + // just a `list` shape with `uniqueItems`, which hasn't been implemented yet. + // val constrainedSetShape = testCase.model.lookup("test#ConstrainedSet") val codegenContext = serverTestCodegenContext(testCase.model) val symbolProvider = codegenContext.symbolProvider val project = TestWorkspace.testProject(symbolProvider) - project.withModule(ModelsModule) { - render(codegenContext, this, constrainedListShape) - - val instantiator = serverInstantiator(codegenContext) - rustBlock("##[cfg(test)] fn build_valid_list() -> std::vec::Vec") { - instantiator.render(this, constrainedListShape, testCase.validList) - } - rustBlock("##[cfg(test)] fn build_invalid_list() -> std::vec::Vec") { - instantiator.render(this, constrainedListShape, testCase.invalidList) + listOf(constrainedListShape /*, constrainedSetShape */).forEach { shape -> + val shapeName = when (shape) { + is ListShape -> "list" + is SetShape -> "set" + else -> UNREACHABLE("Shape is either list or set.") } - unitTest( - name = "try_from_success", - test = """ - let list = build_valid_list(); - let _constrained: ConstrainedList = list.try_into().unwrap(); - """, - ) - unitTest( - name = "try_from_fail", - test = """ - let list = build_invalid_list(); - let constrained_res: Result = list.try_into(); - constrained_res.unwrap_err(); - """, - ) - unitTest( - name = "inner", - test = """ - let list = build_valid_list(); - let constrained = ConstrainedList::try_from(list.clone()).unwrap(); - - assert_eq!(constrained.inner(), &list); - """, - ) - unitTest( - name = "into_inner", - test = """ - let list = build_valid_list(); - let constrained = ConstrainedList::try_from(list.clone()).unwrap(); - - assert_eq!(constrained.into_inner(), list); - """, - ) + project.withModule(ModelsModule) { + render(codegenContext, this, shape) + + val instantiator = serverInstantiator(codegenContext) + rustBlock("##[cfg(test)] fn build_valid_$shapeName() -> std::vec::Vec") { + instantiator.render(this, shape, testCase.validList) + } + rustBlock("##[cfg(test)] fn build_invalid_$shapeName() -> std::vec::Vec") { + instantiator.render(this, shape, testCase.invalidList) + } + + unitTest( + name = "try_from_success", + test = """ + let $shapeName = build_valid_$shapeName(); + let _constrained: ConstrainedList = $shapeName.try_into().unwrap(); + """, + ) + unitTest( + name = "try_from_fail", + test = """ + let $shapeName = build_invalid_$shapeName(); + let constrained_res: Result = $shapeName.try_into(); + constrained_res.unwrap_err(); + """, + ) + unitTest( + name = "inner", + test = """ + let $shapeName = build_valid_$shapeName(); + let constrained = ConstrainedList::try_from($shapeName.clone()).unwrap(); + + assert_eq!(constrained.inner(), &$shapeName); + """, + ) + unitTest( + name = "into_inner", + test = """ + let $shapeName = build_valid_$shapeName(); + let constrained = ConstrainedList::try_from($shapeName.clone()).unwrap(); + + assert_eq!(constrained.into_inner(), $shapeName); + """, + ) + } } project.compileAndTest() From dfa3979cd553e43a249bb9b3a7c61d3bfbe73ba8 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Tue, 29 Nov 2022 13:27:37 +0000 Subject: [PATCH 29/51] Remove mention of `set` in changelog Co-authored-by: david-perez --- CHANGELOG.next.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 30d775cedf..db0bf252db 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -243,7 +243,6 @@ message = """ * The `length` trait on `string` shapes. * The `length` trait on `map` shapes. * The `length` trait on `list` shapes. -* The `length` trait on `set` shapes. * The `range` trait on `integer` shapes. * The `pattern` trait on `string` shapes. From 925c2257ced4fb442f6f372a81aa66f70a7fb96f Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Tue, 29 Nov 2022 13:28:00 +0000 Subject: [PATCH 30/51] Fix styling in `constraints.smithy` Co-authored-by: david-perez --- codegen-core/common-test-models/constraints.smithy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codegen-core/common-test-models/constraints.smithy b/codegen-core/common-test-models/constraints.smithy index 23fdfa7677..71c2cb69fc 100644 --- a/codegen-core/common-test-models/constraints.smithy +++ b/codegen-core/common-test-models/constraints.smithy @@ -582,7 +582,7 @@ list ListOfLengthPatternString { member: LengthPatternString } -@length(min:12, max: 39) +@length(min: 12, max: 39) list LengthListOfPatternString { member: PatternString } From 3b92790a478a8d62fda86dc6492529af6cffc2f0 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Tue, 29 Nov 2022 13:32:07 +0000 Subject: [PATCH 31/51] Use `check` instead of `assert` --- .../codegen/server/smithy/ConstrainedShapeSymbolProvider.kt | 4 +--- .../smithy/generators/ConstrainedCollectionGenerator.kt | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolProvider.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolProvider.kt index 38854feb0c..8666446768 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolProvider.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolProvider.kt @@ -88,7 +88,7 @@ class ConstrainedShapeSymbolProvider( } is CollectionShape -> { if (shape.isDirectlyConstrained(base)) { - assert(constrainedCollectionCheck(shape)) { "Only the `length` constraint trait can be applied to lists" } + check(constrainedCollectionCheck(shape)) { "Only the `length` constraint trait can be applied to lists" } publicConstrainedSymbolForMapOrCollectionShape(shape) } else { val inner = this.toSymbol(shape.member) @@ -111,8 +111,6 @@ class ConstrainedShapeSymbolProvider( * Checks that the collection: * - Has at least 1 supported constraint applied to it, and * - That it has no unsupported constraints applied. - * - * This check is relatively expensive, so we only run it on `assert`s. */ private fun constrainedCollectionCheck(shape: CollectionShape): Boolean { val supportedConstraintTraits = supportedCollectionConstraintTraits.mapNotNull { shape.getTrait(it).orNull() }.toSet() 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 f056468fa1..961a68f846 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 @@ -61,7 +61,7 @@ class ConstrainedCollectionGenerator( private val symbolProvider = codegenContext.symbolProvider fun render() { - assert(constraintsInfo.isNotEmpty()) { + check(constraintsInfo.isNotEmpty()) { "`ConstrainedCollectionGenerator` can only be invoked for constrained collections, but this shape was unconstrained" } From bb24581b3b412c217da3d618058fcbf7ea2171ee Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Tue, 29 Nov 2022 13:34:02 +0000 Subject: [PATCH 32/51] Grammar fix in comment about `Option>` --- .../amazon/smithy/rust/codegen/core/smithy/SymbolVisitor.kt | 2 +- .../server/smithy/PubCrateConstrainedShapeSymbolProvider.kt | 2 +- .../codegen/server/smithy/UnconstrainedShapeSymbolProvider.kt | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolVisitor.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolVisitor.kt index d2e66a87aa..29ce9bced2 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolVisitor.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolVisitor.kt @@ -346,7 +346,7 @@ open class SymbolVisitor( override fun memberShape(shape: MemberShape): Symbol { val target = model.expectShape(shape.target) - // Handle boxing first so we end up with Option>, not Box>. + // Handle boxing first, so we end up with Option>, not Box>. return handleOptionality( handleRustBoxing(toSymbol(target), shape), shape, 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 bb818eaf07..3674185520 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 @@ -108,7 +108,7 @@ class PubCrateConstrainedShapeSymbolProvider( base.toSymbol(shape) } else { val targetSymbol = this.toSymbol(targetShape) - // Handle boxing first so we end up with `Option>`, not `Box>`. + // Handle boxing first, so we end up with `Option>`, not `Box>`. handleOptionality( handleRustBoxing(targetSymbol, shape), shape, diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/UnconstrainedShapeSymbolProvider.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/UnconstrainedShapeSymbolProvider.kt index 5beb183c81..3da3129387 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/UnconstrainedShapeSymbolProvider.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/UnconstrainedShapeSymbolProvider.kt @@ -162,7 +162,7 @@ class UnconstrainedShapeSymbolProvider( if (shape.targetCanReachConstrainedShape(model, base)) { val targetShape = model.expectShape(shape.target) val targetSymbol = this.toSymbol(targetShape) - // Handle boxing first so we end up with `Option>`, not `Box>`. + // Handle boxing first, so we end up with `Option>`, not `Box>`. handleOptionality( handleRustBoxing(targetSymbol, shape), shape, From 5cebcb911964dfd5cbecb48c4aecb7d8d2cf69c5 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Tue, 29 Nov 2022 13:38:14 +0000 Subject: [PATCH 33/51] Rename unsupported length data class for blobs - `UnsupportedLengthTraitOnCollectionOrOnBlobShape` -> `UnsupportedLengthTraitOnBlobShape` --- .../codegen/server/smithy/ValidateUnsupportedConstraints.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt index bf2db05ac6..b1ac21c027 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt @@ -86,7 +86,7 @@ private sealed class UnsupportedConstraintMessageKind { ), ) - is UnsupportedLengthTraitOnCollectionOrOnBlobShape -> LogMessage( + is UnsupportedLengthTraitOnBlobShape -> LogMessage( level, buildMessageShapeHasUnsupportedConstraintTrait(shape, lengthTrait, constraintTraitsUberIssue), ) @@ -117,7 +117,7 @@ private data class UnsupportedLengthTraitOnStreamingBlobShape( val streamingTrait: StreamingTrait, ) : UnsupportedConstraintMessageKind() -private data class UnsupportedLengthTraitOnCollectionOrOnBlobShape(val shape: Shape, val lengthTrait: LengthTrait) : +private data class UnsupportedLengthTraitOnBlobShape(val shape: Shape, val lengthTrait: LengthTrait) : UnsupportedConstraintMessageKind() private data class UnsupportedRangeTraitOnShape(val shape: Shape, val rangeTrait: RangeTrait) : @@ -226,7 +226,7 @@ fun validateUnsupportedConstraints( .asSequence() .filter { it is BlobShape } .filter { it.hasTrait() } - .map { UnsupportedLengthTraitOnCollectionOrOnBlobShape(it, it.expectTrait()) } + .map { UnsupportedLengthTraitOnBlobShape(it, it.expectTrait()) } .toSet() // 5. Range trait used on a non-integer shape. It has not been implemented yet. From 074276a277849dea917a070f4c439b7a02d2e7a6 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Tue, 29 Nov 2022 13:42:17 +0000 Subject: [PATCH 34/51] Add TODO about `uniqueItems` not being implemented yet --- .../server/smithy/generators/ConstrainedCollectionGenerator.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 961a68f846..b76a04ca7a 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 @@ -32,13 +32,14 @@ import software.amazon.smithy.rust.codegen.server.smithy.validationErrorMessage * the constraints. * * The [`length`] and [`uniqueItems`] traits are the only constraint traits applicable to list shapes. + * TODO(https://github.com/awslabs/smithy-rs/issues/1401): * The [`uniqueItems`] trait has not been implemented yet. * * If [unconstrainedSymbol] is provided, the `MaybeConstrained` trait is implemented for the constrained type, using the * [unconstrainedSymbol]'s associated type as the associated type for the trait. * * [`length`]: https://smithy.io/2.0/spec/constraint-traits.html#length-trait - * [`uniqueItem`]: https://smithy.io/2.0/spec/constraint-traits.html#smithy-api-uniqueitems-trait + * [`uniqueItems`]: https://smithy.io/2.0/spec/constraint-traits.html#smithy-api-uniqueitems-trait */ class ConstrainedCollectionGenerator( val codegenContext: ServerCodegenContext, From 28a3397f527e4722d302743e2d5ef69800a21817 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Tue, 29 Nov 2022 13:44:03 +0000 Subject: [PATCH 35/51] Change error message: `unconstrained` -> `not constrained` --- .../server/smithy/generators/ConstrainedCollectionGenerator.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 b76a04ca7a..c9629381db 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 @@ -63,7 +63,7 @@ class ConstrainedCollectionGenerator( fun render() { check(constraintsInfo.isNotEmpty()) { - "`ConstrainedCollectionGenerator` can only be invoked for constrained collections, but this shape was unconstrained" + "`ConstrainedCollectionGenerator` can only be invoked for constrained collections, but this shape was not constrained" } val name = constrainedShapeSymbolProvider.toSymbol(shape).name From b0679c518b4f17b859685835acfde133c33eb5e6 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Tue, 29 Nov 2022 13:49:57 +0000 Subject: [PATCH 36/51] Add imports to fix docs --- .../smithy/generators/UnconstrainedCollectionGenerator.kt | 2 ++ 1 file changed, 2 insertions(+) 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 59bc3aee74..8017c2ec87 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 @@ -6,6 +6,7 @@ package software.amazon.smithy.rust.codegen.server.smithy.generators import software.amazon.smithy.model.shapes.CollectionShape +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.rustTemplate import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType @@ -13,6 +14,7 @@ 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 import software.amazon.smithy.rust.codegen.server.smithy.ServerCodegenContext +import software.amazon.smithy.rust.codegen.server.smithy.UnconstrainedShapeSymbolProvider import software.amazon.smithy.rust.codegen.server.smithy.canReachConstrainedShape import software.amazon.smithy.rust.codegen.server.smithy.isDirectlyConstrained From ebaa3f01ccfcf82ad74859a096e65e9048f446ea Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Tue, 29 Nov 2022 13:53:17 +0000 Subject: [PATCH 37/51] Remove unused `modelsModuleWriter` parameter --- .../codegen/server/smithy/ServerCodegenVisitor.kt | 15 ++++++--------- .../UnconstrainedCollectionGenerator.kt | 1 - 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt index a0e9a9047c..fb12343c82 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt @@ -290,15 +290,12 @@ open class ServerCodegenVisitor( if (renderUnconstrainedList) { logger.info("[rust-server-codegen] Generating an unconstrained type for collection shape $shape") - rustCrate.withModule(UnconstrainedModule) unconstrainedModuleWriter@{ - rustCrate.withModule(ModelsModule) modelsModuleWriter@{ - UnconstrainedCollectionGenerator( - codegenContext, - this@unconstrainedModuleWriter, - this@modelsModuleWriter, - shape, - ).render() - } + rustCrate.withModule(UnconstrainedModule) { + UnconstrainedCollectionGenerator( + codegenContext, + this, + shape, + ).render() } if (!isDirectlyConstrained) { 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 8017c2ec87..b337c6f629 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 @@ -32,7 +32,6 @@ import software.amazon.smithy.rust.codegen.server.smithy.isDirectlyConstrained class UnconstrainedCollectionGenerator( val codegenContext: ServerCodegenContext, private val unconstrainedModuleWriter: RustWriter, - private val modelsModuleWriter: RustWriter, val shape: CollectionShape, ) { private val model = codegenContext.model From 0d5a313872a21e77014748be352e7e2536b8fdd6 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Tue, 29 Nov 2022 14:20:54 +0000 Subject: [PATCH 38/51] Use `filterIsInstance` and coalesce `filter + map` --- .../server/smithy/ValidateUnsupportedConstraints.kt | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt index b1ac21c027..385f6f98ad 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt @@ -24,6 +24,7 @@ import software.amazon.smithy.model.traits.StreamingTrait import software.amazon.smithy.model.traits.Trait import software.amazon.smithy.model.traits.UniqueItemsTrait import software.amazon.smithy.rust.codegen.core.util.expectTrait +import software.amazon.smithy.rust.codegen.core.util.getTrait import software.amazon.smithy.rust.codegen.core.util.hasTrait import software.amazon.smithy.rust.codegen.core.util.inputShape import software.amazon.smithy.rust.codegen.core.util.orNull @@ -224,9 +225,10 @@ fun validateUnsupportedConstraints( val unsupportedLengthTraitOnBlobShapeSet = walker .walkShapes(service) .asSequence() - .filter { it is BlobShape } - .filter { it.hasTrait() } - .map { UnsupportedLengthTraitOnBlobShape(it, it.expectTrait()) } + .filterIsInstance() + .mapNotNull { + it.getTrait()?.let { trait -> UnsupportedLengthTraitOnBlobShape(it, trait) } + } .toSet() // 5. Range trait used on a non-integer shape. It has not been implemented yet. From ac2f73740bd9f6986ef94f02312edaa6b9e8e11d Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Tue, 29 Nov 2022 14:46:59 +0000 Subject: [PATCH 39/51] Add `check` in json customization --- .../BeforeIteratingOverMapOrCollectionJsonCustomization.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/customizations/BeforeIteratingOverMapOrCollectionJsonCustomization.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/customizations/BeforeIteratingOverMapOrCollectionJsonCustomization.kt index e66f6c326a..8dabd4ac02 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/customizations/BeforeIteratingOverMapOrCollectionJsonCustomization.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/customizations/BeforeIteratingOverMapOrCollectionJsonCustomization.kt @@ -5,8 +5,9 @@ package software.amazon.smithy.rust.codegen.server.smithy.customizations +import software.amazon.smithy.model.shapes.CollectionShape +import software.amazon.smithy.model.shapes.MapShape import software.amazon.smithy.rust.codegen.core.rustlang.Writable -import software.amazon.smithy.rust.codegen.core.rustlang.rust import software.amazon.smithy.rust.codegen.core.rustlang.writable import software.amazon.smithy.rust.codegen.core.smithy.protocols.serialize.JsonSerializerCustomization import software.amazon.smithy.rust.codegen.core.smithy.protocols.serialize.JsonSerializerSection @@ -22,6 +23,7 @@ import software.amazon.smithy.rust.codegen.server.smithy.workingWithPublicConstr class BeforeIteratingOverMapOrCollectionJsonCustomization(private val codegenContext: ServerCodegenContext) : JsonSerializerCustomization() { override fun section(section: JsonSerializerSection): Writable = when (section) { is JsonSerializerSection.BeforeIteratingOverMapOrCollection -> writable { + check(section.shape is CollectionShape || section.shape is MapShape) if (workingWithPublicConstrainedWrapperTupleType( section.shape, codegenContext.model, From 57cd6f1e930d755afb24468824cd421013fc954f Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Tue, 29 Nov 2022 14:51:36 +0000 Subject: [PATCH 40/51] Use `Set` instead of `List` for supported contraints --- .../amazon/smithy/rust/codegen/server/smithy/Constraints.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt index 4d00ab3f77..0254d5811c 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt @@ -22,8 +22,8 @@ import software.amazon.smithy.model.traits.LengthTrait import software.amazon.smithy.model.traits.PatternTrait import software.amazon.smithy.model.traits.RangeTrait import software.amazon.smithy.model.traits.RequiredTrait -import software.amazon.smithy.model.traits.Trait import software.amazon.smithy.model.traits.UniqueItemsTrait +import software.amazon.smithy.rust.codegen.core.smithy.CodegenContext import software.amazon.smithy.rust.codegen.core.smithy.isOptional import software.amazon.smithy.rust.codegen.core.util.UNREACHABLE import software.amazon.smithy.rust.codegen.core.util.hasTrait @@ -48,12 +48,12 @@ val allConstraintTraits = setOf( RequiredTrait::class.java, ) -val supportedStringConstraintTraits: List> = listOf(LengthTrait::class.java, PatternTrait::class.java) +val supportedStringConstraintTraits = setOf(LengthTrait::class.java, PatternTrait::class.java) /** * Supported constraint traits for the `list` and `set` shapes. */ -val supportedCollectionConstraintTraits: List> = listOf( +val supportedCollectionConstraintTraits = setOf( LengthTrait::class.java, // TODO(https://github.com/awslabs/smithy-rs/issues/1670): Not yet supported. // UniqueItemsTrait::class.java From 964d78f8df9e2287ad424917ad248866f03cae3b Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Tue, 29 Nov 2022 14:56:04 +0000 Subject: [PATCH 41/51] Use `toMutableList` to avoid creating an extra list --- .../generators/CollectionConstraintViolationGenerator.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt index a294ec4d11..30fa283e42 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt @@ -44,7 +44,7 @@ class CollectionConstraintViolationGenerator( val constraintViolationVisibility = Visibility.publicIf(publicConstrainedTypes, Visibility.PUBCRATE) modelsModuleWriter.withInlineModule(constraintViolationSymbol.module()) { - var constraintViolationVariants = constraintsInfo.map { it.constraintViolationVariant } + val constraintViolationVariants = constraintsInfo.map { it.constraintViolationVariant }.toMutableList() if (isMemberConstrained) { constraintViolationVariants += { rustTemplate( @@ -72,7 +72,7 @@ class CollectionConstraintViolationGenerator( ) if (shape.isReachableFromOperationInput()) { - var validationExceptionFields = constraintsInfo.map { it.asValidationExceptionField } + val validationExceptionFields = constraintsInfo.map { it.asValidationExceptionField }.toMutableList() if (isMemberConstrained) { validationExceptionFields += { rust("""Self::Member(index, member_constraint_violation) => member_constraint_violation.as_validation_exception_field(path + "/" + &index.to_string())""") } } From 838ada5657a3c4fb6ad17cfcbe06b831ecfa1468 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Tue, 29 Nov 2022 15:01:56 +0000 Subject: [PATCH 42/51] Add `@length` list to `ConA` --- codegen-core/common-test-models/constraints.smithy | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/codegen-core/common-test-models/constraints.smithy b/codegen-core/common-test-models/constraints.smithy index 71c2cb69fc..7e718f20a9 100644 --- a/codegen-core/common-test-models/constraints.smithy +++ b/codegen-core/common-test-models/constraints.smithy @@ -418,6 +418,11 @@ structure ConA { // TODO(https://github.com/awslabs/smithy-rs/issues/1401): a `set` shape is // just a `list` shape with `uniqueItems`, which hasn't been implemented yet. // setOfLengthPatternString: SetOfLengthPatternString, + + lengthListOfPatternString: LengthListOfPatternString, + // TODO(https://github.com/awslabs/smithy-rs/issues/1401): a `set` shape is + // just a `list` shape with `uniqueItems`, which hasn't been implemented yet. + // lengthSetOfPatternString: LengthSetOfPatternString, } map MapOfLengthString { @@ -556,6 +561,11 @@ set SetOfLengthPatternString { member: LengthPatternString } +@length(min: 5, max: 9) +set LengthSetOfPatternString { + member: PatternString +} + list ListOfLengthString { member: LengthString } From 1e818e43f345791d94ba09d65ab6c72144a4e629 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Tue, 29 Nov 2022 15:09:44 +0000 Subject: [PATCH 43/51] Add `@httpQuery`-bound `@length` list example --- codegen-core/common-test-models/constraints.smithy | 3 +++ 1 file changed, 3 insertions(+) diff --git a/codegen-core/common-test-models/constraints.smithy b/codegen-core/common-test-models/constraints.smithy index 7e718f20a9..4483b43129 100644 --- a/codegen-core/common-test-models/constraints.smithy +++ b/codegen-core/common-test-models/constraints.smithy @@ -239,6 +239,9 @@ structure ConstrainedHttpBoundShapesOperationInputOutput { @httpQuery("lengthStringList") lengthStringListQuery: ListOfLengthString, + @httpQuery("lengthListPatternString") + lengthListPatternStringQuery: LengthListOfPatternString, + // TODO(https://github.com/awslabs/smithy-rs/issues/1401): a `set` shape is // just a `list` shape with `uniqueItems`, which hasn't been implemented yet. // @httpQuery("lengthStringSet") From 111ee98d8abe3438ac784118e41a713c1aad8577 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Tue, 29 Nov 2022 15:13:39 +0000 Subject: [PATCH 44/51] Add `@httpHeader`-bound `@length` list --- codegen-core/common-test-models/constraints.smithy | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/codegen-core/common-test-models/constraints.smithy b/codegen-core/common-test-models/constraints.smithy index 4483b43129..496a4654e1 100644 --- a/codegen-core/common-test-models/constraints.smithy +++ b/codegen-core/common-test-models/constraints.smithy @@ -209,8 +209,16 @@ structure ConstrainedHttpBoundShapesOperationInputOutput { // @httpHeader("X-Length-Set") // lengthStringSetHeader: SetOfLengthString, - @httpHeader("X-Length-List") - lengthStringListHeader: ListOfLengthString, + @httpHeader("X-List-Length-String") + listLengthStringHeader: ListOfLengthString, + + @httpHeader("X-Length-List-Pattern-String") + lengthListPatternStringHeader: LengthListOfPatternString, + + // TODO(https://github.com/awslabs/smithy-rs/issues/1401): a `set` shape is + // just a `list` shape with `uniqueItems`, which hasn't been implemented yet. + // @httpHeader("X-Length-Set-Pattern-String") + // lengthSetPatternStringHeader: LengthSetOfPatternString, // TODO(https://github.com/awslabs/smithy-rs/issues/1401): a `set` shape is // just a `list` shape with `uniqueItems`, which hasn't been implemented yet. From a0e561e218c3e6c454b5ed7ccf7628e10ddd0dc3 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Tue, 29 Nov 2022 16:52:01 +0000 Subject: [PATCH 45/51] Fix `UnconstrainedCollectionGeneratorTest` test --- .../smithy/generators/UnconstrainedCollectionGeneratorTest.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/UnconstrainedCollectionGeneratorTest.kt b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/UnconstrainedCollectionGeneratorTest.kt index 58b0d27c33..6a02765c9a 100644 --- a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/UnconstrainedCollectionGeneratorTest.kt +++ b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/UnconstrainedCollectionGeneratorTest.kt @@ -65,7 +65,6 @@ class UnconstrainedCollectionGeneratorTest { UnconstrainedCollectionGenerator( codegenContext, this@unconstrainedModuleWriter, - this@modelsModuleWriter, it, ).render() From f759439054d3ae593868e76147666300e7cbc89a Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Tue, 29 Nov 2022 16:54:03 +0000 Subject: [PATCH 46/51] Fix rendering of constrained lists as header values --- .../core/smithy/generators/http/HttpBindingGenerator.kt | 9 ++++++++- .../generators/http/ServerResponseBindingGenerator.kt | 3 ++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/http/HttpBindingGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/http/HttpBindingGenerator.kt index 8ff3880c17..1dfd208ace 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/http/HttpBindingGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/http/HttpBindingGenerator.kt @@ -590,7 +590,14 @@ class HttpBindingGenerator( renderErrorMessage: (String) -> Writable, ) { val loopVariable = ValueExpression.Reference(safeName("inner")) - rustBlock("for ${loopVariable.name} in ${value.asRef()}") { + val context = HeaderValueSerializationContext(value, shape) + for (customization in customizations) { + customization.section( + HttpBindingSection.BeforeRenderingHeaderValue(context), + )(this) + } + + rustBlock("for ${loopVariable.name} in ${context.valueExpression.asRef()}") { this.renderHeaderValue( headerName, loopVariable, diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/http/ServerResponseBindingGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/http/ServerResponseBindingGenerator.kt index 20e9f97363..387d7d03ce 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/http/ServerResponseBindingGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/http/ServerResponseBindingGenerator.kt @@ -6,6 +6,7 @@ package software.amazon.smithy.rust.codegen.server.smithy.generators.http import software.amazon.smithy.codegen.core.Symbol +import software.amazon.smithy.model.shapes.CollectionShape import software.amazon.smithy.model.shapes.OperationShape import software.amazon.smithy.model.shapes.Shape import software.amazon.smithy.model.shapes.StructureShape @@ -90,7 +91,7 @@ class ServerResponseBeforeRenderingHeadersHttpBindingCustomization(val codegenCo codegenContext.settings.codegenConfig.publicConstrainedTypes, ) ) { - if (section.context.shape.isIntegerShape) { + if (section.context.shape.isIntegerShape || section.context.shape is CollectionShape) { section.context.valueExpression = ValueExpression.Reference("&${section.context.valueExpression.name.removePrefix("&")}.0") } From 3f97e7ccdf88be95a005a890aca8a63237fbe8e2 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Tue, 29 Nov 2022 16:59:12 +0000 Subject: [PATCH 47/51] Split very long line --- .../generators/CollectionConstraintViolationGenerator.kt | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt index 30fa283e42..247114f359 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt @@ -74,7 +74,14 @@ class CollectionConstraintViolationGenerator( if (shape.isReachableFromOperationInput()) { val validationExceptionFields = constraintsInfo.map { it.asValidationExceptionField }.toMutableList() if (isMemberConstrained) { - validationExceptionFields += { rust("""Self::Member(index, member_constraint_violation) => member_constraint_violation.as_validation_exception_field(path + "/" + &index.to_string())""") } + validationExceptionFields += { + rust( + """ + Self::Member(index, member_constraint_violation) => + member_constraint_violation.as_validation_exception_field(path + "/" + &index.to_string()) + """, + ) + } } rustTemplate( From 9e8c22b9eb29e401086ab20266b9dc1489fffa60 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Tue, 29 Nov 2022 17:06:40 +0000 Subject: [PATCH 48/51] Add docs for `ConstraintViolation::Member` for lists --- .../generators/CollectionConstraintViolationGenerator.kt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt index 247114f359..7867b045c4 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/CollectionConstraintViolationGenerator.kt @@ -49,6 +49,9 @@ class CollectionConstraintViolationGenerator( constraintViolationVariants += { rustTemplate( """ + /// Constraint violation error when an element doesn't satisfy its own constraints. + /// The first component of the tuple is the index in the collection where the + /// first constraint violation was found. ##[doc(hidden)] Member(usize, #{MemberConstraintViolationSymbol}) """, From 92ceba8a0af16c83d5caf8ea1b076880d43f93f1 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Wed, 30 Nov 2022 15:28:48 +0000 Subject: [PATCH 49/51] Pass `length` variable name to `LengthTrait.rustCondition` --- .../generators/ConstrainedCollectionGenerator.kt | 2 +- .../smithy/generators/ConstrainedMapGenerator.kt | 2 +- .../smithy/generators/ConstrainedStringGenerator.kt | 11 ++--------- .../server/smithy/generators/LenghTraitCommon.kt | 8 ++++---- 4 files changed, 8 insertions(+), 15 deletions(-) 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 c9629381db..857b6aa829 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 @@ -190,7 +190,7 @@ internal sealed class CollectionTraitInfo { rustTemplate( """ fn check_length(length: usize) -> Result<(), #{ConstraintViolation}> { - if ${lengthTrait.rustCondition()} { + if ${lengthTrait.rustCondition("length")} { Ok(()) } else { Err(#{ConstraintViolation}::Length(length)) 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 6379bdf36f..69cfafc698 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 @@ -98,7 +98,7 @@ class ConstrainedMapGenerator( /// ${rustDocsTryFromMethod(name, inner)} fn try_from(value: $inner) -> Result { let length = value.len(); - if ${lengthTrait.rustCondition()} { + if ${lengthTrait.rustCondition("length")} { Ok(Self(value)) } else { Err(#{ConstraintViolation}::Length(length)) 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 38d1a2acc7..6bf017b61f 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 @@ -198,21 +198,14 @@ private data class Length(val lengthTrait: LengthTrait) : StringTraitInfo() { * Renders a `check_length` function to validate the string matches the * required length indicated by the `@length` trait. */ + @Suppress("UNUSED_PARAMETER") private fun renderValidationFunction(constraintViolation: Symbol, unconstrainedTypeName: String): Writable = { - val condition = if (lengthTrait.min.isPresent && lengthTrait.max.isPresent) { - "(${lengthTrait.min.get()}..=${lengthTrait.max.get()}).contains(&length)" - } else if (lengthTrait.min.isPresent) { - "${lengthTrait.min.get()} <= length" - } else { - "length <= ${lengthTrait.max.get()}" - } - rust( """ fn check_length(string: &str) -> Result<(), $constraintViolation> { let length = string.chars().count(); - if $condition { + if ${lengthTrait.rustCondition("length")} { Ok(()) } else { Err($constraintViolation::Length(length)) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/LenghTraitCommon.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/LenghTraitCommon.kt index 8a2763acb9..bc074126ee 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/LenghTraitCommon.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/LenghTraitCommon.kt @@ -7,13 +7,13 @@ package software.amazon.smithy.rust.codegen.server.smithy.generators import software.amazon.smithy.model.traits.LengthTrait -fun LengthTrait.rustCondition(): String { +fun LengthTrait.rustCondition(lengthVariable: String): String { val condition = if (min.isPresent && max.isPresent) { - "(${min.get()}..=${max.get()}).contains(&length)" + "(${min.get()}..=${max.get()}).contains(&$lengthVariable)" } else if (min.isPresent) { - "${min.get()} <= length" + "${min.get()} <= $lengthVariable" } else { - "length <= ${max.get()}" + "$lengthVariable <= ${max.get()}" } return condition From 01cfe2386e855f4dade0e9ae17cb1a156f494997 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Wed, 30 Nov 2022 15:34:37 +0000 Subject: [PATCH 50/51] Refactor long conditional --- .../http/ServerResponseBindingGenerator.kt | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/http/ServerResponseBindingGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/http/ServerResponseBindingGenerator.kt index cc9c576d0d..794f77435a 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/http/ServerResponseBindingGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/http/ServerResponseBindingGenerator.kt @@ -89,16 +89,16 @@ class ServerResponseBeforeRenderingHeadersHttpBindingCustomization(val codegenCo HttpBindingCustomization() { override fun section(section: HttpBindingSection): Writable = when (section) { is HttpBindingSection.BeforeRenderingHeaderValue -> writable { - if (workingWithPublicConstrainedWrapperTupleType( - section.context.shape, - codegenContext.model, - codegenContext.settings.codegenConfig.publicConstrainedTypes, - ) - ) { - if (section.context.shape is IntegerShape || section.context.shape is ShortShape || section.context.shape is LongShape || section.context.shape is ByteShape || section.context.shape is CollectionShape) { - section.context.valueExpression = - ValueExpression.Reference("&${section.context.valueExpression.name.removePrefix("&")}.0") - } + val isIntegral = section.context.shape is ByteShape || section.context.shape is ShortShape || section.context.shape is IntegerShape || section.context.shape is LongShape + val workingWithPublicWrapper = workingWithPublicConstrainedWrapperTupleType( + section.context.shape, + codegenContext.model, + codegenContext.settings.codegenConfig.publicConstrainedTypes, + ) + + if (workingWithPublicWrapper && (isIntegral || section.context.shape is CollectionShape)) { + section.context.valueExpression = + ValueExpression.Reference("&${section.context.valueExpression.name.removePrefix("&")}.0") } } From 1ce32c8d0e68ba850150deab2686f8b9a85f0fc2 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Wed, 30 Nov 2022 15:39:22 +0000 Subject: [PATCH 51/51] Homogenise conditional --- .../smithy/generators/http/ServerResponseBindingGenerator.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/http/ServerResponseBindingGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/http/ServerResponseBindingGenerator.kt index 794f77435a..e30d9cc633 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/http/ServerResponseBindingGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/http/ServerResponseBindingGenerator.kt @@ -90,13 +90,15 @@ class ServerResponseBeforeRenderingHeadersHttpBindingCustomization(val codegenCo override fun section(section: HttpBindingSection): Writable = when (section) { is HttpBindingSection.BeforeRenderingHeaderValue -> writable { val isIntegral = section.context.shape is ByteShape || section.context.shape is ShortShape || section.context.shape is IntegerShape || section.context.shape is LongShape + val isCollection = section.context.shape is CollectionShape + val workingWithPublicWrapper = workingWithPublicConstrainedWrapperTupleType( section.context.shape, codegenContext.model, codegenContext.settings.codegenConfig.publicConstrainedTypes, ) - if (workingWithPublicWrapper && (isIntegral || section.context.shape is CollectionShape)) { + if (workingWithPublicWrapper && (isIntegral || isCollection)) { section.context.valueExpression = ValueExpression.Reference("&${section.context.valueExpression.name.removePrefix("&")}.0") }