Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Derive Eq and Hash wherever possible #2223

Merged
merged 17 commits into from
Jan 19, 2023
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -370,3 +370,15 @@ set_credentials_cache(
references = ["smithy-rs#2122"]
meta = { "breaking" = true, "tada" = false, "bug" = false }
author = "ysaito1001"

[[smithy-rs]]
message = "`aws_smithy_types::date_time::DateTime`, `aws_smithy_types::Blob` now implement the `Eq` and `Hash` traits"
references = ["smithy-rs#2223"]
meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "all"}
author = "david-perez"

[[smithy-rs]]
message = "Code-generated types for server SDKs now implement the `Eq` and `Hash` traits when possible"
references = ["smithy-rs#2223"]
meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "server"}
author = "david-perez"
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class RustClientCodegenPlugin : DecoratableBuildPlugin() {
.let { StreamingShapeSymbolProvider(it, model) }
// Add Rust attributes (like `#[derive(PartialEq)]`) to generated shapes
.let { BaseSymbolMetadataProvider(it, model, additionalAttributes = listOf(NonExhaustive)) }
// Streaming shapes need different derives (e.g. they cannot derive Eq)
// Streaming shapes need different derives (e.g. they cannot derive `PartialEq`)
.let { StreamingShapeMetadataProvider(it, model) }
// Rename shapes that clash with Rust reserved words & and other SDK specific features e.g. `send()` cannot
// be the name of an operation input
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ package software.amazon.smithy.rust.codegen.core.smithy
import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.BlobShape
import software.amazon.smithy.model.shapes.ListShape
import software.amazon.smithy.model.shapes.MapShape
import software.amazon.smithy.model.shapes.MemberShape
import software.amazon.smithy.model.shapes.NumberShape
import software.amazon.smithy.model.shapes.Shape
import software.amazon.smithy.model.shapes.StringShape
import software.amazon.smithy.model.shapes.StructureShape
Expand All @@ -27,7 +30,7 @@ class StreamingShapeSymbolProvider(private val base: RustSymbolProvider, private
WrappingSymbolProvider(base) {
override fun toSymbol(shape: Shape): Symbol {
val initial = base.toSymbol(shape)
// We are only targetting member shapes
// We are only targeting member shapes
if (shape !is MemberShape) {
return initial
}
Expand All @@ -49,7 +52,7 @@ class StreamingShapeSymbolProvider(private val base: RustSymbolProvider, private
}

/**
* SymbolProvider to drop the clone and PartialEq bounds in streaming shapes
* SymbolProvider to drop the `Clone` and `PartialEq` bounds in streaming shapes.
*
* Streaming shapes cannot be cloned and equality cannot be checked without reading the body. Because of this, these shapes
* do not implement `Clone` or `PartialEq`.
Expand All @@ -60,10 +63,6 @@ class StreamingShapeMetadataProvider(
private val base: RustSymbolProvider,
private val model: Model,
) : SymbolMetadataProvider(base) {
override fun memberMeta(memberShape: MemberShape): RustMetadata {
return base.toSymbol(memberShape).expectRustMetadata()
}

override fun structureMeta(structureShape: StructureShape): RustMetadata {
val baseMetadata = base.toSymbol(structureShape).expectRustMetadata()
return if (structureShape.hasStreamingMember(model)) {
Expand All @@ -78,7 +77,12 @@ class StreamingShapeMetadataProvider(
} else baseMetadata
}

override fun enumMeta(stringShape: StringShape): RustMetadata {
return base.toSymbol(stringShape).expectRustMetadata()
}
override fun memberMeta(memberShape: MemberShape) = base.toSymbol(memberShape).expectRustMetadata()
override fun enumMeta(stringShape: StringShape) = base.toSymbol(stringShape).expectRustMetadata()

override fun listMeta(listShape: ListShape) = base.toSymbol(listShape).expectRustMetadata()
override fun mapMeta(mapShape: MapShape) = base.toSymbol(mapShape).expectRustMetadata()
override fun stringMeta(stringShape: StringShape) = base.toSymbol(stringShape).expectRustMetadata()
override fun numberMeta(numberShape: NumberShape) = base.toSymbol(numberShape).expectRustMetadata()
override fun blobMeta(blobShape: BlobShape) = base.toSymbol(blobShape).expectRustMetadata()
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@ package software.amazon.smithy.rust.codegen.core.smithy
import software.amazon.smithy.codegen.core.CodegenException
import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.BlobShape
import software.amazon.smithy.model.shapes.CollectionShape
import software.amazon.smithy.model.shapes.ListShape
import software.amazon.smithy.model.shapes.MapShape
import software.amazon.smithy.model.shapes.MemberShape
import software.amazon.smithy.model.shapes.NumberShape
import software.amazon.smithy.model.shapes.Shape
import software.amazon.smithy.model.shapes.StringShape
import software.amazon.smithy.model.shapes.StructureShape
Expand Down Expand Up @@ -55,9 +60,15 @@ abstract class SymbolMetadataProvider(private val base: RustSymbolProvider) : Wr
is MemberShape -> memberMeta(shape)
is StructureShape -> structureMeta(shape)
is UnionShape -> unionMeta(shape)
is ListShape -> listMeta(shape)
is MapShape -> mapMeta(shape)
is NumberShape -> numberMeta(shape)
is BlobShape -> blobMeta(shape)
is StringShape -> if (shape.hasTrait<EnumTrait>()) {
enumMeta(shape)
} else null
} else {
stringMeta(shape)
}

else -> null
}
Expand All @@ -68,98 +79,101 @@ abstract class SymbolMetadataProvider(private val base: RustSymbolProvider) : Wr
abstract fun structureMeta(structureShape: StructureShape): RustMetadata
abstract fun unionMeta(unionShape: UnionShape): RustMetadata
abstract fun enumMeta(stringShape: StringShape): RustMetadata

abstract fun listMeta(listShape: ListShape): RustMetadata
abstract fun mapMeta(mapShape: MapShape): RustMetadata
abstract fun stringMeta(stringShape: StringShape): RustMetadata
abstract fun numberMeta(numberShape: NumberShape): RustMetadata
abstract fun blobMeta(blobShape: BlobShape): RustMetadata
}

fun containerDefaultMetadata(
shape: Shape,
model: Model,
additionalAttributes: List<Attribute> = emptyList(),
): RustMetadata {
val defaultDerives = setOf(RuntimeType.Debug, RuntimeType.PartialEq, RuntimeType.Clone)

val isSensitive = shape.hasTrait<SensitiveTrait>() ||
// Checking the shape's direct members for the sensitive trait should suffice.
// Whether their descendants, i.e. a member's member, is sensitive does not
// affect the inclusion/exclusion of the derived `Debug` trait of _this_ container
// shape; any sensitive descendant should still be printed as redacted.
shape.members().any { it.getMemberTrait(model, SensitiveTrait::class.java).isPresent }

val setOfDerives = if (isSensitive) {
defaultDerives - RuntimeType.Debug
} else {
defaultDerives
}
return RustMetadata(
setOfDerives,
additionalAttributes,
Visibility.PUBLIC,
)
}

/**
* The base metadata supports a list of attributes that are used by generators to decorate code.
* By default, we apply ```#[non_exhaustive]``` only to client structures since model changes should
* be considered as breaking only when generating server code.
* The base metadata supports a set of attributes that are used by generators to decorate code.
*
* By default we apply `#[non_exhaustive]` in [additionalAttributes] only to client structures since breaking model
* changes are fine when generating server code.
*/
class BaseSymbolMetadataProvider(
base: RustSymbolProvider,
private val model: Model,
private val additionalAttributes: List<Attribute>,
) : SymbolMetadataProvider(base) {
private fun containerDefault(shape: Shape): RustMetadata {
val isSensitive = shape.hasTrait<SensitiveTrait>() ||
// Checking the shape's direct members for the sensitive trait should suffice.
// Whether their descendants, i.e. a member's member, is sensitive does not
// affect the inclusion/exclusion of the derived Debug trait of _this_ container
// shape; any sensitive descendant should still be printed as redacted.
shape.members().any { it.getMemberTrait(model, SensitiveTrait::class.java).isPresent }

val derives = if (isSensitive) {
defaultDerives - RuntimeType.Debug
} else {
defaultDerives
}
return RustMetadata(
derives,
additionalAttributes,
Visibility.PUBLIC,
)
}

override fun memberMeta(memberShape: MemberShape): RustMetadata {
val container = model.expectShape(memberShape.container)
return when {
container.isStructureShape -> {
override fun memberMeta(memberShape: MemberShape): RustMetadata =
when (val container = model.expectShape(memberShape.container)) {
is StructureShape -> {
// TODO(https://github.com/awslabs/smithy-rs/issues/943): Once streaming accessors are usable,
// then also make streaming members `#[doc(hidden)]`
if (memberShape.getMemberTrait(model, StreamingTrait::class.java).isPresent) {
RustMetadata(visibility = Visibility.PUBLIC)
} else {
RustMetadata(
// At some point, visibility will be made PRIVATE, so make these `#[doc(hidden)]` for now
// At some point, visibility _may_ be made `PRIVATE`, so make these `#[doc(hidden)]` for now.
visibility = Visibility.PUBLIC,
additionalAttributes = listOf(Attribute.DocHidden),
)
}
}

container.isUnionShape ||
container.isListShape ||
container.isSetShape ||
container.isMapShape
-> RustMetadata(visibility = Visibility.PUBLIC)

is UnionShape, is CollectionShape, is MapShape -> RustMetadata(visibility = Visibility.PUBLIC)
else -> TODO("Unrecognized container type: $container")
}
}

override fun structureMeta(structureShape: StructureShape): RustMetadata {
return containerDefault(structureShape)
}

override fun unionMeta(unionShape: UnionShape): RustMetadata {
return containerDefault(unionShape)
}
override fun structureMeta(structureShape: StructureShape) = containerDefaultMetadata(structureShape, model, additionalAttributes)
override fun unionMeta(unionShape: UnionShape) = containerDefaultMetadata(unionShape, model, additionalAttributes)

override fun enumMeta(stringShape: StringShape): RustMetadata {
return containerDefault(stringShape).withDerives(
RuntimeType.Hash,
// enums can be Eq because they can only contain ints and strings
override fun enumMeta(stringShape: StringShape): RustMetadata =
containerDefaultMetadata(stringShape, model, additionalAttributes).withDerives(
// Smithy's `enum` shapes can additionally be `Eq`, `PartialOrd`, `Ord`, and `Hash` because they can
// only contain strings.
RuntimeType.Eq,
// enums can be PartialOrd/Ord because they can only contain ints and strings
RuntimeType.PartialOrd,
RuntimeType.Ord,
RuntimeType.Hash,
)
}

companion object {
private val defaultDerives by lazy {
setOf(RuntimeType.Debug, RuntimeType.PartialEq, RuntimeType.Clone)
}
}
// Only the server subproject uses these, so we provide a sane and conservative default implementation here so that
// the rest of symbol metadata providers can just delegate to it.
private val defaultRustMetadata = RustMetadata(visibility = Visibility.PRIVATE)

override fun listMeta(listShape: ListShape) = defaultRustMetadata
override fun mapMeta(mapShape: MapShape) = defaultRustMetadata
override fun stringMeta(stringShape: StringShape) = defaultRustMetadata
override fun numberMeta(numberShape: NumberShape) = defaultRustMetadata
override fun blobMeta(blobShape: BlobShape) = defaultRustMetadata
}

private const val META_KEY = "meta"
fun Symbol.Builder.meta(rustMetadata: RustMetadata?): Symbol.Builder {
return this.putProperty(META_KEY, rustMetadata)
}
fun Symbol.Builder.meta(rustMetadata: RustMetadata?): Symbol.Builder = this.putProperty(META_KEY, rustMetadata)

fun Symbol.expectRustMetadata(): RustMetadata = this.getProperty(META_KEY, RustMetadata::class.java).orElseThrow {
CodegenException(
"Expected $this to have metadata attached but it did not. ",
"Expected `$this` to have metadata attached but it did not.",
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ package software.amazon.smithy.rust.codegen.server.python.smithy
import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.BlobShape
import software.amazon.smithy.model.shapes.ListShape
import software.amazon.smithy.model.shapes.MapShape
import software.amazon.smithy.model.shapes.MemberShape
import software.amazon.smithy.model.shapes.NumberShape
import software.amazon.smithy.model.shapes.ServiceShape
import software.amazon.smithy.model.shapes.Shape
import software.amazon.smithy.model.shapes.StringShape
Expand Down Expand Up @@ -88,10 +91,6 @@ class PythonServerSymbolVisitor(
* Note that since streaming members can only be used on the root shape, this can only impact input and output shapes.
*/
class PythonStreamingShapeMetadataProvider(private val base: RustSymbolProvider, private val model: Model) : SymbolMetadataProvider(base) {
override fun memberMeta(memberShape: MemberShape): RustMetadata {
return base.toSymbol(memberShape).expectRustMetadata()
}

override fun structureMeta(structureShape: StructureShape): RustMetadata {
val baseMetadata = base.toSymbol(structureShape).expectRustMetadata()
return if (structureShape.hasStreamingMember(model)) {
Expand All @@ -106,7 +105,12 @@ class PythonStreamingShapeMetadataProvider(private val base: RustSymbolProvider,
} else baseMetadata
}

override fun enumMeta(stringShape: StringShape): RustMetadata {
return base.toSymbol(stringShape).expectRustMetadata()
}
override fun memberMeta(memberShape: MemberShape) = base.toSymbol(memberShape).expectRustMetadata()
override fun enumMeta(stringShape: StringShape) = base.toSymbol(stringShape).expectRustMetadata()

override fun listMeta(listShape: ListShape) = base.toSymbol(listShape).expectRustMetadata()
override fun mapMeta(mapShape: MapShape) = base.toSymbol(mapShape).expectRustMetadata()
override fun stringMeta(stringShape: StringShape) = base.toSymbol(stringShape).expectRustMetadata()
override fun numberMeta(numberShape: NumberShape) = base.toSymbol(numberShape).expectRustMetadata()
override fun blobMeta(blobShape: BlobShape) = base.toSymbol(blobShape).expectRustMetadata()
david-perez marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

package software.amazon.smithy.rust.codegen.server.smithy

import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.BlobShape
import software.amazon.smithy.model.shapes.ListShape
import software.amazon.smithy.model.shapes.MapShape
import software.amazon.smithy.model.shapes.MemberShape
import software.amazon.smithy.model.shapes.NumberShape
import software.amazon.smithy.model.shapes.Shape
import software.amazon.smithy.model.shapes.StringShape
import software.amazon.smithy.model.shapes.StructureShape
import software.amazon.smithy.model.shapes.UnionShape
import software.amazon.smithy.rust.codegen.core.rustlang.RustMetadata
import software.amazon.smithy.rust.codegen.core.rustlang.Visibility
import software.amazon.smithy.rust.codegen.core.smithy.RustSymbolProvider
import software.amazon.smithy.rust.codegen.core.smithy.SymbolMetadataProvider
import software.amazon.smithy.rust.codegen.core.smithy.containerDefaultMetadata
import software.amazon.smithy.rust.codegen.core.smithy.expectRustMetadata

/**
* This symbol metadata provider adds the usual derives on shapes that are constrained and hence generate newtypes.
*
* It also makes the newtypes `pub(crate)` when `publicConstrainedTypes` is disabled.
*/
class ConstrainedShapeSymbolMetadataProvider(
private val base: RustSymbolProvider,
private val model: Model,
private val constrainedTypes: Boolean,
) : SymbolMetadataProvider(base) {

override fun memberMeta(memberShape: MemberShape) = base.toSymbol(memberShape).expectRustMetadata()
override fun structureMeta(structureShape: StructureShape) = base.toSymbol(structureShape).expectRustMetadata()
override fun unionMeta(unionShape: UnionShape) = base.toSymbol(unionShape).expectRustMetadata()
override fun enumMeta(stringShape: StringShape) = base.toSymbol(stringShape).expectRustMetadata()

private fun addDerivesAndAdjustVisibilityIfConstrained(shape: Shape): RustMetadata {
check(shape is ListShape || shape is MapShape || shape is StringShape || shape is NumberShape || shape is BlobShape)

val baseMetadata = base.toSymbol(shape).expectRustMetadata()
val derives = baseMetadata.derives.toMutableSet()
val additionalAttributes = baseMetadata.additionalAttributes.toMutableList()

if (shape.canReachConstrainedShape(model, base)) {
derives += containerDefaultMetadata(shape, model).derives
}

val visibility = Visibility.publicIf(constrainedTypes, Visibility.PUBCRATE)
return RustMetadata(derives, additionalAttributes, visibility)
}

override fun listMeta(listShape: ListShape): RustMetadata = addDerivesAndAdjustVisibilityIfConstrained(listShape)
override fun mapMeta(mapShape: MapShape): RustMetadata = addDerivesAndAdjustVisibilityIfConstrained(mapShape)
override fun stringMeta(stringShape: StringShape): RustMetadata = addDerivesAndAdjustVisibilityIfConstrained(stringShape)
override fun numberMeta(numberShape: NumberShape): RustMetadata = addDerivesAndAdjustVisibilityIfConstrained(numberShape)
override fun blobMeta(blobShape: BlobShape) = addDerivesAndAdjustVisibilityIfConstrained(blobShape)
}
Loading