Implement @range on short (#2034)

* Extract a ConstrainedNumberGenerator from ConstrainedIntegerGenerator

* Update tests' expectations to assume that we support @range for short

* Add ShortShape to all the key places to get @range support

* Generalize generator tests.

* Add tests for the @range trait on short to our constraints.smithy model.

* Update changelog

* Fix type in constraints.smithy

CHANGELOG.next.toml

@@ -243,13 +243,14 @@ message = """
 * The `length` trait on `string` shapes.
 * The `length` trait on `map` shapes.
 * The `range` trait on `integer` shapes.
+* The `range` trait on `short` 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#2005", "smithy-rs#1998"]
+references = ["smithy-rs#1199", "smithy-rs#1342", "smithy-rs#1401", "smithy-rs#2005", "smithy-rs#1998", "smithy-rs#2034"]
 meta = { "breaking" = true, "tada" = true, "bug" = false, "target" = "server" }
 author = "david-perez"
 

codegen-core/common-test-models/constraints.smithy

@@ -49,7 +49,10 @@ operation ConstrainedShapesOperation {
  errors: [ValidationException]
 }
 
-@http(uri: "/constrained-http-bound-shapes-operation/{rangeIntegerLabel}/{lengthStringLabel}/{enumStringLabel}", method: "POST")
+@http(
+ uri: "/constrained-http-bound-shapes-operation/{rangeIntegerLabel}/{rangeShortLabel}/{lengthStringLabel}/{enumStringLabel}",
+ method: "POST"
+)
 operation ConstrainedHttpBoundShapesOperation {
  input: ConstrainedHttpBoundShapesOperationInputOutput,
  output: ConstrainedHttpBoundShapesOperationInputOutput,
@@ -179,6 +182,10 @@ structure ConstrainedHttpBoundShapesOperationInputOutput {
  @httpLabel
  rangeIntegerLabel: RangeInteger,
 
+ @required
+ @httpLabel
+ rangeShortLabel: RangeShort,
+
  @required
  @httpLabel
  enumStringLabel: EnumString,
@@ -193,6 +200,9 @@ structure ConstrainedHttpBoundShapesOperationInputOutput {
  @httpHeader("X-Range-Integer")
  rangeIntegerHeader: RangeInteger,
 
+ @httpHeader("X-Range-Short")
+ rangeShortHeader: RangeShort,
+
  // @httpHeader("X-Length-MediaType")
  // lengthStringHeaderWithMediaType: MediaTypeLengthString,
 
@@ -208,10 +218,15 @@ structure ConstrainedHttpBoundShapesOperationInputOutput {
  // just a `list` shape with `uniqueItems`, which hasn't been implemented yet.
  // @httpHeader("X-Range-Integer-Set")
  // rangeIntegerSetHeader: SetOfRangeInteger,
+ // @httpHeader("X-Range-Short-Set")
+ // rangeShortSetHeader: SetOfShortInteger,
 
  @httpHeader("X-Range-Integer-List")
  rangeIntegerListHeader: ListOfRangeInteger,
 
+ @httpHeader("X-Range-Short-List")
+ rangeShortListHeader: ListOfRangeShort,
+
  // TODO(https://github.com/awslabs/smithy-rs/issues/1431)
  // @httpHeader("X-Enum")
  //enumStringHeader: EnumString,
@@ -225,6 +240,9 @@ structure ConstrainedHttpBoundShapesOperationInputOutput {
  @httpQuery("rangeInteger")
  rangeIntegerQuery: RangeInteger,
 
+ @httpQuery("rangeShort")
+ rangeShortQuery: RangeShort,
+
  @httpQuery("enumString")
  enumStringQuery: EnumString,
 
@@ -239,10 +257,15 @@ structure ConstrainedHttpBoundShapesOperationInputOutput {
  @httpQuery("rangeIntegerList")
  rangeIntegerListQuery: ListOfRangeInteger,
 
+ @httpQuery("rangeShortList")
+ rangeShortListQuery: ListOfRangeShort,
+
  // 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("rangeIntegerSet")
  // rangeIntegerSetQuery: SetOfRangeInteger,
+ // @httpQuery("rangeShortSet")
+ // rangeShortSetQuery: SetOfRangeShort,
 
  @httpQuery("enumStringList")
  enumStringListQuery: ListOfEnumString,
@@ -364,6 +387,11 @@ structure ConA {
  maxRangeInteger: MaxRangeInteger,
  fixedValueInteger: FixedValueInteger,
 
+ rangeShort: RangeShort,
+ minRangeShort: MinRangeShort,
+ maxRangeShort: MaxRangeShort,
+ fixedValueShort: FixedValueShort,
+
  conBList: ConBList,
  conBList2: ConBList2,
 
@@ -390,6 +418,12 @@ structure ConA {
  // setOfRangeInteger: SetOfRangeInteger,
  mapOfRangeInteger: MapOfRangeInteger,
 
+ listOfRangeShort: ListOfRangeShort,
+ // 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.
+ // setOfRangeShort: SetOfRangeShort,
+ mapOfRangeShort: MapOfRangeShort,
+
  nonStreamingBlob: NonStreamingBlob
 
  patternString: PatternString,
@@ -417,6 +451,11 @@ map MapOfRangeInteger {
  value: RangeInteger,
 }
 
+map MapOfRangeShort {
+ key: String,
+ value: RangeShort,
+}
+
 map MapOfEnumString {
  key: EnumString,
  value: EnumString,
@@ -453,10 +492,17 @@ map MapOfSetOfLengthString {
 // 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.
 // map MapOfSetOfRangeInteger {
-// key: LengthString,
+// key: String,
 // value: SetOfRangeInteger,
 // }
 
+// 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.
+// map MapOfSetOfRangeShort {
+// key: String,
+// value: SetOfRangeShort,
+// }
+
 @length(min: 2, max: 8)
 list LengthListOfLengthString {
  member: LengthString
@@ -497,6 +543,18 @@ integer MaxRangeInteger
 @range(min: 69, max: 69)
 integer FixedValueInteger
 
+@range(min: -0, max: 10)
+short RangeShort
+
+@range(min: -10)
+short MinRangeShort
+
+@range(max: 11)
+short MaxRangeShort
+
+@range(min: 10, max: 10)
+short FixedValueShort
+
 /// A union with constrained members.
 union ConstrainedUnion {
  enumString: EnumString,
@@ -552,6 +610,16 @@ list ListOfRangeInteger {
  member: RangeInteger
 }
 
+// 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.
+// set SetOfRangeShort {
+// member: RangeShort
+// }
+
+list ListOfRangeShort {
+ member: RangeShort
+}
+
 list ListOfEnumString {
  member: EnumString
 }

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolProvider.kt

@@ -14,6 +14,7 @@ import software.amazon.smithy.model.shapes.MapShape
 import software.amazon.smithy.model.shapes.MemberShape
 import software.amazon.smithy.model.shapes.ServiceShape
 import software.amazon.smithy.model.shapes.Shape
+import software.amazon.smithy.model.shapes.ShortShape
 import software.amazon.smithy.model.shapes.StringShape
 import software.amazon.smithy.model.traits.LengthTrait
 import software.amazon.smithy.rust.codegen.core.rustlang.RustType
@@ -96,14 +97,16 @@ class ConstrainedShapeSymbolProvider(
  symbolBuilder(shape, RustType.Vec(inner.rustType())).addReference(inner).build()
  }
  }
- is StringShape, is IntegerShape -> {
+
+ is StringShape, is IntegerShape, is ShortShape -> {
  if (shape.isDirectlyConstrained(base)) {
  val rustType = RustType.Opaque(shape.contextName(serviceShape).toPascalCase())
  symbolBuilder(shape, rustType).locatedIn(ModelsModule).build()
  } else {
  base.toSymbol(shape)
  }
  }
+
  else -> base.toSymbol(shape)
  }
  }

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstraintViolationSymbolProvider.kt

@@ -12,6 +12,7 @@ import software.amazon.smithy.model.shapes.IntegerShape
 import software.amazon.smithy.model.shapes.MapShape
 import software.amazon.smithy.model.shapes.ServiceShape
 import software.amazon.smithy.model.shapes.Shape
+import software.amazon.smithy.model.shapes.ShortShape
 import software.amazon.smithy.model.shapes.StringShape
 import software.amazon.smithy.model.shapes.StructureShape
 import software.amazon.smithy.model.shapes.UnionShape
@@ -113,7 +114,8 @@ class ConstraintViolationSymbolProvider(
  .locatedIn(module)
  .build()
  }
- is StringShape, is IntegerShape -> {
+
+ is StringShape, is IntegerShape, is ShortShape -> {
  val module = shape.shapeModule()
  val rustType = RustType.Opaque(constraintViolationName, module.fullyQualifiedPath())
  Symbol.builder()

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt

@@ -13,6 +13,7 @@ import software.amazon.smithy.model.shapes.IntegerShape
 import software.amazon.smithy.model.shapes.MapShape
 import software.amazon.smithy.model.shapes.MemberShape
 import software.amazon.smithy.model.shapes.Shape
+import software.amazon.smithy.model.shapes.ShortShape
 import software.amazon.smithy.model.shapes.SimpleShape
 import software.amazon.smithy.model.shapes.StringShape
 import software.amazon.smithy.model.shapes.StructureShape
@@ -72,7 +73,7 @@ fun Shape.isDirectlyConstrained(symbolProvider: SymbolProvider): Boolean = when
 
  is MapShape -> this.hasTrait<LengthTrait>()
  is StringShape -> this.hasTrait<EnumTrait>() || supportedStringConstraintTraits.any { this.hasTrait(it) }
- is IntegerShape -> this.hasTrait<RangeTrait>()
+ is IntegerShape, is ShortShape -> this.hasTrait<RangeTrait>()
  else -> false
 }
 
@@ -98,7 +99,7 @@ fun MemberShape.targetCanReachConstrainedShape(model: Model, symbolProvider: Sym
 fun Shape.hasPublicConstrainedWrapperTupleType(model: Model, publicConstrainedTypes: Boolean): Boolean = when (this) {
  is MapShape -> publicConstrainedTypes && this.hasTrait<LengthTrait>()
  is StringShape -> !this.hasTrait<EnumTrait>() && (publicConstrainedTypes && supportedStringConstraintTraits.any(this::hasTrait))
- is IntegerShape -> publicConstrainedTypes && this.hasTrait<RangeTrait>()
+ is IntegerShape, is ShortShape -> publicConstrainedTypes && this.hasTrait<RangeTrait>()
  is MemberShape -> model.expectShape(this.target).hasPublicConstrainedWrapperTupleType(model, publicConstrainedTypes)
  else -> false
 }

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt

@@ -18,6 +18,7 @@ import software.amazon.smithy.model.shapes.ServiceShape
 import software.amazon.smithy.model.shapes.SetShape
 import software.amazon.smithy.model.shapes.Shape
 import software.amazon.smithy.model.shapes.ShapeVisitor
+import software.amazon.smithy.model.shapes.ShortShape
 import software.amazon.smithy.model.shapes.StringShape
 import software.amazon.smithy.model.shapes.StructureShape
 import software.amazon.smithy.model.shapes.UnionShape
@@ -45,6 +46,7 @@ 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.ConstrainedIntegerGenerator
 import software.amazon.smithy.rust.codegen.server.smithy.generators.ConstrainedMapGenerator
+import software.amazon.smithy.rust.codegen.server.smithy.generators.ConstrainedShortGenerator
 import software.amazon.smithy.rust.codegen.server.smithy.generators.ConstrainedStringGenerator
 import software.amazon.smithy.rust.codegen.server.smithy.generators.ConstrainedTraitForEnumGenerator
 import software.amazon.smithy.rust.codegen.server.smithy.generators.MapConstraintViolationGenerator
@@ -361,6 +363,15 @@ open class ServerCodegenVisitor(
  }
  }
 
+ override fun shortShape(shape: ShortShape) {
+ if (shape.isDirectlyConstrained(codegenContext.symbolProvider)) {
+ logger.info("[rust-server-codegen] Generating a constrained short $shape")
+ rustCrate.withModule(ModelsModule) {
+ ConstrainedShortGenerator(codegenContext, this, shape).render()
+ }
+ }
+ }
+
  protected fun stringShape(
  shape: StringShape,
  enumShapeGeneratorFactory: (codegenContext: ServerCodegenContext, writer: RustWriter, shape: StringShape) -> ServerEnumGenerator,

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt

@@ -17,6 +17,7 @@ import software.amazon.smithy.model.shapes.ServiceShape
 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.ShortShape
 import software.amazon.smithy.model.shapes.UnionShape
 import software.amazon.smithy.model.traits.EnumTrait
 import software.amazon.smithy.model.traits.LengthTrait
@@ -245,7 +246,7 @@ fun validateUnsupportedConstraints(
  val unsupportedRangeTraitOnShapeSet = walker
  .walkShapes(service)
  .asSequence()
- .filterNot { it is IntegerShape }
+ .filterNot { it is IntegerShape || it is ShortShape }
  .filterMapShapesToTraits(setOf(RangeTrait::class.java))
  .map { (shape, rangeTrait) -> UnsupportedRangeTraitOnShape(shape, rangeTrait as RangeTrait) }
  .toSet()

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/customizations/BeforeSerializingMemberJsonCustomization.kt

@@ -6,6 +6,7 @@
 package software.amazon.smithy.rust.codegen.server.smithy.customizations
 
 import software.amazon.smithy.model.shapes.IntegerShape
+import software.amazon.smithy.model.shapes.ShortShape
 import software.amazon.smithy.rust.codegen.core.rustlang.Writable
 import software.amazon.smithy.rust.codegen.core.rustlang.writable
 import software.amazon.smithy.rust.codegen.core.smithy.protocols.serialize.JsonSerializerCustomization
@@ -27,7 +28,7 @@ class BeforeSerializingMemberJsonCustomization(private val codegenContext: Serve
  codegenContext.settings.codegenConfig.publicConstrainedTypes,
  )
  ) {
- if (section.shape is IntegerShape) {
+ if (section.shape is IntegerShape || section.shape is ShortShape) {
  section.context.valueExpression =
  ValueExpression.Reference("&${section.context.valueExpression.name}.0")
  }

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedIntegerGenerator.kt

@@ -7,6 +7,8 @@ package software.amazon.smithy.rust.codegen.server.smithy.generators
 
 import software.amazon.smithy.codegen.core.Symbol
 import software.amazon.smithy.model.shapes.IntegerShape
+import software.amazon.smithy.model.shapes.NumberShape
+import software.amazon.smithy.model.shapes.ShortShape
 import software.amazon.smithy.model.traits.RangeTrait
 import software.amazon.smithy.rust.codegen.core.rustlang.Attribute
 import software.amazon.smithy.rust.codegen.core.rustlang.RustMetadata
@@ -31,15 +33,40 @@ 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 ConstrainedIntegerGenerator(
+ val codegenContext: ServerCodegenContext,
+ val writer: RustWriter,
+ val shape: IntegerShape,
+) {
+ val inner = ConstrainedNumberGenerator(codegenContext, writer, shape, RustType.Integer(32))
+
+ fun render() {
+ inner.render()
+ }
+}
+
+class ConstrainedShortGenerator(
+ val codegenContext: ServerCodegenContext,
+ val writer: RustWriter,
+ val shape: ShortShape,
+) {
+ val inner = ConstrainedNumberGenerator(codegenContext, writer, shape, RustType.Integer(16))
+
+ fun render() {
+ inner.render()
+ }
+}
+
 /**
- * [ConstrainedIntegerGenerator] generates a wrapper newtype holding a constrained `i32`.
+ * [ConstrainedNumberGenerator] generates a wrapper newtype holding a constrained number primitive.
  * This type can be built from unconstrained values, yielding a `ConstraintViolation` when the input does not satisfy
  * the constraints.
  */
-class ConstrainedIntegerGenerator(
+class ConstrainedNumberGenerator(
  val codegenContext: ServerCodegenContext,
  val writer: RustWriter,
- val shape: IntegerShape,
+ val shape: NumberShape,
+ private val unconstrainedType: RustType,
 ) {
  val model = codegenContext.model
  val constrainedShapeSymbolProvider = codegenContext.constrainedShapeSymbolProvider
@@ -58,7 +85,7 @@ class ConstrainedIntegerGenerator(
 
  val symbol = constrainedShapeSymbolProvider.toSymbol(shape)
  val constrainedTypeName = symbol.name
- val unconstrainedTypeName = RustType.Integer(32).render()
+ val unconstrainedTypeName = unconstrainedType.render()
  val constraintViolation = constraintViolationSymbolProvider.toSymbol(shape)
  val constraintsInfo = listOf(Range(rangeTrait).toTraitInfo(unconstrainedTypeName))
 
@@ -171,7 +198,7 @@ private data class Range(val rangeTrait: RangeTrait) {
  fun toTraitInfo(unconstrainedTypeName: String): TraitInfo = TraitInfo(
  { rust("Self::check_range(value)?;") },
  {
- docs("Error when an integer doesn't satisfy its `@range` requirements.")
+ docs("Error when a number doesn't satisfy its `@range` requirements.")
  rust("Range($unconstrainedTypeName)")
  },
  {
@@ -188,7 +215,7 @@ private data class Range(val rangeTrait: RangeTrait) {
  )
 
  /**
- * Renders a `check_range` function to validate the integer matches the
+ * Renders a `check_range` function to validate that the value matches the
  * required range indicated by the `@range` trait.
  */
  private fun renderValidationFunction(constraintViolation: Symbol, unconstrainedTypeName: String): Writable = {